From 862518b5e5be996a3b4cba6e8c38b791fc122d14 Mon Sep 17 00:00:00 2001 From: cl0ete Date: Wed, 20 Nov 2024 23:47:09 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9D=20=F0=9F=92=A5Swagger=20tenant=20a?= =?UTF-8?q?dmin=20(#824)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update docstrings for tenant endpoints * formatting * update docstrings * update dacstrings * add response model * exclude group_id * update create docstrings * update delete docstrings * add patch endpoint for access token * update access-token docstrings * update update Tenant docstrings * update get by id docstrings * update fetch all * :art: * undo exclude group_id * assert 204 * assert 204 * edits * :art: * exclude group_id * fix tests * remove group_id and typo * remove group_id * :art: * undo remove of group_id * change to post * re-add group_id asserts * :art: * assert group_id * assert group_id * :art: Re-add group_id to response body in docstrings * :art: * :art: Expand docstring descriptions * :white_check_mark: Unit test coverage for newly duplicated post access token method * :test_tube: Update expected status code to 204 in k6 scripts, for deleting tenants --------- Co-authored-by: ff137 Co-authored-by: cl0ete --- app/routes/admin/tenants.py | 253 +++++++++++++++++- app/tests/e2e/test_auth.py | 4 +- app/tests/e2e/test_tenants.py | 24 +- .../test_get_wallet_auth_token.py | 76 +++++- scripts/k6/main.js | 8 +- scripts/k6/scenarios/delete-holders.js | 4 +- scripts/k6/scenarios/delete-issuers.js | 4 +- 7 files changed, 338 insertions(+), 35 deletions(-) diff --git a/app/routes/admin/tenants.py b/app/routes/admin/tenants.py index cf632c8dc..71e059817 100644 --- a/app/routes/admin/tenants.py +++ b/app/routes/admin/tenants.py @@ -58,12 +58,65 @@ ) -@router.post("", response_model=CreateTenantResponse) +@router.post("", response_model=CreateTenantResponse, summary="Create New Tenant") async def create_tenant( body: CreateTenantRequest, admin_auth: AcaPyAuthVerified = Depends(acapy_auth_tenant_admin), ) -> CreateTenantResponse: - """Create a new tenant.""" + """ + Create a New Tenant + --- + + Use this endpoint to create a new Tenant, which is the same as creating a new Wallet. + + The `wallet_label` is a required field that allows you to assign an alias to the Tenant. + This label is used as the default alias in connections (i.e. the other party will see this name in their records). + + If roles (issuer and/or verifier) are specified, the Tenant will be onboarded with these roles and added to the + trust registry. An issuer or a verifier is referred to as an 'actor' in the trust registry, and the `wallet_label` + will be used as the name for this actor. + + If no roles are provided, then the created wallet represents a regular tenant + (also referred to as a 'user', 'wallet', 'holder', or 'prover', depending on the context). + + The `wallet_label` does not have to be unique for regular tenants, but it may not match the name of an existing + actor in the trust registry, and will be blocked. Conversely, an issuer or verifier may not use a label that is + already in use by another tenant, and will also be blocked. + + The `wallet_name` is an optional field that allows you to assign an additional identifier to the wallet, and is + useful with `get_tenants` to fetch Wallets by name. Wallet names must be unique. + + The `image_url` is an optional field for assigning an image to the Wallet. For actors, this will also be added to + the trust registry. + + `extra_settings` is an optional field intended for advanced users, which allows configuring wallet behaviour. + + Request body: + --- + body: CreateTenantRequest + wallet_label: str + A required alias for the Tenant. + wallet_name: Optional[str] + An optional wallet name. + roles: Optional[List[str]] + A list of roles to assign to the Tenant. + image_url: Optional[str] + An optional image URL for the Tenant. + extra_settings: Optional[dict]] + Optional per-tenant settings to configure wallet behaviour for advanced users. + + Response body: + --- + CreateTenantResponse + wallet_id: str + wallet_label: str + wallet_name: str + created_at: str + image_url: Optional[str] + group_id: Optional[str] + updated_at: str + access_token: str + """ bound_logger = logger.bind(body=body) bound_logger.debug("POST request received: Starting tenant creation") @@ -187,13 +240,29 @@ async def create_tenant( return response -@router.delete("/{wallet_id}") +@router.delete("/{wallet_id}", summary="Delete a Tenant by Wallet ID", status_code=204) async def delete_tenant_by_id( wallet_id: str, group_id: Optional[str] = group_id_query, admin_auth: AcaPyAuthVerified = Depends(acapy_auth_tenant_admin), -): - """Delete tenant by id.""" +) -> None: + """ + Delete Tenant by ID + --- + + Use this endpoint to delete a Tenant by its Wallet ID. This action will remove the Tenant's Wallet, + along with any associated credentials, connections, and other data. If the tenant is an issuer or verifier, + they will also be removed from the trust registry. + + Request Parameters: + --- + wallet_id: str + The Wallet ID of the Tenant to delete. + + Response body: + --- + status_code: 204 No Content + """ bound_logger = logger.bind(body={"wallet_id": wallet_id}) bound_logger.debug("DELETE request received: Deleting tenant by id") @@ -226,12 +295,85 @@ async def delete_tenant_by_id( bound_logger.debug("Successfully deleted tenant.") -@router.get("/{wallet_id}/access-token", response_model=TenantAuth) +@router.get( + "/{wallet_id}/access-token", + response_model=TenantAuth, + summary="(Deprecated) Rotate and Get New Access Token for Wallet", + deprecated=True, +) async def get_wallet_auth_token( wallet_id: str, group_id: Optional[str] = group_id_query, admin_auth: AcaPyAuthVerified = Depends(acapy_auth_tenant_admin), ) -> TenantAuth: + """ + (Deprecated) Rotate and get access token for Wallet + --- + + Calling this endpoint will invalidate the previous access token for the Wallet, and return a new one. + + Request Parameters: + --- + wallet_id: str + The Wallet ID of the tenant for which the access token will be rotated. + + Response Body: + --- + TenantAuth + access_token: str + The new access token for the Wallet. + """ + bound_logger = logger.bind(body={"wallet_id": wallet_id}) + bound_logger.debug("GET request received: Access token for tenant") + + async with get_tenant_admin_controller(admin_auth) as admin_controller: + await get_wallet_and_assert_valid_group( + admin_controller=admin_controller, + wallet_id=wallet_id, + group_id=group_id, + logger=bound_logger, + ) + + bound_logger.debug("Getting auth token for wallet") + response = await handle_acapy_call( + logger=bound_logger, + acapy_call=admin_controller.multitenancy.get_auth_token, + wallet_id=wallet_id, + body=CreateWalletTokenRequest(), + ) + + response = TenantAuth(access_token=tenant_api_key(response.token)) + bound_logger.debug("Successfully retrieved access token.") + return response + + +@router.post( + "/{wallet_id}/access-token", + response_model=TenantAuth, + summary="Rotate and Get New Access Token for Wallet", +) +async def post_wallet_auth_token( + wallet_id: str, + group_id: Optional[str] = group_id_query, + admin_auth: AcaPyAuthVerified = Depends(acapy_auth_tenant_admin), +) -> TenantAuth: + """ + Rotate and get new access token for Wallet + --- + + Calling this endpoint will invalidate the previous access token for the Wallet, and return a new one. + + Request Parameters: + --- + wallet_id: str + The Wallet ID of the tenant for which the access token will be rotated. + + Response Body: + --- + TenantAuth + access_token: str + The new access token for the Wallet. + """ bound_logger = logger.bind(body={"wallet_id": wallet_id}) bound_logger.debug("GET request received: Access token for tenant") @@ -256,14 +398,49 @@ async def get_wallet_auth_token( return response -@router.put("/{wallet_id}", response_model=Tenant) +@router.put("/{wallet_id}", response_model=Tenant, summary="Update Tenant by Wallet ID") async def update_tenant( wallet_id: str, body: UpdateTenantRequest, group_id: Optional[str] = group_id_query, admin_auth: AcaPyAuthVerified = Depends(acapy_auth_tenant_admin), ) -> Tenant: - """Update tenant by id.""" + """ + Update Tenant by Wallet ID + --- + + Update a Tenant's details based on their Wallet ID. + + Issuers or verifiers can be promoted to hold both roles, but this endpoint does not support revoking roles. + + Holders cannot have their roles updated. Attempting to assign issuer or verifier + roles to a holder will result in a 409 conflict error. + + Updates to `wallet_label` or `image_url` for issuers and verifiers will be reflected on the trust registry. + + Request body: + --- + body: UpdateTenantRequest + wallet_label: Optional[str] + An optional alias for the Tenant. + roles: Optional[List[str]] + A list of roles to assign to the Tenant. + image_url: Optional[str] + An optional image URL for the Tenant. + extra_settings: Optional[Dict[str, Union[bool, str]]] + Optional per-tenant settings to configure wallet behaviour for advanced users. + + Response body: + --- + Tenant + wallet_id: str + wallet_label: str + wallet_name: str + created_at: str + updated_at: Optional[str] + image_url: Optional[str] + group_id: Optional[str] + """ bound_logger = logger.bind(body={"wallet_id": wallet_id, "body": body}) bound_logger.debug("PUT request received: Update tenant") @@ -284,13 +461,34 @@ async def update_tenant( return response -@router.get("/{wallet_id}", response_model=Tenant) +@router.get("/{wallet_id}", response_model=Tenant, summary="Get Tenant by Wallet ID") async def get_tenant( wallet_id: str, group_id: Optional[str] = group_id_query, admin_auth: AcaPyAuthVerified = Depends(acapy_auth_tenant_admin), ) -> Tenant: - """Get tenant by id.""" + """ + Fetch Tenant info by ID + --- + + Use this endpoint to fetch Tenant info by Wallet ID. + + Request parameters: + --- + wallet_id: str + The Wallet ID of the Tenant to fetch. + + Response body: + --- + Tenant + wallet_id: str + wallet_label: str + wallet_name: str + created_at: str + updated_at: Optional[str] + image_url: Optional[str] + group_id: Optional[str] + """ bound_logger = logger.bind(body={"wallet_id": wallet_id}) bound_logger.debug("GET request received: Fetch tenant by id") @@ -307,7 +505,7 @@ async def get_tenant( return response -@router.get("", response_model=List[Tenant]) +@router.get("", response_model=List[Tenant], summary="Fetch Tenants") async def get_tenants( wallet_name: Optional[str] = None, group_id: Optional[str] = group_id_query, @@ -317,7 +515,38 @@ async def get_tenants( descending: bool = descending_query_parameter, admin_auth: AcaPyAuthVerified = Depends(acapy_auth_tenant_admin), ) -> List[Tenant]: - """Get all tenants, or fetch by wallet name.""" + """ + Fetch all Tenants (paginated), or Fetch by Wallet Name + --- + + Use this endpoint to fetch all tenants (using pagination), or filter by wallet name. + + The default for `limit` is 1000, with a maximum of 10000. + + Results are ordered by creation time (newest first), and can be controlled to be in ascending order (oldest first). + + Optional Request Parameters: + --- + wallet_name: str + Filter by wallet name. + limit: int + Number of results to return. + offset: int + Number of results to skip. + descending: bool + Whether to return results in descending order or not. Defaults to true (newest first). + + Response body: + --- + List[Tenant] + wallet_id: str + wallet_label: str + wallet_name: str + created_at: str + updated_at: Optional[str] + image_url: Optional[str] + group_id: Optional[str] + """ bound_logger = logger.bind(body={"wallet_name": wallet_name, "group_id": group_id}) bound_logger.debug( "GET request received: Fetch tenants by wallet name and/or group id" diff --git a/app/tests/e2e/test_auth.py b/app/tests/e2e/test_auth.py index ee6ffc52f..5376d349b 100644 --- a/app/tests/e2e/test_auth.py +++ b/app/tests/e2e/test_auth.py @@ -91,7 +91,7 @@ async def test_jwt_invalid_token_error(tenant_admin_client: RichAsyncClient): delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -149,4 +149,4 @@ async def test_invalid_token_error_after_rotation(tenant_admin_client: RichAsync delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 diff --git a/app/tests/e2e/test_tenants.py b/app/tests/e2e/test_tenants.py index f28fced0e..b9f196575 100644 --- a/app/tests/e2e/test_tenants.py +++ b/app/tests/e2e/test_tenants.py @@ -70,7 +70,7 @@ async def test_get_wallet_auth_token(tenant_admin_client: RichAsyncClient): delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -114,7 +114,7 @@ async def test_create_tenant_member_wo_wallet_name( delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -169,7 +169,7 @@ async def test_create_tenant_member_w_wallet_name( delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -251,7 +251,7 @@ async def test_create_tenant_issuer( delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -313,7 +313,7 @@ async def test_create_tenant_verifier( delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -456,7 +456,7 @@ async def test_update_tenant_verifier_to_issuer( delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{verifier_wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -517,7 +517,7 @@ async def test_get_tenants(tenant_admin_client: RichAsyncClient): delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -562,7 +562,7 @@ async def test_get_tenants_by_group(tenant_admin_client: RichAsyncClient): delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -620,7 +620,7 @@ async def test_get_tenants_by_wallet_name(tenant_admin_client: RichAsyncClient): delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -678,7 +678,7 @@ async def test_get_tenant(tenant_admin_client: RichAsyncClient): delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio @@ -724,7 +724,7 @@ async def test_delete_tenant( response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{wallet_id}?group_id={group_id}" ) - assert response.status_code == 200 + assert response.status_code == 204 # Actor doesn't exist any more actor = await trust_registry.fetch_actor_by_id(wallet_id) @@ -792,7 +792,7 @@ async def test_extra_settings( delete_response = await tenant_admin_client.delete( f"{TENANTS_BASE_PATH}/{created_wallet_id}" ) - assert delete_response.status_code == 200 + assert delete_response.status_code == 204 @pytest.mark.anyio diff --git a/app/tests/routes/admin_tenants/test_get_wallet_auth_token.py b/app/tests/routes/admin_tenants/test_get_wallet_auth_token.py index c93a298b2..855a6062f 100644 --- a/app/tests/routes/admin_tenants/test_get_wallet_auth_token.py +++ b/app/tests/routes/admin_tenants/test_get_wallet_auth_token.py @@ -5,7 +5,7 @@ from fastapi import HTTPException from app.dependencies.acapy_clients import TENANT_ADMIN_AUTHED -from app.routes.admin.tenants import get_wallet_auth_token +from app.routes.admin.tenants import get_wallet_auth_token, post_wallet_auth_token wallet_id = "some_wallet_id" wallet_name = "some_wallet_name" @@ -83,3 +83,77 @@ async def test_get_wallet_auth_token_fail_wrong_group(): admin_controller_mock.multitenancy.get_wallet.assert_awaited_with( wallet_id=wallet_id ) + + +@pytest.mark.anyio +@pytest.mark.parametrize("group_id", [None, "some_group"]) +async def test_post_wallet_auth_token_success(group_id): + # Mock the structure of admin_controller -> multitenancy -> get_auth_token + multitenancy_mock = AsyncMock(get_auth_token=AsyncMock()) + admin_controller_mock = AsyncMock(multitenancy=multitenancy_mock) + + with patch( + "app.routes.admin.tenants.get_wallet_and_assert_valid_group", + return_value=AsyncMock(), + ) as mock_assert_valid_group, patch( + "app.routes.admin.tenants.fetch_actor_by_id", return_value=None + ), patch( + "app.routes.admin.tenants.remove_actor_by_id", return_value=AsyncMock() + ), patch( + "app.routes.admin.tenants.get_tenant_admin_controller" + ) as mock_get_admin_controller: + + # Configure get_tenant_admin_controller to return our mocked admin_controller on enter + mock_get_admin_controller.return_value.__aenter__.return_value = ( + admin_controller_mock + ) + + # Execute the function under test + await post_wallet_auth_token( + wallet_id=wallet_id, group_id=group_id, admin_auth=TENANT_ADMIN_AUTHED + ) + + mock_assert_valid_group.assert_awaited_once_with( + admin_controller=admin_controller_mock, + wallet_id=wallet_id, + group_id=group_id, + logger=ANY, + ) + + # Assert that get_auth_token was called with the correct wallet_id + admin_controller_mock.multitenancy.get_auth_token.assert_awaited_once_with( + wallet_id=wallet_id, body=CreateWalletTokenRequest() + ) + + +@pytest.mark.anyio +async def test_post_wallet_auth_token_fail_wrong_group(): + # Setup wallet return object including the group_id it belongs to + wallet_return_obj = AsyncMock() + wallet_return_obj.settings = {"wallet.group_id": "correct_group"} + + # Mock the get_wallet call to return the above object + multitenancy_mock = AsyncMock(get_wallet=AsyncMock(return_value=wallet_return_obj)) + admin_controller_mock = AsyncMock(multitenancy=multitenancy_mock) + + with patch( + "app.routes.admin.tenants.get_tenant_admin_controller" + ) as mock_get_admin_controller: + + mock_get_admin_controller.return_value.__aenter__.return_value = ( + admin_controller_mock + ) + + # Expect an HTTPException due to group mismatch + with pytest.raises(HTTPException) as exc_info: + await post_wallet_auth_token( + wallet_id=wallet_id, + group_id="wrong_group", + admin_auth=TENANT_ADMIN_AUTHED, + ) + + assert exc_info.value.status_code == 404 + # Ensure the get_wallet was called as expected + admin_controller_mock.multitenancy.get_wallet.assert_awaited_with( + wallet_id=wallet_id + ) diff --git a/scripts/k6/main.js b/scripts/k6/main.js index 5804a73f1..693a3ff3f 100644 --- a/scripts/k6/main.js +++ b/scripts/k6/main.js @@ -299,8 +299,8 @@ export function teardown(data) { for (const issuer of issuers) { const deleteIssuerResponse = deleteTenant(bearerToken, issuer.walletId); check(deleteIssuerResponse, { - "Delete Issuer Tenant Response status code is 200": (r) => { - if (r.status !== 200) { + "Delete Issuer Tenant Response status code is 204": (r) => { + if (r.status !== 204) { console.error( `Unexpected response status while deleting issuer tenant ${issuer.walletId}: ${r.status}` ); @@ -319,8 +319,8 @@ export function teardown(data) { const walletId = getWalletIdByWalletName(bearerToken, wallet.wallet_name); const deleteHolderResponse = deleteTenant(bearerToken, walletId); check(deleteHolderResponse, { - "Delete Holder Tenant Response status code is 200": (r) => { - if (r.status !== 200) { + "Delete Holder Tenant Response status code is 204": (r) => { + if (r.status !== 204) { console.error( `Unexpected response status while deleting holder tenant ${walletId}: ${r.status}` ); diff --git a/scripts/k6/scenarios/delete-holders.js b/scripts/k6/scenarios/delete-holders.js index 12c08176f..efec85ac4 100644 --- a/scripts/k6/scenarios/delete-holders.js +++ b/scripts/k6/scenarios/delete-holders.js @@ -78,8 +78,8 @@ export default function (data) { const walletId = getWalletIdByWalletName(bearerToken, wallet.wallet_name); const deleteHolderResponse = deleteTenant(bearerToken, walletId); check(deleteHolderResponse, { - "Delete Holder Tenant Response status code is 200": (r) => { - if (r.status !== 200) { + "Delete Holder Tenant Response status code is 204": (r) => { + if (r.status !== 204) { console.error( `Unexpected response status while deleting holder tenant ${walletId}: ${r.status}` ); diff --git a/scripts/k6/scenarios/delete-issuers.js b/scripts/k6/scenarios/delete-issuers.js index 5cc0a484e..3b4e6a746 100644 --- a/scripts/k6/scenarios/delete-issuers.js +++ b/scripts/k6/scenarios/delete-issuers.js @@ -74,8 +74,8 @@ export default function (data) { const walletId = getWalletIdByWalletName(bearerToken, wallet.wallet_name); const deleteHolderResponse = deleteTenant(bearerToken, walletId); check(deleteHolderResponse, { - "Delete Issuer Tenant Response status code is 200": (r) => { - if (r.status !== 200) { + "Delete Issuer Tenant Response status code is 204": (r) => { + if (r.status !== 204) { console.error( `Unexpected response status while deleting issuer tenant ${walletId}: ${r.status}` );