From 61a1ff159151052d6bfc61390fe47eb605e298d4 Mon Sep 17 00:00:00 2001 From: juanifioren Date: Thu, 26 Dec 2024 21:48:10 -0300 Subject: [PATCH] Request parameter + max_age parameter --- docs/sections/changelog.rst | 2 + oidc_provider/lib/endpoints/authorize.py | 33 +++- oidc_provider/tests/app/utils.py | 3 +- .../tests/cases/test_authorize_endpoint.py | 187 +++++++++++++----- .../tests/cases/test_token_endpoint.py | 3 +- oidc_provider/views.py | 54 +++-- tox.ini | 11 +- 7 files changed, 212 insertions(+), 81 deletions(-) diff --git a/docs/sections/changelog.rst b/docs/sections/changelog.rst index 2ad4ca02..2b5fd7d0 100644 --- a/docs/sections/changelog.rst +++ b/docs/sections/changelog.rst @@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. Unreleased ========== +* Added: support of max_age parameter on authorization request. +* Added: Passing Request Parameters as JWTs now returning request_not_supported error. * Changed: Django 5 added to test matrix. * Changed: ID Token JSON encoder improved using DjangoJSONEncoder. * Changed: Use unittest.mock in tests. Remove mock library. diff --git a/oidc_provider/lib/endpoints/authorize.py b/oidc_provider/lib/endpoints/authorize.py index c14d7689..719da792 100644 --- a/oidc_provider/lib/endpoints/authorize.py +++ b/oidc_provider/lib/endpoints/authorize.py @@ -3,6 +3,8 @@ from hashlib import md5 from hashlib import sha256 +from oidc_provider.compat import get_attr_or_callable + try: from urllib import urlencode @@ -16,6 +18,7 @@ from urllib.parse import urlunsplit from uuid import uuid4 +from django.utils import dateformat from django.utils import timezone from oidc_provider import settings @@ -74,11 +77,12 @@ def _extract_params(self): self.params["scope"] = query_dict.get("scope", "").split() self.params["state"] = query_dict.get("state", "") self.params["nonce"] = query_dict.get("nonce", "") - + # https://openid.net/specs/openid-connect-core-1_0.html#RequestObject + self.params["request"] = query_dict.get("request", "") self.params["prompt"] = self._allowed_prompt_params.intersection( set(query_dict.get("prompt", "").split()) ) - + self.params["max_age"] = query_dict.get("max_age", "") self.params["code_challenge"] = query_dict.get("code_challenge", "") self.params["code_challenge_method"] = query_dict.get("code_challenge_method", "") @@ -105,6 +109,12 @@ def validate_params(self): self.params["redirect_uri"], "unsupported_response_type", self.grant_type ) + # Passing Request Parameters as JWT not supported. + if self.params["request"]: + raise AuthorizeError( + self.params["redirect_uri"], "request_not_supported", self.grant_type + ) + if not self.is_authentication and ( self.grant_type == "hybrid" or self.params["response_type"] in ["id_token", "id_token token"] @@ -302,6 +312,25 @@ def is_client_allowed_to_skip_consent(self): or self.params["response_type"] in implicit_flow_resp_types ) + def is_authentication_age_is_greater_than_max_age(self): + """ + If the End-User authentication age is greater than the max_age value present in the + Authorization request, the OP MUST attempt to actively re-authenticate the End-User. + """ + if not get_attr_or_callable(self.request.user, "is_authenticated"): + return False + try: + max_age = int(self.params["max_age"]) + except ValueError: + return False + + auth_time = int( + dateformat.format(self.request.user.last_login or self.request.user.date_joined, "U") + ) + max_allowed_time = int(dateformat.format(timezone.now(), "U")) - max_age + + return auth_time < max_allowed_time + def get_scopes_information(self): """ Return a list with the description of all the scopes requested. diff --git a/oidc_provider/tests/app/utils.py b/oidc_provider/tests/app/utils.py index 491af0e5..5c3608cc 100644 --- a/oidc_provider/tests/app/utils.py +++ b/oidc_provider/tests/app/utils.py @@ -26,6 +26,7 @@ ) FAKE_CODE_CHALLENGE = "YlYXEqXuRm-Xgi2BOUiK50JW1KsGTX6F1TDnZSC8VTg" FAKE_CODE_VERIFIER = "SmxGa0XueyNh5bDgTcSrqzAh2_FmXEqU8kDT6CuXicw" +FAKE_USER_PASSWORD = "1234" def create_fake_user(): @@ -39,7 +40,7 @@ def create_fake_user(): user.email = "johndoe@example.com" user.first_name = "John" user.last_name = "Doe" - user.set_password("1234") + user.set_password(FAKE_USER_PASSWORD) user.save() diff --git a/oidc_provider/tests/cases/test_authorize_endpoint.py b/oidc_provider/tests/cases/test_authorize_endpoint.py index e5cfeb11..5846a423 100644 --- a/oidc_provider/tests/cases/test_authorize_endpoint.py +++ b/oidc_provider/tests/cases/test_authorize_endpoint.py @@ -1,3 +1,5 @@ +from datetime import datetime + try: from urllib.parse import quote from urllib.parse import urlencode @@ -14,13 +16,14 @@ from unittest.mock import Mock from unittest.mock import patch -from django.contrib.auth.models import AnonymousUser -from django.core.management import call_command +from freezegun import freeze_time try: from django.urls import reverse except ImportError: from django.core.urlresolvers import reverse +from django.contrib.auth.models import AnonymousUser +from django.core.management import call_command from django.test import RequestFactory from django.test import TestCase from django.test import override_settings @@ -70,7 +73,7 @@ def setUp(self): call_command("creatersakey") self.factory = RequestFactory() self.user = create_fake_user() - self.client = create_fake_client(response_type="code") + self.client_code = create_fake_client(response_type="code") self.client_with_no_consent = create_fake_client( response_type="code", require_consent=False ) @@ -103,9 +106,9 @@ def test_invalid_response_type(self): """ # Create an authorize request with an unsupported response_type. data = { - "client_id": self.client.client_id, + "client_id": self.client_code.client_id, "response_type": "something_wrong", - "redirect_uri": self.client.default_redirect_uri, + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, } @@ -118,6 +121,31 @@ def test_invalid_response_type(self): # Should be an 'error' component in query. self.assertIn("error=", response["Location"]) + def test_passing_request_parameters_as_jwt_not_supported(self): + """ + The OP MUST return the request_not_supported error if the parameter value is a + Request Object value. + + See: https://openid.net/specs/openid-connect-core-1_0.html#JWTRequests + """ + # Create an authorize request with an unsupported response_type. + data = { + "client_id": self.client_code.client_id, + "response_type": "code", + "redirect_uri": self.client_code.default_redirect_uri, + "scope": "openid email", + "state": self.state, + "request": "eyJhbGciOiJub25lIn0...", + } + + response = self._auth_request("get", data) + + self.assertEqual(response.status_code, 302) + self.assertEqual(response.has_header("Location"), True) + + # Should be an 'error' component in query. + self.assertIn("error=request_not_supported", response["Location"]) + def test_user_not_logged(self): """ The Authorization Server attempts to Authenticate the End-User by @@ -126,9 +154,9 @@ def test_user_not_logged(self): See: http://openid.net/specs/openid-connect-core-1_0.html#Authenticates """ data = { - "client_id": self.client.client_id, + "client_id": self.client_code.client_id, "response_type": "code", - "redirect_uri": self.client.default_redirect_uri, + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, } @@ -147,9 +175,9 @@ def test_user_consent_inputs(self): See: http://openid.net/specs/openid-connect-core-1_0.html#Consent """ data = { - "client_id": self.client.client_id, + "client_id": self.client_code.client_id, "response_type": "code", - "redirect_uri": self.client.default_redirect_uri, + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, # PKCE parameters. @@ -164,8 +192,8 @@ def test_user_consent_inputs(self): input_html = '' to_check = { - "client_id": self.client.client_id, - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "redirect_uri": self.client_code.default_redirect_uri, "response_type": "code", "code_challenge": FAKE_CODE_CHALLENGE, "code_challenge_method": "S256", @@ -188,8 +216,8 @@ def test_user_consent_response(self): by adding them as query parameters to the redirect_uri. """ data = { - "client_id": self.client.client_id, - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "redirect_uri": self.client_code.default_redirect_uri, "response_type": "code", "scope": "openid email", "state": self.state, @@ -212,7 +240,9 @@ def test_user_consent_response(self): response = self._auth_request("post", data, is_user_authenticated=True) - is_code_ok = is_code_valid(url=response["Location"], user=self.user, client=self.client) + is_code_ok = is_code_valid( + url=response["Location"], user=self.user, client=self.client_code + ) self.assertEqual(is_code_ok, True, msg="Code returned is invalid.") # Check if the state is returned. @@ -263,8 +293,8 @@ def test_response_uri_is_properly_constructed(self): Only 'state' and 'code' should be appended. """ data = { - "client_id": self.client.client_id, - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "redirect_uri": self.client_code.default_redirect_uri, "response_type": "code", "scope": "openid email", "state": self.state, @@ -278,7 +308,9 @@ def test_response_uri_is_properly_constructed(self): state = params["state"][0] self.assertEqual(self.state, state, msg="State returned is invalid or missing") - is_code_ok = is_code_valid(url=response["Location"], user=self.user, client=self.client) + is_code_ok = is_code_valid( + url=response["Location"], user=self.user, client=self.client_code + ) self.assertTrue(is_code_ok, msg="Code returned is invalid or missing") self.assertEqual( @@ -288,7 +320,7 @@ def test_response_uri_is_properly_constructed(self): ) self.assertTrue( - response["Location"].startswith(self.client.default_redirect_uri), + response["Location"].startswith(self.client_code.default_redirect_uri), msg="Different redirect_uri returned", ) @@ -298,7 +330,7 @@ def test_unknown_redirect_uris_are_rejected(self): See http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest. """ data = { - "client_id": self.client.client_id, + "client_id": self.client_code.client_id, "response_type": "code", "redirect_uri": "http://neverseenthis.com", "scope": "openid email", @@ -316,9 +348,9 @@ def test_manipulated_redirect_uris_are_rejected(self): See http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest. """ data = { - "client_id": self.client.client_id, + "client_id": self.client_code.client_id, "response_type": "code", - "redirect_uri": self.client.default_redirect_uri + "?some=query", + "redirect_uri": self.client_code.default_redirect_uri + "?some=query", "scope": "openid email", "state": self.state, } @@ -352,9 +384,9 @@ def test_prompt_none_parameter(self): See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest """ data = { - "client_id": self.client.client_id, - "response_type": next(self.client.response_type_values()), - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, "prompt": "none", @@ -372,16 +404,16 @@ def test_prompt_none_parameter(self): self.assertIn("consent_required", response["Location"]) @patch("oidc_provider.views.django_user_logout") - def test_prompt_login_parameter(self, logout_function): + def test_prompt_login_parameter(self, logout_patched): """ Specifies whether the Authorization Server prompts the End-User for reauthentication and consent. See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest """ data = { - "client_id": self.client.client_id, - "response_type": next(self.client.response_type_values()), - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, "prompt": "login", @@ -392,18 +424,16 @@ def test_prompt_login_parameter(self, logout_function): self.assertNotIn( quote("prompt=login"), response["Location"], - "Found prompt=login, this leads to infinite login loop. See " - "https://github.com/juanifioren/django-oidc-provider/issues/197.", + "Found prompt=login, this leads to infinite login loop.", ) response = self._auth_request("get", data, is_user_authenticated=True) self.assertIn(settings.get("OIDC_LOGIN_URL"), response["Location"]) - logout_function.assert_called_once() + logout_patched.assert_called_once() self.assertNotIn( quote("prompt=login"), response["Location"], - "Found prompt=login, this leads to infinite login loop. See " - "https://github.com/juanifioren/django-oidc-provider/issues/197.", + "Found prompt=login, this leads to infinite login loop.", ) def test_prompt_login_none_parameter(self): @@ -413,9 +443,9 @@ def test_prompt_login_none_parameter(self): See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest """ data = { - "client_id": self.client.client_id, - "response_type": next(self.client.response_type_values()), - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, "prompt": "login none", @@ -435,9 +465,9 @@ def test_prompt_consent_parameter(self, render_patched): See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest """ data = { - "client_id": self.client.client_id, - "response_type": next(self.client.response_type_values()), - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, "prompt": "consent", @@ -457,9 +487,9 @@ def test_prompt_consent_none_parameter(self): See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest """ data = { - "client_id": self.client.client_id, - "response_type": next(self.client.response_type_values()), - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, "prompt": "consent none", @@ -471,6 +501,59 @@ def test_prompt_consent_none_parameter(self): response = self._auth_request("get", data, is_user_authenticated=True) self.assertIn("consent_required", response["Location"]) + @patch("oidc_provider.views.django_user_logout") + @freeze_time("2024-01-20 00:00:00", tz_offset=0, as_kwarg="frozen_time") + def test_max_age_should_re_authenticate_user(self, logout_patched, frozen_time): + """ + Authentication age is greater than the max_age value present in the + Authorization request, the OP MUST attempt to actively re-authenticate the End-User. + See: https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + """ + self.user.last_login = datetime.now() + self.user.save() + + frozen_time.move_to("2024-01-20 00:15:00") + + data = { + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, + "scope": "openid email", + "state": self.state, + "max_age": "600", + } + + response = self._auth_request("get", data, is_user_authenticated=True) + + self.assertIn(settings.get("OIDC_LOGIN_URL"), response["Location"]) + logout_patched.assert_called_once() + + @freeze_time("2024-01-20 00:00:00", tz_offset=0, as_kwarg="frozen_time") + @patch("oidc_provider.views.render") + @patch("oidc_provider.views.django_user_logout") + def test_max_age_should_not_re_authenticate_user( + self, logout_patched, render_patched, frozen_time + ): + self.user.last_login = datetime.now() + self.user.save() + + frozen_time.move_to("2024-01-20 00:08:00") + + data = { + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, + "scope": "openid email", + "state": self.state, + "max_age": "600", + } + + self._auth_request("get", data, is_user_authenticated=True) + + logout_patched.assert_not_called() + render_patched.assert_called_once() + self.assertTrue(render_patched.call_args[0][1], settings.get("OIDC_TEMPLATES")["authorize"]) + def test_strip_prompt_login(self): """ Test for helper method test_strip_prompt_login. @@ -510,7 +593,7 @@ def setUp(self): call_command("creatersakey") self.factory = RequestFactory() self.user = create_fake_user() - self.client = create_fake_client(response_type="id_token token") + self.client_code = create_fake_client(response_type="id_token token") self.client_public = create_fake_client(response_type="id_token token", is_public=True) self.client_public_no_consent = create_fake_client( response_type="id_token token", is_public=True, require_consent=False @@ -528,9 +611,9 @@ def test_missing_nonce(self): The `nonce` parameter is REQUIRED if you use the Implicit Flow. """ data = { - "client_id": self.client.client_id, - "response_type": next(self.client.response_type_values()), - "redirect_uri": self.client.default_redirect_uri, + "client_id": self.client_code.client_id, + "response_type": next(self.client_code.response_type_values()), + "redirect_uri": self.client_code.default_redirect_uri, "scope": "openid email", "state": self.state, } @@ -545,9 +628,9 @@ def test_idtoken_token_response(self): and access token as the result of the authorization request. """ data = { - "client_id": self.client.client_id, - "redirect_uri": self.client.default_redirect_uri, - "response_type": next(self.client.response_type_values()), + "client_id": self.client_code.client_id, + "redirect_uri": self.client_code.default_redirect_uri, + "response_type": next(self.client_code.response_type_values()), "scope": "openid email", "state": self.state, "nonce": self.nonce, @@ -605,9 +688,9 @@ def test_idtoken_token_at_hash(self): `at_hash` in `id_token`. """ data = { - "client_id": self.client.client_id, - "redirect_uri": self.client.default_redirect_uri, - "response_type": next(self.client.response_type_values()), + "client_id": self.client_code.client_id, + "redirect_uri": self.client_code.default_redirect_uri, + "response_type": next(self.client_code.response_type_values()), "scope": "openid email", "state": self.state, "nonce": self.nonce, diff --git a/oidc_provider/tests/cases/test_token_endpoint.py b/oidc_provider/tests/cases/test_token_endpoint.py index a32187df..ee60a0c7 100644 --- a/oidc_provider/tests/cases/test_token_endpoint.py +++ b/oidc_provider/tests/cases/test_token_endpoint.py @@ -33,6 +33,7 @@ from oidc_provider.tests.app.utils import FAKE_CODE_VERIFIER from oidc_provider.tests.app.utils import FAKE_NONCE from oidc_provider.tests.app.utils import FAKE_RANDOM_STRING +from oidc_provider.tests.app.utils import FAKE_USER_PASSWORD from oidc_provider.tests.app.utils import create_fake_client from oidc_provider.tests.app.utils import create_fake_user from oidc_provider.views import JwksView @@ -60,7 +61,7 @@ def setUp(self): def _password_grant_post_data(self, scope=None): result = { "username": "johndoe", - "password": "1234", + "password": FAKE_USER_PASSWORD, "grant_type": "password", "scope": TokenTestCase.SCOPE, } diff --git a/oidc_provider/views.py b/oidc_provider/views.py index 45798e59..a102ab6f 100644 --- a/oidc_provider/views.py +++ b/oidc_provider/views.py @@ -4,9 +4,14 @@ try: from urllib import urlencode - from urlparse import parse_qs, urlsplit, urlunsplit + from urlparse import parse_qs + from urlparse import urlsplit + from urlparse import urlunsplit except ImportError: - from urllib.parse import urlsplit, parse_qs, urlunsplit, urlencode + from urllib.parse import parse_qs + from urllib.parse import urlencode + from urllib.parse import urlsplit + from urllib.parse import urlunsplit from Cryptodome.PublicKey import RSA from django.contrib.auth.views import redirect_to_login @@ -19,40 +24,41 @@ from django.contrib.auth import logout as django_user_logout from django.core.cache import cache from django.db import transaction -from django.http import HttpResponse, JsonResponse +from django.http import HttpResponse +from django.http import JsonResponse from django.shortcuts import render from django.template.loader import render_to_string from django.utils.decorators import method_decorator from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods -from django.views.generic import TemplateView, View +from django.views.generic import TemplateView +from django.views.generic import View from jwkest import long_to_base64 -from oidc_provider import settings, signals +from oidc_provider import settings +from oidc_provider import signals from oidc_provider.compat import get_attr_or_callable from oidc_provider.lib.claims import StandardScopeClaims from oidc_provider.lib.endpoints.authorize import AuthorizeEndpoint from oidc_provider.lib.endpoints.introspection import TokenIntrospectionEndpoint from oidc_provider.lib.endpoints.token import TokenEndpoint -from oidc_provider.lib.errors import ( - AuthorizeError, - ClientIdError, - RedirectUriError, - TokenError, - TokenIntrospectionError, - UserAuthError, -) +from oidc_provider.lib.errors import AuthorizeError +from oidc_provider.lib.errors import ClientIdError +from oidc_provider.lib.errors import RedirectUriError +from oidc_provider.lib.errors import TokenError +from oidc_provider.lib.errors import TokenIntrospectionError +from oidc_provider.lib.errors import UserAuthError from oidc_provider.lib.utils.authorize import strip_prompt_login -from oidc_provider.lib.utils.common import ( - cors_allow_any, - get_issuer, - get_site_url, - redirect, -) +from oidc_provider.lib.utils.common import cors_allow_any +from oidc_provider.lib.utils.common import get_issuer +from oidc_provider.lib.utils.common import get_site_url +from oidc_provider.lib.utils.common import redirect from oidc_provider.lib.utils.oauth2 import protected_resource_view from oidc_provider.lib.utils.token import client_id_from_id_token -from oidc_provider.models import Client, ResponseType, RSAKey +from oidc_provider.models import Client +from oidc_provider.models import ResponseType +from oidc_provider.models import RSAKey logger = logging.getLogger(__name__) @@ -106,6 +112,12 @@ def get(self, request, *args, **kwargs): authorize.params["redirect_uri"], "consent_required", authorize.grant_type ) + if authorize.is_authentication_age_is_greater_than_max_age(): + django_user_logout(request) + return redirect_to_login( + request.get_full_path(), settings.get("OIDC_LOGIN_URL") + ) + if not authorize.client.require_consent and ( authorize.is_client_allowed_to_skip_consent() and "consent" not in authorize.params["prompt"] @@ -297,6 +309,8 @@ def _build_response_dict(self, request): dic["token_endpoint_auth_methods_supported"] = ["client_secret_post", "client_secret_basic"] + dic["request_parameter_supported"] = False + if settings.get("OIDC_SESSION_MANAGEMENT_ENABLE"): dic["check_session_iframe"] = site_url + reverse("oidc_provider:check-session-iframe") diff --git a/tox.ini b/tox.ini index 4710a1ba..26091bc5 100644 --- a/tox.ini +++ b/tox.ini @@ -11,17 +11,18 @@ envlist= changedir= oidc_provider deps = - psycopg2-binary - pytest - pytest-django - pytest-flake8 - pytest-cov django32: django>=3.2,<3.3 django40: django>=4.0,<4.1 django41: django>=4.1,<4.2 django42: django>=4.2,<4.3 django50: django>=5.0,<5.1 django51: django>=5.1,<5.2 + freezegun + psycopg2-binary + pytest + pytest-django + pytest-flake8 + pytest-cov commands = pytest --cov=oidc_provider {posargs}