From 3404c17c670c693db5898512364bce1954d074cd Mon Sep 17 00:00:00 2001 From: Michael Pisman Date: Sat, 14 Oct 2023 22:59:11 -0600 Subject: [PATCH 1/2] fix: Fixed routes to get/update policies Updated Policy endpoints in Workspace and Group routes: Added account_id query parameter to GET resource/{resource_id}/policies, deleted GET resource/{resource_id}/policy, changed PUT resource/{resource_id}/policy to resource/{resource_id}/policies/{policy_id} and removed all the conditions inside --- src/unipoll_api/routes/group.py | 54 ++++------------------------- src/unipoll_api/routes/workspace.py | 51 ++++----------------------- 2 files changed, 13 insertions(+), 92 deletions(-) diff --git a/src/unipoll_api/routes/group.py b/src/unipoll_api/routes/group.py index 11417b3..798a347 100644 --- a/src/unipoll_api/routes/group.py +++ b/src/unipoll_api/routes/group.py @@ -2,7 +2,6 @@ from typing import Annotated, Literal from fastapi import APIRouter, Body, Depends, HTTPException, Query, status from unipoll_api import dependencies as Dependencies -from unipoll_api import AccountManager from unipoll_api.actions import GroupActions, PermissionsActions, MembersActions, PolicyActions from unipoll_api.exceptions.resource import APIException from unipoll_api.schemas import GroupSchemas, PolicySchemas, MemberSchemas @@ -128,34 +127,22 @@ async def remove_group_member(group: Group = Depends(Dependencies.get_group), # List all policies in the workspace @router.get("/{group_id}/policies", response_description="List of all policies", - response_model=PolicySchemas.PolicyList,) -async def get_group_policies(group: Group = Depends(Dependencies.get_group)) -> PolicySchemas.PolicyList: + response_model=PolicySchemas.PolicyList) +async def get_group_policies(group: Group = Depends(Dependencies.get_group), + account_id: ResourceID = Query(None)) -> PolicySchemas.PolicyList: try: - return await PolicyActions.get_policies(resource=group) - except APIException as e: - raise HTTPException(status_code=e.code, detail=str(e)) - - -# List user's permissions in the group -@router.get("/{group_id}/policy", - response_description="List of all member policies", - response_model=PolicySchemas.PolicyOutput) -async def get_group_policy(group: Group = Depends(Dependencies.get_group), - account_id: ResourceID | None = None): - try: - account = await Dependencies.get_account(account_id) if account_id else AccountManager.active_user.get() - policy_list = await PolicyActions.get_policies(resource=group, policy_holder=account) - policy = policy_list.policies[0] - return PolicySchemas.PolicyOutput(**policy.model_dump()) + account = await Dependencies.get_account(account_id) if account_id else None + return await PolicyActions.get_policies(resource=group, policy_holder=account) except APIException as e: raise HTTPException(status_code=e.code, detail=str(e)) # Set permissions for a user in a group -@router.put("/{group_id}/policy", +@router.put("/{group_id}/policies/{policy_id}", response_description="Updated policy", response_model=PolicySchemas.PolicyOutput) async def set_group_policy(group: Group = Depends(Dependencies.get_group), + policy: Policy = Depends(Dependencies.get_policy), permissions: PolicySchemas.PolicyInput = Body(...)): """ Sets the permissions for a user in a workspace. @@ -166,33 +153,6 @@ async def set_group_policy(group: Group = Depends(Dependencies.get_group), - **permissions** (int): new permissions for the user """ try: - # return await GroupActions.set_group_policy(group, permissions) - - policy = None - if permissions.policy_id: - policy = await Dependencies.get_policy(permissions.policy_id) # type: ignore - elif permissions.account_id: - account = await Dependencies.get_account(permissions.account_id) - # policy = await Policy.find_one(Policy.policy_holder.id == account.id, fetch_links=True) - # Temporarily workaround - policy_list = await PolicyActions.get_policies(resource=group, policy_holder=account) # type: ignore - policy = policy_list.policies[0] # type: ignore - policy = await Policy.get(policy.id, fetch_links=True) # type: ignore - elif permissions.group_id: - # Temporarily workaround - group = await Dependencies.get_group(permissions.group_id) - policy_list = await PolicyActions.get_policies(resource=group, policy_holder=group) # type: ignore - policy = policy_list.policies[0] # type: ignore - policy = await Policy.get(policy.id, fetch_links=True) # type: ignore - else: - account = AccountManager.active_user.get() - policy_list = await PolicyActions.get_policies(resource=group, policy_holder=account) - policy = policy_list.policies[0] - policy = await Policy.get(policy.id, fetch_links=True) - - if not policy: - raise APIException(404, "Policy not found 404") - return await PolicyActions.update_policy(policy, new_permissions=permissions.permissions) except APIException as e: raise HTTPException(status_code=e.code, detail=str(e)) diff --git a/src/unipoll_api/routes/workspace.py b/src/unipoll_api/routes/workspace.py index 2d5ef20..0bf246b 100644 --- a/src/unipoll_api/routes/workspace.py +++ b/src/unipoll_api/routes/workspace.py @@ -6,7 +6,6 @@ from unipoll_api.exceptions.resource import APIException from unipoll_api.documents import Account, Workspace, ResourceID, Policy from unipoll_api.schemas import WorkspaceSchemas, PolicySchemas, GroupSchemas, MemberSchemas, PollSchemas -from unipoll_api import AccountManager # APIRouter creates path operations for user module open_router: APIRouter = APIRouter() @@ -219,33 +218,21 @@ async def remove_workspace_member(workspace: Workspace = Depends(Dependencies.ge @router.get("/{workspace_id}/policies", response_description="List of all policies", response_model=PolicySchemas.PolicyList) -async def get_workspace_policies(workspace: Workspace = Depends(Dependencies.get_workspace)): +async def get_workspace_policies(workspace: Workspace = Depends(Dependencies.get_workspace), + account_id: ResourceID = Query(None)): try: - return await actions.PolicyActions.get_policies(resource=workspace) - except APIException as e: - raise HTTPException(status_code=e.code, detail=str(e)) - - -# List member's permissions in the workspace -@router.get("/{workspace_id}/policy", - response_description="List member policy(permissions)", - response_model=PolicySchemas.PolicyOutput) -async def get_workspace_policy(workspace: Workspace = Depends(Dependencies.get_workspace), - account_id: ResourceID | None = None): - try: - account = await Dependencies.get_account(account_id) if account_id else AccountManager.active_user.get() - policy_list = await actions.PolicyActions.get_policies(resource=workspace, policy_holder=account) - policy = policy_list.policies[0] - return PolicySchemas.PolicyOutput(**policy.model_dump()) + account = await Dependencies.get_account(account_id) if account_id else None + return await actions.PolicyActions.get_policies(resource=workspace, policy_holder=account) except APIException as e: raise HTTPException(status_code=e.code, detail=str(e)) # Set permissions for a member in a workspace -@router.put("/{workspace_id}/policy", +@router.put("/{workspace_id}/policies/{policy_id}", response_description="Updated permissions", response_model=PolicySchemas.PolicyOutput) async def set_workspace_policy(workspace: Workspace = Depends(Dependencies.get_workspace), + policy: Policy = Depends(Dependencies.get_policy), permissions: PolicySchemas.PolicyInput = Body(...)): """ Sets the permissions for a user in a workspace. @@ -258,32 +245,6 @@ async def set_workspace_policy(workspace: Workspace = Depends(Dependencies.get_w Returns the updated workspace. """ try: - # return await WorkspaceActions.set_workspace_policy(workspace, permissions) - policy = None - if permissions.policy_id: - policy = await Dependencies.get_policy(permissions.policy_id) # type: ignore - elif permissions.account_id: - account = await Dependencies.get_account(permissions.account_id) - # policy = await Policy.find_one(Policy.policy_holder.id == account.id, fetch_links=True) - # Temporarily workaround - policy_list = await actions.PolicyActions.get_policies(resource=workspace, policy_holder=account) - policy = policy_list.policies[0] # type: ignore - policy = await Policy.get(policy.id, fetch_links=True) # type: ignore - elif permissions.group_id: - # Temporarily workaround - group = await Dependencies.get_group(permissions.group_id) - policy_list = await actions.PolicyActions.get_policies(resource=workspace, policy_holder=group) - policy = policy_list.policies[0] # type: ignore - policy = await Policy.get(policy.id, fetch_links=True) # type: ignore - else: - account = AccountManager.active_user.get() - policy_list = await actions.PolicyActions.get_policies(resource=workspace, policy_holder=account) - policy = policy_list.policies[0] # type: ignore - policy = await Policy.get(policy.id, fetch_links=True) # type: ignore - - if not policy: - raise APIException(404, "Policy not found 404") - return await actions.PolicyActions.update_policy(policy, new_permissions=permissions.permissions) except APIException as e: raise HTTPException(status_code=e.code, detail=str(e)) From b65dc018971d3f4a1a87811069cd2ba5ea10e10d Mon Sep 17 00:00:00 2001 From: Michael Pisman Date: Sat, 14 Oct 2023 23:00:17 -0600 Subject: [PATCH 2/2] test: Updated policies in workspace/group tests Updated tests to accomodate workspace/group route changes to get/update policies --- tests/test_2_workspaces.py | 50 +++++++++++++++++++++++------------ tests/test_3_groups.py | 54 ++++++++++++++++++++------------------ 2 files changed, 62 insertions(+), 42 deletions(-) diff --git a/tests/test_2_workspaces.py b/tests/test_2_workspaces.py index c031e0a..c5c4490 100644 --- a/tests/test_2_workspaces.py +++ b/tests/test_2_workspaces.py @@ -242,25 +242,30 @@ async def test_get_workspace_members(client_test: AsyncClient): async def test_get_permissions(client_test: AsyncClient): print("\n") - colored_dbg.test_info("Getting list of member permissions in workspace [GET /workspaces/{workspace.id}/policy]") + colored_dbg.test_info("Getting list of member permissions in workspace" + + "[GET /workspaces/{workspace.id}/policies?account_id={account_id}]") workspace = workspaces[0] active_user = accounts[0] # Check permission of the user who created the workspace - response = await client_test.get(f"/workspaces/{workspace.id}/policy", - headers={"Authorization": f"Bearer {active_user.token}"}) + response = await client_test.get(f"/workspaces/{workspace.id}/policies", + headers={"Authorization": f"Bearer {active_user.token}"}, + params={"account_id": str(active_user.id)}) assert response.status_code == status.HTTP_200_OK response = response.json() # Creator of the workspace should have all permissions - assert response["permissions"] == Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|") # type: ignore + + policy = response['policies'][0] + assert policy["permissions"] == Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|") # type: ignore # Check permission of the rest of the members for i in range(1, len(accounts)): - response = await client_test.get(f"/workspaces/{workspace.id}/policy", + response = await client_test.get(f"/workspaces/{workspace.id}/policies", params={"account_id": accounts[i].id}, # type: ignore headers={"Authorization": f"Bearer {active_user.token}"}) response = response.json() - assert response["permissions"] == Permissions.WORKSPACE_BASIC_PERMISSIONS.name.split("|") # type: ignore + policy = response['policies'][0] + assert policy["permissions"] == Permissions.WORKSPACE_BASIC_PERMISSIONS.name.split("|") # type: ignore colored_dbg.test_success("All members have the correct permissions") @@ -340,12 +345,13 @@ async def test_permissions(client_test: AsyncClient): assert res.status_code == status.HTTP_403_FORBIDDEN # Try to get workspace permissions - res = await client_test.get(f"/workspaces/{workspace.id}/policy", headers=headers) + res = await client_test.get(f"/workspaces/{workspace.id}/policies", headers=headers) # assert res.status_code == status.HTTP_403_FORBIDDEN assert res.status_code == status.HTTP_200_OK + policy = res.json()["policies"][0] - # Try to set workspace permissions - res = await client_test.put(f"/workspaces/{workspace.id}/policy", + # # Try to set workspace permissions + res = await client_test.put(f"/workspaces/{workspace.id}/policies/{policy['id']}", json={"permissions": Permissions.WORKSPACE_BASIC_PERMISSIONS.name.split("|")}, headers=headers) assert res.status_code == status.HTTP_403_FORBIDDEN @@ -357,32 +363,42 @@ async def test_permissions(client_test: AsyncClient): async def test_set_permissions(client_test: AsyncClient): print("\n") - colored_dbg.test_info("Setting permissions of workspace members [PUT /workspaces/{workspace.id}/policy]") + colored_dbg.test_info("Setting permissions of members [PUT /workspaces/{workspace.id}/policy/{policy_id}]") active_user = accounts[0] workspace = workspaces[0] + # Get policy of another member + response = await client_test.get(f"/workspaces/{workspace.id}/policies", + params={"account_id": str(accounts[1].id)}, + headers={"Authorization": f"Bearer {active_user.token}"}) + assert response.status_code == status.HTTP_200_OK + response = response.json() + policy = response["policies"][0] + # Update policy of another member - response = await client_test.put(f"/workspaces/{workspace.id}/policy?", - json={"account_id": accounts[1].id, - "permissions": Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|")}, + response = await client_test.put(f"/workspaces/{workspace.id}/policies/{policy['id']}", + json={"permissions": Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|")}, headers={"Authorization": f"Bearer {active_user.token}"}) assert response.status_code == status.HTTP_200_OK response = response.json() assert response["permissions"] == Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|") # Check permissions - response = await client_test.get(f"/workspaces/{workspace.id}/policy?account_id={accounts[1].id}", + response = await client_test.get(f"/workspaces/{workspace.id}/policies", + params={"account_id": str(accounts[1].id)}, headers={"Authorization": f"Bearer {active_user.token}"}) assert response.status_code == status.HTTP_200_OK response = response.json() - assert response["permissions"] == Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|") + policy = response["policies"][0] + assert policy["permissions"] == Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|") # Now the member should be able to get their policy information - response = await client_test.get(f"/workspaces/{workspace.id}/policy", + response = await client_test.get(f"/workspaces/{workspace.id}/policies", headers={"Authorization": f"Bearer {accounts[1].token}"}) assert response.status_code == status.HTTP_200_OK response = response.json() - assert response["permissions"] == Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|") + policy = response["policies"][0] + assert policy["permissions"] == Permissions.WORKSPACE_ALL_PERMISSIONS.name.split("|") colored_dbg.test_success("All members have the correct permissions") diff --git a/tests/test_3_groups.py b/tests/test_3_groups.py index 8dff4f2..06edb4a 100644 --- a/tests/test_3_groups.py +++ b/tests/test_3_groups.py @@ -261,26 +261,29 @@ async def test_get_group_members(client_test: AsyncClient): async def test_get_policy(client_test: AsyncClient): print("\n") - colored_dbg.test_info("Getting list of member permissions in group [GET /groups/{group.id}/policy]") + colored_dbg.test_info("Getting list of member permissions in group [GET /groups/{group.id}/policies]") group = groups[0] active_user = accounts[0] # Check permission of the user who created the group - response = await client_test.get(f"/groups/{group.id}/policy", + response = await client_test.get(f"/groups/{group.id}/policies", + params={"account_id": str(active_user.id)}, headers={"Authorization": f"Bearer {active_user.token}"}) assert response.status_code == status.HTTP_200_OK response = response.json() + policy = response["policies"][0] # Creator of the group should have all permissions - assert response["permissions"] == Permissions.GROUP_ALL_PERMISSIONS.name.split("|") # type: ignore + assert policy["permissions"] == Permissions.GROUP_ALL_PERMISSIONS.name.split("|") # type: ignore # Check permission of the rest of the members students = accounts[1:10] for account in students: - response = await client_test.get(f"/groups/{group.id}/policy", + response = await client_test.get(f"/groups/{group.id}/policies", params={"account_id": account.id}, # type: ignore headers={"Authorization": f"Bearer {active_user.token}"}) response = response.json() - assert response["permissions"] == Permissions.GROUP_BASIC_PERMISSIONS.name.split("|") # type: ignore + policy = response["policies"][0] + assert policy["permissions"] == Permissions.GROUP_BASIC_PERMISSIONS.name.split("|") # type: ignore colored_dbg.test_success("All members have the correct permissions") @@ -342,27 +345,18 @@ async def test_permissions(client_test: AsyncClient): headers=headers) assert res.status_code == status.HTTP_403_FORBIDDEN - # # Try to get group list - # res = await client_test.get(f"/groups/{group.id}/groups", headers=headers) - # assert res.status_code == status.HTTP_403_FORBIDDEN - - # # Try to create group - # res = await client_test.post(f"/groups/{group.id}/groups", - # json={"name": "New group", "description": "New description"}, - # headers=headers) - # assert res.status_code == status.HTTP_403_FORBIDDEN - # Try to delete members from group res = await client_test.delete(f"/groups/{group.id}/members/{accounts[2].id}", headers=headers) assert res.status_code == status.HTTP_403_FORBIDDEN # Try to get group permissions - res = await client_test.get(f"/groups/{group.id}/policy", headers=headers) + res = await client_test.get(f"/groups/{group.id}/policies", headers=headers) assert res.status_code == status.HTTP_200_OK + policy = res.json()["policies"][0] # Try to set group permissions - res = await client_test.put(f"/groups/{group.id}/policy", + res = await client_test.put(f"/groups/{group.id}/policies/{policy['id']}", json={"permissions": Permissions.GROUP_BASIC_PERMISSIONS.name.split("|")}, headers=headers) assert res.status_code == status.HTTP_403_FORBIDDEN @@ -374,35 +368,45 @@ async def test_permissions(client_test: AsyncClient): async def test_set_permissions(client_test: AsyncClient): print("\n") - colored_dbg.test_info("Setting permissions of group members [PUT /groups/{group.id}/policy]") + colored_dbg.test_info("Setting permissions of group members [PUT /groups/{group.id}/policies/{policy.id}]") active_user = accounts[0] group = groups[0] + # Get policy of another member + response = await client_test.get(f"/groups/{group.id}/policies", + params={"account_id": str(accounts[1].id)}, + headers={"Authorization": f"Bearer {active_user.token}"}) + assert response.status_code == status.HTTP_200_OK + response = response.json() + policy = response["policies"][0] + # Update policy of another member - response = await client_test.put(f"/groups/{group.id}/policy?", - json={"account_id": accounts[1].id, - "permissions": Permissions.GROUP_ALL_PERMISSIONS.name.split("|")}, + response = await client_test.put(f"/groups/{group.id}/policies/{policy['id']}", + json={"permissions": Permissions.GROUP_ALL_PERMISSIONS.name.split("|")}, headers={"Authorization": f"Bearer {active_user.token}"}) assert response.status_code == status.HTTP_200_OK response = response.json() assert response["permissions"] == Permissions.GROUP_ALL_PERMISSIONS.name.split("|") # Check permissions - response = await client_test.get(f"/groups/{group.id}/policy?account_id={accounts[1].id}", + response = await client_test.get(f"/groups/{group.id}/policies?account_id={accounts[1].id}", headers={"Authorization": f"Bearer {active_user.token}"}) assert response.status_code == status.HTTP_200_OK response = response.json() - assert response["permissions"] == Permissions.GROUP_ALL_PERMISSIONS.name.split("|") + policy = response["policies"][0] + assert policy["permissions"] == Permissions.GROUP_ALL_PERMISSIONS.name.split("|") # Now the member should be able to get their policy information await test_1_accounts.test_login(client_test, accounts[1]) # Login the active_user colored_dbg.test_success("Signed in under {} {} ({})".format( accounts[1].first_name, accounts[1].last_name, accounts[1].email)) - response = await client_test.get(f"/groups/{group.id}/policy", + response = await client_test.get(f"/groups/{group.id}/policies", + params={"account_id": str(accounts[1].id)}, headers={"Authorization": f"Bearer {accounts[1].token}"}) assert response.status_code == status.HTTP_200_OK response = response.json() - assert response["permissions"] == Permissions.GROUP_ALL_PERMISSIONS.name.split("|") + policy = response["policies"][0] + assert policy["permissions"] == Permissions.GROUP_ALL_PERMISSIONS.name.split("|") colored_dbg.test_success("All members have the correct permissions")