aliev / aioauth

Asynchronous OAuth 2.0 provider for Python 3
https://aliev.me/aioauth
MIT License
214 stars 19 forks source link

Add state parameter to authorization error redirects #94

Closed shawnz closed 3 months ago

shawnz commented 3 months ago

The spec states that when redirecting back to the client after an authorization request error, the state parameter should be included if it was present in the request:

state REQUIRED if a "state" parameter was present in the client authorization request. The exact value received from the client.

https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2.1

This adds the state parameter to redirect responses for authorization request errors. Additionally this modifies the check_query_values test utility function to inject the state into the expected redirect URI, so that the behaviour is tested.

Before the change:

FAILED tests/test_flow.py::test_authorization_code_flow - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=fs0DxDPJbC'}) != Response(content={}, status_code...
FAILED tests/test_flow.py::test_authorization_code_flow_credentials_in_post - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=LJ8oByKgl0'}) != Response(con...
FAILED tests/test_flow.py::test_multiple_response_types - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=Up4ghYWkwM'}) != Response(content={}, status_code...
FAILED tests/test_flow.py::test_response_type_none - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=sxQqEiMdU3'}) != Response(content={}, status_code=<HTT...
FAILED tests/test_flow.py::test_response_type_id_token[query] - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=4k3ccmIZEJ'}) != Response(content={}, statu...
FAILED tests/test_flow.py::test_response_type_id_token[form_post] - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=9vdYmr2n1o'}) != Response(content={}, s...
FAILED tests/test_flow.py::test_response_type_id_token[fragment] - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=hxiKrAuT9z'}) != Response(content={}, st...
FAILED tests/test_flow.py::test_response_type_id_token[None] - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20parameter.&state=t4CM6WShmm'}) != Response(content={}, status...
FAILED tests/oidc/core/test_flow.py::test_authorization_endpoint_allows_prompt_query_param[username-HTTPStatus.FOUND] - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20para...
FAILED tests/oidc/core/test_flow.py::test_authorization_endpoint_allows_prompt_query_param[None-HTTPStatus.UNAUTHORIZED] - AssertionError: Response(content={}, status_code=<HTTPStatus.FOUND: 302>, headers={'location': 'https://ownauth.com/callback?error=invalid_request&error_description=Missing%20response_type%20p...

Results (0.85s):
      33 passed
      10 failed
         - tests/test_flow.py:342 test_authorization_code_flow
         - tests/test_flow.py:400 test_authorization_code_flow_credentials_in_post
         - tests/test_flow.py:502 test_multiple_response_types
         - tests/test_flow.py:543 test_response_type_none
         - tests/test_flow.py:578 test_response_type_id_token[query]
         - tests/test_flow.py:578 test_response_type_id_token[form_post]
         - tests/test_flow.py:578 test_response_type_id_token[fragment]
         - tests/test_flow.py:578 test_response_type_id_token[None]
         - tests/oidc/core/test_flow.py:14 test_authorization_endpoint_allows_prompt_query_param[username-HTTPStatus.FOUND]
         - tests/oidc/core/test_flow.py:14 test_authorization_endpoint_allows_prompt_query_param[None-HTTPStatus.UNAUTHORIZED]

After the change:

Results (0.70s):
      43 passed

I think the way I implemented the tests for this change is a little bit hackish. Let me know what you think and if you have any better ideas how it could be implemented. Thanks, Shawn