improve user and org deletion

wundergraph · Nov 19, 2024 · 76379ca · 76379ca
1 parent 8beaec0
commit 76379ca
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 57 deletions.
diff --git a/controlplane/src/core/repositories/UserRepository.ts b/controlplane/src/core/repositories/UserRepository.ts
@@ -4,12 +4,12 @@ import { FastifyBaseLogger } from 'fastify';
 import * as schema from '../../db/schema.js';
 import { users } from '../../db/schema.js';
 import { UserDTO } from '../../types/index.js';
+import { BlobStorage } from '../blobstorage/index.js';
 import Keycloak from '../services/Keycloak.js';
 import OidcProvider from '../services/OidcProvider.js';
-import { BlobStorage } from '../blobstorage/index.js';
-import { OrganizationRepository } from './OrganizationRepository.js';
 import { BillingRepository } from './BillingRepository.js';
 import { OidcRepository } from './OidcRepository.js';
+import { OrganizationRepository } from './OrganizationRepository.js';
 
 /**
  * Repository for user related operations.
@@ -71,53 +71,33 @@ export class UserRepository {
     blobStorage: BlobStorage,
   ) {
     const orgRepo = new OrganizationRepository(this.logger, this.db);
-    const billingRepo = new BillingRepository(this.db);
-
-    const { soloAdminSoloMemberOrgs, memberships } = await orgRepo.adminMemberships({ userId: input.id });
+    const oidcRepo = new OidcRepository(this.db);
 
-    // Cancel subscriptions and remove oidc providers
-    for (const org of soloAdminSoloMemberOrgs) {
-      await billingRepo.cancelSubscription(org.id);
-
-      const oidcRepo = new OidcRepository(this.db);
-      const oidcProvider = new OidcProvider();
+    // get all memberships
+    const orgMemberships = await orgRepo.adminMemberships({ userId: input.id });
 
+    // get all providers
+    const oidcProviders = [];
+    for (const org of orgMemberships.soloAdminSoloMemberOrgs) {
       const provider = await oidcRepo.getOidcProvider({ organizationId: org.id });
       if (provider) {
-        await oidcProvider.deleteOidcProvider({
-          kcClient: input.keycloakClient,
-          kcRealm: input.keycloakRealm,
-          organizationSlug: org.slug,
-          alias: provider.alias,
-        });
+        oidcProviders.push({ ...provider, orgSlug: org.slug });
       }
     }
 
-    // Remove keycloak user from all org groups
-    for (const org of memberships) {
-      const orgMember = await orgRepo.getOrganizationMember({
-        organizationID: org.id,
-        userID: input.id,
-      });
-
-      if (!orgMember) {
-        throw new Error('Organization member not found');
-      }
-
-      await input.keycloakClient.removeUserFromOrganization({
-        realm: input.keycloakRealm,
-        userID: input.id,
-        groupName: org.slug,
-        roles: orgMember.roles,
-      });
-    }
-
+    // First perform DB mutations
     await this.db.transaction(async (tx) => {
       const orgRepo = new OrganizationRepository(this.logger, tx);
+      const billingRepo = new BillingRepository(tx);
+
+      // Cancel subscriptions and remove oidc providers
+      for (const org of orgMemberships.soloAdminSoloMemberOrgs) {
+        await billingRepo.cancelSubscription(org.id);
+      }
 
       // Delete all solo organizations of the user
       const deleteOrgs: Promise<void>[] = [];
-      for (const org of soloAdminSoloMemberOrgs) {
+      for (const org of orgMemberships.soloAdminSoloMemberOrgs) {
         deleteOrgs.push(orgRepo.deleteOrganization(org.id, blobStorage));
       }
       await Promise.all(deleteOrgs);
@@ -126,18 +106,67 @@ export class UserRepository {
       await tx.delete(users).where(eq(users.id, input.id)).execute();
     });
 
-    for (const org of soloAdminSoloMemberOrgs) {
-      await input.keycloakClient.deleteOrganizationGroup({
+    // Perform Keycloak deletions.
+    await this.deleteUserFromKeycloak({ ...input, oidcProviders, orgMemberships });
+  }
+
+  private async deleteUserFromKeycloak(input: {
+    id: string;
+    oidcProviders: { alias: string; orgSlug: string }[];
+    orgMemberships: {
+      memberships: { slug: string; roles: string[] }[];
+      soloAdminSoloMemberOrgs: { slug: string }[];
+    };
+    keycloakClient: Keycloak;
+    keycloakRealm: string;
+  }) {
+    try {
+      const oidcProvider = new OidcProvider();
+
+      // Remove OIDC providers
+      for (const provider of input.oidcProviders) {
+        await oidcProvider.deleteOidcProvider({
+          kcClient: input.keycloakClient,
+          kcRealm: input.keycloakRealm,
+          organizationSlug: provider.orgSlug,
+          alias: provider.alias,
+        });
+      }
+
+      // Remove keycloak user from all org groups
+      for (const org of input.orgMemberships.memberships) {
+        await input.keycloakClient.removeUserFromOrganization({
+          realm: input.keycloakRealm,
+          userID: input.id,
+          groupName: org.slug,
+          roles: org.roles,
+        });
+      }
+
+      // Delete keycloak organization groups
+      for (const org of input.orgMemberships.soloAdminSoloMemberOrgs) {
+        await input.keycloakClient.deleteOrganizationGroup({
+          realm: input.keycloakRealm,
+          organizationSlug: org.slug,
+        });
+      }
+
+      // Delete user from keycloak
+      await input.keycloakClient.client.users.del({
+        id: input.id,
         realm: input.keycloakRealm,
-        organizationSlug: org.slug,
       });
+    } catch (e: any) {
+      this.logger.error(
+        {
+          userId: input.id,
+          error: e.message,
+          oidcProviders: input.oidcProviders,
+          ...input.orgMemberships,
+        },
+        'Error deleting user details from keycloak.',
+      );
     }
-
-    // Delete user from keycloak
-    await input.keycloakClient.client.users.del({
-      id: input.id,
-      realm: input.keycloakRealm,
-    });
   }
 
   // only to update the active attribute

diff --git a/controlplane/src/core/services/Keycloak.ts b/controlplane/src/core/services/Keycloak.ts
@@ -278,13 +278,13 @@ export default class Keycloak {
     });
 
     if (orgGroups.length === 0) {
-      throw new Error(`Organization group '${organizationSlug}' not found`);
+      return;
     }
 
     const orgGroup = orgGroups.find((group) => group.name === organizationSlug);
 
     if (!orgGroup) {
-      throw new Error(`Organization group '${organizationSlug}' not found`);
+      return;
     }
 
     await this.client.groups.del({

diff --git a/controlplane/src/core/workers/DeleteOrganizationWorker.ts b/controlplane/src/core/workers/DeleteOrganizationWorker.ts
@@ -92,19 +92,12 @@ class DeleteOrganizationWorker {
         });
       }
 
+      await orgRepo.deleteOrganization(job.data.organizationId, this.input.blobStorage);
+
       await this.input.keycloakClient.deleteOrganizationGroup({
         realm: this.input.keycloakRealm,
         organizationSlug: org.slug,
       });
-
-      await this.input.db.transaction(async (tx) => {
-        const orgRepo = new OrganizationRepository(this.input.logger, tx);
-        const oidcRepo = new OidcRepository(tx);
-
-        await oidcRepo.deleteOidcProvider({ organizationId: job.data.organizationId });
-
-        await orgRepo.deleteOrganization(job.data.organizationId, this.input.blobStorage);
-      });
     } catch (err) {
       this.input.logger.error(err, `Failed to delete organization with id ${job.data.organizationId}`);
       throw err;