Add localhost to allowed loopback addresses #1423
Conversation
| @@ -205,9 +205,8 @@ def test_validate_authorization_request_unsafe_query(self): | |||
|
|
|||
| @pytest.mark.parametrize( | |||
| "uri, expected_result", | |||
| # localhost is _not_ a loopback URI | |||
There was a problem hiding this comment.
Ugh I'm sorry I put you through creating this PR, but according to RFC8252:
While redirect URIs using localhost (i.e.,
"http://localhost:{port}/{path}") function similarly to loopback IP
redirects described in Section 7.3, the use of localhost is NOT
RECOMMENDED. Specifying a redirect URI with the loopback IP literal
rather than localhost avoids inadvertently listening on network
interfaces other than the loopback interface. It is also less
susceptible to client-side firewalls and misconfigured host name
resolution on the user's device.
Given this, I think the default should remain as is, perhaps with a setting added to allow one to override the default to allow localhost.
There was a problem hiding this comment.
Ugh I'm sorry I put you through creating this PR
Oh it's fine, no worries at all. Thanks for taking the time looking at it!
| @@ -56,7 +56,7 @@ 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 | |||
| Application (``[::1]`` or ``127.0.0.1`` or ``localhost``). In this case the ``redirect_uri`` can be | |||
There was a problem hiding this comment.
Per comment referencing RFC8252, please change this to a setting with the default as is without localhost.
| @@ -778,7 +778,7 @@ def redirect_to_uri_allowed(uri, allowed_uris): | |||
|
|
|||
| allowed_uri_is_loopback = ( | |||
| parsed_allowed_uri.scheme == "http" | |||
| and parsed_allowed_uri.hostname in ["127.0.0.1", "::1"] | |||
| and parsed_allowed_uri.hostname in ["127.0.0.1", "::1", "localhost"] | |||
There was a problem hiding this comment.
Please add a setting along these lines"
| and parsed_allowed_uri.hostname in ["127.0.0.1", "::1", "localhost"] | |
| and parsed_allowed_uri.hostname in settings.LOOPBACK_URIS |
| @@ -8,7 +8,7 @@ | |||
| @pytest.mark.usefixtures("oauth2_settings") | |||
| class TestValidators(TestCase): | |||
| def test_validate_good_uris(self): | |||
| validator = RedirectURIValidator(allowed_schemes=["https"]) | |||
| validator = RedirectURIValidator(allowed_schemes=["https", "http"]) | |||
There was a problem hiding this comment.
Not sure why you changed this to add http.
Fixes #1416
Description
This pull request adds
localhostto the list of allowed loopback addresses for redirect URIs in thedjango-oauth-toolkitlibrary.Changes Made
localhostas a valid loopback address.localhostas a valid URI.localhostentries.Rationale
This change improves usability by allowing the use of
localhostin development environments, aligning with common practices.Related Issue
Add localhost to Allowed Loopback Addresses for Redirect URIs