From 4711dc241c9551f5d6b1ed83a8ca75d492203b4c Mon Sep 17 00:00:00 2001 From: Robbe Sneyders Date: Thu, 2 Nov 2023 22:55:59 +0100 Subject: [PATCH] Fail on invalid token when checking multiple OR security schemes --- connexion/security.py | 54 ++++--------------------------- docs/security.rst | 23 +++++++++++-- tests/api/test_secure_api.py | 4 +-- tests/decorators/test_security.py | 29 ----------------- tests/fakeapi/hello/__init__.py | 5 ++- 5 files changed, 31 insertions(+), 84 deletions(-) diff --git a/connexion/security.py b/connexion/security.py index ddfd14b16..250b459e9 100644 --- a/connexion/security.py +++ b/connexion/security.py @@ -558,24 +558,15 @@ async def wrapper(request): @classmethod def verify_security(cls, auth_funcs): async def verify_fn(request): - token_info = NO_VALUE - errors = [] for func in auth_funcs: - try: - token_info = func(request) - while asyncio.iscoroutine(token_info): - token_info = await token_info - if token_info is not NO_VALUE: - break - except Exception as err: - errors.append(err) - + token_info = func(request) + while asyncio.iscoroutine(token_info): + token_info = await token_info + if token_info is not NO_VALUE: + break else: - if errors != []: - cls._raise_most_specific(errors) - else: - logger.info("... No auth provided. Aborting with 401.") - raise OAuthProblem(detail="No authorization token provided") + logger.info("... No auth provided. Aborting with 401.") + raise OAuthProblem(detail="No authorization token provided") request.context.update( { @@ -586,34 +577,3 @@ async def verify_fn(request): ) return verify_fn - - @staticmethod - def _raise_most_specific(exceptions: t.List[Exception]) -> None: - """Raises the most specific error from a list of exceptions by status code. - - The status codes are expected to be either in the `code` - or in the `status` attribute of the exceptions. - - The order is as follows: - - 403: valid credentials but not enough privileges - - 401: no or invalid credentials - - for other status codes, the smallest one is selected - - :param errors: List of exceptions. - :type errors: t.List[Exception] - """ - if not exceptions: - return - # We only use status code attributes from exceptions - # We use 600 as default because 599 is highest valid status code - status_to_exc = { - getattr(exc, "status_code", getattr(exc, "status", 600)): exc - for exc in exceptions - } - if 403 in status_to_exc: - raise status_to_exc[403] - elif 401 in status_to_exc: - raise status_to_exc[401] - else: - lowest_status_code = min(status_to_exc) - raise status_to_exc[lowest_status_code] diff --git a/docs/security.rst b/docs/security.rst index fac723a1d..d140d0d65 100644 --- a/docs/security.rst +++ b/docs/security.rst @@ -152,13 +152,30 @@ Multiple Authentication Schemes ------------------------------- With Connexion, it is also possible to combine multiple authentication schemes -as described in the `OpenAPI specification`_. When multiple authentication -schemes are combined using logical AND, the ``token_info`` argument will -consist of a dictionary mapping the names of the security scheme to their +as described in the `OpenAPI specification`_. + +AND +``` + +When multiple authentication schemes are combined using logical AND, the ``token_info`` argument +will consist of a dictionary mapping the names of the security scheme to their corresponding ``token_info``. Multiple OAuth2 security schemes in AND fashion are not supported. +OR +`` + +When multiple authentication schemes are combined using logical OR, the schemes are checked in +the order they are listed in the OpenAPI spec. For each security scheme the following logic is +applied: + +- If a token matching the security schem is found in the request and is valid, allow the request + to pass +- If a token matching the security scheme is found in the request and is invalid, return an error +- If no token matching the security scheme is found in the request, move on to the next scheme, + or return a 401 error if this was the last scheme + .. _OpenAPI specification: https://swagger.io/docs/specification/authentication/#multiple Custom security handlers diff --git a/tests/api/test_secure_api.py b/tests/api/test_secure_api.py index cf547965f..9b1b385e2 100644 --- a/tests/api/test_secure_api.py +++ b/tests/api/test_secure_api.py @@ -162,8 +162,8 @@ def test_security(oauth_requests, secure_endpoint_app): assert response.text == '"Authenticated"\n' headers = {"X-AUTH": "wrong-key"} response = app_client.get("/v1.0/optional-auth", headers=headers) - assert response.text == '"Unauthenticated"\n' - assert response.status_code == 200 + assert response.json()["title"] == "Unauthorized" + assert response.status_code == 401 # security function throws exception response = app_client.get("/v1.0/auth-exception", headers={"X-Api-Key": "foo"}) diff --git a/tests/decorators/test_security.py b/tests/decorators/test_security.py index abb88eb01..3d7796be8 100644 --- a/tests/decorators/test_security.py +++ b/tests/decorators/test_security.py @@ -299,32 +299,3 @@ async def test_verify_security_oauthproblem(): assert exc_info.value.status_code == 401 assert exc_info.value.detail == "No authorization token provided" - - -@pytest.mark.parametrize( - "errors, most_specific", - [ - ([OAuthProblem()], OAuthProblem), - ([OAuthProblem(), OAuthScopeProblem([], [])], OAuthScopeProblem), - ( - [OAuthProblem(), OAuthScopeProblem([], []), BadRequestProblem], - OAuthScopeProblem, - ), - ( - [ - OAuthProblem(), - OAuthScopeProblem([], []), - BadRequestProblem, - ConnexionException, - ], - OAuthScopeProblem, - ), - ([BadRequestProblem(), ConnexionException()], BadRequestProblem), - ([ConnexionException()], ConnexionException), - ], -) -def test_raise_most_specific(errors, most_specific): - """Tests whether most specific exception is raised from a list.""" - security_handler_factory = SecurityHandlerFactory() - with pytest.raises(most_specific): - security_handler_factory._raise_most_specific(errors) diff --git a/tests/fakeapi/hello/__init__.py b/tests/fakeapi/hello/__init__.py index 1921ac930..3e342f4b8 100644 --- a/tests/fakeapi/hello/__init__.py +++ b/tests/fakeapi/hello/__init__.py @@ -526,9 +526,8 @@ def more_than_one_scope_defined(**kwargs): return "OK" -def optional_auth(**kwargs): - key = apikey_info(request.headers.get("X-AUTH")) - if key is None: +def optional_auth(context_): + if not context_.get("token_info"): return "Unauthenticated" else: return "Authenticated"