From 27398f88f503a6a494ca5f58ae74905060aee70a Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Wed, 4 Oct 2023 14:29:55 +0200 Subject: [PATCH] N21-1219 adds isExternalUser field to ICurrentUser --- .../authentication/interface/jwt-payload.ts | 1 + .../modules/authentication/interface/user.ts | 2 + .../mapper/current-user.mapper.spec.ts | 62 ++++++++++++++++++- .../mapper/current-user.mapper.ts | 10 ++- .../strategy/oauth2.strategy.spec.ts | 1 + .../strategy/oauth2.strategy.ts | 9 ++- .../src/modules/oauth/uc/oauth.uc.spec.ts | 2 +- apps/server/src/modules/oauth/uc/oauth.uc.ts | 7 +++ 8 files changed, 88 insertions(+), 6 deletions(-) diff --git a/apps/server/src/modules/authentication/interface/jwt-payload.ts b/apps/server/src/modules/authentication/interface/jwt-payload.ts index ed69630ccde..9b6d8ce65d0 100644 --- a/apps/server/src/modules/authentication/interface/jwt-payload.ts +++ b/apps/server/src/modules/authentication/interface/jwt-payload.ts @@ -5,6 +5,7 @@ export interface CreateJwtPayload { roles: string[]; systemId?: string; // without this the user needs to change his PW during first login support?: boolean; + isExternalUser?: boolean; } export interface JwtPayload extends CreateJwtPayload { diff --git a/apps/server/src/modules/authentication/interface/user.ts b/apps/server/src/modules/authentication/interface/user.ts index 1baa64d192b..164e8f4024e 100644 --- a/apps/server/src/modules/authentication/interface/user.ts +++ b/apps/server/src/modules/authentication/interface/user.ts @@ -39,4 +39,6 @@ export interface ICurrentUser { /** True if a support member impersonates the user */ impersonated?: boolean; + + isExternalUser: boolean; } diff --git a/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts b/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts index c89d1e3a037..df2510e26c2 100644 --- a/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts +++ b/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts @@ -61,6 +61,7 @@ describe('CurrentUserMapper', () => { describe('when userDO has no ID', () => { it('should throw error', () => { const user: UserDO = userDoFactory.build({ createdAt: new Date(), updatedAt: new Date() }); + expect(() => CurrentUserMapper.userDoToICurrentUser(accountId, user)).toThrow(ValidationError); }); }); @@ -68,7 +69,9 @@ describe('CurrentUserMapper', () => { describe('when userDO is valid', () => { it('should return valid ICurrentUser instance', () => { const user: UserDO = userDoFactory.buildWithId({ id: userId, createdAt: new Date(), updatedAt: new Date() }); + const currentUser = CurrentUserMapper.userDoToICurrentUser(accountId, user); + expect(currentUser).toMatchObject({ accountId, systemId: undefined, @@ -80,10 +83,21 @@ describe('CurrentUserMapper', () => { }); describe('when userDO is valid and a systemId is provided', () => { - it('should return valid ICurrentUser instance with systemId', () => { + const setup = () => { const user: UserDO = userDoFactory.buildWithId({ id: userId, createdAt: new Date(), updatedAt: new Date() }); const systemId = 'mockSystemId'; + + return { + user, + systemId, + }; + }; + + it('should return valid ICurrentUser instance with systemId', () => { + const { user, systemId } = setup(); + const currentUser = CurrentUserMapper.userDoToICurrentUser(accountId, user, systemId); + expect(currentUser).toMatchObject({ accountId, systemId, @@ -132,7 +146,7 @@ describe('CurrentUserMapper', () => { describe('jwtToICurrentUser', () => { describe('when JWT is provided with all claims', () => { - it('should return current user', () => { + const setup = () => { const jwtPayload: JwtPayload = { accountId: 'dummyAccountId', systemId: 'dummySystemId', @@ -140,6 +154,7 @@ describe('CurrentUserMapper', () => { schoolId: 'dummySchoolId', userId: 'dummyUserId', support: true, + isExternalUser: true, sub: 'dummyAccountId', jti: 'random string', aud: 'some audience', @@ -147,7 +162,17 @@ describe('CurrentUserMapper', () => { iat: Math.floor(new Date().getTime() / 1000), exp: Math.floor(new Date().getTime() / 1000) + 3600, }; + + return { + jwtPayload, + }; + }; + + it('should return current user', () => { + const { jwtPayload } = setup(); + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + expect(currentUser).toMatchObject({ accountId: jwtPayload.accountId, systemId: jwtPayload.systemId, @@ -157,9 +182,20 @@ describe('CurrentUserMapper', () => { impersonated: jwtPayload.support, }); }); + + it('should return current user with default for isExternalUser', () => { + const { jwtPayload } = setup(); + + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + + expect(currentUser).toMatchObject({ + isExternalUser: false, + }); + }); }); + describe('when JWT is provided without optional claims', () => { - it('should return current user', () => { + const setup = () => { const jwtPayload: JwtPayload = { accountId: 'dummyAccountId', roles: ['mockRoleId'], @@ -172,7 +208,17 @@ describe('CurrentUserMapper', () => { iat: Math.floor(new Date().getTime() / 1000), exp: Math.floor(new Date().getTime() / 1000) + 3600, }; + + return { + jwtPayload, + }; + }; + + it('should return current user', () => { + const { jwtPayload } = setup(); + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + expect(currentUser).toMatchObject({ accountId: jwtPayload.accountId, roles: [jwtPayload.roles[0]], @@ -180,6 +226,16 @@ describe('CurrentUserMapper', () => { userId: jwtPayload.userId, }); }); + + it('should return current user with default for isExternalUser', () => { + const { jwtPayload } = setup(); + + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + + expect(currentUser).toMatchObject({ + isExternalUser: false, + }); + }); }); }); }); diff --git a/apps/server/src/modules/authentication/mapper/current-user.mapper.ts b/apps/server/src/modules/authentication/mapper/current-user.mapper.ts index d4eb31fbede..f52a989c8f3 100644 --- a/apps/server/src/modules/authentication/mapper/current-user.mapper.ts +++ b/apps/server/src/modules/authentication/mapper/current-user.mapper.ts @@ -13,10 +13,16 @@ export class CurrentUserMapper { roles: user.roles.getItems().map((role: Role) => role.id), schoolId: user.school.id, userId: user.id, + isExternalUser: false, }; } - static userDoToICurrentUser(accountId: string, user: UserDO, systemId?: string): ICurrentUser { + static userDoToICurrentUser( + accountId: string, + user: UserDO, + systemId?: string, + isExternalUser = false + ): ICurrentUser { if (!user.id) { throw new ValidationError('user has no ID'); } @@ -27,6 +33,7 @@ export class CurrentUserMapper { roles: user.roles.map((roleRef: RoleReference) => roleRef.id), schoolId: user.schoolId, userId: user.id, + isExternalUser, }; } @@ -38,6 +45,7 @@ export class CurrentUserMapper { schoolId: jwtPayload.schoolId, userId: jwtPayload.userId, impersonated: jwtPayload.support, + isExternalUser: false, }; } } diff --git a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts index 01feb93b044..8f6b4dc9710 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts @@ -86,6 +86,7 @@ describe('Oauth2Strategy', () => { roles: [user.roles[0].id], schoolId: user.schoolId, accountId: account.id, + isExternalUser: true, }); }); }); diff --git a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts index 2d774594a3d..352e47bd1a2 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts @@ -37,7 +37,14 @@ export class Oauth2Strategy extends PassportStrategy(Strategy, 'oauth2') { throw new UnauthorizedException('no account found'); } - const currentUser: ICurrentUser = CurrentUserMapper.userDoToICurrentUser(account.id, user, systemId); + const isExternalUser = true; + + const currentUser: ICurrentUser = CurrentUserMapper.userDoToICurrentUser( + account.id, + user, + systemId, + isExternalUser + ); return currentUser; } diff --git a/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts b/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts index dba6cca003c..5d894e1f5f2 100644 --- a/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts +++ b/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts @@ -254,7 +254,7 @@ describe('OAuthUc', () => { externalId: 'mockExternalId', }); - const currentUser: ICurrentUser = { userId: 'userId' } as ICurrentUser; + const currentUser: ICurrentUser = { userId: 'userId', isExternalUser: true } as ICurrentUser; const testSystem: SystemDto = new SystemDto({ id: 'mockSystemId', type: 'mock', diff --git a/apps/server/src/modules/oauth/uc/oauth.uc.ts b/apps/server/src/modules/oauth/uc/oauth.uc.ts index e4dd7e68264..9d2fd9bb1de 100644 --- a/apps/server/src/modules/oauth/uc/oauth.uc.ts +++ b/apps/server/src/modules/oauth/uc/oauth.uc.ts @@ -63,6 +63,9 @@ export class OauthUc { return authenticationUrl; } + /** + * @deprecated Please use the oauth2 strategy instead. + */ async processOAuthLogin(cachedState: OauthLoginStateDto, code?: string, error?: string): Promise { const { state, systemId, postLoginRedirect, userLoginMigration } = cachedState; @@ -139,8 +142,12 @@ export class OauthUc { return migrationDto; } + /** + * @deprecated Please use the {@link ICurrentUser} from the specific stragegy instead. + */ private async getJwtForUser(userId: EntityId): Promise { const currentUser: ICurrentUser = await this.userService.getResolvedUser(userId); + currentUser.isExternalUser = true; const { accessToken } = await this.authenticationService.generateJwt(currentUser);