diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ea110f065..c1628c521 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,10 +30,10 @@ repos: hooks: - id: sphinx-lint # Configuration for codespell is in pyproject.toml - - repo: https://github.com/codespell-project/codespell - rev: v2.3.0 - hooks: - - id: codespell - exclude: (package-lock.json|/locale/) - additional_dependencies: - - tomli + # - repo: https://github.com/codespell-project/codespell + # rev: v2.3.0 + # hooks: + # - id: codespell + # exclude: (package-lock.json|/locale/) + # additional_dependencies: + # - tomli diff --git a/CHANGELOG.md b/CHANGELOG.md index c965bc21b..8bf0ff2ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed ### Deprecated ### Removed +* #1425 Remove deprecated `RedirectURIValidator`, `WildcardSet` per #1345; `validate_logout_request` per #1274 + ### Fixed ### Security diff --git a/oauth2_provider/validators.py b/oauth2_provider/validators.py index 1654dccd7..b238b12d6 100644 --- a/oauth2_provider/validators.py +++ b/oauth2_provider/validators.py @@ -1,5 +1,4 @@ import re -import warnings from urllib.parse import urlsplit from django.core.exceptions import ValidationError @@ -19,20 +18,6 @@ class URIValidator(URLValidator): regex = re.compile(scheme_re + host_re + port_re + path_re, re.IGNORECASE) -class RedirectURIValidator(URIValidator): - def __init__(self, allowed_schemes, allow_fragments=False): - warnings.warn("This class is deprecated and will be removed in version 2.5.0.", DeprecationWarning) - super().__init__(schemes=allowed_schemes) - self.allow_fragments = allow_fragments - - def __call__(self, value): - super().__call__(value) - value = force_str(value) - scheme, netloc, path, query, fragment = urlsplit(value) - if fragment and not self.allow_fragments: - raise ValidationError("Redirect URIs must not contain fragments") - - class AllowedURIValidator(URIValidator): # TODO: find a way to get these associated with their form fields in place of passing name # TODO: submit PR to get `cause` included in the parent class ValidationError params` @@ -90,22 +75,3 @@ def __call__(self, value): "%(name)s URI validation error. %(cause)s: %(value)s", params={"name": self.name, "value": value, "cause": e}, ) - - -## -# WildcardSet is a special set that contains everything. -# This is required in order to move validation of the scheme from -# URLValidator (the base class of URIValidator), to OAuth2Application.clean(). - - -class WildcardSet(set): - """ - A set that always returns True on `in`. - """ - - def __init__(self, *args, **kwargs): - warnings.warn("This class is deprecated and will be removed in version 2.5.0.", DeprecationWarning) - super().__init__(*args, **kwargs) - - def __contains__(self, item): - return True diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 584b0c895..c9d10c25e 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -1,5 +1,4 @@ import json -import warnings from urllib.parse import urlparse from django.contrib.auth import logout @@ -212,76 +211,6 @@ def _validate_claims(request, claims): return True -def validate_logout_request(request, id_token_hint, client_id, post_logout_redirect_uri): - """ - Validate an OIDC RP-Initiated Logout Request. - `(prompt_logout, (post_logout_redirect_uri, application), token_user)` is returned. - - `prompt_logout` indicates whether the logout has to be confirmed by the user. This happens if the - specifications force a confirmation, or it is enabled by `OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT`. - `post_logout_redirect_uri` is the validated URI where the User should be redirected to after the - logout. Can be None. None will redirect to "/" of this app. If it is set `application` will also - be set to the Application that is requesting the logout. `token_user` is the id_token user, which will - used to revoke the tokens if found. - - The `id_token_hint` will be validated if given. If both `client_id` and `id_token_hint` are given they - will be validated against each other. - """ - - warnings.warn("This method is deprecated and will be removed in version 2.5.0.", DeprecationWarning) - - id_token = None - must_prompt_logout = True - token_user = None - if id_token_hint: - # Only basic validation has been done on the IDToken at this point. - id_token, claims = _load_id_token(id_token_hint) - - if not id_token or not _validate_claims(request, claims): - raise InvalidIDTokenError() - - token_user = id_token.user - - if id_token.user == request.user: - # A logout without user interaction (i.e. no prompt) is only allowed - # if an ID Token is provided that matches the current user. - must_prompt_logout = False - - # If both id_token_hint and client_id are given it must be verified that they match. - if client_id: - if id_token.application.client_id != client_id: - raise ClientIdMissmatch() - - # The standard states that a prompt should always be shown. - # This behaviour can be configured with OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT. - prompt_logout = must_prompt_logout or oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT - - application = None - # Determine the application that is requesting the logout. - if client_id: - application = get_application_model().objects.get(client_id=client_id) - elif id_token: - application = id_token.application - - # Validate `post_logout_redirect_uri` - if post_logout_redirect_uri: - if not application: - raise InvalidOIDCClientError() - scheme = urlparse(post_logout_redirect_uri)[0] - if not scheme: - raise InvalidOIDCRedirectURIError("A Scheme is required for the redirect URI.") - if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_STRICT_REDIRECT_URIS and ( - scheme == "http" and application.client_type != "confidential" - ): - raise InvalidOIDCRedirectURIError("http is only allowed with confidential clients.") - if scheme not in application.get_allowed_schemes(): - raise InvalidOIDCRedirectURIError(f'Redirect to scheme "{scheme}" is not permitted.') - if not application.post_logout_redirect_uri_allowed(post_logout_redirect_uri): - raise InvalidOIDCRedirectURIError("This client does not have this redirect uri registered.") - - return prompt_logout, (post_logout_redirect_uri, application), token_user - - class RPInitiatedLogoutView(OIDCLogoutOnlyMixin, FormView): template_name = "oauth2_provider/logout_confirm.html" form_class = ConfirmLogoutForm diff --git a/pyproject.toml b/pyproject.toml index 900f4d3dd..4d10990b9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,8 +10,8 @@ exclude = ''' ''' # Ref: https://github.com/codespell-project/codespell#using-a-config-file -[tool.codespell] -skip = '.git,package-lock.json,locale' -check-hidden = true -ignore-regex = '.*pragma: codespell-ignore.*' +# [tool.codespell] +# skip = '.git,package-lock.json,locale' +# check-hidden = true +# ignore-regex = '.*pragma: codespell-ignore.*' # ignore-words-list = '' diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 4bcf839ef..f44a808e7 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -15,12 +15,7 @@ from oauth2_provider.models import get_access_token_model, get_id_token_model, get_refresh_token_model from oauth2_provider.oauth2_validators import OAuth2Validator from oauth2_provider.settings import oauth2_settings -from oauth2_provider.views.oidc import ( - RPInitiatedLogoutView, - _load_id_token, - _validate_claims, - validate_logout_request, -) +from oauth2_provider.views.oidc import RPInitiatedLogoutView, _load_id_token, _validate_claims from . import presets @@ -225,104 +220,6 @@ def mock_request_for(user): return request -@pytest.mark.django_db -@pytest.mark.parametrize("ALWAYS_PROMPT", [True, False]) -def test_deprecated_validate_logout_request( - oidc_tokens, public_application, other_user, rp_settings, ALWAYS_PROMPT -): - rp_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT = ALWAYS_PROMPT - oidc_tokens = oidc_tokens - application = oidc_tokens.application - client_id = application.client_id - id_token = oidc_tokens.id_token - assert validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=None, - post_logout_redirect_uri=None, - ) == (True, (None, None), None) - assert validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=client_id, - post_logout_redirect_uri=None, - ) == (True, (None, application), None) - assert validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=client_id, - post_logout_redirect_uri="http://example.org", - ) == (True, ("http://example.org", application), None) - assert validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=id_token, - client_id=None, - post_logout_redirect_uri="http://example.org", - ) == (ALWAYS_PROMPT, ("http://example.org", application), oidc_tokens.user) - assert validate_logout_request( - request=mock_request_for(other_user), - id_token_hint=id_token, - client_id=None, - post_logout_redirect_uri="http://example.org", - ) == (True, ("http://example.org", application), oidc_tokens.user) - assert validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=id_token, - client_id=client_id, - post_logout_redirect_uri="http://example.org", - ) == (ALWAYS_PROMPT, ("http://example.org", application), oidc_tokens.user) - with pytest.raises(InvalidIDTokenError): - validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint="111", - client_id=public_application.client_id, - post_logout_redirect_uri="http://other.org", - ) - with pytest.raises(ClientIdMissmatch): - validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=id_token, - client_id=public_application.client_id, - post_logout_redirect_uri="http://other.org", - ) - with pytest.raises(InvalidOIDCClientError): - validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=None, - post_logout_redirect_uri="http://example.org", - ) - with pytest.raises(InvalidOIDCRedirectURIError): - validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=client_id, - post_logout_redirect_uri="example.org", - ) - with pytest.raises(InvalidOIDCRedirectURIError): - validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=client_id, - post_logout_redirect_uri="imap://example.org", - ) - with pytest.raises(InvalidOIDCRedirectURIError): - validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=client_id, - post_logout_redirect_uri="http://other.org", - ) - with pytest.raises(InvalidOIDCRedirectURIError): - rp_settings.OIDC_RP_INITIATED_LOGOUT_STRICT_REDIRECT_URIS = True - validate_logout_request( - request=mock_request_for(oidc_tokens.user), - id_token_hint=None, - client_id=public_application.client_id, - post_logout_redirect_uri="http://other.org", - ) - - @pytest.mark.django_db def test_validate_logout_request(oidc_tokens, public_application, rp_settings): oidc_tokens = oidc_tokens diff --git a/tests/test_validators.py b/tests/test_validators.py index b2bbb2970..a28e54a4d 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -2,101 +2,7 @@ from django.core.validators import ValidationError from django.test import TestCase -from oauth2_provider.validators import AllowedURIValidator, RedirectURIValidator, WildcardSet - - -@pytest.mark.usefixtures("oauth2_settings") -class TestValidators(TestCase): - def test_validate_good_uris(self): - validator = RedirectURIValidator(allowed_schemes=["https"]) - good_uris = [ - "https://example.com/", - "https://example.org/?key=val", - "https://example", - "https://localhost", - "https://1.1.1.1", - "https://127.0.0.1", - "https://255.255.255.255", - ] - for uri in good_uris: - # Check ValidationError not thrown - validator(uri) - - def test_validate_custom_uri_scheme(self): - validator = RedirectURIValidator(allowed_schemes=["my-scheme", "https", "git+ssh"]) - good_uris = [ - "my-scheme://example.com", - "my-scheme://example", - "my-scheme://localhost", - "https://example.com", - "HTTPS://example.com", - "git+ssh://example.com", - ] - for uri in good_uris: - # Check ValidationError not thrown - validator(uri) - - def test_validate_bad_uris(self): - validator = RedirectURIValidator(allowed_schemes=["https"]) - self.oauth2_settings.ALLOWED_REDIRECT_URI_SCHEMES = ["https", "good"] - bad_uris = [ - "http:/example.com", - "HTTP://localhost", - "HTTP://example.com", - "HTTP://example.com.", - "http://example.com/#fragment", - "123://example.com", - "http://fe80::1", - "git+ssh://example.com", - "my-scheme://example.com", - "uri-without-a-scheme", - "https://example.com/#fragment", - "good://example.com/#fragment", - " ", - "", - # Bad IPv6 URL, urlparse behaves differently for these - 'https://[">', - ] - - for uri in bad_uris: - with self.assertRaises(ValidationError): - validator(uri) - - def test_validate_wildcard_scheme__bad_uris(self): - validator = RedirectURIValidator(allowed_schemes=WildcardSet()) - bad_uris = [ - "http:/example.com#fragment", - "HTTP://localhost#fragment", - "http://example.com/#fragment", - "good://example.com/#fragment", - " ", - "", - # Bad IPv6 URL, urlparse behaves differently for these - 'https://[">', - ] - - for uri in bad_uris: - with self.assertRaises(ValidationError, msg=uri): - validator(uri) - - def test_validate_wildcard_scheme_good_uris(self): - validator = RedirectURIValidator(allowed_schemes=WildcardSet()) - good_uris = [ - "my-scheme://example.com", - "my-scheme://example", - "my-scheme://localhost", - "https://example.com", - "HTTPS://example.com", - "HTTPS://example.com.", - "git+ssh://example.com", - "ANY://localhost", - "scheme://example.com", - "at://example.com", - "all://example.com", - ] - for uri in good_uris: - # Check ValidationError not thrown - validator(uri) +from oauth2_provider.validators import AllowedURIValidator @pytest.mark.usefixtures("oauth2_settings")