diff --git a/AUTHORS b/AUTHORS index d8973f29f..37f2cbdde 100644 --- a/AUTHORS +++ b/AUTHORS @@ -36,6 +36,7 @@ Jonas Nygaard Pedersen Jonathan Steffan Jun Zhou Kristian Rune Larsen +Paul Dekkers Paul Oswald Pavel TvrdĂ­k Rodney Richardson diff --git a/CHANGELOG.md b/CHANGELOG.md index c28031a26..af3fdae3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * #524 Restrict usage of timezone aware expire dates to Django projects with USE_TZ set to True. * #955 Avoid doubling of `oauth2_provider` urls mountpath in json response for OIDC view `ConnectDiscoveryInfoView`. Breaks existing OIDC discovery output +* #953 Allow loopback redirect URIs with random ports using http scheme, localhost address and no explicit port + configuration in the allowed redirect_uris for Oauth2 Applications (RFC8252) ## [1.5.0] 2021-03-18 diff --git a/docs/settings.rst b/docs/settings.rst index 67ea7b37a..de7bcf85c 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -52,6 +52,11 @@ Default: ``["http", "https"]`` A list of schemes that the ``redirect_uri`` field will be validated against. Setting this to ``["https"]`` only in production is strongly recommended. +For Native Apps the ``http`` scheme can be safely used with loopback addresses in the +Application (``[::1]`` or ``127.0.0.1``). In this case the ``redirect_uri`` can be +configured without explicit port specification, so that the Application accepts randomly +assigned ports. + Note that you may override ``Application.get_allowed_schemes()`` to set this on a per-application basis. diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index a21cb868b..aa10eca16 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -125,23 +125,7 @@ def redirect_uri_allowed(self, uri): :param uri: Url to check """ - parsed_uri = urlparse(uri) - uqs_set = set(parse_qsl(parsed_uri.query)) - for allowed_uri in self.redirect_uris.split(): - parsed_allowed_uri = urlparse(allowed_uri) - - if ( - parsed_allowed_uri.scheme == parsed_uri.scheme - and parsed_allowed_uri.netloc == parsed_uri.netloc - and parsed_allowed_uri.path == parsed_uri.path - ): - - aqs_set = set(parse_qsl(parsed_allowed_uri.query)) - - if aqs_set.issubset(uqs_set): - return True - - return False + return redirect_to_uri_allowed(uri, self.redirect_uris.split()) def clean(self): from django.core.exceptions import ValidationError @@ -674,3 +658,50 @@ def clear_expired(): access_tokens.delete() grants.delete() + + +def redirect_to_uri_allowed(uri, allowed_uris): + """ + Checks if a given uri can be redirected to based on the provided allowed_uris configuration. + + On top of exact matches, this function also handles loopback IPs based on RFC 8252. + + :param uri: URI to check + :param allowed_uris: A list of URIs that are allowed + """ + + parsed_uri = urlparse(uri) + uqs_set = set(parse_qsl(parsed_uri.query)) + for allowed_uri in allowed_uris: + parsed_allowed_uri = urlparse(allowed_uri) + + # From RFC 8252 (Section 7.3) + # + # Loopback redirect URIs use the "http" scheme + # [...] + # The authorization server MUST allow any port to be specified at the + # time of the request for loopback IP redirect URIs, to accommodate + # clients that obtain an available ephemeral port from the operating + # system at the time of the request. + + allowed_uri_is_loopback = ( + parsed_allowed_uri.scheme == "http" + and parsed_allowed_uri.hostname in ["127.0.0.1", "::1"] + and parsed_allowed_uri.port is None + ) + if ( + allowed_uri_is_loopback + and parsed_allowed_uri.scheme == parsed_uri.scheme + and parsed_allowed_uri.hostname == parsed_uri.hostname + and parsed_allowed_uri.path == parsed_uri.path + ) or ( + parsed_allowed_uri.scheme == parsed_uri.scheme + and parsed_allowed_uri.netloc == parsed_uri.netloc + and parsed_allowed_uri.path == parsed_uri.path + ): + + aqs_set = set(parse_qsl(parsed_allowed_uri.query)) + if aqs_set.issubset(uqs_set): + return True + + return False diff --git a/tests/test_oauth2_backends.py b/tests/test_oauth2_backends.py index 860cbb461..acff2cae9 100644 --- a/tests/test_oauth2_backends.py +++ b/tests/test_oauth2_backends.py @@ -4,6 +4,7 @@ from django.test import RequestFactory, TestCase from oauth2_provider.backends import get_oauthlib_core +from oauth2_provider.models import redirect_to_uri_allowed from oauth2_provider.oauth2_backends import JSONOAuthLibCore, OAuthLibCore @@ -110,3 +111,23 @@ def test_validate_authorization_request_unsafe_query(self): oauthlib_core = get_oauthlib_core() oauthlib_core.verify_request(request, scopes=[]) + + +@pytest.mark.parametrize( + "uri, expected_result", + # localhost is _not_ a loopback URI + [ + ("http://localhost:3456", False), + # only http scheme is supported for loopback URIs + ("https://127.0.0.1:3456", False), + ("http://127.0.0.1:3456", True), + ("http://[::1]", True), + ("http://[::1]:34", True), + ], +) +def test_uri_loopback_redirect_check(uri, expected_result): + allowed_uris = ["http://127.0.0.1", "http://[::1]"] + if expected_result: + assert redirect_to_uri_allowed(uri, allowed_uris) + else: + assert not redirect_to_uri_allowed(uri, allowed_uris)