Delete owned user on member removal (#19215)

components/dashboard/src/components/error-boundaries/QueryErrorBoundary.tsx

@@ -11,6 +11,7 @@ import { ErrorBoundary, FallbackProps } from "react-error-boundary";
 import { hasLoggedInBefore, Login } from "../../Login";
 import { isGitpodIo } from "../../utils";
 import { CaughtError } from "./ReloadPageErrorBoundary";
+import { gitpodHostUrl } from "../../service/service";
 
 // Error boundary intended to catch and handle expected errors from api calls
 export const QueryErrorBoundary: FC = ({ children }) => {
@@ -32,6 +33,29 @@ const ExpectedQueryErrorsFallback: FC<FallbackProps> = ({ error, resetErrorBound
     // adjust typing, as we may have caught an api error here w/ a code property
     const caughtError = error as CaughtError;
 
+    // user deleted needs a n explicit logout to destroy the session
+    if (caughtError.code === ErrorCodes.USER_DELETED) {
+        console.log("clearing query cache for deleted user");
+        client.clear();
+
+        // redirect to <domain>/logout
+        const loginUrl = gitpodHostUrl
+            .withApi({
+                pathname: "/login",
+                search: `returnTo=${encodeURIComponent(window.location.href)}`,
+            })
+            .toString();
+
+        const logoutUrl = gitpodHostUrl
+            .withApi({
+                pathname: "/logout",
+                search: `returnTo=${encodeURIComponent(loginUrl)}`,
+            })
+            .toString();
+        window.location.href = logoutUrl;
+        return <div></div>;
+    }
+
     // User needs to Login
     if (caughtError.code === ErrorCodes.NOT_AUTHENTICATED) {
         console.log("clearing query cache for unauthenticated user");

components/dashboard/src/data/organizations/members-query.ts

@@ -33,7 +33,6 @@ export function useListOrganizationMembers() {
             return response.members;
         },
         {
-            staleTime: 1000 * 60 * 5, // 5 minutes
             enabled: !!organizationId,
         },
     );

components/dashboard/src/hooks/use-user-loader.ts

@@ -33,7 +33,7 @@ export const useUserLoader = () => {
             if (!doRetryUserLoader) {
                 return false;
             }
-            return error.code !== ErrorCodes.NOT_AUTHENTICATED;
+            return error.code !== ErrorCodes.NOT_AUTHENTICATED && error.code !== ErrorCodes.USER_DELETED;
         },
         // docs: https://tanstack.com/query/v4/docs/react/guides/query-retries
         // backoff by doubling, max. 10s

components/dashboard/src/teams/Members.tsx

@@ -36,6 +36,7 @@ export default function MembersPage() {
     const [showInviteModal, setShowInviteModal] = useState<boolean>(false);
     const [searchText, setSearchText] = useState<string>("");
     const [roleFilter, setRoleFilter] = useState<OrganizationRole | undefined>();
+    const [memberToRemove, setMemberToRemove] = useState<OrganizationMember | undefined>(undefined);
     const inviteId = useInvitationId().data;
 
     const inviteUrl = useMemo(() => {
@@ -67,11 +68,6 @@ export default function MembersPage() {
         invalidateMembers();
     };
 
-    const removeTeamMember = async (userId: string) => {
-        await organizationClient.deleteOrganizationMember({ organizationId: org.data?.id, userId });
-        invalidateMembers();
-    };
-
     const isRemainingOwner = useMemo(() => {
         const owners = members.filter((m) => m.role === OrganizationRole.OWNER);
         return owners?.length === 1 && owners[0].userId === user?.id;
@@ -252,8 +248,7 @@ export default function MembersPage() {
                                                           customFontStyle: !isRemainingOwner
                                                               ? "text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300"
                                                               : "text-gray-400 dark:text-gray-200",
-                                                          onClick: () =>
-                                                              !isRemainingOwner && removeTeamMember(m.userId),
+                                                          onClick: () => !isRemainingOwner && setMemberToRemove(m),
                                                       },
                                                   ]
                                                 : isOwner
@@ -262,7 +257,7 @@ export default function MembersPage() {
                                                           title: "Remove",
                                                           customFontStyle:
                                                               "text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300",
-                                                          onClick: () => removeTeamMember(m.userId),
+                                                          onClick: () => setMemberToRemove(m),
                                                       },
                                                   ]
                                                 : []
@@ -295,6 +290,38 @@ export default function MembersPage() {
                     </ModalFooter>
                 </Modal>
             )}
+            {memberToRemove && (
+                // TODO: Use title and buttons props
+                <Modal visible={true} onClose={() => setMemberToRemove(undefined)}>
+                    <ModalHeader>Remove Members</ModalHeader>
+                    <ModalBody>
+                        You are about to remove <b>{memberToRemove.fullName}</b> from this organization.
+                        <br />
+                        <br />
+                        {memberToRemove.ownedByOrganization ? (
+                            <>This will delete the user account and all associated data.</>
+                        ) : null}
+                    </ModalBody>
+                    <ModalFooter>
+                        <Button variant="secondary" onClick={() => setMemberToRemove(undefined)}>
+                            Cancel
+                        </Button>
+                        <Button
+                            variant="default"
+                            onClick={async () => {
+                                await organizationClient.deleteOrganizationMember({
+                                    organizationId: org.data?.id,
+                                    userId: memberToRemove.userId,
+                                });
+                                invalidateMembers();
+                                setMemberToRemove(undefined);
+                            }}
+                        >
+                            Remove
+                        </Button>
+                    </ModalFooter>
+                </Modal>
+            )}
         </>
     );
 }

components/server/src/orgs/organization-service.spec.db.ts

@@ -23,6 +23,7 @@ const expect = chai.expect;
 describe("OrganizationService", async () => {
     let container: Container;
     let os: OrganizationService;
+    let userService: UserService;
 
     let owner: User;
     let member: User;
@@ -45,7 +46,7 @@ describe("OrganizationService", async () => {
                 }
             });
         os = container.get(OrganizationService);
-        const userService = container.get<UserService>(UserService);
+        userService = container.get<UserService>(UserService);
         owner = await userService.createUser({
             identity: {
                 authId: "github|1234",
@@ -158,6 +159,30 @@ describe("OrganizationService", async () => {
         await expectError(ErrorCodes.NOT_FOUND, os.removeOrganizationMember(member.id, org.id, member.id));
     });
 
+    it("should delete owned user when removing it", async () => {
+        const ownedMember = await userService.createUser({
+            organizationId: org.id,
+            identity: {
+                authId: "github|1234",
+                authName: "github",
+                authProviderId: "github",
+            },
+        });
+        await os.addOrUpdateMember(owner.id, org.id, ownedMember.id, "member");
+
+        const members = await os.listMembers(owner.id, org.id);
+        expect(members.some((m) => m.userId === ownedMember.id)).to.be.true;
+
+        // remove it and assert it's gone
+        await os.removeOrganizationMember(owner.id, org.id, ownedMember.id);
+        const members2 = await os.listMembers(owner.id, org.id);
+        expect(members2.some((m) => m.userId === ownedMember.id)).to.be.false;
+        // also assert that the user is gone
+        const deleted = await userService.findUserById(ownedMember.id, ownedMember.id);
+        // await expectError(ErrorCodes.NOT_FOUND, () => deleted);
+        expect(deleted.markedDeleted).to.be.true;
+    });
+
     it("should listOrganizationsByMember", async () => {
         await os.createOrganization(owner.id, "org1");
         await os.createOrganization(owner.id, "org2");

components/server/src/orgs/organization-service.ts

@@ -21,12 +21,14 @@ import { ProjectsService } from "../projects/projects-service";
 import { TransactionalContext } from "@gitpod/gitpod-db/lib/typeorm/transactional-db-impl";
 import { DefaultWorkspaceImageValidator } from "./default-workspace-image-validator";
 import { getPrimaryEmail } from "@gitpod/public-api-common/lib/user-utils";
+import { UserService } from "../user/user-service";
 
 @injectable()
 export class OrganizationService {
     constructor(
         @inject(TeamDB) private readonly teamDB: TeamDB,
         @inject(UserDB) private readonly userDB: UserDB,
+        @inject(UserService) private readonly userService: UserService,
         @inject(ProjectsService) private readonly projectsService: ProjectsService,
         @inject(Authorizer) private readonly auth: Authorizer,
         @inject(IAnalyticsWriter) private readonly analytics: IAnalyticsWriter,
@@ -344,12 +346,8 @@ export class OrganizationService {
                 }
                 // Only invited members can be removed from the Org, but organizational accounts cannot.
                 if (userToBeRemoved.organizationId && orgId === userToBeRemoved.organizationId) {
-                    throw new ApplicationError(
-                        ErrorCodes.PERMISSION_DENIED,
-                        `User's account '${memberId}' belongs to the organization '${orgId}'`,
-                    );
+                    await this.userService.deleteUser(userId, memberId);
                 }
-
                 await db.removeMemberFromTeam(userToBeRemoved.id, orgId);
                 await this.auth.removeOrganizationRole(orgId, memberId, "member");
             });