SPSH-983 Send attributes to keycloak (#624)

* Send attributes to keycloak * Fix unrelated files * Fix some tests * Fix coverage * Make external system attributes optional * Remove attributes from keycloak response * Revert some changes
dBildungsplattform · Aug 26, 2024 · c189c83 · c189c83
1 parent 3762006
commit c189c83
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 25 deletions.
diff --git a/src/console/dbseed/domain/db-seed.service.spec.ts b/src/console/dbseed/domain/db-seed.service.spec.ts
@@ -391,6 +391,9 @@ describe('DbSeedService', () => {
                     'testusername',
                     '[email protected]',
                     faker.date.recent(),
+                    {
+                        ID_ITSLEARNING: faker.string.uuid(),
+                    },
                 );
 
                 kcUserService.findOne.mockResolvedValueOnce({ ok: true, value: existingUser });

diff --git a/src/modules/keycloak-administration/domain/keycloak-user.service.spec.ts b/src/modules/keycloak-administration/domain/keycloak-user.service.spec.ts
@@ -69,6 +69,7 @@ describe('KeycloakUserService', () => {
                     createdDate: undefined,
                     username: user.username,
                     email: user.email,
+                    externalSystemIDs: user.externalSystemIDs,
                 });
 
                 expect(res).toStrictEqual<Result<string>>({
@@ -90,6 +91,7 @@ describe('KeycloakUserService', () => {
                         createdDate: undefined,
                         username: user.username,
                         email: user.email,
+                        externalSystemIDs: user.externalSystemIDs,
                     },
                     password,
                 );
@@ -99,6 +101,9 @@ describe('KeycloakUserService', () => {
                     email: user.email,
                     enabled: true,
                     credentials: [{ type: 'password', value: password, temporary: false }],
+                    attributes: {
+                        ID_ITSLEARNING: user.externalSystemIDs.ID_ITSLEARNING,
+                    },
                 });
             });
         });
@@ -120,6 +125,7 @@ describe('KeycloakUserService', () => {
                     createdDate: undefined,
                     username: user.username,
                     email: user.email,
+                    externalSystemIDs: user.externalSystemIDs,
                 });
 
                 expect(res).toStrictEqual<Result<string>>({
@@ -172,6 +178,7 @@ describe('KeycloakUserService', () => {
                         createdDate: undefined,
                         username: user.username,
                         email: user.email,
+                        externalSystemIDs: user.externalSystemIDs,
                     },
                     `{BCRYPT}$2b$12$hqG5T3z8v0Ou8Lmmr2mhW.lNP0DQGO9MS6PQT/CzCJP8Fcx
                     GgKOau`,
@@ -194,6 +201,7 @@ describe('KeycloakUserService', () => {
                         createdDate: undefined,
                         username: user.username,
                         email: user.email,
+                        externalSystemIDs: user.externalSystemIDs,
                     },
                     `{crypt}$6$M.L8yO/PSWLRRhe6$CXj2g0wgWhiAnfROIdqJROrgbjmcmin02M1
                     sM1Z25N7H3puT6qlgsDIM.60brf1csn0Zk9GxS8sILpJvmvFi11`,
@@ -215,6 +223,7 @@ describe('KeycloakUserService', () => {
                         createdDate: undefined,
                         username: user.username,
                         email: user.email,
+                        externalSystemIDs: user.externalSystemIDs,
                     },
                     `{BCRYPT}xxxxxhqG5T3$z8v0Ou8Lmmr2mhW.lNP0DQGO9MS6PQT/CzCJP8Fcx
                     GgKOau`,
@@ -236,6 +245,7 @@ describe('KeycloakUserService', () => {
                         createdDate: undefined,
                         username: user.username,
                         email: user.email,
+                        externalSystemIDs: user.externalSystemIDs,
                     },
                     `{crypt}$$x$$M.L8yO/PSWLRRhe6$CXj2g0wgWhiAnfROIdqJROrgbjmcmin02M1
                     sM1Z25N7H3puT6qlgsDIM.60brf1csn0Zk9GxS8sILpJvmvFi11`,
@@ -257,6 +267,7 @@ describe('KeycloakUserService', () => {
                         createdDate: undefined,
                         username: user.username,
                         email: user.email,
+                        externalSystemIDs: user.externalSystemIDs,
                     },
                     `{notsupported}$6$M.L8yO/PSWLRRhe6$CXj2g0wgWhiAnfROIdqJROrgbjmcmin02M1
                     sM1Z25N7H3puT6qlgsDIM.60brf1csn0Zk9GxS8sILpJvmvFi11`,
@@ -287,6 +298,7 @@ describe('KeycloakUserService', () => {
                         createdDate: undefined,
                         username: user.username,
                         email: user.email,
+                        externalSystemIDs: user.externalSystemIDs,
                     },
                     `{BCRYPT}$2b$12$hqG5T3z8v0Ou8Lmmr2mhW.lNP0DQGO9MS6PQT/CzCJP8Fcx
                     GgKOau`,

diff --git a/src/modules/keycloak-administration/domain/keycloak-user.service.ts b/src/modules/keycloak-administration/domain/keycloak-user.service.ts
@@ -54,6 +54,7 @@ export class KeycloakUserService {
             const userRepresentation: UserRepresentation = {
                 username: user.username,
                 enabled: true,
+                attributes: user.externalSystemIDs,
             };
 
             if (user.email) {
@@ -150,6 +151,7 @@ export class KeycloakUserService {
                         type: 'password',
                     },
                 ],
+                attributes: user.externalSystemIDs,
             };
 
             const response: { id: string } = await kcAdminClientResult.value.users.create(userRepresentation);
@@ -269,6 +271,7 @@ export class KeycloakUserService {
             userReprDto.username,
             userReprDto.email,
             new Date(userReprDto.createdTimestamp),
+            {}, // UserAttributes
         );
 
         return { ok: true, value: userDo };

diff --git a/src/modules/keycloak-administration/domain/user.ts b/src/modules/keycloak-administration/domain/user.ts
@@ -1,21 +1,31 @@
+export type ExternalSystemIDs = {
+    ID_ITSLEARNING?: string;
+};
+
 export class User<WasPersisted extends boolean> {
     private constructor(
         public id: Persisted<string, WasPersisted>,
         public username: string,
         public email: string | undefined,
         public createdDate: Persisted<Date, WasPersisted>,
+        public externalSystemIDs: ExternalSystemIDs,
     ) {}
 
-    public static createNew(username: string, email: string | undefined): User<false> {
-        return new User(undefined, username, email, undefined);
+    public static createNew(
+        username: string,
+        email: string | undefined,
+        externalSystemIDs: ExternalSystemIDs,
+    ): User<false> {
+        return new User(undefined, username, email, undefined, externalSystemIDs);
     }
 
     public static construct<WasPersisted extends boolean = true>(
         id: string,
         username: string,
         email: string | undefined,
         createdDate: Date,
+        externalSystemIDs: ExternalSystemIDs,
     ): User<WasPersisted> {
-        return new User(id, username, email, createdDate);
+        return new User(id, username, email, createdDate, externalSystemIDs);
     }
 }
diff --git a/src/modules/person/persistence/person.repository.integration-spec.ts b/src/modules/person/persistence/person.repository.integration-spec.ts
@@ -516,6 +516,26 @@ describe('PersonRepository Integration', () => {
                 });
             });
         });
+
+        describe('When an unexpected error occurs', () => {
+            it('should rollback transaction and rethrow', async () => {
+                usernameGeneratorService.generateUsername.mockResolvedValue({ ok: true, value: 'testusername' });
+                const person: Person<false> | DomainError = await Person.createNew(usernameGeneratorService, {
+                    familienname: faker.person.lastName(),
+                    vorname: faker.person.firstName(),
+                });
+                if (person instanceof DomainError) {
+                    throw person;
+                }
+
+                const dummyError: Error = new Error('Unexpected');
+                kcUserServiceMock.create.mockRejectedValueOnce(dummyError);
+
+                const promise: Promise<unknown> = sut.create(person);
+
+                await expect(promise).rejects.toBe(dummyError);
+            });
+        });
     });
 
     describe('update', () => {

diff --git a/src/modules/person/persistence/person.repository.ts b/src/modules/person/persistence/person.repository.ts
@@ -1,3 +1,4 @@
+import { randomUUID } from 'node:crypto';
 import { EntityManager, Loaded, RequiredEntityData } from '@mikro-orm/postgresql';
 import { Injectable } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
@@ -231,23 +232,50 @@ export class PersonRepository {
     }
 
     public async create(person: Person<false>, hashedPassword?: string): Promise<Person<true> | DomainError> {
-        let personWithKeycloakUser: Person<false> | DomainError;
-        if (!hashedPassword) {
-            personWithKeycloakUser = await this.createKeycloakUser(person, this.kcUserService);
-        } else {
-            personWithKeycloakUser = await this.createKeycloakUserWithHashedPassword(
-                person,
-                hashedPassword,
-                this.kcUserService,
-            );
-        }
-        if (personWithKeycloakUser instanceof DomainError) {
-            return personWithKeycloakUser;
-        }
-        const personEntity: PersonEntity = this.em.create(PersonEntity, mapAggregateToData(personWithKeycloakUser));
-        await this.em.persistAndFlush(personEntity);
+        const transaction: EntityManager = this.em.fork();
+        await transaction.begin();
+
+        try {
+            // Create DB person
+            const personEntity: PersonEntity = transaction.create(PersonEntity, mapAggregateToData(person)).assign({
+                id: randomUUID(), // Generate ID here instead of at insert-time
+            });
+            transaction.persist(personEntity);
+
+            const persistedPerson: Person<true> = mapEntityToAggregateInplace(personEntity, person);
+
+            // Take ID from person to create keycloak user
+            let personWithKeycloakUser: Person<true> | DomainError;
+            if (!hashedPassword) {
+                personWithKeycloakUser = await this.createKeycloakUser(persistedPerson, this.kcUserService);
+            } else {
+                personWithKeycloakUser = await this.createKeycloakUserWithHashedPassword(
+                    persistedPerson,
+                    hashedPassword,
+                    this.kcUserService,
+                );
+            }
 
-        return mapEntityToAggregateInplace(personEntity, personWithKeycloakUser);
+            // -> When keycloak fails, rollback
+            if (personWithKeycloakUser instanceof DomainError) {
+                await transaction.rollback();
+                return personWithKeycloakUser;
+            }
+
+            // take ID from keycloak and update user
+            personEntity.assign(mapAggregateToData(personWithKeycloakUser));
+
+            // Commit
+            await transaction.commit();
+
+            // Return mapped person
+            return mapEntityToAggregateInplace(personEntity, personWithKeycloakUser);
+        } catch (e) {
+            // Any other errors
+            // -> rollback and rethrow
+            await transaction.rollback();
+            throw e;
+        }
     }
 
     public async update(person: Person<true>): Promise<Person<true> | DomainError> {
@@ -287,15 +315,17 @@ export class PersonRepository {
     }
 
     private async createKeycloakUser(
-        person: Person<boolean>,
+        person: Person<true>,
         kcUserService: KeycloakUserService,
-    ): Promise<Person<boolean> | DomainError> {
+    ): Promise<Person<true> | DomainError> {
         if (person.keycloakUserId || !person.newPassword || !person.username) {
             return new EntityCouldNotBeCreated('Person');
         }
 
         person.referrer = person.username;
-        const userDo: User<false> = User.createNew(person.username, undefined);
+        const userDo: User<false> = User.createNew(person.username, undefined, {
+            ID_ITSLEARNING: person.id,
+        });
 
         const creationResult: Result<string, DomainError> = await kcUserService.create(userDo);
         if (!creationResult.ok) {
@@ -324,15 +354,17 @@ export class PersonRepository {
     }
 
     private async createKeycloakUserWithHashedPassword(
-        person: Person<boolean>,
+        person: Person<true>,
         hashedPassword: string,
         kcUserService: KeycloakUserService,
-    ): Promise<Person<boolean> | DomainError> {
+    ): Promise<Person<true> | DomainError> {
         if (person.keycloakUserId || !person.username) {
             return new EntityCouldNotBeCreated('Person');
         }
         person.referrer = person.username;
-        const userDo: User<false> = User.createNew(person.username, undefined);
+        const userDo: User<false> = User.createNew(person.username, undefined, {
+            ID_ITSLEARNING: person.id,
+        });
 
         const creationResult: Result<string, DomainError> = await kcUserService.createWithHashedPassword(
             userDo,

diff --git a/test/utils/do-factory.ts b/test/utils/do-factory.ts
@@ -79,6 +79,7 @@ export class DoFactory {
             createdDate: withId ? faker.date.past() : undefined,
             username: faker.internet.userName(),
             email: faker.internet.email(),
+            externalSystemIDs: {},
         };
 
         return Object.assign(Object.create(User.prototype) as User<boolean>, user, props);