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 e9e8a30dd2e..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'); }); }); diff --git a/apps/server/src/modules/authentication/controllers/login.controller.ts b/apps/server/src/modules/authentication/controllers/login.controller.ts index 00f9c0f7bc8..68af396233e 100644 --- a/apps/server/src/modules/authentication/controllers/login.controller.ts +++ b/apps/server/src/modules/authentication/controllers/login.controller.ts @@ -3,7 +3,7 @@ import { AuthGuard } from '@nestjs/passport'; import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; import { ForbiddenOperationError, ValidationError } from '@shared/common'; import { CurrentUser } from '../decorator'; -import type { ICurrentUser, LdapCurrentUser, OauthCurrentUser } from '../interface'; +import type { ICurrentUser, OauthCurrentUser } from '../interface'; import { LoginDto } from '../uc/dto'; import { LoginUc } from '../uc/login.uc'; import { @@ -27,11 +27,8 @@ export class LoginController { @ApiResponse({ status: 200, type: LoginResponse, description: 'Login was successful.' }) @ApiResponse({ status: 400, type: ValidationError, description: 'Request data has invalid format.' }) @ApiResponse({ status: 403, type: ForbiddenOperationError, description: 'Invalid user credentials.' }) - async loginLdap( - @CurrentUser() user: LdapCurrentUser, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - @Body() _: LdapAuthorizationBodyParams - ): Promise { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + async loginLdap(@CurrentUser() user: ICurrentUser, @Body() _: LdapAuthorizationBodyParams): Promise { const loginDto: LoginDto = await this.loginUc.getLoginData(user); const mapped: LoginResponse = LoginResponseMapper.mapToLoginResponse(loginDto); diff --git a/apps/server/src/modules/authentication/interface/index.ts b/apps/server/src/modules/authentication/interface/index.ts index 857059841c4..a9de8109a5b 100644 --- a/apps/server/src/modules/authentication/interface/index.ts +++ b/apps/server/src/modules/authentication/interface/index.ts @@ -1,3 +1,2 @@ export * from './user'; -export * from './ldap-current-user'; export * from './oauth-current-user'; diff --git a/apps/server/src/modules/authentication/interface/ldap-current-user.ts b/apps/server/src/modules/authentication/interface/ldap-current-user.ts deleted file mode 100644 index 4f9991314d4..00000000000 --- a/apps/server/src/modules/authentication/interface/ldap-current-user.ts +++ /dev/null @@ -1,3 +0,0 @@ -import { ICurrentUser } from './user'; - -export interface LdapCurrentUser extends ICurrentUser {} 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 2b2f8e6fbb5..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, + }); }); }); }); @@ -304,33 +340,4 @@ describe('CurrentUserMapper', () => { }); }); }); - - describe('mapToLdapCurrentUser', () => { - const setup = () => { - const user = userFactory.buildWithId({ - school: schoolFactory.buildWithId(), - }); - const systemId = 'mockSystemId'; - - return { - user, - systemId, - }; - }; - - it('should map to ldap current user', () => { - const { user, systemId } = setup(); - - const currentUser: ICurrentUser = CurrentUserMapper.mapToLdapCurrentUser(accountId, user, systemId); - - expect(currentUser).toMatchObject({ - accountId, - systemId, - roles: [], - schoolId: user.school.id, - userId: user.id, - isExternalUser: true, - }); - }); - }); }); 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 5e2baafd5d3..35dd6c5fe7c 100644 --- a/apps/server/src/modules/authentication/mapper/current-user.mapper.ts +++ b/apps/server/src/modules/authentication/mapper/current-user.mapper.ts @@ -2,25 +2,21 @@ import { ValidationError } from '@shared/common'; import { Role, User } from '@shared/domain'; import { RoleReference } from '@shared/domain/domainobject'; import { UserDO } from '@shared/domain/domainobject/user.do'; -import { ICurrentUser, LdapCurrentUser, OauthCurrentUser } from '../interface'; +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, }; } - static mapToLdapCurrentUser(accountId: string, user: User, systemId?: string): LdapCurrentUser { - return { ...CurrentUserMapper.userToICurrentUser(accountId, user, systemId), isExternalUser: true }; - } - static mapToOauthCurrentUser( accountId: string, user: UserDO, diff --git a/apps/server/src/modules/authentication/strategy/ldap.strategy.ts b/apps/server/src/modules/authentication/strategy/ldap.strategy.ts index 02f34fff36b..6f33e92f21a 100644 --- a/apps/server/src/modules/authentication/strategy/ldap.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/ldap.strategy.ts @@ -1,13 +1,13 @@ +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, LdapCurrentUser } from '../interface'; +import { ICurrentUser } from '../interface'; import { CurrentUserMapper } from '../mapper'; import { AuthenticationService } from '../services/authentication.service'; import { LdapService } from '../services/ldap.service'; @@ -48,7 +48,7 @@ export class LdapStrategy extends PassportStrategy(Strategy, 'ldap') { await this.checkCredentials(account, system, ldapDn, password); - const currentUser: LdapCurrentUser = CurrentUserMapper.mapToLdapCurrentUser(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; }