Skip to content

Commit

Permalink
Add API-key scope checking (#1837)
Browse files Browse the repository at this point in the history
Fixes # N/A

The current documentation mentions that API-key
security supports scopes: "
The function should accept the following arguments:
- apikey
- required_scopes (optional) "

However, the scopes were not passed to the checker.

Changes proposed in this pull request:

 - Add missing parameter routing to ApiKeySecurityHandler
 - Add a unit test for API-key scopes

---------

Co-authored-by: Robbe Sneyders <[email protected]>
  • Loading branch information
etvahala and RobbeSneyders authored Oct 23, 2024
1 parent 17f5ed0 commit c89e2d2
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 10 deletions.
5 changes: 3 additions & 2 deletions connexion/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,10 @@ def get_fn(self, security_scheme, required_scopes):
apikey_info_func,
security_scheme["in"],
security_scheme["name"],
required_scopes,
)

def _get_verify_func(self, api_key_info_func, loc, name):
def _get_verify_func(self, api_key_info_func, loc, name, required_scopes):
check_api_key_func = self.check_api_key(api_key_info_func)

def wrapper(request: ConnexionRequest):
Expand All @@ -264,7 +265,7 @@ def wrapper(request: ConnexionRequest):
if api_key is None:
return NO_VALUE

return check_api_key_func(request, api_key)
return check_api_key_func(request, api_key, required_scopes=required_scopes)

return wrapper

Expand Down
40 changes: 35 additions & 5 deletions tests/decorators/test_security.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import json
from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch

import pytest
import requests
Expand Down Expand Up @@ -209,7 +209,7 @@ def apikey_info(apikey, required_scopes=None):

security_handler_factory = ApiKeySecurityHandler()
wrapped_func = security_handler_factory._get_verify_func(
apikey_info, "query", "auth"
apikey_info, "query", "auth", None
)

request = ConnexionRequest(scope={"type": "http", "query_string": b"auth=foobar"})
Expand All @@ -225,7 +225,7 @@ def apikey_info(apikey, required_scopes=None):

security_handler_factory = ApiKeySecurityHandler()
wrapped_func = security_handler_factory._get_verify_func(
apikey_info, "header", "X-Auth"
apikey_info, "header", "X-Auth", None
)

request = ConnexionRequest(
Expand All @@ -235,6 +235,36 @@ def apikey_info(apikey, required_scopes=None):
assert await wrapped_func(request) is not None


async def test_verify_apikey_scopes():
def apikey_info(apikey, required_scopes=None):
if apikey == "admin foobar" and required_scopes == ["admin"]:
return {"sub": "foo"}
return None

security_handler_factory = ApiKeySecurityHandler()

scheme_apikey = {
"type": "apiKey",
"name": "x-auth",
"in": "header",
"scopes": {"admin"},
}

with patch.object(
security_handler_factory,
f"{security_handler_factory._resolve_func.__name__}",
return_value=apikey_info,
) as mock_resolve_func:
wrapped_func = security_handler_factory.get_fn(scheme_apikey, ["admin"])
mock_resolve_func.assert_called_once()

request = ConnexionRequest(
scope={"type": "http", "headers": [[b"x-auth", b"admin foobar"]]}
)

assert await wrapped_func(request) == {"sub": "foo"}


async def test_multiple_schemes():
def apikey1_info(apikey, required_scopes=None):
if apikey == "foobar":
Expand All @@ -249,10 +279,10 @@ def apikey2_info(apikey, required_scopes=None):
security_handler_factory = SecurityHandlerFactory()
apikey_security_handler_factory = ApiKeySecurityHandler()
wrapped_func_key1 = apikey_security_handler_factory._get_verify_func(
apikey1_info, "header", "X-Auth-1"
apikey1_info, "header", "X-Auth-1", []
)
wrapped_func_key2 = apikey_security_handler_factory._get_verify_func(
apikey2_info, "header", "X-Auth-2"
apikey2_info, "header", "X-Auth-2", []
)
schemes = {
"key1": wrapped_func_key1,
Expand Down
6 changes: 3 additions & 3 deletions tests/test_operation2.py
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ def test_no_token_info():
def test_multiple_security_schemes_and():
"""Tests an operation with multiple security schemes in AND fashion."""

def return_api_key_name(func, in_, name):
def return_api_key_name(func, in_, name, scopes):
return name

class MockApiKeyHandler(ApiKeySecurityHandler):
Expand All @@ -610,8 +610,8 @@ class MockApiKeyHandler(ApiKeySecurityHandler):
)

assert verify_api_key.call_count == 2
verify_api_key.assert_any_call(math.ceil, "header", "X-Auth-1")
verify_api_key.assert_any_call(math.ceil, "header", "X-Auth-2")
verify_api_key.assert_any_call(math.ceil, "header", "X-Auth-1", [])
verify_api_key.assert_any_call(math.ceil, "header", "X-Auth-2", [])
# Assert verify_multiple_schemes is called with mapping from scheme name
# to result of security_handler_factory.verify_api_key()
verify_multiple.assert_called_with({"key1": "X-Auth-1", "key2": "X-Auth-2"})
Expand Down

0 comments on commit c89e2d2

Please sign in to comment.