From 9438179f18145d69a140820dba0e2ebadb5442c5 Mon Sep 17 00:00:00 2001 From: Ruwan Date: Sun, 20 Feb 2022 01:39:49 +0100 Subject: [PATCH 01/14] WIP: rework required_scopes checking --- connexion/operations/secure.py | 8 ++-- .../async_security_handler_factory.py | 4 +- .../security/security_handler_factory.py | 45 ++++++++++++------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/connexion/operations/secure.py b/connexion/operations/secure.py index bc16cb914..5dbd90d91 100644 --- a/connexion/operations/secure.py +++ b/connexion/operations/secure.py @@ -75,7 +75,6 @@ def security_decorator(self): return self._api.security_handler_factory.security_passthrough auth_funcs = [] - required_scopes = None for security_req in self.security: if not security_req: auth_funcs.append(self._api.security_handler_factory.verify_none()) @@ -83,7 +82,7 @@ def security_decorator(self): sec_req_funcs = {} oauth = False - for scheme_name, scopes in security_req.items(): + for scheme_name, required_scopes in security_req.items(): security_scheme = self.security_schemes[scheme_name] if security_scheme['type'] == 'oauth2': @@ -91,7 +90,6 @@ def security_decorator(self): logger.warning("... multiple OAuth2 security schemes in AND fashion not supported", extra=vars(self)) break oauth = True - required_scopes = scopes token_info_func = self._api.security_handler_factory.get_tokeninfo_func(security_scheme) scope_validate_func = self._api.security_handler_factory.get_scope_validate_func(security_scheme) if not token_info_func: @@ -99,7 +97,7 @@ def security_decorator(self): break sec_req_funcs[scheme_name] = self._api.security_handler_factory.verify_oauth( - token_info_func, scope_validate_func) + token_info_func, scope_validate_func, required_scopes) # Swagger 2.0 elif security_scheme['type'] == 'basic': @@ -159,7 +157,7 @@ def security_decorator(self): else: auth_funcs.append(self._api.security_handler_factory.verify_multiple_schemes(sec_req_funcs)) - return functools.partial(self._api.security_handler_factory.verify_security, auth_funcs, required_scopes) + return functools.partial(self._api.security_handler_factory.verify_security, auth_funcs) def get_mimetype(self): return DEFAULT_MIMETYPE diff --git a/connexion/security/async_security_handler_factory.py b/connexion/security/async_security_handler_factory.py index c7d707d06..ca9f23d6f 100644 --- a/connexion/security/async_security_handler_factory.py +++ b/connexion/security/async_security_handler_factory.py @@ -62,12 +62,12 @@ async def wrapper(request, token, required_scopes): return wrapper @classmethod - def verify_security(cls, auth_funcs, required_scopes, function): + def verify_security(cls, auth_funcs, function): @functools.wraps(function) async def wrapper(request): token_info = cls.no_value for func in auth_funcs: - token_info = func(request, required_scopes) + token_info = func(request) while asyncio.iscoroutine(token_info): token_info = await token_info if token_info is not cls.no_value: diff --git a/connexion/security/security_handler_factory.py b/connexion/security/security_handler_factory.py index a6504c3b3..9323c4ef8 100644 --- a/connexion/security/security_handler_factory.py +++ b/connexion/security/security_handler_factory.py @@ -173,10 +173,10 @@ def get_auth_header_value(request): raise OAuthProblem(description='Invalid authorization header') return auth_type.lower(), value - def verify_oauth(self, token_info_func, scope_validate_func): + def verify_oauth(self, token_info_func, scope_validate_func, required_scopes): check_oauth_func = self.check_oauth_func(token_info_func, scope_validate_func) - def wrapper(request, required_scopes): + def wrapper(request): auth_type, token = self.get_auth_header_value(request) if auth_type != 'bearer': return self.no_value @@ -188,7 +188,7 @@ def wrapper(request, required_scopes): def verify_basic(self, basic_info_func): check_basic_info_func = self.check_basic_auth(basic_info_func) - def wrapper(request, required_scopes): + def wrapper(request): auth_type, user_pass = self.get_auth_header_value(request) if auth_type != 'basic': return self.no_value @@ -198,7 +198,7 @@ def wrapper(request, required_scopes): except Exception: raise OAuthProblem(description='Invalid authorization header') - return check_basic_info_func(request, username, password, required_scopes=required_scopes) + return check_basic_info_func(request, username, password) return wrapper @@ -221,7 +221,7 @@ def get_cookie_value(cookies, name): def verify_api_key(self, api_key_info_func, loc, name): check_api_key_func = self.check_api_key(api_key_info_func) - def wrapper(request, required_scopes): + def wrapper(request): def _immutable_pop(_dict, key): """ @@ -252,7 +252,7 @@ def _immutable_pop(_dict, key): if api_key is None: return self.no_value - return check_api_key_func(request, api_key, required_scopes=required_scopes) + return check_api_key_func(request, api_key) return wrapper @@ -263,11 +263,11 @@ def verify_bearer(self, token_info_func): """ check_bearer_func = self.check_bearer_token(token_info_func) - def wrapper(request, required_scopes): + def wrapper(request): auth_type, token = self.get_auth_header_value(request) if auth_type != 'bearer': return self.no_value - return check_bearer_func(request, token, required_scopes=required_scopes) + return check_bearer_func(request, token) return wrapper @@ -281,10 +281,10 @@ def verify_multiple_schemes(self, schemes): :rtype: types.FunctionType """ - def wrapper(request, required_scopes): + def wrapper(request): token_info = {} for scheme_name, func in schemes.items(): - result = func(request, required_scopes) + result = func(request) if result is self.no_value: return self.no_value token_info[scheme_name] = result @@ -299,7 +299,7 @@ def verify_none(): :rtype: types.FunctionType """ - def wrapper(request, required_scopes): + def wrapper(request): return {} return wrapper @@ -320,6 +320,9 @@ def wrapper(request, *args, required_scopes=None): if need_to_add_required_scopes: kwargs[self.required_scopes_kw] = required_scopes token_info = func(*args, **kwargs) + # TODO: Multiple OAuth schemes defined in OR fashion + # This currently doesn't work because if the first one doesn't apply, it will raise an error + # and the second OAuth security func is never checked if token_info is self.no_value: return self.no_value if token_info is None: @@ -362,18 +365,26 @@ def wrapper(request, token, required_scopes): return wrapper @classmethod - def verify_security(cls, auth_funcs, required_scopes, function): + def verify_security(cls, auth_funcs, function): @functools.wraps(function) def wrapper(request): token_info = cls.no_value + problem = None for func in auth_funcs: - token_info = func(request, required_scopes) - if token_info is not cls.no_value: - break + try: + token_info = func(request) + if token_info is not cls.no_value: + break + # TODO: Catch any error that might be raised instead of specific ones? + except (OAuthProblem, OAuthResponseProblem, OAuthScopeProblem) as err: + problem = err if token_info is cls.no_value: - logger.info("... No auth provided. Aborting with 401.") - raise OAuthProblem(description='No authorization token provided') + if problem is not None: + raise problem + else: + logger.info("... No auth provided. Aborting with 401.") + raise OAuthProblem(description='No authorization token provided') # Fallback to 'uid' for backward compatibility request.context['user'] = token_info.get('sub', token_info.get('uid')) From fe9e813628eb86bb4c8e5472a2209d03040e9e77 Mon Sep 17 00:00:00 2001 From: Ruwan Date: Sat, 5 Mar 2022 00:34:27 +0100 Subject: [PATCH 02/14] Update tests for security scopes --- tests/test_operation2.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/test_operation2.py b/tests/test_operation2.py index 6976c1410..6437e9c58 100644 --- a/tests/test_operation2.py +++ b/tests/test_operation2.py @@ -296,8 +296,7 @@ def test_operation(api, security_handler_factory): security_decorator = operation.security_decorator assert len(security_decorator.args[0]) == 1 assert security_decorator.args[0][0] == 'verify_oauth_result' - assert security_decorator.args[1] == ['uid'] - verify_oauth.assert_called_with('get_token_info_remote_result',security_handler_factory.validate_scope) + verify_oauth.assert_called_with('get_token_info_remote_result', security_handler_factory.validate_scope, ['uid']) security_handler_factory.get_token_info_remote.assert_called_with('https://oauth.example/token_info') assert operation.method == 'GET' @@ -384,8 +383,7 @@ def test_operation_local_security_oauth2(api): security_decorator = operation.security_decorator assert len(security_decorator.args[0]) == 1 assert security_decorator.args[0][0] == 'verify_oauth_result' - assert security_decorator.args[1] == ['uid'] - verify_oauth.assert_called_with(math.ceil, api.security_handler_factory.validate_scope) + verify_oauth.assert_called_with(math.ceil, api.security_handler_factory.validate_scope, ['uid']) assert operation.method == 'GET' assert operation.produces == ['application/json'] @@ -418,8 +416,7 @@ def test_operation_local_security_duplicate_token_info(api): security_decorator = operation.security_decorator assert len(security_decorator.args[0]) == 1 assert security_decorator.args[0][0] == 'verify_oauth_result' - assert security_decorator.args[1] == ['uid'] - verify_oauth.call_args.assert_called_with(math.ceil, api.security_handler_factory.validate_scope) + verify_oauth.call_args.assert_called_with(math.ceil, api.security_handler_factory.validate_scope, ['uid']) assert operation.method == 'GET' assert operation.produces == ['application/json'] @@ -514,7 +511,6 @@ def return_api_key_name(func, in_, name): security_decorator = operation.security_decorator assert len(security_decorator.args[0]) == 1 assert security_decorator.args[0][0] == 'verify_multiple_result' - assert security_decorator.args[1] is None assert operation.method == 'GET' assert operation.produces == ['application/json'] @@ -548,7 +544,6 @@ def test_multiple_oauth_in_and(api, caplog): security_decorator = operation.security_decorator assert len(security_decorator.args[0]) == 0 assert security_decorator.args[0] == [] - assert security_decorator.args[1] == ['uid'] assert '... multiple OAuth2 security schemes in AND fashion not supported' in caplog.text From 003531b09bfae5a73443a7060f79314adeccd1bc Mon Sep 17 00:00:00 2001 From: Ruwan Date: Sat, 5 Mar 2022 00:35:17 +0100 Subject: [PATCH 03/14] Add test for oauth security scheme with multiple possible scopes --- tests/test_operation2.py | 42 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/test_operation2.py b/tests/test_operation2.py index 6437e9c58..692f5fbb4 100644 --- a/tests/test_operation2.py +++ b/tests/test_operation2.py @@ -215,6 +215,11 @@ 'responses': {'200': {'description': 'OK'}}, 'security': [{'oauth_1': ['uid'], 'oauth_2': ['uid']}]} +OPERATION11 = {'description': 'operation secured with an oauth schemes with 2 possible scopes (in OR)', + 'operationId': 'fakeapi.hello.post_greeting', + 'responses': {'200': {'description': 'OK'}}, + 'security': [{'oauth': ['myscope']}, {'oauth': ['myscope2']}]} + SECURITY_DEFINITIONS_REMOTE = {'oauth': {'type': 'oauth2', 'flow': 'password', 'x-tokenInfoUrl': 'https://oauth.example/token_info', @@ -223,7 +228,8 @@ SECURITY_DEFINITIONS_LOCAL = {'oauth': {'type': 'oauth2', 'flow': 'password', 'x-tokenInfoFunc': 'math.ceil', - 'scopes': {'myscope': 'can do stuff'}}} + 'scopes': {'myscope': 'can do stuff', + 'myscope2': 'can do other stuff'}}} SECURITY_DEFINITIONS_BOTH = {'oauth': {'type': 'oauth2', 'flow': 'password', @@ -612,3 +618,37 @@ def test_get_path_parameter_types(api): ) assert {'int_path': 'int', 'string_path': 'string', 'path_path': 'path'} == operation.get_path_parameter_types() + + +def test_oauth_scopes_in_or(api): + """Tests whether an OAuth security scheme with 2 different possible scopes is correctly handled.""" + verify_oauth = mock.MagicMock(return_value='verify_oauth_result') + api.security_handler_factory.verify_oauth = verify_oauth + + op_spec = make_operation(OPERATION11) + operation = Swagger2Operation(api=api, + method='GET', + path='endpoint', + path_parameters=[], + operation=op_spec, + app_produces=['application/json'], + app_consumes=['application/json'], + app_security=[], + security_definitions=SECURITY_DEFINITIONS_LOCAL, + definitions=DEFINITIONS, + parameter_definitions=PARAMETER_DEFINITIONS, + resolver=Resolver()) + assert isinstance(operation.function, types.FunctionType) + security_decorator = operation.security_decorator + assert len(security_decorator.args[0]) == 2 + assert security_decorator.args[0][0] == 'verify_oauth_result' + assert security_decorator.args[0][1] == 'verify_oauth_result' + verify_oauth.assert_has_calls([ + mock.call(math.ceil, api.security_handler_factory.validate_scope, ['myscope']), + mock.call(math.ceil, api.security_handler_factory.validate_scope, ['myscope2']), + ]) + + assert operation.method == 'GET' + assert operation.produces == ['application/json'] + assert operation.consumes == ['application/json'] + assert operation.security == [{'oauth': ['myscope']}, {'oauth': ['myscope2']}] From 496f83fe11db093b3e04e8b2b3c506c6eaf5cc50 Mon Sep 17 00:00:00 2001 From: Ruwan Date: Sat, 5 Mar 2022 00:56:16 +0100 Subject: [PATCH 04/14] Update security tests --- tests/decorators/test_security.py | 44 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/decorators/test_security.py b/tests/decorators/test_security.py index 44a39b86c..4a21a5e75 100644 --- a/tests/decorators/test_security.py +++ b/tests/decorators/test_security.py @@ -34,12 +34,12 @@ def test_verify_oauth_missing_auth_header(security_handler_factory): def somefunc(token): return None - wrapped_func = security_handler_factory.verify_oauth(somefunc, security_handler_factory.validate_scope) + wrapped_func = security_handler_factory.verify_oauth(somefunc, security_handler_factory.validate_scope, ['admin']) request = MagicMock() request.headers = {} - assert wrapped_func(request, ['admin']) is security_handler_factory.no_value + assert wrapped_func(request) is security_handler_factory.no_value def test_verify_oauth_scopes_remote(monkeypatch, security_handler_factory): @@ -52,7 +52,7 @@ def get_tokeninfo_response(*args, **kwargs): return tokeninfo_response token_info_func = security_handler_factory.get_tokeninfo_func({'x-tokenInfoUrl': 'https://example.org/tokeninfo'}) - wrapped_func = security_handler_factory.verify_oauth(token_info_func, security_handler_factory.validate_scope) + wrapped_func = security_handler_factory.verify_oauth(token_info_func, security_handler_factory.validate_scope, ['admin']) request = MagicMock() request.headers = {"Authorization": "Bearer 123"} @@ -62,30 +62,30 @@ def get_tokeninfo_response(*args, **kwargs): monkeypatch.setattr('connexion.security.flask_security_handler_factory.session', session) with pytest.raises(OAuthScopeProblem, match="Provided token doesn't have the required scope"): - wrapped_func(request, ['admin']) + wrapped_func(request) tokeninfo["scope"] += " admin" - assert wrapped_func(request, ['admin']) is not None + assert wrapped_func(request) is not None tokeninfo["scope"] = ["foo", "bar"] with pytest.raises(OAuthScopeProblem, match="Provided token doesn't have the required scope"): - wrapped_func(request, ['admin']) + wrapped_func(request) tokeninfo["scope"].append("admin") - assert wrapped_func(request, ['admin']) is not None + assert wrapped_func(request) is not None def test_verify_oauth_invalid_local_token_response_none(security_handler_factory): def somefunc(token): return None - wrapped_func = security_handler_factory.verify_oauth(somefunc, security_handler_factory.validate_scope) + wrapped_func = security_handler_factory.verify_oauth(somefunc, security_handler_factory.validate_scope, ['admin']) request = MagicMock() request.headers = {"Authorization": "Bearer 123"} with pytest.raises(OAuthResponseProblem): - wrapped_func(request, ['admin']) + wrapped_func(request) def test_verify_oauth_scopes_local(security_handler_factory): @@ -94,23 +94,23 @@ def test_verify_oauth_scopes_local(security_handler_factory): def token_info(token): return tokeninfo - wrapped_func = security_handler_factory.verify_oauth(token_info, security_handler_factory.validate_scope) + wrapped_func = security_handler_factory.verify_oauth(token_info, security_handler_factory.validate_scope, ['admin']) request = MagicMock() request.headers = {"Authorization": "Bearer 123"} with pytest.raises(OAuthScopeProblem, match="Provided token doesn't have the required scope"): - wrapped_func(request, ['admin']) + wrapped_func(request) tokeninfo["scope"] += " admin" - assert wrapped_func(request, ['admin']) is not None + assert wrapped_func(request) is not None tokeninfo["scope"] = ["foo", "bar"] with pytest.raises(OAuthScopeProblem, match="Provided token doesn't have the required scope"): - wrapped_func(request, ['admin']) + wrapped_func(request) tokeninfo["scope"].append("admin") - assert wrapped_func(request, ['admin']) is not None + assert wrapped_func(request) is not None def test_verify_basic_missing_auth_header(security_handler_factory): @@ -122,7 +122,7 @@ def somefunc(username, password, required_scopes=None): request = MagicMock() request.headers = {"Authorization": "Bearer 123"} - assert wrapped_func(request, ['admin']) is security_handler_factory.no_value + assert wrapped_func(request) is security_handler_factory.no_value def test_verify_basic(security_handler_factory): @@ -136,7 +136,7 @@ def basic_info(username, password, required_scopes=None): request = MagicMock() request.headers = {"Authorization": 'Basic Zm9vOmJhcg=='} - assert wrapped_func(request, ['admin']) is not None + assert wrapped_func(request) is not None def test_verify_apikey_query(security_handler_factory): @@ -150,7 +150,7 @@ def apikey_info(apikey, required_scopes=None): request = MagicMock() request.query = {"auth": 'foobar'} - assert wrapped_func(request, ['admin']) is not None + assert wrapped_func(request) is not None def test_verify_apikey_header(security_handler_factory): @@ -164,7 +164,7 @@ def apikey_info(apikey, required_scopes=None): request = MagicMock() request.headers = {"X-Auth": 'foobar'} - assert wrapped_func(request, ['admin']) is not None + assert wrapped_func(request) is not None def test_multiple_schemes(security_handler_factory): @@ -189,12 +189,12 @@ def apikey2_info(apikey, required_scopes=None): request = MagicMock() request.headers = {"X-Auth-1": 'foobar'} - assert wrapped_func(request, ['admin']) is security_handler_factory.no_value + assert wrapped_func(request) is security_handler_factory.no_value request = MagicMock() request.headers = {"X-Auth-2": 'bar'} - assert wrapped_func(request, ['admin']) is security_handler_factory.no_value + assert wrapped_func(request) is security_handler_factory.no_value # Supplying both keys does succeed request = MagicMock() @@ -207,13 +207,13 @@ def apikey2_info(apikey, required_scopes=None): 'key1': {'sub': 'foo'}, 'key2': {'sub': 'bar'}, } - assert wrapped_func(request, ['admin']) == expected_token_info + assert wrapped_func(request) == expected_token_info def test_verify_security_oauthproblem(security_handler_factory): """Tests whether verify_security raises an OAuthProblem if there are no auth_funcs.""" func_to_secure = MagicMock(return_value='func') - secured_func = security_handler_factory.verify_security([], [], func_to_secure) + secured_func = security_handler_factory.verify_security([], func_to_secure) request = MagicMock() with pytest.raises(OAuthProblem) as exc_info: From 0b39289eae488709b71d25660ffbfa3f3d7ce4ba Mon Sep 17 00:00:00 2001 From: Ruwan Date: Sat, 5 Mar 2022 01:19:42 +0100 Subject: [PATCH 05/14] Change optional auth test to correct behaviour --- tests/api/test_secure_api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/api/test_secure_api.py b/tests/api/test_secure_api.py index a2729eeb4..195de9851 100644 --- a/tests/api/test_secure_api.py +++ b/tests/api/test_secure_api.py @@ -96,7 +96,8 @@ def test_security(oauth_requests, secure_endpoint_app): assert response.data == b'"Authenticated"\n' headers = {"X-AUTH": "wrong-key"} response = app_client.get('/v1.0/optional-auth', headers=headers) # type: flask.Response - assert response.status_code == 401 + assert response.data == b'"Unauthenticated"\n' + assert response.status_code == 200 def test_checking_that_client_token_has_all_necessary_scopes( From 7d52eb61229e4c8e16790a1ca90889059acbe843 Mon Sep 17 00:00:00 2001 From: Ruwan Date: Sat, 5 Mar 2022 21:29:52 +0100 Subject: [PATCH 06/14] Update security documentation --- docs/security.rst | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/security.rst b/docs/security.rst index 6d4b6c111..4ce56e134 100644 --- a/docs/security.rst +++ b/docs/security.rst @@ -57,11 +57,7 @@ Basic Authentication With Connexion, the API security definition **must** include a ``x-basicInfoFunc`` or set ``BASICINFO_FUNC`` env var. It uses the same semantics as for ``x-tokenInfoFunc``, but the function accepts three -parameters: username, password and required_scopes. If the security declaration -of the operation also has an oauth security requirement, required_scopes is -taken from there, otherwise it's None. This allows authorizing individual -operations with `oauth scope`_ while using basic authentication for -authentication. +parameters: username, password and required_scopes. You can find a `minimal Basic Auth example application`_ in Connexion's "examples" folder. From 6d7b8e5bf5dad8bd9e5ed42c3965ca66d5d8c14f Mon Sep 17 00:00:00 2001 From: Ruwan Date: Wed, 9 Mar 2022 20:25:56 +0100 Subject: [PATCH 07/14] Remove TODOs --- connexion/security/security_handler_factory.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/connexion/security/security_handler_factory.py b/connexion/security/security_handler_factory.py index 9323c4ef8..a368ba240 100644 --- a/connexion/security/security_handler_factory.py +++ b/connexion/security/security_handler_factory.py @@ -320,9 +320,6 @@ def wrapper(request, *args, required_scopes=None): if need_to_add_required_scopes: kwargs[self.required_scopes_kw] = required_scopes token_info = func(*args, **kwargs) - # TODO: Multiple OAuth schemes defined in OR fashion - # This currently doesn't work because if the first one doesn't apply, it will raise an error - # and the second OAuth security func is never checked if token_info is self.no_value: return self.no_value if token_info is None: @@ -375,7 +372,6 @@ def wrapper(request): token_info = func(request) if token_info is not cls.no_value: break - # TODO: Catch any error that might be raised instead of specific ones? except (OAuthProblem, OAuthResponseProblem, OAuthScopeProblem) as err: problem = err From 3cf5ffe6fdd795d6c1f525131d58234f282860e8 Mon Sep 17 00:00:00 2001 From: Ruwan Date: Sat, 12 Mar 2022 01:35:57 +0100 Subject: [PATCH 08/14] Catch possible exceptions from failed checks in async security factory --- .../async_security_handler_factory.py | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/connexion/security/async_security_handler_factory.py b/connexion/security/async_security_handler_factory.py index ca9f23d6f..5ff02a27c 100644 --- a/connexion/security/async_security_handler_factory.py +++ b/connexion/security/async_security_handler_factory.py @@ -66,16 +66,23 @@ def verify_security(cls, auth_funcs, function): @functools.wraps(function) async def wrapper(request): token_info = cls.no_value + problem = None for func in auth_funcs: - token_info = func(request) - while asyncio.iscoroutine(token_info): - token_info = await token_info - if token_info is not cls.no_value: - break + try: + token_info = func(request) + while asyncio.iscoroutine(token_info): + token_info = await token_info + if token_info is not cls.no_value: + break + except (OAuthProblem, OAuthResponseProblem, OAuthScopeProblem) as err: + problem = err if token_info is cls.no_value: - logger.info("... No auth provided. Aborting with 401.") - raise OAuthProblem(description='No authorization token provided') + if problem is not None: + raise problem + else: + logger.info("... No auth provided. Aborting with 401.") + raise OAuthProblem(description='No authorization token provided') # Fallback to 'uid' for backward compatibility request.context['user'] = token_info.get('sub', token_info.get('uid')) From 18b9f49f681f5e1c28f3ef060171334be8cd30d5 Mon Sep 17 00:00:00 2001 From: Ruwan Lambrichts Date: Fri, 18 Mar 2022 18:30:59 +0100 Subject: [PATCH 09/14] Add .venv/ to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 916e23e41..cd64b3131 100644 --- a/.gitignore +++ b/.gitignore @@ -13,4 +13,5 @@ htmlcov/ .idea/ .vscode/ venv/ +.venv/ src/ From 836cf68e4f21fed43874f7ecf31c399b6f008852 Mon Sep 17 00:00:00 2001 From: Ruwan Lambrichts Date: Fri, 18 Mar 2022 18:31:55 +0100 Subject: [PATCH 10/14] Try to raise most specific exception --- .../security/security_handler_factory.py | 41 ++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/connexion/security/security_handler_factory.py b/connexion/security/security_handler_factory.py index a368ba240..3fb2aef7c 100644 --- a/connexion/security/security_handler_factory.py +++ b/connexion/security/security_handler_factory.py @@ -366,18 +366,18 @@ def verify_security(cls, auth_funcs, function): @functools.wraps(function) def wrapper(request): token_info = cls.no_value - problem = None + errors = [] for func in auth_funcs: try: token_info = func(request) if token_info is not cls.no_value: break - except (OAuthProblem, OAuthResponseProblem, OAuthScopeProblem) as err: - problem = err + except Exception as err: + errors += err if token_info is cls.no_value: - if problem is not None: - raise problem + if errors != []: + cls._raise_most_specific(errors) else: logger.info("... No auth provided. Aborting with 401.") raise OAuthProblem(description='No authorization token provided') @@ -389,6 +389,37 @@ def wrapper(request): return wrapper + @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, '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] + @abc.abstractmethod def get_token_info_remote(self, token_info_url): """ From 84c772bf7cbb5e8a5d5c5ce9c6a66e9ff6425922 Mon Sep 17 00:00:00 2001 From: Ruwan Lambrichts Date: Fri, 18 Mar 2022 19:10:37 +0100 Subject: [PATCH 11/14] Add test for raising most specific error --- tests/decorators/test_security.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/decorators/test_security.py b/tests/decorators/test_security.py index 4a21a5e75..412f90e6e 100644 --- a/tests/decorators/test_security.py +++ b/tests/decorators/test_security.py @@ -4,7 +4,8 @@ import pytest import requests from connexion.exceptions import (OAuthProblem, OAuthResponseProblem, - OAuthScopeProblem) + OAuthScopeProblem, BadRequestProblem, + ConnexionException) def test_get_tokeninfo_url(monkeypatch, security_handler_factory): @@ -220,3 +221,20 @@ def test_verify_security_oauthproblem(security_handler_factory): secured_func(request) assert str(exc_info.value) == '401 Unauthorized: 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, security_handler_factory): + """Tests whether most specific exception is raised from a list.""" + + with pytest.raises(most_specific): + security_handler_factory._raise_most_specific(errors) From fa54e557352c10b8aec2c81908358bfd68bd4c17 Mon Sep 17 00:00:00 2001 From: Ruwan Lambrichts Date: Fri, 18 Mar 2022 19:23:16 +0100 Subject: [PATCH 12/14] Update async security handler factory --- connexion/security/async_security_handler_factory.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/connexion/security/async_security_handler_factory.py b/connexion/security/async_security_handler_factory.py index 5ff02a27c..6c12fb9c9 100644 --- a/connexion/security/async_security_handler_factory.py +++ b/connexion/security/async_security_handler_factory.py @@ -66,7 +66,7 @@ def verify_security(cls, auth_funcs, function): @functools.wraps(function) async def wrapper(request): token_info = cls.no_value - problem = None + errors = [] for func in auth_funcs: try: token_info = func(request) @@ -74,12 +74,12 @@ async def wrapper(request): token_info = await token_info if token_info is not cls.no_value: break - except (OAuthProblem, OAuthResponseProblem, OAuthScopeProblem) as err: - problem = err + except Exception as err: + errors += err if token_info is cls.no_value: - if problem is not None: - raise problem + if errors != []: + cls._raise_most_specific(errors) else: logger.info("... No auth provided. Aborting with 401.") raise OAuthProblem(description='No authorization token provided') From 7390dbb6f03e4919014b4bf719653ceb9c1313a0 Mon Sep 17 00:00:00 2001 From: Ruwan Lambrichts Date: Fri, 18 Mar 2022 21:01:06 +0100 Subject: [PATCH 13/14] Fix security handler error catching --- connexion/security/async_security_handler_factory.py | 2 +- connexion/security/security_handler_factory.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/connexion/security/async_security_handler_factory.py b/connexion/security/async_security_handler_factory.py index 6c12fb9c9..4eeb927e1 100644 --- a/connexion/security/async_security_handler_factory.py +++ b/connexion/security/async_security_handler_factory.py @@ -75,7 +75,7 @@ async def wrapper(request): if token_info is not cls.no_value: break except Exception as err: - errors += err + errors.append(err) if token_info is cls.no_value: if errors != []: diff --git a/connexion/security/security_handler_factory.py b/connexion/security/security_handler_factory.py index 3fb2aef7c..5a09888c0 100644 --- a/connexion/security/security_handler_factory.py +++ b/connexion/security/security_handler_factory.py @@ -373,7 +373,7 @@ def wrapper(request): if token_info is not cls.no_value: break except Exception as err: - errors += err + errors.append(err) if token_info is cls.no_value: if errors != []: From ca64f8591a4240352bacf5dc95ab93539f93c8a4 Mon Sep 17 00:00:00 2001 From: Ruwan Lambrichts Date: Fri, 18 Mar 2022 21:11:39 +0100 Subject: [PATCH 14/14] Fix imports order --- tests/decorators/test_security.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/decorators/test_security.py b/tests/decorators/test_security.py index 412f90e6e..60ab26260 100644 --- a/tests/decorators/test_security.py +++ b/tests/decorators/test_security.py @@ -3,9 +3,9 @@ import pytest import requests -from connexion.exceptions import (OAuthProblem, OAuthResponseProblem, - OAuthScopeProblem, BadRequestProblem, - ConnexionException) +from connexion.exceptions import (BadRequestProblem, ConnexionException, + OAuthProblem, OAuthResponseProblem, + OAuthScopeProblem) def test_get_tokeninfo_url(monkeypatch, security_handler_factory):