Merge pull request #150 from unisoncomputing/cp/patch-org-membership

Add/remove users from org memberships when roles change

Author: ChrisPenner

src/Share/Web/Share/Orgs/Impl.hs

@@ -4,6 +4,7 @@ module Share.Web.Share.Orgs.Impl (server) where
 
 import Control.Lens
 import Data.Either (isRight)
+import Data.Map qualified as Map
 import Data.Set qualified as Set
 import Servant
 import Servant.Server.Generic
@@ -104,17 +105,39 @@ addRolesEndpoint orgHandle caller (AddRolesRequest {roleAssignments}) = do
   assertNoOrgSubjects roleAssignments
   PG.runTransaction do
     orgRoles <- OrgQ.addOrgRoles orgId roleAssignments
+    let newMembers =
+          (computeOrgMembershipChanges roleAssignments)
+            & Map.filter id
+            & Map.keysSet
+    OrgQ.addOrgMembers orgId newMembers
     ListRolesResponse True . canonicalRoleAssignmentOrdering <$> displaySubjectsOf (traversed . traversed) orgRoles
 
 removeRolesEndpoint :: UserHandle -> UserId -> RemoveRolesRequest -> WebApp ListRolesResponse
 removeRolesEndpoint orgHandle caller (RemoveRolesRequest {roleAssignments}) = do
   orgId <- orgIdByHandle orgHandle
   _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkEditOrgRoles caller orgId
   PG.runTransactionOrRespondError do
+    let updatedUsersMap =
+          roleAssignments
+            & foldMap
+              ( \RoleAssignment {subject} ->
+                  case subject of
+                    UserSubject userId -> Map.singleton userId Set.empty
+                    _ -> Map.empty
+              )
     orgRoles <- OrgQ.removeOrgRoles orgId roleAssignments
     OrgQ.doesOrgHaveOwner orgId >>= \case
       False -> throwError OrgMustHaveOwnerError
       True -> pure ()
+    let remainingRolesMap = computeOrgMembershipChanges orgRoles
+    let usersWithNoRemainingRoles = Map.keysSet updatedUsersMap `Set.difference` Map.keysSet remainingRolesMap
+    let evictedMembers =
+          remainingRolesMap
+            -- Only keep users who should no longer be members
+            & Map.filter not
+            & Map.keysSet
+            & Set.union usersWithNoRemainingRoles
+    OrgQ.removeOrgMembers orgId evictedMembers
 
     ListRolesResponse True . canonicalRoleAssignmentOrdering <$> displaySubjectsOf (traversed . traversed) orgRoles
 
@@ -158,3 +181,35 @@ assertNoOrgSubjects roleAssignments = do
             )
   when hasOrgSubject do
     respondError OrgMemberOfOrgError
+
+-- | This is part of a hack to temporarily fix org membership issues
+-- until we have time for a more robust solution.
+shouldRoleBeOrgMember :: RoleRef -> Bool
+shouldRoleBeOrgMember = \case
+  RoleOrgViewer -> False
+  RoleOrgContributor -> True
+  RoleOrgMaintainer -> True
+  RoleOrgAdmin -> True
+  RoleOrgOwner -> True
+  RoleOrgDefault -> True
+  RoleTeamAdmin -> True
+  RoleProjectViewer -> False
+  RoleProjectContributor -> True
+  RoleProjectMaintainer -> True
+  RoleProjectAdmin -> True
+  RoleProjectOwner -> True
+  RoleProjectPublicAccess -> False
+
+-- | Returns a list of users and whether they should end up as members of the org or not
+computeOrgMembershipChanges :: [RoleAssignment ResolvedAuthSubject] -> Map UserId Bool
+computeOrgMembershipChanges roleAssignments =
+  roleAssignments
+    & foldMap
+      ( \RoleAssignment {subject, roles} ->
+          case subject of
+            UserSubject userId ->
+              let shouldBeMember = any shouldRoleBeOrgMember roles
+               in [(userId, shouldBeMember)]
+            _ -> mempty
+      )
+    & Map.fromListWith (||)

transcripts/share-apis/roles/check-contributor-membership-addition.json

@@ -0,0 +1,35 @@
+{
+  "body": {
+    "members": [
+      {
+        "avatarUrl": "https://www.gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y&d=retro",
+        "handle": "admin",
+        "name": "Admin User",
+        "userId": "U-<UUID>"
+      },
+      {
+        "avatarUrl": null,
+        "handle": "some-user",
+        "name": null,
+        "userId": "U-<UUID>"
+      },
+      {
+        "avatarUrl": null,
+        "handle": "test",
+        "name": null,
+        "userId": "U-<UUID>"
+      },
+      {
+        "avatarUrl": "https://www.gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y&d=retro",
+        "handle": "transcripts",
+        "name": "The Transcript User",
+        "userId": "U-<UUID>"
+      }
+    ]
+  },
+  "status": [
+    {
+      "status_code": 200
+    }
+  ]
+}

transcripts/share-apis/roles/check-contributor-membership-removal.json

@@ -0,0 +1,29 @@
+{
+  "body": {
+    "members": [
+      {
+        "avatarUrl": "https://www.gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y&d=retro",
+        "handle": "admin",
+        "name": "Admin User",
+        "userId": "U-<UUID>"
+      },
+      {
+        "avatarUrl": null,
+        "handle": "test",
+        "name": null,
+        "userId": "U-<UUID>"
+      },
+      {
+        "avatarUrl": "https://www.gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y&d=retro",
+        "handle": "transcripts",
+        "name": "The Transcript User",
+        "userId": "U-<UUID>"
+      }
+    ]
+  },
+  "status": [
+    {
+      "status_code": 200
+    }
+  ]
+}

transcripts/share-apis/roles/maintainer-project-view.json

@@ -20,7 +20,18 @@
     },
     "permissions": [
       "project:view",
-      "project:contribute"
+      "project:contribute",
+      "project:maintain",
+      "project:create",
+      "org:view",
+      "org:edit",
+      "team:view",
+      "notification_hub_entry:view",
+      "notification_hub_entry:update",
+      "notification_subscription:view",
+      "notification_subscription:manage",
+      "notification_delivery_method:view",
+      "notification_delivery_method:manage"
     ],
     "releaseDownloads": [],
     "slug": "privateorgproject",

transcripts/share-apis/roles/run.zsh

@@ -36,6 +36,9 @@ fetch "$test_user" POST grant-project-contributor '/orgs/unison/roles' "
     ]
 }"
 
+# Now the user should be a member of the org
+fetch "$test_user" GET check-contributor-membership-addition '/orgs/unison/members'
+
 fetch "$some_user" GET maintainer-project-view '/users/unison/projects/privateorgproject'
 
 # Remove the user from the org again
@@ -49,3 +52,6 @@ fetch "$test_user" DELETE revoke-project-contributor '/orgs/unison/roles' "
 }"
 
 fetch "$some_user" GET non-maintainer-project-view '/users/unison/projects/privateorgproject'
+
+# Now the user should be no longer be a member of the org
+fetch "$test_user" GET check-contributor-membership-removal '/orgs/unison/members'