diff --git a/apps/server/src/modules/authentication/interface/jwt-payload.ts b/apps/server/src/modules/authentication/interface/jwt-payload.ts index aad11700e60..ca46acbe761 100644 --- a/apps/server/src/modules/authentication/interface/jwt-payload.ts +++ b/apps/server/src/modules/authentication/interface/jwt-payload.ts @@ -6,6 +6,7 @@ export interface CreateJwtPayload { systemId?: string; // without this the user needs to change his PW during first login support?: boolean; // support UserId is missed see featherJS + 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 a070367a43b..cc8423f69b7 100644 --- a/apps/server/src/modules/authentication/interface/user.ts +++ b/apps/server/src/modules/authentication/interface/user.ts @@ -15,6 +15,9 @@ 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 */ + isExternalUser: boolean; } export interface OauthCurrentUser 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 09a76c1ebfb..d06bea6d080 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.mapToOauthCurrentUser(accountId, user, undefined, 'idToken')).toThrow( ValidationError ); @@ -100,6 +101,7 @@ describe('CurrentUserMapper', () => { schoolId: user.schoolId, userId, externalIdToken: idToken, + isExternalUser: true, }); }); }); @@ -139,6 +141,7 @@ describe('CurrentUserMapper', () => { schoolId: user.schoolId, userId, externalIdToken: idToken, + isExternalUser: true, }); }); }); @@ -181,7 +184,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', @@ -189,6 +192,7 @@ describe('CurrentUserMapper', () => { schoolId: 'dummySchoolId', userId: 'dummyUserId', support: true, + isExternalUser: true, sub: 'dummyAccountId', jti: 'random string', aud: 'some audience', @@ -196,7 +200,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, @@ -206,15 +220,26 @@ 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: jwtPayload.isExternalUser, + }); + }); }); describe('when JWT is provided without optional claims', () => { - it('should return current user', () => { + const setup = () => { const jwtPayload: JwtPayload = { accountId: 'dummyAccountId', roles: ['mockRoleId'], schoolId: 'dummySchoolId', userId: 'dummyUserId', + isExternalUser: false, sub: 'dummyAccountId', jti: 'random string', aud: 'some audience', @@ -222,12 +247,33 @@ 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]], schoolId: jwtPayload.schoolId, userId: jwtPayload.userId, + isExternalUser: false, + }); + }); + + it('should return current user with default for isExternalUser', () => { + const { jwtPayload } = setup(); + + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + + expect(currentUser).toMatchObject({ + isExternalUser: false, }); }); }); @@ -242,6 +288,7 @@ describe('CurrentUserMapper', () => { schoolId: 'dummySchoolId', userId: 'dummyUserId', impersonated: true, + isExternalUser: false, }; const createJwtPayload: CreateJwtPayload = CurrentUserMapper.mapCurrentUserToCreateJwtPayload(currentUser); @@ -253,6 +300,7 @@ describe('CurrentUserMapper', () => { schoolId: currentUser.schoolId, userId: currentUser.userId, support: currentUser.impersonated, + 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 80ca91b56b0..ab832b70d8c 100644 --- a/apps/server/src/modules/authentication/mapper/current-user.mapper.ts +++ b/apps/server/src/modules/authentication/mapper/current-user.mapper.ts @@ -13,6 +13,7 @@ export class CurrentUserMapper { roles: user.roles.getItems().map((role: Role) => role.id), schoolId: user.school.id, userId: user.id, + isExternalUser: false, }; } @@ -33,6 +34,7 @@ export class CurrentUserMapper { schoolId: user.schoolId, userId: user.id, externalIdToken, + isExternalUser: true, }; } @@ -44,6 +46,7 @@ export class CurrentUserMapper { roles: currentUser.roles, systemId: currentUser.systemId, support: currentUser.impersonated, + isExternalUser: currentUser.isExternalUser, }; } @@ -55,6 +58,7 @@ export class CurrentUserMapper { schoolId: jwtPayload.schoolId, userId: jwtPayload.userId, impersonated: jwtPayload.support, + isExternalUser: jwtPayload.isExternalUser, }; } } diff --git a/apps/server/src/modules/authentication/services/authentication.service.spec.ts b/apps/server/src/modules/authentication/services/authentication.service.spec.ts index 3d5b6d3a1b7..1e5c69ecfb1 100644 --- a/apps/server/src/modules/authentication/services/authentication.service.spec.ts +++ b/apps/server/src/modules/authentication/services/authentication.service.spec.ts @@ -99,6 +99,7 @@ describe('AuthenticationService', () => { roles: ['student'], schoolId: 'mockSchoolId', userId: 'mockUserId', + isExternalUser: false, }; await authenticationService.generateJwt(mockCurrentUser); expect(jwtService.sign).toBeCalledWith( 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 d686dfcac72..78f445ce5b0 100644 --- a/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts @@ -436,6 +436,7 @@ describe('LdapStrategy', () => { schoolId: school.id, systemId: system.id, accountId: account.id, + isExternalUser: false, }); }); }); @@ -500,6 +501,7 @@ describe('LdapStrategy', () => { schoolId: school.id, systemId: system.id, accountId: account.id, + 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 8fd8f096dbe..f67f620175d 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts @@ -88,6 +88,7 @@ describe('Oauth2Strategy', () => { schoolId: user.schoolId, accountId: account.id, externalIdToken: idToken, + isExternalUser: true, }); }); }); diff --git a/apps/server/src/modules/authentication/uc/login.uc.spec.ts b/apps/server/src/modules/authentication/uc/login.uc.spec.ts index a14b741ae88..c0f1d924876 100644 --- a/apps/server/src/modules/authentication/uc/login.uc.spec.ts +++ b/apps/server/src/modules/authentication/uc/login.uc.spec.ts @@ -35,6 +35,7 @@ describe('LoginUc', () => { userId: '', systemId: '', impersonated: false, + isExternalUser: false, someProperty: 'shouldNotBeMapped', }; const loginDto: LoginDto = new LoginDto({ accessToken: 'accessToken' }); @@ -58,6 +59,7 @@ describe('LoginUc', () => { roles: userInfo.roles, systemId: userInfo.systemId, support: userInfo.impersonated, + isExternalUser: userInfo.isExternalUser, }); }); 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 4323cd5bc85..1e888abd5f1 100644 --- a/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts +++ b/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts @@ -1,17 +1,9 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { UnauthorizedException, UnprocessableEntityException } from '@nestjs/common'; -import { Test, TestingModule } from '@nestjs/testing'; -import { LegacySchoolDo, UserDO } from '@shared/domain'; -import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; -import { ISession } from '@shared/domain/types/session'; -import { legacySchoolDoFactory, setupEntities } from '@shared/testing'; -import { LegacyLogger } from '@src/core/logger'; -import { ICurrentUser } from '@modules/authentication'; import { AuthenticationService } from '@modules/authentication/services/authentication.service'; +import { LegacySchoolService } from '@modules/legacy-school'; import { OauthUc } from '@modules/oauth/uc/oauth.uc'; import { ProvisioningService } from '@modules/provisioning'; import { ExternalUserDto, OauthDataDto, ProvisioningSystemDto } from '@modules/provisioning/dto'; -import { LegacySchoolService } from '@modules/legacy-school'; import { SystemService } from '@modules/system'; import { OauthConfigDto, SystemDto } from '@modules/system/service'; import { UserService } from '@modules/user'; @@ -19,9 +11,17 @@ import { UserMigrationService } from '@modules/user-login-migration'; import { OAuthMigrationError } from '@modules/user-login-migration/error/oauth-migration.error'; import { SchoolMigrationService } from '@modules/user-login-migration/service'; import { MigrationDto } from '@modules/user-login-migration/service/dto'; -import { OAuthSSOError } from '../loggable/oauth-sso.error'; +import { UnauthorizedException, UnprocessableEntityException } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { LegacySchoolDo, UserDO } from '@shared/domain'; +import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; +import { ISession } from '@shared/domain/types/session'; +import { legacySchoolDoFactory, setupEntities } from '@shared/testing'; +import { LegacyLogger } from '@src/core/logger'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { AuthorizationParams } from '../controller/dto'; import { OAuthTokenDto } from '../interface'; +import { OAuthSSOError } from '../loggable/oauth-sso.error'; import { OAuthProcessDto } from '../service/dto'; import { OAuthService } from '../service/oauth.service'; import { OauthLoginStateDto } from './dto/oauth-login-state.dto'; @@ -254,7 +254,7 @@ describe('OAuthUc', () => { externalId: 'mockExternalId', }); - const currentUser: ICurrentUser = { userId: 'userId' } as ICurrentUser; + const currentUser: OauthCurrentUser = { userId: 'userId', isExternalUser: true } as OauthCurrentUser; 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 53d986bf029..c495e7be05d 100644 --- a/apps/server/src/modules/oauth/uc/oauth.uc.ts +++ b/apps/server/src/modules/oauth/uc/oauth.uc.ts @@ -2,7 +2,6 @@ import { Injectable, UnauthorizedException, UnprocessableEntityException } from import { EntityId, LegacySchoolDo, UserDO } from '@shared/domain'; import { ISession } from '@shared/domain/types/session'; import { LegacyLogger } from '@src/core/logger'; -import { ICurrentUser } from '@modules/authentication'; import { AuthenticationService } from '@modules/authentication/services/authentication.service'; import { ProvisioningService } from '@modules/provisioning'; import { OauthDataDto } from '@modules/provisioning/dto'; @@ -13,6 +12,7 @@ import { UserMigrationService } from '@modules/user-login-migration'; import { SchoolMigrationService } from '@modules/user-login-migration/service'; import { MigrationDto } from '@modules/user-login-migration/service/dto'; import { nanoid } from 'nanoid'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { AuthorizationParams } from '../controller/dto'; import { OAuthTokenDto } from '../interface'; import { OAuthProcessDto } from '../service/dto'; @@ -140,9 +140,9 @@ export class OauthUc { } private async getJwtForUser(userId: EntityId): Promise { - const currentUser: ICurrentUser = await this.userService.getResolvedUser(userId); + const oauthCurrentUser: OauthCurrentUser = await this.userService.getResolvedUser(userId); - const { accessToken } = await this.authenticationService.generateJwt(currentUser); + const { accessToken } = await this.authenticationService.generateJwt(oauthCurrentUser); return accessToken; } diff --git a/apps/server/src/modules/user/service/user.service.spec.ts b/apps/server/src/modules/user/service/user.service.spec.ts index 1b208764c8f..223fa1f0c88 100644 --- a/apps/server/src/modules/user/service/user.service.spec.ts +++ b/apps/server/src/modules/user/service/user.service.spec.ts @@ -9,10 +9,10 @@ import { UserDORepo } from '@shared/repo/user/user-do.repo'; import { roleFactory, setupEntities, userDoFactory, userFactory } from '@shared/testing'; import { AccountService } from '@modules/account/services/account.service'; import { AccountDto } from '@modules/account/services/dto'; -import { ICurrentUser } from '@modules/authentication'; import { RoleService } from '@modules/role/service/role.service'; import { UserService } from '@modules/user/service/user.service'; import { UserDto } from '@modules/user/uc/dto/user.dto'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { UserQuery } from './user-query.type'; describe('UserService', () => { @@ -136,13 +136,13 @@ describe('UserService', () => { describe('getResolvedUser is called', () => { describe('when a resolved user is requested', () => { - it('should return an ICurrentUser', async () => { + const setup = () => { const systemId = 'systemId'; const role: Role = roleFactory.buildWithId({ name: RoleName.STUDENT, permissions: [Permission.DASHBOARD_VIEW], }); - const user: User = userFactory.buildWithId({ roles: [role] }); + const user: UserDO = userDoFactory.buildWithId({ roles: [role] }); const account: AccountDto = new AccountDto({ id: 'accountId', systemId, @@ -152,17 +152,30 @@ describe('UserService', () => { activated: true, }); - userRepo.findById.mockResolvedValue(user); + userDORepo.findById.mockResolvedValue(user); accountService.findByUserIdOrFail.mockResolvedValue(account); - const result: ICurrentUser = await service.getResolvedUser(user.id); + return { + userId: user.id as string, + user, + account, + role, + systemId, + }; + }; + + it('should return the current user', async () => { + const { userId, user, account, role, systemId } = setup(); - expect(result).toEqual({ - userId: user.id, + const result: OauthCurrentUser = await service.getResolvedUser(userId); + + expect(result).toEqual({ + userId, systemId, - schoolId: user.school.id, + schoolId: user.schoolId, accountId: account.id, roles: [role.id], + isExternalUser: true, }); }); }); @@ -177,30 +190,24 @@ describe('UserService', () => { }); it('should return only the last name when the user has a protected role', async () => { - // Arrange const user: UserDO = userDoFactory.withRoles([{ id: role.id, name: RoleName.STUDENT }]).buildWithId({ lastName: 'lastName', }); - // Act const result: string = await service.getDisplayName(user); - // Assert expect(result).toEqual(user.lastName); expect(roleService.getProtectedRoles).toHaveBeenCalled(); }); it('should return the first name and last name when the user has no protected role', async () => { - // Arrange const user: UserDO = userDoFactory.withRoles([{ id: 'unprotectedId', name: RoleName.STUDENT }]).buildWithId({ lastName: 'lastName', firstName: 'firstName', }); - // Act const result: string = await service.getDisplayName(user); - // Assert expect(result).toEqual(`${user.firstName} ${user.lastName}`); expect(roleService.getProtectedRoles).toHaveBeenCalled(); }); diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index cc15404fc63..2cc95991f96 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -1,16 +1,16 @@ -import { ConfigService } from '@nestjs/config'; -import { EntityId, IFindOptions, LanguageType, User } from '@shared/domain'; -import { RoleReference, Page, UserDO } from '@shared/domain/domainobject'; -import { UserRepo } from '@shared/repo'; -import { UserDORepo } from '@shared/repo/user/user-do.repo'; import { AccountService } from '@modules/account'; import { AccountDto } from '@modules/account/services/dto'; -import { ICurrentUser } from '@modules/authentication'; // invalid import import { CurrentUserMapper } from '@modules/authentication/mapper'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { RoleService } from '@modules/role/service/role.service'; import { BadRequestException, Injectable } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { EntityId, IFindOptions, LanguageType, User } from '@shared/domain'; +import { Page, RoleReference, UserDO } from '@shared/domain/domainobject'; +import { UserRepo } from '@shared/repo'; +import { UserDORepo } from '@shared/repo/user/user-do.repo'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { IUserConfig } from '../interfaces'; import { UserMapper } from '../mapper/user.mapper'; import { UserDto } from '../uc/dto/user.dto'; @@ -34,7 +34,7 @@ export class UserService { } /** - * @deprecated + * @deprecated use {@link UserService.findById} instead */ async getUser(id: string): Promise { const userEntity = await this.userRepo.findById(id, true); @@ -43,11 +43,11 @@ export class UserService { return userDto; } - async getResolvedUser(userId: EntityId): Promise { - const user: User = await this.userRepo.findById(userId, true); + async getResolvedUser(userId: EntityId): Promise { + const user: UserDO = await this.findById(userId); const account: AccountDto = await this.accountService.findByUserIdOrFail(userId); - const resolvedUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(account.id, user, account.systemId); + const resolvedUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser(account.id, user, account.systemId); return resolvedUser; } diff --git a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts index 4d15397548b..994c8042a6d 100644 --- a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts +++ b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts @@ -172,6 +172,7 @@ describe('VideoConferenceUc', () => { roles: [], schoolId: 'schoolId', accountId: 'accountId', + isExternalUser: false, }; defaultOptions = { everybodyJoinsAsModerator: false, diff --git a/apps/server/src/shared/testing/map-user-to-current-user.ts b/apps/server/src/shared/testing/map-user-to-current-user.ts index d835c822066..b8c975f125d 100644 --- a/apps/server/src/shared/testing/map-user-to-current-user.ts +++ b/apps/server/src/shared/testing/map-user-to-current-user.ts @@ -15,6 +15,7 @@ export const mapUserToCurrentUser = ( accountId: account ? account.id : new ObjectId().toHexString(), systemId, impersonated, + isExternalUser: false, }; return currentUser;