diff --git a/.gitignore b/.gitignore index 5564038d3..a22702e96 100644 --- a/.gitignore +++ b/.gitignore @@ -13,5 +13,6 @@ htmlcov/ .idea/ .vscode/ venv/ +.venv/ src/ *.un~ 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..4eeb927e1 100644 --- a/connexion/security/async_security_handler_factory.py +++ b/connexion/security/async_security_handler_factory.py @@ -62,20 +62,27 @@ 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 + errors = [] for func in auth_funcs: - token_info = func(request, required_scopes) - 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 Exception as err: + errors.append(err) if token_info is cls.no_value: - logger.info("... No auth provided. Aborting with 401.") - raise OAuthProblem(description='No authorization token provided') + if errors != []: + cls._raise_most_specific(errors) + 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')) diff --git a/connexion/security/security_handler_factory.py b/connexion/security/security_handler_factory.py index a6504c3b3..5a09888c0 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 @@ -362,18 +362,25 @@ 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 + errors = [] 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 + except Exception as err: + errors.append(err) if token_info is cls.no_value: - logger.info("... No auth provided. Aborting with 401.") - raise OAuthProblem(description='No authorization token provided') + if errors != []: + cls._raise_most_specific(errors) + 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')) @@ -382,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): """ diff --git a/docs/security.rst b/docs/security.rst index 3d4b20cc6..02b929d83 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. 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( diff --git a/tests/decorators/test_security.py b/tests/decorators/test_security.py index 44a39b86c..60ab26260 100644 --- a/tests/decorators/test_security.py +++ b/tests/decorators/test_security.py @@ -3,7 +3,8 @@ import pytest import requests -from connexion.exceptions import (OAuthProblem, OAuthResponseProblem, +from connexion.exceptions import (BadRequestProblem, ConnexionException, + OAuthProblem, OAuthResponseProblem, OAuthScopeProblem) @@ -34,12 +35,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 +53,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 +63,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 +95,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 +123,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 +137,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 +151,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 +165,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 +190,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,16 +208,33 @@ 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: 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) diff --git a/tests/test_operation2.py b/tests/test_operation2.py index 6976c1410..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', @@ -296,8 +302,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 +389,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 +422,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 +517,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 +550,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 @@ -617,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']}]