Support "client credentials" tokens (#42)

src/requestor/routes/manage.py

@@ -158,7 +158,12 @@ async def create_request(
     if not data.get("username"):
         logger.debug("No username provided in body, using token username")
         token_claims = await auth.get_token_claims()
-        token_username = token_claims["context"]["user"]["name"]
+        token_username = token_claims.get("context", {}).get("user", {}).get("name")
+        if not token_username:
+            raise HTTPException(
+                HTTP_400_BAD_REQUEST,
+                "Must provide a username in the request body or token",
+            )
         logger.debug(f"Got username from token: {token_username}")
         data["username"] = token_username
 

src/requestor/routes/query.py

@@ -5,6 +5,7 @@
 from starlette.status import (
     HTTP_200_OK,
     HTTP_400_BAD_REQUEST,
+    HTTP_403_FORBIDDEN,
     HTTP_404_NOT_FOUND,
 )
 
@@ -105,6 +106,8 @@ async def list_requests(
 
     # get the resources the current user has access to see
     token_claims = await auth.get_token_claims()
+    # TODO update this endpoint to accept client tokens. We need to get the
+    # auth mapping for the client instead of the user
     username = token_claims["context"]["user"]["name"]
     authz_mapping = await api_request.app.arborist_client.auth_mapping(username)
     authorized_resource_paths = [
@@ -171,7 +174,12 @@ async def list_user_requests(api_request: Request, auth=Depends(Auth)) -> dict:
     # their own requests.
     filter_dict, active = populate_filters_from_query_params(api_request.query_params)
     token_claims = await auth.get_token_claims()
-    username = token_claims["context"]["user"]["name"]
+    username = token_claims.get("context", {}).get("user", {}).get("name")
+    if not username:
+        raise HTTPException(
+            HTTP_403_FORBIDDEN,
+            "This endpoint does not support tokens that are not linked to a user",
+        )
     logger.debug(f"Getting requests for user '{username}' with active = '{active}'")
     user_requests = await get_filtered_requests(
         # if we only want active requests, filter out requests in a final status
@@ -237,7 +245,12 @@ async def check_user_resource_paths(
     # no authz checks because we assume the current user can read
     # their own requests.
     token_claims = await auth.get_token_claims()
-    username = token_claims["context"]["user"]["name"]
+    username = token_claims.get("context", {}).get("user", {}).get("name")
+    if not username:
+        raise HTTPException(
+            HTTP_403_FORBIDDEN,
+            "This endpoint does not support tokens that are not linked to a user",
+        )
     user_requests = await get_filtered_requests(username, draft=False, final=False)
     positive_requests = [r for r in user_requests if not r.revoke]
     existing_policies = await arborist.list_policies(

tests/conftest.py

@@ -170,8 +170,39 @@ def list_roles_patcher():
     role_patch.stop()
 
 
-@pytest.fixture(autouse=True, scope="function")
-def access_token_patcher(client, request):
+@pytest.fixture(autouse=True, scope="function", params=["user_token", "client_token"])
+def access_token_user_client_patcher(client, request):
+    """
+    The `access_token` function will return first a token linked to a test
+    user, then a token linked to a test client.
+    """
+
+    async def get_access_token(*args, **kwargs):
+        if request.param == "user_token":
+            return {"sub": "1", "context": {"user": {"name": "requestor_user"}}}
+        if request.param == "client_token":
+            return {"context": {}, "azp": "test-client-id"}
+
+    access_token_mock = MagicMock()
+    access_token_mock.return_value = get_access_token
+
+    access_token_patch = patch("requestor.auth.access_token", access_token_mock)
+    access_token_patch.start()
+
+    yield access_token_mock
+
+    access_token_patch.stop()
+
+
+@pytest.fixture(scope="function")
+def access_token_user_only_patcher(client, request):
+    """
+    The `access_token` function will return a token linked to a test user.
+    This fixture should be used explicitely instead of the automatic
+    `access_token_user_client_patcher` fixture for endpoints that do not
+    support client tokens.
+    """
+
     async def get_access_token(*args, **kwargs):
         return {"sub": "1", "context": {"user": {"name": "requestor_user"}}}
 

tests/test_actions.py

@@ -227,7 +227,7 @@ def test_backoff_retry(client):
         assert mock_requests.post.call_count == config["DEFAULT_MAX_RETRIES"]
 
 
-def test_create_request_failure_revert(client):
+def test_create_request_failure_revert(client, access_token_user_only_patcher):
     """
     If something goes wrong during an external call, access should not be
     granted, the request should not be created and we should get a 500.

tests/test_manage.py

@@ -43,7 +43,7 @@ def test_create_request_with_unallowed_params(client, data):
     assert data["err_msg"] in res.json()["detail"]
 
 
-def test_create_request_without_username(client):
+def test_create_request_without_username(client, access_token_user_only_patcher):
     """
     When a username is not provided in the body, the request is created
     using the username from the provided access token.
@@ -146,7 +146,9 @@ def test_create_duplicate_request(client):
     assert res.status_code == 201, res.text
 
 
-def test_create_request_without_access(client, mock_arborist_requests):
+def test_create_request_without_access(
+    client, mock_arborist_requests, access_token_user_only_patcher
+):
     fake_jwt = "1.2.3"
     mock_arborist_requests(authorized=False)
 

tests/test_manage_resource_path.py

@@ -287,7 +287,7 @@ def test_create_request_with_resource_paths_and_role_ids(
         },
     ],
 )
-def test_create_request_without_username(client, data):
+def test_create_request_without_username(client, data, access_token_user_only_patcher):
     """
     When a username is not provided in the body, the request is created
     using the username from the provided access token.
@@ -441,7 +441,11 @@ def test_create_duplicate_request(client, data):
     ],
 )
 def test_create_request_without_access(
-    client, mock_arborist_requests, list_roles_patcher, data
+    client,
+    mock_arborist_requests,
+    list_roles_patcher,
+    data,
+    access_token_user_only_patcher,
 ):
     fake_jwt = "1.2.3"
     mock_arborist_requests(authorized=False)

tests/test_query.py

@@ -3,14 +3,9 @@
 from requestor.config import config
 
 
-def test_create_get_and_list_request(client):
+def test_create_and_get_request(client):
     fake_jwt = "1.2.3"
 
-    # list requests: empty
-    res = client.get("/request", headers={"Authorization": f"bearer {fake_jwt}"})
-    assert res.status_code == 200
-    assert res.json() == []
-
     # create a request
     data = {
         "username": "requestor_user",
@@ -43,6 +38,42 @@ def test_create_get_and_list_request(client):
     assert res.status_code == 200, res.text
     assert res.json() == request_data
 
+
+def test_create_and_list_request(client, access_token_user_only_patcher):
+    fake_jwt = "1.2.3"
+
+    # list requests: empty
+    res = client.get("/request", headers={"Authorization": f"bearer {fake_jwt}"})
+    assert res.status_code == 200
+    assert res.json() == []
+
+    # create a request
+    data = {
+        "username": "requestor_user",
+        "policy_id": "test-policy",
+        "resource_id": "uniqid",
+        "resource_display_name": "My Resource",
+    }
+    res = client.post(
+        "/request", json=data, headers={"Authorization": f"bearer {fake_jwt}"}
+    )
+    assert res.status_code == 201, res.text
+    request_data = res.json()
+    request_id = request_data.get("request_id")
+    assert request_id, "POST /request did not return a request_id"
+    assert request_data == {
+        "request_id": request_id,
+        "username": data["username"],
+        "policy_id": data["policy_id"],
+        "resource_id": data["resource_id"],
+        "resource_display_name": data["resource_display_name"],
+        "status": config["DEFAULT_INITIAL_STATUS"],
+        # just ensure revoke, created_time and updated_time are there:
+        "revoke": False,
+        "created_time": request_data["created_time"],
+        "updated_time": request_data["updated_time"],
+    }
+
     # list requests
     res = client.get("/request", headers={"Authorization": f"bearer {fake_jwt}"})
     assert res.status_code == 200, res.text
@@ -92,7 +123,7 @@ def test_get_request_without_access(client, mock_arborist_requests):
     assert not_found_err == unauthorized_err
 
 
-def test_get_filtered_requests(client):
+def test_get_filtered_requests(client, access_token_user_only_patcher):
 
     fake_jwt = "1.2.3"
     filtered_requests = []
@@ -173,7 +204,7 @@ def test_get_filtered_requests(client):
     assert res.status_code == 400, res.text
 
 
-def test_get_user_requests(client):
+def test_get_user_requests(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     # create a request for the current user
@@ -205,7 +236,7 @@ def test_get_user_requests(client):
     assert res.status_code == 401, res.text
 
 
-def test_get_active_user_requests(client):
+def test_get_active_user_requests(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     # create a request with a DRAFT status
@@ -260,7 +291,7 @@ def test_get_active_user_requests(client):
     assert res.json() == [active_request1, active_request2]
 
 
-def test_get_filtered_user_requests(client):
+def test_get_filtered_user_requests(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
     filtered_requests = []
 
@@ -337,7 +368,7 @@ def test_get_filtered_user_requests(client):
     assert res.status_code == 400, res.text
 
 
-def test_list_requests_with_access(client):
+def test_list_requests_with_access(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     # create requests
@@ -403,7 +434,9 @@ def test_list_requests_with_access(client):
         },
     ],
 )
-def test_check_user_resource_paths_prefixes(client, list_policies_patcher, test_data):
+def test_check_user_resource_paths_prefixes(
+    client, list_policies_patcher, test_data, access_token_user_only_patcher
+):
     """
     Test if having requested access to the resource path in
     test_data["resource_path"] means having requested access to
@@ -448,7 +481,9 @@ def test_check_user_resource_paths_prefixes(client, list_policies_patcher, test_
         }
     ],
 )
-def test_check_user_resource_paths_multiple(client, list_policies_patcher, test_data):
+def test_check_user_resource_paths_multiple(
+    client, list_policies_patcher, test_data, access_token_user_only_patcher
+):
     fake_jwt = "1.2.3"
     expected_matches = {
         "/a/b": True,
@@ -481,7 +516,7 @@ def test_check_user_resource_paths_multiple(client, list_policies_patcher, test_
     assert res.json() == expected_matches
 
 
-def test_check_user_resource_paths_username(client):
+def test_check_user_resource_paths_username(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     resource_path_to_match = "/a/b"
@@ -534,7 +569,9 @@ def test_check_user_resource_paths_username(client):
         },
     ],
 )
-def test_check_user_resource_paths_status(client, list_policies_patcher, test_data):
+def test_check_user_resource_paths_status(
+    client, list_policies_patcher, test_data, access_token_user_only_patcher
+):
     fake_jwt = "1.2.3"
 
     # create a request with the status to test
@@ -588,7 +625,9 @@ def test_check_user_resource_paths_status(client, list_policies_patcher, test_da
         },
     ],
 )
-def test_check_permissions_mismatch(client, list_policies_patcher, test_data):
+def test_check_permissions_mismatch(
+    client, list_policies_patcher, test_data, access_token_user_only_patcher
+):
     fake_jwt = "1.2.3"
 
     # create a request with an active status

tests/test_query_resource_path.py

@@ -10,7 +10,7 @@
 from requestor.config import config
 
 
-def test_create_get_and_list_request(client):
+def test_create_get_and_list_request(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     # list requests: empty
@@ -99,7 +99,7 @@ def test_get_request_without_access(client, mock_arborist_requests):
     assert not_found_err == unauthorized_err
 
 
-def test_get_user_requests(client):
+def test_get_user_requests(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     # create a request for the current user
@@ -132,7 +132,7 @@ def test_get_user_requests(client):
     assert res.status_code == 401, res.text
 
 
-def test_list_requests_with_access(client):
+def test_list_requests_with_access(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     # create requests
@@ -190,7 +190,9 @@ def test_list_requests_with_access(client):
         },
     ],
 )
-def test_check_user_resource_paths_prefixes(client, list_policies_patcher, test_data):
+def test_check_user_resource_paths_prefixes(
+    client, list_policies_patcher, test_data, access_token_user_only_patcher
+):
     """
     Test if having requested access to the resource path in
     test_data["resource_path"] means having requested access to
@@ -227,7 +229,9 @@ def test_check_user_resource_paths_prefixes(client, list_policies_patcher, test_
 
 
 @pytest.mark.parametrize("test_data", [{"resource_paths": ["/a/b", "/c"]}])
-def test_check_user_resource_paths_multiple(client, list_policies_patcher, test_data):
+def test_check_user_resource_paths_multiple(
+    client, list_policies_patcher, test_data, access_token_user_only_patcher
+):
     fake_jwt = "1.2.3"
     existing_resource_paths = test_data["resource_paths"]
     expected_matches = {
@@ -262,7 +266,7 @@ def test_check_user_resource_paths_multiple(client, list_policies_patcher, test_
     assert res.json() == expected_matches
 
 
-def test_check_user_resource_paths_username(client):
+def test_check_user_resource_paths_username(client, access_token_user_only_patcher):
     fake_jwt = "1.2.3"
 
     resource_path_to_match = "/a/b"
@@ -312,7 +316,9 @@ def test_check_user_resource_paths_username(client):
         },
     ],
 )
-def test_check_user_resource_paths_status(client, list_policies_patcher, test_data):
+def test_check_user_resource_paths_status(
+    client, list_policies_patcher, test_data, access_token_user_only_patcher
+):
     fake_jwt = "1.2.3"
     resource_path_to_match = test_data["resource_path"]