Allow loopback redirect URIs using ports as described in RFC8252 (#953)

* Allow loopback redirect URIs using ports as described in RFC8252

* Update Changelog and Authors

* Docs update and adjustment for explicit port config on loopback

* Wrap and clarify Changelog

* Clarify documentation

* Split out redirect uri logic for easier testing

 This adds some unit tests for loopback IP code in particular, as part
 of reviewing the change

Co-authored-by: Raphael Gaschignard <[email protected]>
Co-authored-by: Asif Saif Uddin <[email protected]>
Co-authored-by: Raphael Gaschignard <[email protected]>

Author: 4 people

AUTHORS

@@ -36,6 +36,7 @@ Jonas Nygaard Pedersen
 Jonathan Steffan
 Jun Zhou
 Kristian Rune Larsen
+Paul Dekkers
 Paul Oswald
 Pavel Tvrdík
 Rodney Richardson

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
 

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.
 

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

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)