From 33381e6834014743f33866d1c02def649930d466 Mon Sep 17 00:00:00 2001 From: agnisa-cap Date: Wed, 8 Nov 2023 14:09:48 +0100 Subject: [PATCH] N21-1398 hide mail section for ldap (#4535) * makes ldap to external system for skipping email section on first login --- .../controllers/api-test/login.api.spec.ts | 3 +- .../modules/authentication/interface/index.ts | 1 + .../interface/oauth-current-user.ts | 6 ++ .../modules/authentication/interface/user.ts | 7 +- .../mapper/current-user.mapper.spec.ts | 96 +++++++++++++------ .../mapper/current-user.mapper.ts | 4 +- .../strategy/ldap.strategy.spec.ts | 4 +- .../authentication/strategy/ldap.strategy.ts | 4 +- .../authentication/strategy/local.strategy.ts | 2 +- 9 files changed, 83 insertions(+), 44 deletions(-) create mode 100644 apps/server/src/modules/authentication/interface/oauth-current-user.ts diff --git a/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts b/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts index 7da3c21eab9..04683e182a8 100644 --- a/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts +++ b/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts @@ -192,6 +192,7 @@ describe('Login Controller (api)', () => { expect(decodedToken).toHaveProperty('accountId'); expect(decodedToken).toHaveProperty('schoolId'); expect(decodedToken).toHaveProperty('roles'); + expect(decodedToken).toHaveProperty('isExternalUser'); expect(decodedToken).not.toHaveProperty('externalIdToken'); }); }); @@ -287,7 +288,7 @@ describe('Login Controller (api)', () => { roles: [studentRole.id], schoolId: school.id, accountId: account.id, - isExternalUser: false, + isExternalUser: true, }); expect(decodedToken).not.toHaveProperty('externalIdToken'); }); diff --git a/apps/server/src/modules/authentication/interface/index.ts b/apps/server/src/modules/authentication/interface/index.ts index e5abc856509..a9de8109a5b 100644 --- a/apps/server/src/modules/authentication/interface/index.ts +++ b/apps/server/src/modules/authentication/interface/index.ts @@ -1 +1,2 @@ export * from './user'; +export * from './oauth-current-user'; diff --git a/apps/server/src/modules/authentication/interface/oauth-current-user.ts b/apps/server/src/modules/authentication/interface/oauth-current-user.ts new file mode 100644 index 00000000000..ddf15e1ca5d --- /dev/null +++ b/apps/server/src/modules/authentication/interface/oauth-current-user.ts @@ -0,0 +1,6 @@ +import { ICurrentUser } from './user'; + +export interface OauthCurrentUser extends ICurrentUser { + /** Contains the idToken of the external idp. Will be set during oAuth2 login and used for rp initiated logout */ + externalIdToken?: string; +} diff --git a/apps/server/src/modules/authentication/interface/user.ts b/apps/server/src/modules/authentication/interface/user.ts index cc8423f69b7..82b6d292d50 100644 --- a/apps/server/src/modules/authentication/interface/user.ts +++ b/apps/server/src/modules/authentication/interface/user.ts @@ -16,11 +16,6 @@ export interface ICurrentUser { /** True if a support member impersonates the user */ impersonated?: boolean; - /** True if the user is an external user e.g. an oauth user */ + /** True if the user is an external user e.g. an oauth user or ldap user */ isExternalUser: boolean; } - -export interface OauthCurrentUser extends ICurrentUser { - /** Contains the idToken of the external idp. Will be set during oAuth2 login and used for rp initiated logout */ - externalIdToken?: string; -} 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 d06bea6d080..104ca1219a4 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 @@ -15,42 +15,78 @@ describe('CurrentUserMapper', () => { describe('userToICurrentUser', () => { describe('when mapping from a user entity to the current user object', () => { - it('should map with roles', () => { - const teacherRole = roleFactory.buildWithId({ name: RoleName.TEACHER, permissions: [Permission.STUDENT_EDIT] }); - const user = userFactory.buildWithId({ - roles: [teacherRole], - }); - const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(accountId, user); - expect(currentUser).toMatchObject({ - accountId, - systemId: undefined, - roles: [teacherRole.id], - schoolId: null, + describe('when user has roles', () => { + const setup = () => { + const teacherRole = roleFactory.buildWithId({ + name: RoleName.TEACHER, + permissions: [Permission.STUDENT_EDIT], + }); + const user = userFactory.buildWithId({ + roles: [teacherRole], + }); + + return { + teacherRole, + user, + }; + }; + + it('should map with roles', () => { + const { teacherRole, user } = setup(); + + const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(accountId, user, false); + + expect(currentUser).toMatchObject({ + accountId, + systemId: undefined, + roles: [teacherRole.id], + schoolId: null, + isExternalUser: false, + }); }); }); - it('should map without roles', () => { - const user = userFactory.buildWithId(); - const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(accountId, user); - expect(currentUser).toMatchObject({ - accountId, - systemId: undefined, - roles: [], - schoolId: null, + describe('when user has no roles', () => { + it('should map without roles', () => { + const user = userFactory.buildWithId(); + + const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(accountId, user, true); + + expect(currentUser).toMatchObject({ + accountId, + systemId: undefined, + roles: [], + schoolId: null, + isExternalUser: true, + }); }); }); - it('should map system and school', () => { - const user = userFactory.buildWithId({ - school: schoolFactory.buildWithId(), - }); - const systemId = 'mockSystemId'; - const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(accountId, user, systemId); - expect(currentUser).toMatchObject({ - accountId, - systemId, - roles: [], - schoolId: user.school.id, + describe('when systemId is provided', () => { + const setup = () => { + const user = userFactory.buildWithId({ + school: schoolFactory.buildWithId(), + }); + const systemId = 'mockSystemId'; + + return { + user, + systemId, + }; + }; + + it('should map system and school', () => { + const { user, systemId } = setup(); + + const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(accountId, user, false, systemId); + + expect(currentUser).toMatchObject({ + accountId, + systemId, + roles: [], + schoolId: user.school.id, + 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 ab832b70d8c..35dd6c5fe7c 100644 --- a/apps/server/src/modules/authentication/mapper/current-user.mapper.ts +++ b/apps/server/src/modules/authentication/mapper/current-user.mapper.ts @@ -6,14 +6,14 @@ import { ICurrentUser, OauthCurrentUser } from '../interface'; import { CreateJwtPayload, JwtPayload } from '../interface/jwt-payload'; export class CurrentUserMapper { - static userToICurrentUser(accountId: string, user: User, systemId?: string): ICurrentUser { + static userToICurrentUser(accountId: string, user: User, isExternalUser: boolean, systemId?: string): ICurrentUser { return { accountId, systemId, roles: user.roles.getItems().map((role: Role) => role.id), schoolId: user.school.id, userId: user.id, - isExternalUser: false, + isExternalUser, }; } diff --git a/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts b/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts index 78f445ce5b0..5b23be9eb74 100644 --- a/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts @@ -436,7 +436,7 @@ describe('LdapStrategy', () => { schoolId: school.id, systemId: system.id, accountId: account.id, - isExternalUser: false, + isExternalUser: true, }); }); }); @@ -501,7 +501,7 @@ describe('LdapStrategy', () => { schoolId: school.id, systemId: system.id, accountId: account.id, - isExternalUser: false, + isExternalUser: true, }); }); }); diff --git a/apps/server/src/modules/authentication/strategy/ldap.strategy.ts b/apps/server/src/modules/authentication/strategy/ldap.strategy.ts index 1622e434310..6f33e92f21a 100644 --- a/apps/server/src/modules/authentication/strategy/ldap.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/ldap.strategy.ts @@ -1,10 +1,10 @@ +import { AccountDto } from '@modules/account/services/dto'; import { Injectable, UnauthorizedException } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { LegacySchoolDo, SystemEntity, User } from '@shared/domain'; import { LegacySchoolRepo, SystemRepo, UserRepo } from '@shared/repo'; import { ErrorLoggable } from '@src/core/error/loggable/error.loggable'; import { Logger } from '@src/core/logger'; -import { AccountDto } from '@modules/account/services/dto'; import { Strategy } from 'passport-custom'; import { LdapAuthorizationBodyParams } from '../controllers/dto'; import { ICurrentUser } from '../interface'; @@ -48,7 +48,7 @@ export class LdapStrategy extends PassportStrategy(Strategy, 'ldap') { await this.checkCredentials(account, system, ldapDn, password); - const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(account.id, user, systemId); + const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(account.id, user, true, systemId); return currentUser; } diff --git a/apps/server/src/modules/authentication/strategy/local.strategy.ts b/apps/server/src/modules/authentication/strategy/local.strategy.ts index 28ed573b45e..1d31a86d833 100644 --- a/apps/server/src/modules/authentication/strategy/local.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/local.strategy.ts @@ -39,7 +39,7 @@ export class LocalStrategy extends PassportStrategy(Strategy) { new Error(`login failing, because account ${account.id} has no userId`) ); const user = await this.userRepo.findById(accountUserId, true); - const currentUser = CurrentUserMapper.userToICurrentUser(account.id, user); + const currentUser = CurrentUserMapper.userToICurrentUser(account.id, user, false); return currentUser; }