From 48d3dc417bc89768cddc508b1d2d4f6f43368c84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Fri, 10 Nov 2023 13:01:51 +0100 Subject: [PATCH] fix oauth service and test --- .../modules/authentication/errors/index.ts | 1 - .../modules/authentication/loggable/index.ts | 1 + ...school-in-migration.loggable-exception.ts} | 15 +- .../strategy/oauth2.strategy.spec.ts | 18 +- .../strategy/oauth2.strategy.ts | 16 +- .../controller/api-test/oauth-sso.api.spec.ts | 253 ------- .../controller/dto/authorization.params.ts | 3 - .../src/modules/oauth/controller/dto/index.ts | 3 - .../oauth/controller/dto/sso-login.query.ts | 14 - .../oauth/controller/dto/system-id.params.ts | 12 - .../controller/dto/user-migration.response.ts | 7 - .../controller/oauth-sso.controller.spec.ts | 11 +- .../oauth/controller/oauth-sso.controller.ts | 108 +-- .../oauth/mapper/oauth-login-state.mapper.ts | 9 - .../oauth/mapper/user-migration.mapper.ts | 12 - .../src/modules/oauth/oauth-api.module.ts | 23 +- apps/server/src/modules/oauth/oauth.module.ts | 2 + .../oauth/service/oauth.service.spec.ts | 689 +++++++++++------- .../modules/oauth/service/oauth.service.ts | 93 +-- .../oauth/uc/dto/oauth-login-state.dto.ts | 21 - apps/server/src/modules/oauth/uc/index.ts | 1 - .../src/modules/oauth/uc/oauth.uc.spec.ts | 309 -------- apps/server/src/modules/oauth/uc/oauth.uc.ts | 95 --- .../src/modules/server/server.module.ts | 10 +- .../api-test/user-login-migration.api.spec.ts | 21 +- .../controller/dto/index.ts | 2 - .../src/modules/user-login-migration/index.ts | 1 - .../user-login-migration/mapper/index.ts | 1 - .../mapper/page-content.mapper.spec.ts | 38 - .../mapper/page-content.mapper.ts | 15 - .../service/school-migration.service.spec.ts | 44 +- .../service/user-migration.service.spec.ts | 10 +- .../uc/close-user-login-migration.uc.spec.ts | 8 +- .../uc/user-login-migration.uc.spec.ts | 2 +- .../user-login-migration-api.module.ts | 11 +- 35 files changed, 511 insertions(+), 1368 deletions(-) create mode 100644 apps/server/src/modules/authentication/loggable/index.ts rename apps/server/src/modules/authentication/{errors/school-in-migration.error.ts => loggable/school-in-migration.loggable-exception.ts} (51%) delete mode 100644 apps/server/src/modules/oauth/controller/api-test/oauth-sso.api.spec.ts delete mode 100644 apps/server/src/modules/oauth/controller/dto/sso-login.query.ts delete mode 100644 apps/server/src/modules/oauth/controller/dto/system-id.params.ts delete mode 100644 apps/server/src/modules/oauth/controller/dto/user-migration.response.ts delete mode 100644 apps/server/src/modules/oauth/mapper/oauth-login-state.mapper.ts delete mode 100644 apps/server/src/modules/oauth/mapper/user-migration.mapper.ts delete mode 100644 apps/server/src/modules/oauth/uc/dto/oauth-login-state.dto.ts delete mode 100644 apps/server/src/modules/oauth/uc/oauth.uc.spec.ts delete mode 100644 apps/server/src/modules/oauth/uc/oauth.uc.ts delete mode 100644 apps/server/src/modules/user-login-migration/mapper/page-content.mapper.spec.ts delete mode 100644 apps/server/src/modules/user-login-migration/mapper/page-content.mapper.ts diff --git a/apps/server/src/modules/authentication/errors/index.ts b/apps/server/src/modules/authentication/errors/index.ts index d87d53df8bf..d345cf9b0ce 100644 --- a/apps/server/src/modules/authentication/errors/index.ts +++ b/apps/server/src/modules/authentication/errors/index.ts @@ -1,4 +1,3 @@ export * from './brute-force.error'; export * from './ldap-connection.error'; -export * from './school-in-migration.error'; export * from './unauthorized.loggable-exception'; diff --git a/apps/server/src/modules/authentication/loggable/index.ts b/apps/server/src/modules/authentication/loggable/index.ts new file mode 100644 index 00000000000..7e6fcda9db1 --- /dev/null +++ b/apps/server/src/modules/authentication/loggable/index.ts @@ -0,0 +1 @@ +export * from './school-in-migration.loggable-exception'; diff --git a/apps/server/src/modules/authentication/errors/school-in-migration.error.ts b/apps/server/src/modules/authentication/loggable/school-in-migration.loggable-exception.ts similarity index 51% rename from apps/server/src/modules/authentication/errors/school-in-migration.error.ts rename to apps/server/src/modules/authentication/loggable/school-in-migration.loggable-exception.ts index ed507b07656..76f5baecb61 100644 --- a/apps/server/src/modules/authentication/errors/school-in-migration.error.ts +++ b/apps/server/src/modules/authentication/loggable/school-in-migration.loggable-exception.ts @@ -1,16 +1,23 @@ import { HttpStatus } from '@nestjs/common'; import { BusinessError } from '@shared/common'; +import { ErrorLogMessage, Loggable } from '@src/core/logger'; -export class SchoolInMigrationError extends BusinessError { - constructor(details?: Record) { +export class SchoolInMigrationLoggableException extends BusinessError implements Loggable { + constructor() { super( { type: 'SCHOOL_IN_MIGRATION', title: 'Login failed because school is in migration', defaultMessage: 'Login failed because creation of user is not possible during migration', }, - HttpStatus.UNAUTHORIZED, - details + HttpStatus.UNAUTHORIZED ); } + + getLogMessage(): ErrorLogMessage { + return { + type: this.type, + stack: this.stack, + }; + } } 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 f67f620175d..e6bf4ef2fa6 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts @@ -1,15 +1,15 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { AccountService } from '@modules/account/services/account.service'; +import { AccountDto } from '@modules/account/services/dto'; +import { OAuthTokenDto } from '@modules/oauth'; +import { OAuthService } from '@modules/oauth/service/oauth.service'; import { UnauthorizedException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { EntityId, RoleName } from '@shared/domain'; import { UserDO } from '@shared/domain/domainobject/user.do'; import { userDoFactory } from '@shared/testing'; -import { AccountService } from '@modules/account/services/account.service'; -import { AccountDto } from '@modules/account/services/dto'; -import { OAuthTokenDto } from '@modules/oauth'; -import { OAuthService } from '@modules/oauth/service/oauth.service'; -import { SchoolInMigrationError } from '../errors/school-in-migration.error'; import { ICurrentUser, OauthCurrentUser } from '../interface'; +import { SchoolInMigrationLoggableException } from '../loggable'; import { Oauth2Strategy } from './oauth2.strategy'; describe('Oauth2Strategy', () => { @@ -68,7 +68,7 @@ describe('Oauth2Strategy', () => { refreshToken: 'refreshToken', }) ); - oauthService.provisionUser.mockResolvedValue({ user, redirect: '' }); + oauthService.provisionUser.mockResolvedValue(user); accountService.findByUserId.mockResolvedValue(account); return { systemId, user, account, idToken }; @@ -102,7 +102,7 @@ describe('Oauth2Strategy', () => { refreshToken: 'refreshToken', }) ); - oauthService.provisionUser.mockResolvedValue({ user: undefined, redirect: '' }); + oauthService.provisionUser.mockResolvedValue(null); }; it('should throw a SchoolInMigrationError', async () => { @@ -111,7 +111,7 @@ describe('Oauth2Strategy', () => { const func = async () => strategy.validate({ body: { code: 'code', redirectUri: 'redirectUri', systemId: 'systemId' } }); - await expect(func).rejects.toThrow(new SchoolInMigrationError()); + await expect(func).rejects.toThrow(new SchoolInMigrationLoggableException()); }); }); @@ -126,7 +126,7 @@ describe('Oauth2Strategy', () => { refreshToken: 'refreshToken', }) ); - oauthService.provisionUser.mockResolvedValue({ user, redirect: '' }); + oauthService.provisionUser.mockResolvedValue(user); accountService.findByUserId.mockResolvedValue(null); }; diff --git a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts index 599744cc1a7..e83e9174abc 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts @@ -1,14 +1,14 @@ -import { Injectable, UnauthorizedException } from '@nestjs/common'; -import { PassportStrategy } from '@nestjs/passport'; -import { UserDO } from '@shared/domain/domainobject/user.do'; import { AccountService } from '@modules/account/services/account.service'; import { AccountDto } from '@modules/account/services/dto'; import { OAuthTokenDto } from '@modules/oauth'; import { OAuthService } from '@modules/oauth/service/oauth.service'; +import { Injectable, UnauthorizedException } from '@nestjs/common'; +import { PassportStrategy } from '@nestjs/passport'; +import { UserDO } from '@shared/domain/domainobject/user.do'; import { Strategy } from 'passport-custom'; import { Oauth2AuthorizationBodyParams } from '../controllers/dto'; -import { SchoolInMigrationError } from '../errors/school-in-migration.error'; import { ICurrentUser, OauthCurrentUser } from '../interface'; +import { SchoolInMigrationLoggableException } from '../loggable'; import { CurrentUserMapper } from '../mapper'; @Injectable() @@ -22,14 +22,10 @@ export class Oauth2Strategy extends PassportStrategy(Strategy, 'oauth2') { const tokenDto: OAuthTokenDto = await this.oauthService.authenticateUser(systemId, redirectUri, code); - const { user }: { user?: UserDO; redirect: string } = await this.oauthService.provisionUser( - systemId, - tokenDto.idToken, - tokenDto.accessToken - ); + const user: UserDO | null = await this.oauthService.provisionUser(systemId, tokenDto.idToken, tokenDto.accessToken); if (!user || !user.id) { - throw new SchoolInMigrationError(); + throw new SchoolInMigrationLoggableException(); } const account: AccountDto | null = await this.accountService.findByUserId(user.id); diff --git a/apps/server/src/modules/oauth/controller/api-test/oauth-sso.api.spec.ts b/apps/server/src/modules/oauth/controller/api-test/oauth-sso.api.spec.ts deleted file mode 100644 index ed027ad26c6..00000000000 --- a/apps/server/src/modules/oauth/controller/api-test/oauth-sso.api.spec.ts +++ /dev/null @@ -1,253 +0,0 @@ -import { Configuration } from '@hpi-schul-cloud/commons/lib'; -import { KeycloakAdministrationService } from '@infra/identity-management/keycloak-administration/service/keycloak-administration.service'; -import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; -import { ICurrentUser } from '@modules/authentication'; -import { JwtAuthGuard } from '@modules/authentication/guard/jwt-auth.guard'; -import { ServerTestModule } from '@modules/server'; -import { ExecutionContext, INestApplication } from '@nestjs/common'; -import { Test, TestingModule } from '@nestjs/testing'; -import { Account, EntityId, SchoolEntity, SystemEntity, User } from '@shared/domain'; -import { accountFactory, cleanupCollections, schoolFactory, systemFactory, userFactory } from '@shared/testing'; -import { JwtTestFactory } from '@shared/testing/factory/jwt.test.factory'; -import axios from 'axios'; -import MockAdapter from 'axios-mock-adapter'; -import { Request } from 'express'; -import request, { Response } from 'supertest'; -import { SSOAuthenticationError } from '../../interface/sso-authentication-error.enum'; -import { OauthTokenResponse } from '../../service/dto'; -import { AuthorizationParams, SSOLoginQuery } from '../dto'; - -jest.mock('jwks-rsa', () => () => { - return { - getKeys: jest.fn(), - getSigningKey: jest.fn().mockResolvedValue({ - kid: 'kid', - alg: 'RS256', - getPublicKey: jest.fn().mockReturnValue(JwtTestFactory.getPublicKey()), - rsaPublicKey: JwtTestFactory.getPublicKey(), - }), - getSigningKeys: jest.fn(), - }; -}); - -describe('OAuth SSO Controller (API)', () => { - let app: INestApplication; - let em: EntityManager; - let currentUser: ICurrentUser; - let axiosMock: MockAdapter; - - const sessionCookieName: string = Configuration.get('SESSION__NAME') as string; - beforeAll(async () => { - Configuration.set('PUBLIC_BACKEND_URL', 'http://localhost:3030/api'); - const schulcloudJwt: string = JwtTestFactory.createJwt(); - - const moduleRef: TestingModule = await Test.createTestingModule({ - imports: [ServerTestModule], - }) - .overrideGuard(JwtAuthGuard) - .useValue({ - canActivate(context: ExecutionContext) { - const req: Request = context.switchToHttp().getRequest(); - req.user = currentUser; - req.headers.authorization = schulcloudJwt; - return true; - }, - }) - .compile(); - - axiosMock = new MockAdapter(axios); - app = moduleRef.createNestApplication(); - await app.init(); - em = app.get(EntityManager); - const kcAdminService = app.get(KeycloakAdministrationService); - - axiosMock.onGet(kcAdminService.getWellKnownUrl()).reply(200, { - issuer: 'issuer', - token_endpoint: 'token_endpoint', - authorization_endpoint: 'authorization_endpoint', - end_session_endpoint: 'end_session_endpoint', - jwks_uri: 'jwks_uri', - }); - }); - - afterAll(async () => { - await app.close(); - }); - - afterEach(async () => { - await cleanupCollections(em); - }); - - const setupSessionState = async (systemId: EntityId, migration: boolean) => { - const query: SSOLoginQuery = { - migration, - }; - - const response: Response = await request(app.getHttpServer()) - .get(`/sso/login/${systemId}`) - .query(query) - .expect(302) - .expect('set-cookie', new RegExp(`^${sessionCookieName}`)); - - const cookies: string[] = response.get('Set-Cookie'); - const redirect: string = response.get('Location'); - const matchState: RegExpMatchArray | null = redirect.match(/(?<=state=)([^&]+)/); - const state = matchState ? matchState[0] : ''; - - return { - cookies, - state, - }; - }; - - const setup = async () => { - const externalUserId = 'externalUserId'; - const system: SystemEntity = systemFactory.withOauthConfig().buildWithId(); - const school: SchoolEntity = schoolFactory.buildWithId({ systems: [system] }); - const user: User = userFactory.buildWithId({ externalId: externalUserId, school }); - const account: Account = accountFactory.buildWithId({ systemId: system.id, userId: user.id }); - - await em.persistAndFlush([system, user, school, account]); - em.clear(); - - const query: AuthorizationParams = new AuthorizationParams(); - query.code = 'code'; - query.state = 'state'; - - return { - system, - user, - externalUserId, - school, - query, - }; - }; - - describe('[GET] sso/login/:systemId', () => { - describe('when no error occurs', () => { - it('should redirect to the authentication url and set a session cookie', async () => { - const { system } = await setup(); - - await request(app.getHttpServer()) - .get(`/sso/login/${system.id}`) - .expect(302) - .expect('set-cookie', new RegExp(`^${sessionCookieName}`)) - .expect( - 'Location', - /^http:\/\/mock.de\/auth\?client_id=12345&redirect_uri=http%3A%2F%2Flocalhost%3A3030%2Fapi%2Fv3%2Fsso%2Foauth&response_type=code&scope=openid\+uuid&state=\w*/ - ); - }); - }); - - describe('when an error occurs', () => { - it('should redirect to the login page', async () => { - const unknownSystemId: string = new ObjectId().toHexString(); - const clientUrl: string = Configuration.get('HOST') as string; - - await request(app.getHttpServer()) - .get(`/sso/login/${unknownSystemId}`) - .expect(302) - .expect('Location', `${clientUrl}/login?error=sso_login_failed`); - }); - }); - }); - - describe('[GET] sso/oauth', () => { - describe('when the session has no oauthLoginState', () => { - it('should return 401 Unauthorized', async () => { - await setup(); - const query: AuthorizationParams = new AuthorizationParams(); - query.code = 'code'; - query.state = 'state'; - - await request(app.getHttpServer()).get(`/sso/oauth`).query(query).expect(401); - }); - }); - - describe('when the session and the request have a different state', () => { - it('should return 401 Unauthorized', async () => { - const { system } = await setup(); - const { cookies } = await setupSessionState(system.id, false); - const query: AuthorizationParams = new AuthorizationParams(); - query.code = 'code'; - query.state = 'wrongState'; - - await request(app.getHttpServer()).get(`/sso/oauth`).set('Cookie', cookies).query(query).expect(401); - }); - }); - - describe('when code and state are valid', () => { - it('should set a jwt and redirect', async () => { - const { system, externalUserId, query } = await setup(); - const { state, cookies } = await setupSessionState(system.id, false); - const baseUrl: string = Configuration.get('HOST') as string; - query.code = 'code'; - query.state = state; - - const idToken: string = JwtTestFactory.createJwt({ - sub: 'testUser', - iss: system.oauthConfig?.issuer, - aud: system.oauthConfig?.clientId, - // For OIDC provisioning strategy - external_sub: externalUserId, - }); - - axiosMock.onPost(system.oauthConfig?.tokenEndpoint).reply(200, { - id_token: idToken, - refresh_token: 'refreshToken', - access_token: 'accessToken', - }); - - await request(app.getHttpServer()) - .get(`/sso/oauth`) - .set('Cookie', cookies) - .query(query) - .expect(302) - .expect('Location', `${baseUrl}/dashboard`) - .expect( - (res: Response) => res.get('Set-Cookie').filter((value: string) => value.startsWith('jwt')).length === 1 - ); - }); - }); - - describe('when an error occurs during the login process', () => { - it('should redirect to the login page', async () => { - const { system, query } = await setup(); - const { state, cookies } = await setupSessionState(system.id, false); - const clientUrl: string = Configuration.get('HOST') as string; - query.error = SSOAuthenticationError.ACCESS_DENIED; - query.state = state; - - await request(app.getHttpServer()) - .get(`/sso/oauth`) - .set('Cookie', cookies) - .query(query) - .expect(302) - .expect( - 'Location', - `${clientUrl}/login?error=access_denied&provider=${system.oauthConfig?.provider as string}` - ); - }); - }); - - describe('when a faulty query is passed', () => { - it('should redirect to the login page with an error', async () => { - const { system, query } = await setup(); - const { state, cookies } = await setupSessionState(system.id, false); - const clientUrl: string = Configuration.get('HOST') as string; - query.state = state; - query.code = undefined; - - await request(app.getHttpServer()) - .get(`/sso/oauth`) - .set('Cookie', cookies) - .query(query) - .expect(302) - .expect( - 'Location', - `${clientUrl}/login?error=sso_auth_code_step&provider=${system.oauthConfig?.provider as string}` - ); - }); - }); - }); -}); diff --git a/apps/server/src/modules/oauth/controller/dto/authorization.params.ts b/apps/server/src/modules/oauth/controller/dto/authorization.params.ts index 1a20985ce43..af76d0799e4 100644 --- a/apps/server/src/modules/oauth/controller/dto/authorization.params.ts +++ b/apps/server/src/modules/oauth/controller/dto/authorization.params.ts @@ -1,9 +1,6 @@ import { IsEnum, IsNotEmpty, IsOptional, IsString } from 'class-validator'; import { SSOAuthenticationError } from '../../interface/sso-authentication-error.enum'; -/** - * @deprecated - */ export class AuthorizationParams { @IsOptional() @IsString() diff --git a/apps/server/src/modules/oauth/controller/dto/index.ts b/apps/server/src/modules/oauth/controller/dto/index.ts index 9fc38145be5..7f679725bd3 100644 --- a/apps/server/src/modules/oauth/controller/dto/index.ts +++ b/apps/server/src/modules/oauth/controller/dto/index.ts @@ -1,4 +1 @@ export * from './authorization.params'; -export * from './system-id.params'; -export * from './sso-login.query'; -export * from './user-migration.response'; diff --git a/apps/server/src/modules/oauth/controller/dto/sso-login.query.ts b/apps/server/src/modules/oauth/controller/dto/sso-login.query.ts deleted file mode 100644 index 092380cbcf2..00000000000 --- a/apps/server/src/modules/oauth/controller/dto/sso-login.query.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { ApiProperty } from '@nestjs/swagger'; -import { IsBoolean, IsOptional, IsString } from 'class-validator'; - -export class SSOLoginQuery { - @IsOptional() - @IsString() - @ApiProperty() - postLoginRedirect?: string; - - @IsOptional() - @IsBoolean() - @ApiProperty() - migration?: boolean; -} diff --git a/apps/server/src/modules/oauth/controller/dto/system-id.params.ts b/apps/server/src/modules/oauth/controller/dto/system-id.params.ts deleted file mode 100644 index 04ba0017284..00000000000 --- a/apps/server/src/modules/oauth/controller/dto/system-id.params.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { ApiProperty } from '@nestjs/swagger'; -import { IsMongoId } from 'class-validator'; - -export class SystemIdParams { - @IsMongoId() - @ApiProperty({ - description: 'The id of the system.', - required: true, - nullable: false, - }) - systemId!: string; -} diff --git a/apps/server/src/modules/oauth/controller/dto/user-migration.response.ts b/apps/server/src/modules/oauth/controller/dto/user-migration.response.ts deleted file mode 100644 index 88c0fdef3b4..00000000000 --- a/apps/server/src/modules/oauth/controller/dto/user-migration.response.ts +++ /dev/null @@ -1,7 +0,0 @@ -export class UserMigrationResponse { - constructor(props: UserMigrationResponse) { - this.redirect = props.redirect; - } - - redirect: string; -} diff --git a/apps/server/src/modules/oauth/controller/oauth-sso.controller.spec.ts b/apps/server/src/modules/oauth/controller/oauth-sso.controller.spec.ts index 3d1a470e227..e98100d3e43 100644 --- a/apps/server/src/modules/oauth/controller/oauth-sso.controller.spec.ts +++ b/apps/server/src/modules/oauth/controller/oauth-sso.controller.spec.ts @@ -1,14 +1,13 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons'; +import { ICurrentUser } from '@modules/authentication'; +import { HydraOauthUc } from '@modules/oauth/uc/hydra-oauth.uc'; import { UnauthorizedException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { LegacyLogger } from '@src/core/logger'; -import { ICurrentUser } from '@modules/authentication'; -import { HydraOauthUc } from '@modules/oauth/uc/hydra-oauth.uc'; import { Request } from 'express'; -import { OauthSSOController } from './oauth-sso.controller'; import { StatelessAuthorizationParams } from './dto/stateless-authorization.params'; -import { OauthUc } from '../uc'; +import { OauthSSOController } from './oauth-sso.controller'; describe('OAuthController', () => { let module: TestingModule; @@ -52,10 +51,6 @@ describe('OAuthController', () => { provide: LegacyLogger, useValue: createMock(), }, - { - provide: OauthUc, - useValue: createMock(), - }, { provide: HydraOauthUc, useValue: createMock(), diff --git a/apps/server/src/modules/oauth/controller/oauth-sso.controller.ts b/apps/server/src/modules/oauth/controller/oauth-sso.controller.ts index 2dad85b5862..b5549cf2d98 100644 --- a/apps/server/src/modules/oauth/controller/oauth-sso.controller.ts +++ b/apps/server/src/modules/oauth/controller/oauth-sso.controller.ts @@ -1,116 +1,18 @@ -import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { Authenticate, CurrentUser, ICurrentUser } from '@modules/authentication'; -import { Controller, Get, Param, Query, Req, Res, Session, UnauthorizedException } from '@nestjs/common'; +import { Controller, Get, Param, Query, Req, UnauthorizedException } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; -import { ISession } from '@shared/domain/types/session'; import { LegacyLogger } from '@src/core/logger'; -import { CookieOptions, Request, Response } from 'express'; +import { Request } from 'express'; import { OAuthTokenDto } from '../interface'; -import { OAuthSSOError } from '../loggable'; -import { OauthLoginStateMapper } from '../mapper/oauth-login-state.mapper'; -import { OAuthProcessDto } from '../service/dto'; -import { HydraOauthUc, OauthUc } from '../uc'; -import { OauthLoginStateDto } from '../uc/dto/oauth-login-state.dto'; -import { AuthorizationParams, SSOLoginQuery, SystemIdParams } from './dto'; +import { HydraOauthUc } from '../uc'; +import { AuthorizationParams } from './dto'; import { StatelessAuthorizationParams } from './dto/stateless-authorization.params'; @ApiTags('SSO') @Controller('sso') export class OauthSSOController { - private readonly clientUrl: string; - - constructor( - private readonly oauthUc: OauthUc, - private readonly hydraUc: HydraOauthUc, - private readonly logger: LegacyLogger - ) { + constructor(private readonly hydraUc: HydraOauthUc, private readonly logger: LegacyLogger) { this.logger.setContext(OauthSSOController.name); - this.clientUrl = Configuration.get('HOST') as string; - } - - private errorHandler(error: unknown, session: ISession, res: Response, provider?: string) { - this.logger.error(error); - const ssoError: OAuthSSOError = error instanceof OAuthSSOError ? error : new OAuthSSOError(); - - session.destroy((err) => { - this.logger.log(err); - }); - - const errorRedirect: URL = new URL('/login', this.clientUrl); - errorRedirect.searchParams.append('error', ssoError.errorcode); - - if (provider) { - errorRedirect.searchParams.append('provider', provider); - } - - res.redirect(errorRedirect.toString()); - } - - private sessionHandler(session: ISession, query: AuthorizationParams): OauthLoginStateDto { - if (!session.oauthLoginState) { - throw new UnauthorizedException('Oauth session not found'); - } - - const oauthLoginState: OauthLoginStateDto = OauthLoginStateMapper.mapSessionToDto(session); - - if (oauthLoginState.state !== query.state) { - throw new UnauthorizedException(`Invalid state. Got: ${query.state} Expected: ${oauthLoginState.state}`); - } - - return oauthLoginState; - } - - @Get('login/:systemId') - async getAuthenticationUrl( - @Session() session: ISession, - @Res() res: Response, - @Param() params: SystemIdParams, - @Query() query: SSOLoginQuery - ): Promise { - try { - const redirect: string = await this.oauthUc.startOauthLogin( - session, - params.systemId, - query.migration || false, - query.postLoginRedirect - ); - - res.redirect(redirect); - } catch (error) { - this.errorHandler(error, session, res); - } - } - - @Get('oauth') - async startOauthAuthorizationCodeFlow( - @Session() session: ISession, - @Res() res: Response, - @Query() query: AuthorizationParams - ): Promise { - const oauthLoginState: OauthLoginStateDto = this.sessionHandler(session, query); - - try { - const oauthProcessDto: OAuthProcessDto = await this.oauthUc.processOAuthLogin( - oauthLoginState, - query.code, - query.error - ); - - if (oauthProcessDto.jwt) { - const cookieDefaultOptions: CookieOptions = { - httpOnly: Configuration.get('COOKIE__HTTP_ONLY') as boolean, - sameSite: Configuration.get('COOKIE__SAME_SITE') as 'lax' | 'strict' | 'none', - secure: Configuration.get('COOKIE__SECURE') as boolean, - expires: new Date(Date.now() + (Configuration.get('COOKIE__EXPIRES_SECONDS') as number)), - }; - - res.cookie('jwt', oauthProcessDto.jwt, cookieDefaultOptions); - } - - res.redirect(oauthProcessDto.redirect); - } catch (error) { - this.errorHandler(error, session, res, oauthLoginState.provider); - } } @Get('hydra/:oauthClientId') diff --git a/apps/server/src/modules/oauth/mapper/oauth-login-state.mapper.ts b/apps/server/src/modules/oauth/mapper/oauth-login-state.mapper.ts deleted file mode 100644 index 67c1ae8a6ef..00000000000 --- a/apps/server/src/modules/oauth/mapper/oauth-login-state.mapper.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { ISession } from '@shared/domain/types/session'; -import { OauthLoginStateDto } from '../uc/dto/oauth-login-state.dto'; - -export class OauthLoginStateMapper { - static mapSessionToDto(session: ISession): OauthLoginStateDto { - const dto = new OauthLoginStateDto(session.oauthLoginState as OauthLoginStateDto); - return dto; - } -} diff --git a/apps/server/src/modules/oauth/mapper/user-migration.mapper.ts b/apps/server/src/modules/oauth/mapper/user-migration.mapper.ts deleted file mode 100644 index 42134d0b4d2..00000000000 --- a/apps/server/src/modules/oauth/mapper/user-migration.mapper.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { MigrationDto } from '@modules/user-login-migration/service/dto'; -import { UserMigrationResponse } from '../controller/dto'; - -export class UserMigrationMapper { - static mapDtoToResponse(dto: MigrationDto): UserMigrationResponse { - const response: UserMigrationResponse = new UserMigrationResponse({ - redirect: dto.redirect, - }); - - return response; - } -} diff --git a/apps/server/src/modules/oauth/oauth-api.module.ts b/apps/server/src/modules/oauth/oauth-api.module.ts index 98e62d87eca..880f11dc731 100644 --- a/apps/server/src/modules/oauth/oauth-api.module.ts +++ b/apps/server/src/modules/oauth/oauth-api.module.ts @@ -1,29 +1,12 @@ import { Module } from '@nestjs/common'; import { LoggerModule } from '@src/core/logger'; -import { AuthenticationModule } from '@modules/authentication/authentication.module'; -import { AuthorizationModule } from '@modules/authorization'; -import { ProvisioningModule } from '@modules/provisioning'; -import { LegacySchoolModule } from '@modules/legacy-school'; -import { SystemModule } from '@modules/system'; -import { UserModule } from '@modules/user'; -import { UserLoginMigrationModule } from '@modules/user-login-migration'; import { OauthSSOController } from './controller/oauth-sso.controller'; import { OauthModule } from './oauth.module'; -import { HydraOauthUc, OauthUc } from './uc'; +import { HydraOauthUc } from './uc'; @Module({ - imports: [ - OauthModule, - AuthenticationModule, - AuthorizationModule, - ProvisioningModule, - LegacySchoolModule, - UserLoginMigrationModule, - SystemModule, - UserModule, - LoggerModule, - ], + imports: [OauthModule, LoggerModule], controllers: [OauthSSOController], - providers: [OauthUc, HydraOauthUc], + providers: [HydraOauthUc], }) export class OauthApiModule {} diff --git a/apps/server/src/modules/oauth/oauth.module.ts b/apps/server/src/modules/oauth/oauth.module.ts index 13518db0d9a..3898a2e8547 100644 --- a/apps/server/src/modules/oauth/oauth.module.ts +++ b/apps/server/src/modules/oauth/oauth.module.ts @@ -5,6 +5,7 @@ import { LegacySchoolModule } from '@modules/legacy-school'; import { ProvisioningModule } from '@modules/provisioning'; import { SystemModule } from '@modules/system'; import { UserModule } from '@modules/user'; +import { UserLoginMigrationModule } from '@modules/user-login-migration'; import { HttpModule } from '@nestjs/axios'; import { Module } from '@nestjs/common'; import { LtiToolRepo } from '@shared/repo'; @@ -23,6 +24,7 @@ import { OAuthService } from './service/oauth.service'; ProvisioningModule, SystemModule, CacheWrapperModule, + UserLoginMigrationModule, LegacySchoolModule, ], providers: [OAuthService, OauthAdapterService, HydraSsoService, LtiToolRepo], diff --git a/apps/server/src/modules/oauth/service/oauth.service.spec.ts b/apps/server/src/modules/oauth/service/oauth.service.spec.ts index 9c4b45582df..adfa9603bc6 100644 --- a/apps/server/src/modules/oauth/service/oauth.service.spec.ts +++ b/apps/server/src/modules/oauth/service/oauth.service.spec.ts @@ -1,23 +1,23 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons'; -import { Test, TestingModule } from '@nestjs/testing'; -import { LegacySchoolDo, OauthConfig, SchoolFeatures, SystemEntity } from '@shared/domain'; -import { UserDO } from '@shared/domain/domainobject/user.do'; -import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; import { DefaultEncryptionService, IEncryptionService, SymetricKeyEncryptionService } from '@infra/encryption'; -import { legacySchoolDoFactory, setupEntities, systemFactory, userDoFactory } from '@shared/testing'; -import { LegacyLogger } from '@src/core/logger'; -import { ProvisioningDto, ProvisioningService } from '@modules/provisioning'; -import { ExternalSchoolDto, ExternalUserDto, OauthDataDto, ProvisioningSystemDto } from '@modules/provisioning/dto'; +import { ObjectId } from '@mikro-orm/mongodb'; import { LegacySchoolService } from '@modules/legacy-school'; +import { ProvisioningService } from '@modules/provisioning'; import { OauthConfigDto } from '@modules/system/service'; import { SystemDto } from '@modules/system/service/dto/system.dto'; import { SystemService } from '@modules/system/service/system.service'; import { UserService } from '@modules/user'; -import { MigrationCheckService, UserMigrationService } from '@modules/user-login-migration'; +import { MigrationCheckService } from '@modules/user-login-migration'; +import { Test, TestingModule } from '@nestjs/testing'; +import { LegacySchoolDo, OauthConfig, SchoolFeatures, SystemEntity, UserDO } from '@shared/domain'; +import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; +import { legacySchoolDoFactory, setupEntities, systemFactory, userDoFactory } from '@shared/testing'; +import { LegacyLogger } from '@src/core/logger'; +import { OauthDataDto } from '@src/modules/provisioning/dto'; import jwt, { JwtPayload } from 'jsonwebtoken'; -import { OAuthSSOError, UserNotFoundAfterProvisioningLoggableException } from '../loggable'; import { OAuthTokenDto } from '../interface'; +import { OAuthSSOError, UserNotFoundAfterProvisioningLoggableException } from '../loggable'; import { OauthTokenResponse } from './dto'; import { OauthAdapterService } from './oauth-adapter.service'; import { OAuthService } from './oauth.service'; @@ -45,7 +45,6 @@ describe('OAuthService', () => { let provisioningService: DeepMocked; let userService: DeepMocked; let systemService: DeepMocked; - let userMigrationService: DeepMocked; let oauthAdapterService: DeepMocked; let migrationCheckService: DeepMocked; let schoolService: DeepMocked; @@ -85,10 +84,6 @@ describe('OAuthService', () => { provide: SystemService, useValue: createMock(), }, - { - provide: UserMigrationService, - useValue: createMock(), - }, { provide: OauthAdapterService, useValue: createMock(), @@ -105,7 +100,6 @@ describe('OAuthService', () => { provisioningService = module.get(ProvisioningService); userService = module.get(UserService); systemService = module.get(SystemService); - userMigrationService = module.get(UserMigrationService); oauthAdapterService = module.get(OauthAdapterService); migrationCheckService = module.get(MigrationCheckService); schoolService = module.get(LegacySchoolService); @@ -197,49 +191,6 @@ describe('OAuthService', () => { }); }); - describe('getPostLoginRedirectUrl is called', () => { - describe('when the oauth provider is iserv', () => { - it('should return an iserv login url string', async () => { - const system: SystemDto = new SystemDto({ - type: 'oauth', - oauthConfig: { - provider: 'iserv', - logoutEndpoint: 'http://iserv.de/logout', - } as OauthConfigDto, - }); - systemService.findById.mockResolvedValue(system); - - const result: string = await service.getPostLoginRedirectUrl('idToken', 'systemId'); - - expect(result).toEqual( - `http://iserv.de/logout?id_token_hint=idToken&post_logout_redirect_uri=https%3A%2F%2Fmock.de%2Fdashboard` - ); - }); - }); - - describe('when it is called with a postLoginRedirect and the provider is not iserv', () => { - it('should return the postLoginRedirect', async () => { - const system: SystemDto = new SystemDto({ type: 'oauth' }); - systemService.findById.mockResolvedValue(system); - - const result: string = await service.getPostLoginRedirectUrl('idToken', 'systemId', 'postLoginRedirect'); - - expect(result).toEqual('postLoginRedirect'); - }); - }); - - describe('when it is called with any other oauth provider', () => { - it('should return a login url string', async () => { - const system: SystemDto = new SystemDto({ type: 'oauth' }); - systemService.findById.mockResolvedValue(system); - - const result: string = await service.getPostLoginRedirectUrl('idToken', 'systemId'); - - expect(result).toEqual(`${hostUri}/dashboard`); - }); - }); - }); - describe('authenticateUser is called', () => { const setup = () => { const authCode = '43534543jnj543342jn2'; @@ -332,307 +283,489 @@ describe('OAuthService', () => { }); }); - describe('provisionUser is called', () => { - describe('when only provisioning a user', () => { - it('should return the user and a redirect', async () => { + describe('provisionUser', () => { + describe('when provisioning a user and a school without official school number', () => { + const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; const externalUserId = 'externalUserId'; - const user: UserDO = userDoFactory.buildWithId({ externalId: externalUserId }); - const oauthData: OauthDataDto = new OauthDataDto({ - system: new ProvisioningSystemDto({ - systemId: 'systemId', - provisioningStrategy: SystemProvisioningStrategy.OIDC, - }), - externalUser: new ExternalUserDto({ - externalId: externalUserId, - }), - }); - const provisioningDto: ProvisioningDto = new ProvisioningDto({ - externalUserId, + + const user: UserDO = userDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalUserId, }); - provisioningService.getData.mockResolvedValue(oauthData); - provisioningService.provisionData.mockResolvedValue(provisioningDto); - userService.findByExternalId.mockResolvedValue(user); + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { + externalId: externalUserId, + }, + externalSchool: { + externalId: 'externalSchoolId', + name: 'External School', + }, + }); - const result: { user?: UserDO; redirect: string } = await service.provisionUser( - 'systemId', - 'idToken', - 'accessToken' - ); + provisioningService.getData.mockResolvedValueOnce(provisioningData); + userService.findByExternalId.mockResolvedValueOnce(user); - expect(result).toEqual<{ user?: UserDO; redirect: string }>({ + return { + systemId, + idToken, + accessToken, + provisioningData, user, - redirect: `${hostUri}/dashboard`, - }); + }; + }; + + it('should provision the data', async () => { + const { systemId, idToken, accessToken, provisioningData } = setup(); + + await service.provisionUser(systemId, idToken, accessToken); + + expect(provisioningService.provisionData).toHaveBeenCalledWith(provisioningData); + }); + + it('should return the user', async () => { + const { systemId, idToken, accessToken, user } = setup(); + + const result = await service.provisionUser(systemId, idToken, accessToken); + + expect(result).toEqual(user); }); }); - describe('when provisioning a user that should migrate, but the user does not exist', () => { - it('should return a redirect to the migration page and skip provisioning', async () => { - const migrationRedirectUrl = 'migrationRedirectUrl'; - const oauthData: OauthDataDto = new OauthDataDto({ - system: new ProvisioningSystemDto({ - systemId: 'systemId', - provisioningStrategy: SystemProvisioningStrategy.OIDC, - }), - externalUser: new ExternalUserDto({ - externalId: 'externalUserId', - }), - externalSchool: new ExternalSchoolDto({ - externalId: 'schoolExternalId', - name: 'externalSchool', - officialSchoolNumber: 'officialSchoolNumber', - }), + describe('when provisioning a user and a school without official school number, but the user cannot be found after the provisioning', () => { + const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; + const externalUserId = 'externalUserId'; + + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { + externalId: externalUserId, + }, + externalSchool: { + externalId: 'externalSchoolId', + name: 'External School', + }, }); - provisioningService.getData.mockResolvedValue(oauthData); - schoolService.getSchoolBySchoolNumber.mockResolvedValue(null); - migrationCheckService.shouldUserMigrate.mockResolvedValue(true); - userMigrationService.getMigrationConsentPageRedirect.mockResolvedValue(migrationRedirectUrl); - userService.findByExternalId.mockResolvedValue(null); + provisioningService.getData.mockResolvedValueOnce(provisioningData); + userService.findByExternalId.mockResolvedValueOnce(null); - const result: { user?: UserDO; redirect: string } = await service.provisionUser( - 'systemId', - 'idToken', - 'accessToken' - ); + return { + systemId, + idToken, + accessToken, + provisioningData, + }; + }; - expect(result).toEqual<{ user?: UserDO; redirect: string }>({ - user: undefined, - redirect: migrationRedirectUrl, - }); - expect(provisioningService.provisionData).not.toHaveBeenCalled(); + it('should throw an error', async () => { + const { systemId, idToken, accessToken } = setup(); + + await expect(service.provisionUser(systemId, idToken, accessToken)).rejects.toThrow( + UserNotFoundAfterProvisioningLoggableException + ); }); }); - describe('when provisioning an existing user that should migrate', () => { - it('should return a redirect to the migration page and provision the user', async () => { - const migrationRedirectUrl = 'migrationRedirectUrl'; + describe('when provisioning a user and a new school with official school number', () => { + const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; const externalUserId = 'externalUserId'; - const user: UserDO = userDoFactory.buildWithId({ externalId: externalUserId }); - const oauthData: OauthDataDto = new OauthDataDto({ - system: new ProvisioningSystemDto({ - systemId: 'systemId', - provisioningStrategy: SystemProvisioningStrategy.OIDC, - }), - externalUser: new ExternalUserDto({ + + const user: UserDO = userDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalUserId, + }); + + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { externalId: externalUserId, - }), - externalSchool: new ExternalSchoolDto({ - externalId: 'schoolExternalId', - name: 'externalSchool', + }, + externalSchool: { + externalId: 'externalSchoolId', + name: 'External School', officialSchoolNumber: 'officialSchoolNumber', - }), + }, + }); + + provisioningService.getData.mockResolvedValueOnce(provisioningData); + schoolService.getSchoolBySchoolNumber.mockResolvedValueOnce(null); + migrationCheckService.shouldUserMigrate.mockResolvedValueOnce(false); + userService.findByExternalId.mockResolvedValueOnce(user); + + return { + systemId, + idToken, + accessToken, + provisioningData, + user, + }; + }; + + it('should provision the data', async () => { + const { systemId, idToken, accessToken, provisioningData } = setup(); + + await service.provisionUser(systemId, idToken, accessToken); + + expect(provisioningService.provisionData).toHaveBeenCalledWith(provisioningData); + }); + + it('should return the user', async () => { + const { systemId, idToken, accessToken, user } = setup(); + + const result = await service.provisionUser(systemId, idToken, accessToken); + + expect(result).toEqual(user); + }); + }); + + describe('when provisioning a user and an existing school with official school number, that has provisioning enabled', () => { + const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; + const externalUserId = 'externalUserId'; + const externalSchoolId = 'externalSchoolId'; + const officialSchoolNumber = 'officialSchoolNumber'; + + const user: UserDO = userDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalUserId, }); - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ + + const school: LegacySchoolDo = legacySchoolDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalSchoolId, + officialSchoolNumber, features: [SchoolFeatures.OAUTH_PROVISIONING_ENABLED], }); - provisioningService.getData.mockResolvedValue(oauthData); - schoolService.getSchoolBySchoolNumber.mockResolvedValue(school); - migrationCheckService.shouldUserMigrate.mockResolvedValue(true); - userMigrationService.getMigrationConsentPageRedirect.mockResolvedValue(migrationRedirectUrl); - userService.findByExternalId.mockResolvedValue(user); + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { + externalId: externalUserId, + }, + externalSchool: { + externalId: externalSchoolId, + name: school.name, + officialSchoolNumber, + }, + }); - const result: { user?: UserDO; redirect: string } = await service.provisionUser( - 'systemId', - 'idToken', - 'accessToken' - ); + provisioningService.getData.mockResolvedValueOnce(provisioningData); + schoolService.getSchoolBySchoolNumber.mockResolvedValueOnce(school); + migrationCheckService.shouldUserMigrate.mockResolvedValueOnce(false); + userService.findByExternalId.mockResolvedValueOnce(user); - expect(result).toEqual<{ user?: UserDO; redirect: string }>({ + return { + systemId, + idToken, + accessToken, + provisioningData, user, - redirect: migrationRedirectUrl, - }); - expect(provisioningService.provisionData).toHaveBeenCalled(); + }; + }; + + it('should provision the data', async () => { + const { systemId, idToken, accessToken, provisioningData } = setup(); + + await service.provisionUser(systemId, idToken, accessToken); + + expect(provisioningService.provisionData).toHaveBeenCalledWith(provisioningData); + }); + + it('should return the user', async () => { + const { systemId, idToken, accessToken, user } = setup(); + + const result = await service.provisionUser(systemId, idToken, accessToken); + + expect(result).toEqual(user); }); }); - describe('when provisioning an existing user, that is in a school with provisioning disabled', () => { + describe('when provisioning an existing user and an existing school with official school number, that has provisioning disabled', () => { const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; const externalUserId = 'externalUserId'; - const user: UserDO = userDoFactory.buildWithId({ externalId: externalUserId }); - const oauthData: OauthDataDto = new OauthDataDto({ - system: new ProvisioningSystemDto({ - systemId: 'systemId', - provisioningStrategy: SystemProvisioningStrategy.OIDC, - }), - externalUser: new ExternalUserDto({ + const externalSchoolId = 'externalSchoolId'; + const officialSchoolNumber = 'officialSchoolNumber'; + + const user: UserDO = userDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalUserId, + }); + + const school: LegacySchoolDo = legacySchoolDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalSchoolId, + officialSchoolNumber, + features: [], + }); + + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { externalId: externalUserId, - }), - externalSchool: new ExternalSchoolDto({ - externalId: 'schoolExternalId', - name: 'externalSchool', - officialSchoolNumber: 'officialSchoolNumber', - }), + }, + externalSchool: { + externalId: externalSchoolId, + name: school.name, + officialSchoolNumber, + }, }); - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ features: [] }); - provisioningService.getData.mockResolvedValue(oauthData); - schoolService.getSchoolBySchoolNumber.mockResolvedValue(school); - migrationCheckService.shouldUserMigrate.mockResolvedValue(false); - userService.findByExternalId.mockResolvedValue(user); + provisioningService.getData.mockResolvedValueOnce(provisioningData); + schoolService.getSchoolBySchoolNumber.mockResolvedValueOnce(school); + migrationCheckService.shouldUserMigrate.mockResolvedValueOnce(false); + userService.findByExternalId.mockResolvedValueOnce(user); return { + systemId, + idToken, + accessToken, + provisioningData, user, }; }; - it('should not provision the user, but find it', async () => { - const { user } = setup(); + it('should not provision the data', async () => { + const { systemId, idToken, accessToken } = setup(); - const result: { user?: UserDO; redirect: string } = await service.provisionUser( - 'systemId', - 'idToken', - 'accessToken' - ); + await service.provisionUser(systemId, idToken, accessToken); - expect(result).toEqual<{ user?: UserDO; redirect: string }>({ - user, - redirect: 'https://mock.de/dashboard', - }); expect(provisioningService.provisionData).not.toHaveBeenCalled(); }); + + it('should return the user', async () => { + const { systemId, idToken, accessToken, user } = setup(); + + const result = await service.provisionUser(systemId, idToken, accessToken); + + expect(result).toEqual(user); + }); }); - describe('when provisioning a new user, that is in a school with provisioning disabled', () => { + describe('when provisioning a new user and an existing school with official school number, that has provisioning disabled', () => { const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; const externalUserId = 'externalUserId'; - const oauthData: OauthDataDto = new OauthDataDto({ - system: new ProvisioningSystemDto({ - systemId: 'systemId', - provisioningStrategy: SystemProvisioningStrategy.OIDC, - }), - externalUser: new ExternalUserDto({ + const externalSchoolId = 'externalSchoolId'; + const officialSchoolNumber = 'officialSchoolNumber'; + + const school: LegacySchoolDo = legacySchoolDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalSchoolId, + officialSchoolNumber, + features: [], + }); + + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { externalId: externalUserId, - }), - externalSchool: new ExternalSchoolDto({ - externalId: 'schoolExternalId', - name: 'externalSchool', - officialSchoolNumber: 'officialSchoolNumber', - }), + }, + externalSchool: { + externalId: externalSchoolId, + name: school.name, + officialSchoolNumber, + }, }); - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ features: [] }); - provisioningService.getData.mockResolvedValue(oauthData); - schoolService.getSchoolBySchoolNumber.mockResolvedValue(school); - migrationCheckService.shouldUserMigrate.mockResolvedValue(false); - userService.findByExternalId.mockResolvedValue(null); + provisioningService.getData.mockResolvedValueOnce(provisioningData); + schoolService.getSchoolBySchoolNumber.mockResolvedValueOnce(school); + migrationCheckService.shouldUserMigrate.mockResolvedValueOnce(false); + userService.findByExternalId.mockResolvedValueOnce(null); + + return { + systemId, + idToken, + accessToken, + provisioningData, + }; }; - it('should throw UserNotFoundAfterProvisioningLoggableException', async () => { - setup(); + it('should not provision the data', async () => { + const { systemId, idToken, accessToken } = setup(); - const func = () => service.provisionUser('systemId', 'idToken', 'accessToken'); + await expect(service.provisionUser(systemId, idToken, accessToken)).rejects.toThrow(); - await expect(func).rejects.toThrow(UserNotFoundAfterProvisioningLoggableException); expect(provisioningService.provisionData).not.toHaveBeenCalled(); }); + + it('should throw an error', async () => { + const { systemId, idToken, accessToken } = setup(); + + await expect(service.provisionUser(systemId, idToken, accessToken)).rejects.toThrow( + UserNotFoundAfterProvisioningLoggableException + ); + }); }); - describe('when the user cannot be found after provisioning', () => { + describe('when provisioning a new user and an existing school with official school number, that is currently migrating', () => { const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; const externalUserId = 'externalUserId'; - const oauthData: OauthDataDto = new OauthDataDto({ - system: new ProvisioningSystemDto({ - systemId: 'systemId', - provisioningStrategy: SystemProvisioningStrategy.OIDC, - }), - externalUser: new ExternalUserDto({ - externalId: externalUserId, - }), + const externalSchoolId = 'externalSchoolId'; + const officialSchoolNumber = 'officialSchoolNumber'; + + const school: LegacySchoolDo = legacySchoolDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalSchoolId, + officialSchoolNumber, + features: [SchoolFeatures.OAUTH_PROVISIONING_ENABLED], }); - const provisioningDto: ProvisioningDto = new ProvisioningDto({ - externalUserId, + + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { + externalId: externalUserId, + }, + externalSchool: { + externalId: externalSchoolId, + name: school.name, + officialSchoolNumber, + }, }); - provisioningService.getData.mockResolvedValue(oauthData); - provisioningService.provisionData.mockResolvedValue(provisioningDto); - userService.findByExternalId.mockResolvedValue(null); + provisioningService.getData.mockResolvedValueOnce(provisioningData); + schoolService.getSchoolBySchoolNumber.mockResolvedValueOnce(school); + migrationCheckService.shouldUserMigrate.mockResolvedValueOnce(true); + userService.findByExternalId.mockResolvedValueOnce(null); + + return { + systemId, + idToken, + accessToken, + provisioningData, + }; }; - it('should throw an error', async () => { - setup(); + it('should not provision the data', async () => { + const { systemId, idToken, accessToken } = setup(); - const func = () => service.provisionUser('systemId', 'idToken', 'accessToken'); + await service.provisionUser(systemId, idToken, accessToken); - await expect(func).rejects.toThrow(UserNotFoundAfterProvisioningLoggableException); + expect(provisioningService.provisionData).not.toHaveBeenCalled(); }); - }); - }); - describe('getAuthenticationUrl is called', () => { - describe('when a normal authentication url is requested', () => { - it('should return a authentication url', () => { - const oauthConfig: OauthConfig = new OauthConfig({ - clientId: '12345', - clientSecret: 'mocksecret', - tokenEndpoint: 'http://mock.de/mock/auth/public/mockToken', - grantType: 'authorization_code', - redirectUri: 'http://mockhost:3030/api/v3/sso/oauth/testsystemId', - scope: 'openid uuid', - responseType: 'code', - authEndpoint: 'http://mock.de/auth', - provider: 'mock_type', - logoutEndpoint: 'http://mock.de/logout', - issuer: 'mock_issuer', - jwksEndpoint: 'http://mock.de/jwks', - }); + it('should return null', async () => { + const { systemId, idToken, accessToken } = setup(); - const result: string = service.getAuthenticationUrl(oauthConfig, 'state', false); + const result = await service.provisionUser(systemId, idToken, accessToken); - expect(result).toEqual( - 'http://mock.de/auth?client_id=12345&redirect_uri=https%3A%2F%2Fmock.de%2Fapi%2Fv3%2Fsso%2Foauth&response_type=code&scope=openid+uuid&state=state' - ); + expect(result).toBeNull(); }); }); - describe('when a migration authentication url is requested', () => { - it('should return a authentication url', () => { - const oauthConfig: OauthConfig = new OauthConfig({ - clientId: '12345', - clientSecret: 'mocksecret', - tokenEndpoint: 'http://mock.de/mock/auth/public/mockToken', - grantType: 'authorization_code', - redirectUri: 'http://mockhost.de/api/v3/sso/oauth/testsystemId', - scope: 'openid uuid', - responseType: 'code', - authEndpoint: 'http://mock.de/auth', - provider: 'mock_type', - logoutEndpoint: 'http://mock.de/logout', - issuer: 'mock_issuer', - jwksEndpoint: 'http://mock.de/jwks', + describe('when provisioning an existing user and an existing school with official school number, that is currently migrating', () => { + const setup = () => { + const systemId = new ObjectId().toHexString(); + const idToken = 'idToken'; + const accessToken = 'accessToken'; + const externalUserId = 'externalUserId'; + const externalSchoolId = 'externalSchoolId'; + const officialSchoolNumber = 'officialSchoolNumber'; + + const user: UserDO = userDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalUserId, + }); + + const school: LegacySchoolDo = legacySchoolDoFactory.build({ + id: new ObjectId().toHexString(), + externalId: externalSchoolId, + officialSchoolNumber, + features: [SchoolFeatures.OAUTH_PROVISIONING_ENABLED], }); - const result: string = service.getAuthenticationUrl(oauthConfig, 'state', true); + const provisioningData: OauthDataDto = new OauthDataDto({ + system: { + systemId, + provisioningStrategy: SystemProvisioningStrategy.SANIS, + provisioningUrl: 'https://mock.person-info.de/', + }, + externalUser: { + externalId: externalUserId, + }, + externalSchool: { + externalId: externalSchoolId, + name: school.name, + officialSchoolNumber, + }, + }); - expect(result).toEqual( - 'http://mock.de/auth?client_id=12345&redirect_uri=https%3A%2F%2Fmock.de%2Fapi%2Fv3%2Fsso%2Foauth%2Fmigration&response_type=code&scope=openid+uuid&state=state' - ); + provisioningService.getData.mockResolvedValueOnce(provisioningData); + schoolService.getSchoolBySchoolNumber.mockResolvedValueOnce(school); + migrationCheckService.shouldUserMigrate.mockResolvedValueOnce(true); + userService.findByExternalId.mockResolvedValueOnce(user).mockResolvedValueOnce(user); + + return { + systemId, + idToken, + accessToken, + provisioningData, + user, + }; + }; + + it('should provision the data', async () => { + const { systemId, idToken, accessToken, provisioningData } = setup(); + + await service.provisionUser(systemId, idToken, accessToken); + + expect(provisioningService.provisionData).toHaveBeenCalledWith(provisioningData); }); - it('should return add an idp hint if existing authentication url', () => { - const oauthConfig: OauthConfig = new OauthConfig({ - clientId: '12345', - clientSecret: 'mocksecret', - tokenEndpoint: 'http://mock.de/mock/auth/public/mockToken', - grantType: 'authorization_code', - redirectUri: 'http://mockhost.de/api/v3/sso/oauth/testsystemId', - scope: 'openid uuid', - responseType: 'code', - authEndpoint: 'http://mock.de/auth', - provider: 'mock_type', - logoutEndpoint: 'http://mock.de/logout', - issuer: 'mock_issuer', - jwksEndpoint: 'http://mock.de/jwks', - idpHint: 'TheIdpHint', - }); + it('should return the user', async () => { + const { systemId, idToken, accessToken, user } = setup(); - const result: string = service.getAuthenticationUrl(oauthConfig, 'state', true); + const result = await service.provisionUser(systemId, idToken, accessToken); - expect(result).toEqual( - 'http://mock.de/auth?client_id=12345&redirect_uri=https%3A%2F%2Fmock.de%2Fapi%2Fv3%2Fsso%2Foauth%2Fmigration&response_type=code&scope=openid+uuid&state=state&kc_idp_hint=TheIdpHint' - ); + expect(result).toEqual(user); }); }); }); diff --git a/apps/server/src/modules/oauth/service/oauth.service.ts b/apps/server/src/modules/oauth/service/oauth.service.ts index 190c962cd97..9a484a52d47 100644 --- a/apps/server/src/modules/oauth/service/oauth.service.ts +++ b/apps/server/src/modules/oauth/service/oauth.service.ts @@ -1,19 +1,18 @@ -import { Configuration } from '@hpi-schul-cloud/commons'; -import { Inject } from '@nestjs/common'; -import { Injectable } from '@nestjs/common/decorators/core/injectable.decorator'; -import { EntityId, LegacySchoolDo, OauthConfig, SchoolFeatures, UserDO } from '@shared/domain'; import { DefaultEncryptionService, IEncryptionService } from '@infra/encryption'; -import { LegacyLogger } from '@src/core/logger'; +import { LegacySchoolService } from '@modules/legacy-school'; import { ProvisioningService } from '@modules/provisioning'; import { OauthDataDto } from '@modules/provisioning/dto'; -import { LegacySchoolService } from '@modules/legacy-school'; import { SystemService } from '@modules/system'; import { SystemDto } from '@modules/system/service'; import { UserService } from '@modules/user'; -import { MigrationCheckService, UserMigrationService } from '@modules/user-login-migration'; +import { MigrationCheckService } from '@modules/user-login-migration'; +import { Inject } from '@nestjs/common'; +import { Injectable } from '@nestjs/common/decorators/core/injectable.decorator'; +import { EntityId, LegacySchoolDo, OauthConfig, SchoolFeatures, UserDO } from '@shared/domain'; +import { LegacyLogger } from '@src/core/logger'; import jwt, { JwtPayload } from 'jsonwebtoken'; -import { OAuthSSOError, SSOErrorCode, UserNotFoundAfterProvisioningLoggableException } from '../loggable'; import { OAuthTokenDto } from '../interface'; +import { OAuthSSOError, SSOErrorCode, UserNotFoundAfterProvisioningLoggableException } from '../loggable'; import { TokenRequestMapper } from '../mapper/token-request.mapper'; import { AuthenticationCodeGrantTokenRequest, OauthTokenResponse } from './dto'; import { OauthAdapterService } from './oauth-adapter.service'; @@ -27,7 +26,6 @@ export class OAuthService { private readonly logger: LegacyLogger, private readonly provisioningService: ProvisioningService, private readonly systemService: SystemService, - private readonly userMigrationService: UserMigrationService, private readonly migrationCheckService: MigrationCheckService, private readonly schoolService: LegacySchoolService ) { @@ -60,22 +58,16 @@ export class OAuthService { return oauthTokens; } - async provisionUser( - systemId: string, - idToken: string, - accessToken: string, - postLoginRedirect?: string - ): Promise<{ user?: UserDO; redirect: string }> { + async provisionUser(systemId: string, idToken: string, accessToken: string): Promise { const data: OauthDataDto = await this.provisioningService.getData(systemId, idToken, accessToken); const externalUserId: string = data.externalUser.externalId; const officialSchoolNumber: string | undefined = data.externalSchool?.officialSchoolNumber; - let provisioning = true; - let migrationConsentRedirect: string | undefined; + let isProvisioningEnabled = true; if (officialSchoolNumber) { - provisioning = await this.isOauthProvisioningEnabledForSchool(officialSchoolNumber); + isProvisioningEnabled = await this.isOauthProvisioningEnabledForSchool(officialSchoolNumber); const shouldUserMigrate: boolean = await this.migrationCheckService.shouldUserMigrate( externalUserId, @@ -84,33 +76,21 @@ export class OAuthService { ); if (shouldUserMigrate) { - // TODO: https://ticketsystem.dbildungscloud.de/browse/N21-632 Move Redirect Logic URLs to Client - migrationConsentRedirect = await this.userMigrationService.getMigrationConsentPageRedirect( - officialSchoolNumber, - systemId - ); - const existingUser: UserDO | null = await this.userService.findByExternalId(externalUserId, systemId); + if (!existingUser) { - return { user: undefined, redirect: migrationConsentRedirect }; + return null; } } } - if (provisioning) { + if (isProvisioningEnabled) { await this.provisioningService.provisionData(data); } const user: UserDO = await this.findUserAfterProvisioningOrThrow(externalUserId, systemId, officialSchoolNumber); - // TODO: https://ticketsystem.dbildungscloud.de/browse/N21-632 Move Redirect Logic URLs to Client - const redirect: string = await this.getPostLoginRedirectUrl( - idToken, - systemId, - postLoginRedirect || migrationConsentRedirect - ); - - return { user, redirect }; + return user; } private async findUserAfterProvisioningOrThrow( @@ -166,51 +146,6 @@ export class OAuthService { return decodedJWT; } - async getPostLoginRedirectUrl(idToken: string, systemId: string, postLoginRedirect?: string): Promise { - const clientUrl: string = Configuration.get('HOST') as string; - const dashboardUrl: URL = new URL('/dashboard', clientUrl); - const system: SystemDto = await this.systemService.findById(systemId); - - let redirect: string; - if (system.oauthConfig?.provider === 'iserv' && system.oauthConfig?.logoutEndpoint) { - const iservLogoutUrl: URL = new URL(system.oauthConfig.logoutEndpoint); - iservLogoutUrl.searchParams.append('id_token_hint', idToken); - iservLogoutUrl.searchParams.append('post_logout_redirect_uri', postLoginRedirect || dashboardUrl.toString()); - redirect = iservLogoutUrl.toString(); - } else if (postLoginRedirect) { - redirect = postLoginRedirect; - } else { - redirect = dashboardUrl.toString(); - } - - return redirect; - } - - getAuthenticationUrl(oauthConfig: OauthConfig, state: string, migration: boolean): string { - const redirectUri: string = this.getRedirectUri(migration); - - const authenticationUrl: URL = new URL(oauthConfig.authEndpoint); - authenticationUrl.searchParams.append('client_id', oauthConfig.clientId); - authenticationUrl.searchParams.append('redirect_uri', redirectUri); - authenticationUrl.searchParams.append('response_type', oauthConfig.responseType); - authenticationUrl.searchParams.append('scope', oauthConfig.scope); - authenticationUrl.searchParams.append('state', state); - if (oauthConfig.idpHint) { - authenticationUrl.searchParams.append('kc_idp_hint', oauthConfig.idpHint); - } - - return authenticationUrl.toString(); - } - - getRedirectUri(migration: boolean) { - const publicBackendUrl: string = Configuration.get('PUBLIC_BACKEND_URL') as string; - - const path: string = migration ? 'api/v3/sso/oauth/migration' : 'api/v3/sso/oauth'; - const redirectUri: URL = new URL(path, publicBackendUrl); - - return redirectUri.toString(); - } - private buildTokenRequestPayload( code: string, oauthConfig: OauthConfig, diff --git a/apps/server/src/modules/oauth/uc/dto/oauth-login-state.dto.ts b/apps/server/src/modules/oauth/uc/dto/oauth-login-state.dto.ts deleted file mode 100644 index 10d01b596d2..00000000000 --- a/apps/server/src/modules/oauth/uc/dto/oauth-login-state.dto.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { EntityId } from '@shared/domain'; - -export class OauthLoginStateDto { - state: string; - - systemId: EntityId; - - provider: string; - - postLoginRedirect?: string; - - userLoginMigration: boolean; - - constructor(props: OauthLoginStateDto) { - this.state = props.state; - this.systemId = props.systemId; - this.postLoginRedirect = props.postLoginRedirect; - this.provider = props.provider; - this.userLoginMigration = props.userLoginMigration; - } -} diff --git a/apps/server/src/modules/oauth/uc/index.ts b/apps/server/src/modules/oauth/uc/index.ts index 32e4dce0f74..e1a569e5f88 100644 --- a/apps/server/src/modules/oauth/uc/index.ts +++ b/apps/server/src/modules/oauth/uc/index.ts @@ -1,2 +1 @@ -export * from './oauth.uc'; export * from './hydra-oauth.uc'; diff --git a/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts b/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts deleted file mode 100644 index 67a6d269f87..00000000000 --- a/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts +++ /dev/null @@ -1,309 +0,0 @@ -import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { OauthCurrentUser } from '@modules/authentication/interface'; -import { AuthenticationService } from '@modules/authentication/services/authentication.service'; -import { LegacySchoolService } from '@modules/legacy-school'; -import { OauthUc } from '@modules/oauth/uc/oauth.uc'; -import { SystemService } from '@modules/system'; -import { OauthConfigDto, SystemDto } from '@modules/system/service'; -import { UserService } from '@modules/user'; -import { UnprocessableEntityException } from '@nestjs/common'; -import { Test, TestingModule } from '@nestjs/testing'; -import { UserDO } from '@shared/domain'; -import { ISession } from '@shared/domain/types/session'; -import { setupEntities } from '@shared/testing'; -import { LegacyLogger } from '@src/core/logger'; -import { OAuthTokenDto } from '../interface'; -import { OAuthSSOError } from '../loggable'; -import { OAuthProcessDto } from '../service/dto'; -import { OAuthService } from '../service/oauth.service'; -import { OauthLoginStateDto } from './dto/oauth-login-state.dto'; -import resetAllMocks = jest.resetAllMocks; - -jest.mock('nanoid', () => { - return { - nanoid: () => 'mockNanoId', - }; -}); - -describe(OauthUc.name, () => { - let module: TestingModule; - let uc: OauthUc; - - let authenticationService: DeepMocked; - let oauthService: DeepMocked; - let systemService: DeepMocked; - let userService: DeepMocked; - - beforeAll(async () => { - await setupEntities(); - - module = await Test.createTestingModule({ - providers: [ - OauthUc, - { - provide: LegacyLogger, - useValue: createMock(), - }, - { - provide: SystemService, - useValue: createMock(), - }, - { - provide: OAuthService, - useValue: createMock(), - }, - { - provide: AuthenticationService, - useValue: createMock(), - }, - { - provide: UserService, - useValue: createMock(), - }, - { - provide: LegacySchoolService, - useValue: createMock(), - }, - { - provide: AuthenticationService, - useValue: createMock(), - }, - ], - }).compile(); - - uc = module.get(OauthUc); - systemService = module.get(SystemService); - authenticationService = module.get(AuthenticationService); - oauthService = module.get(OAuthService); - userService = module.get(UserService); - authenticationService = module.get(AuthenticationService); - }); - - afterAll(async () => { - await module.close(); - }); - - afterEach(() => { - resetAllMocks(); - }); - - const createOAuthTestData = () => { - const oauthConfig: OauthConfigDto = new OauthConfigDto({ - clientId: '12345', - clientSecret: 'mocksecret', - tokenEndpoint: 'https://mock.de/mock/auth/public/mockToken', - grantType: 'authorization_code', - scope: 'openid uuid', - responseType: 'code', - authEndpoint: 'mock_authEndpoint', - provider: 'mock_provider', - logoutEndpoint: 'mock_logoutEndpoint', - issuer: 'mock_issuer', - jwksEndpoint: 'mock_jwksEndpoint', - redirectUri: 'mock_codeRedirectUri', - }); - const system: SystemDto = new SystemDto({ - id: 'systemId', - type: 'oauth', - oauthConfig, - }); - - return { - system, - systemId: system.id as string, - oauthConfig, - }; - }; - - describe('startOauthLogin', () => { - describe('when starting an oauth login without migration', () => { - const setup = () => { - const { system, systemId } = createOAuthTestData(); - - const session: DeepMocked = createMock(); - const authenticationUrl = 'authenticationUrl'; - - systemService.findById.mockResolvedValue(system); - oauthService.getAuthenticationUrl.mockReturnValue(authenticationUrl); - - return { - systemId, - session, - authenticationUrl, - }; - }; - - it('should return the authentication url for the system', async () => { - const { systemId, session, authenticationUrl } = setup(); - - const result: string = await uc.startOauthLogin(session, systemId, false); - - expect(result).toEqual(authenticationUrl); - }); - }); - - describe('when starting an oauth login during a migration', () => { - const setup = () => { - const { system, systemId } = createOAuthTestData(); - - const session: DeepMocked = createMock(); - const authenticationUrl = 'authenticationUrl'; - const postLoginRedirect = 'postLoginRedirect'; - - systemService.findById.mockResolvedValue(system); - oauthService.getAuthenticationUrl.mockReturnValue(authenticationUrl); - - return { - system, - systemId, - postLoginRedirect, - session, - }; - }; - - it('should save data to the session', async () => { - const { systemId, system, session, postLoginRedirect } = setup(); - - await uc.startOauthLogin(session, systemId, false, postLoginRedirect); - - expect(session.oauthLoginState).toEqual({ - systemId, - state: 'mockNanoId', - postLoginRedirect, - provider: system.oauthConfig?.provider as string, - userLoginMigration: false, - }); - }); - }); - - describe('when the system cannot be found', () => { - const setup = () => { - const { systemId, system } = createOAuthTestData(); - system.oauthConfig = undefined; - const session: DeepMocked = createMock(); - const authenticationUrl = 'authenticationUrl'; - - systemService.findById.mockResolvedValue(system); - oauthService.getAuthenticationUrl.mockReturnValue(authenticationUrl); - - return { - systemId, - session, - authenticationUrl, - }; - }; - - it('should throw UnprocessableEntityException', async () => { - const { systemId, session } = setup(); - - const func = async () => uc.startOauthLogin(session, systemId, false); - - await expect(func).rejects.toThrow(UnprocessableEntityException); - }); - }); - }); - - describe('processOAuth', () => { - const setup = () => { - const postLoginRedirect = 'postLoginRedirect'; - const cachedState: OauthLoginStateDto = new OauthLoginStateDto({ - state: 'state', - systemId: 'systemId', - postLoginRedirect, - provider: 'mock_provider', - userLoginMigration: false, - }); - const code = 'code'; - const error = 'error'; - - const jwt = 'schulcloudJwt'; - const redirect = 'redirect'; - const user: UserDO = new UserDO({ - id: 'mockUserId', - firstName: 'firstName', - lastName: 'lastame', - email: '', - roles: [], - schoolId: 'mockSchoolId', - externalId: 'mockExternalId', - }); - - const currentUser: OauthCurrentUser = { userId: 'userId', isExternalUser: true } as OauthCurrentUser; - const testSystem: SystemDto = new SystemDto({ - id: 'mockSystemId', - type: 'mock', - oauthConfig: { provider: 'testProvider' } as OauthConfigDto, - }); - const tokenDto: OAuthTokenDto = new OAuthTokenDto({ - idToken: 'idToken', - refreshToken: 'refreshToken', - accessToken: 'accessToken', - }); - - return { cachedState, code, error, jwt, redirect, user, currentUser, testSystem, tokenDto }; - }; - - describe('when a user is returned', () => { - it('should return a response with a valid jwt', async () => { - const { cachedState, code, error, jwt, redirect, user, currentUser, tokenDto } = setup(); - - userService.getResolvedUser.mockResolvedValue(currentUser); - authenticationService.generateJwt.mockResolvedValue({ accessToken: jwt }); - oauthService.authenticateUser.mockResolvedValue(tokenDto); - oauthService.provisionUser.mockResolvedValue({ user, redirect }); - - const response: OAuthProcessDto = await uc.processOAuthLogin(cachedState, code, error); - expect(response).toEqual( - expect.objectContaining({ - jwt, - redirect, - }) - ); - }); - }); - - describe('when no user is returned', () => { - it('should return a response without a jwt', async () => { - const { cachedState, code, error, redirect, tokenDto } = setup(); - oauthService.authenticateUser.mockResolvedValue(tokenDto); - oauthService.provisionUser.mockResolvedValue({ redirect }); - - const response: OAuthProcessDto = await uc.processOAuthLogin(cachedState, code, error); - - expect(response).toEqual({ - redirect, - }); - }); - }); - - describe('when an error occurs', () => { - it('should return an OAuthProcessDto with error', async () => { - const { cachedState, code, error, testSystem } = setup(); - oauthService.authenticateUser.mockRejectedValue(new OAuthSSOError('Testmessage')); - systemService.findById.mockResolvedValue(testSystem); - - const response = uc.processOAuthLogin(cachedState, code, error); - - await expect(response).rejects.toThrow(OAuthSSOError); - }); - }); - - describe('when the process runs successfully', () => { - it('should return a valid jwt', async () => { - const { cachedState, code, user, currentUser, jwt, redirect, tokenDto } = setup(); - - userService.getResolvedUser.mockResolvedValue(currentUser); - authenticationService.generateJwt.mockResolvedValue({ accessToken: jwt }); - oauthService.authenticateUser.mockResolvedValue(tokenDto); - oauthService.provisionUser.mockResolvedValue({ user, redirect }); - - const response: OAuthProcessDto = await uc.processOAuthLogin(cachedState, code); - - expect(response).toEqual({ - jwt, - redirect, - }); - }); - }); - }); -}); diff --git a/apps/server/src/modules/oauth/uc/oauth.uc.ts b/apps/server/src/modules/oauth/uc/oauth.uc.ts deleted file mode 100644 index bc48eb79a31..00000000000 --- a/apps/server/src/modules/oauth/uc/oauth.uc.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { OauthCurrentUser } from '@modules/authentication/interface'; -import { AuthenticationService } from '@modules/authentication/services/authentication.service'; -import { SystemService } from '@modules/system'; -import { SystemDto } from '@modules/system/service/dto/system.dto'; -import { UserService } from '@modules/user'; -import { Injectable, UnprocessableEntityException } from '@nestjs/common'; -import { EntityId, UserDO } from '@shared/domain'; -import { ISession } from '@shared/domain/types/session'; -import { LegacyLogger } from '@src/core/logger'; -import { nanoid } from 'nanoid'; -import { OAuthTokenDto } from '../interface'; -import { OAuthProcessDto } from '../service/dto'; -import { OAuthService } from '../service/oauth.service'; -import { OauthLoginStateDto } from './dto/oauth-login-state.dto'; - -/** - * @deprecated remove after login via oauth moved to authentication module - */ -@Injectable() -export class OauthUc { - constructor( - private readonly oauthService: OAuthService, - private readonly authenticationService: AuthenticationService, - private readonly systemService: SystemService, - private readonly userService: UserService, - private readonly logger: LegacyLogger - ) { - this.logger.setContext(OauthUc.name); - } - - async startOauthLogin( - session: ISession, - systemId: EntityId, - migration: boolean, - postLoginRedirect?: string - ): Promise { - const state = nanoid(16); - - const system: SystemDto = await this.systemService.findById(systemId); - if (!system.oauthConfig) { - throw new UnprocessableEntityException(`Requested system ${systemId} has no oauth configured`); - } - - const authenticationUrl: string = this.oauthService.getAuthenticationUrl(system.oauthConfig, state, migration); - - session.oauthLoginState = new OauthLoginStateDto({ - state, - systemId, - provider: system.oauthConfig.provider, - postLoginRedirect, - userLoginMigration: migration, - }); - - return authenticationUrl; - } - - async processOAuthLogin(cachedState: OauthLoginStateDto, code?: string, error?: string): Promise { - const { state, systemId, postLoginRedirect, userLoginMigration } = cachedState; - - this.logger.debug(`Oauth login process started. [state: ${state}, system: ${systemId}]`); - - const redirectUri: string = this.oauthService.getRedirectUri(userLoginMigration); - - const tokenDto: OAuthTokenDto = await this.oauthService.authenticateUser(systemId, redirectUri, code, error); - - const { user, redirect }: { user?: UserDO; redirect: string } = await this.oauthService.provisionUser( - systemId, - tokenDto.idToken, - tokenDto.accessToken, - postLoginRedirect - ); - - this.logger.debug(`Generating jwt for user. [state: ${state}, system: ${systemId}]`); - - let jwt: string | undefined; - if (user && user.id) { - jwt = await this.getJwtForUser(user.id); - } - - const response = new OAuthProcessDto({ - jwt, - redirect, - }); - - return response; - } - - private async getJwtForUser(userId: EntityId): Promise { - const oauthCurrentUser: OauthCurrentUser = await this.userService.getResolvedUser(userId); - - const { accessToken } = await this.authenticationService.generateJwt(oauthCurrentUser); - - return accessToken; - } -} diff --git a/apps/server/src/modules/server/server.module.ts b/apps/server/src/modules/server/server.module.ts index a812ff773b2..c048156d08b 100644 --- a/apps/server/src/modules/server/server.module.ts +++ b/apps/server/src/modules/server/server.module.ts @@ -1,4 +1,8 @@ import { Configuration } from '@hpi-schul-cloud/commons'; +import { MongoDatabaseModuleOptions, MongoMemoryDatabaseModule } from '@infra/database'; +import { MailModule } from '@infra/mail'; +import { RabbitMQWrapperModule, RabbitMQWrapperTestModule } from '@infra/rabbitmq'; +import { REDIS_CLIENT, RedisModule } from '@infra/redis'; import { Dictionary, IPrimaryKey } from '@mikro-orm/core'; import { MikroOrmModule, MikroOrmModuleSyncOptions } from '@mikro-orm/nestjs'; import { AccountApiModule } from '@modules/account/account-api.module'; @@ -8,7 +12,6 @@ import { CollaborativeStorageModule } from '@modules/collaborative-storage'; import { FilesStorageClientModule } from '@modules/files-storage-client'; import { GroupApiModule } from '@modules/group/group-api.module'; import { LearnroomApiModule } from '@modules/learnroom/learnroom-api.module'; -import { LegacySchoolApiModule } from '@modules/legacy-school/legacy-school-api.module'; import { LessonApiModule } from '@modules/lesson/lesson-api.module'; import { MetaTagExtractorApiModule, MetaTagExtractorModule } from '@modules/meta-tag-extractor'; import { NewsModule } from '@modules/news'; @@ -28,10 +31,6 @@ import { VideoConferenceApiModule } from '@modules/video-conference/video-confer import { DynamicModule, Inject, MiddlewareConsumer, Module, NestModule, NotFoundException } from '@nestjs/common'; import { ConfigModule } from '@nestjs/config'; import { ALL_ENTITIES } from '@shared/domain'; -import { MongoDatabaseModuleOptions, MongoMemoryDatabaseModule } from '@infra/database'; -import { MailModule } from '@infra/mail'; -import { RabbitMQWrapperModule, RabbitMQWrapperTestModule } from '@infra/rabbitmq'; -import { RedisModule, REDIS_CLIENT } from '@infra/redis'; import { createConfigModuleOptions, DB_PASSWORD, DB_URL, DB_USERNAME } from '@src/config'; import { CoreModule } from '@src/core'; import { LegacyLogger, LoggerModule } from '@src/core/logger'; @@ -68,7 +67,6 @@ const serverModules = [ adminUser: Configuration.get('ROCKET_CHAT_ADMIN_USER') as string, adminPassword: Configuration.get('ROCKET_CHAT_ADMIN_PASSWORD') as string, }), - LegacySchoolApiModule, VideoConferenceApiModule, OauthProviderApiModule, SharingApiModule, diff --git a/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts b/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts index 154148c1fa0..f23795a35ab 100644 --- a/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts +++ b/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts @@ -688,7 +688,11 @@ describe('UserLoginMigrationController (API)', () => { const setup = async () => { const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - await em.persistAndFlush([teacherAccount, teacherUser]); + const userLoginMigration: UserLoginMigrationEntity = userLoginMigrationFactory.buildWithId({ + school: teacherUser.school, + }); + + await em.persistAndFlush([teacherAccount, teacherUser, userLoginMigration]); em.clear(); const loggedInClient = await testApiClient.login(teacherAccount); @@ -1156,9 +1160,22 @@ describe('UserLoginMigrationController (API)', () => { closedAt: new Date(2023, 1, 5), }); + const migratedUser = userFactory.buildWithId({ + school, + lastLoginSystemChange: new Date(2023, 1, 4), + }); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); - await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); + await em.persistAndFlush([ + sourceSystem, + targetSystem, + school, + adminAccount, + adminUser, + userLoginMigration, + migratedUser, + ]); em.clear(); const loggedInClient = await testApiClient.login(adminAccount); diff --git a/apps/server/src/modules/user-login-migration/controller/dto/index.ts b/apps/server/src/modules/user-login-migration/controller/dto/index.ts index a158f06bb75..bcbb7e46f4e 100644 --- a/apps/server/src/modules/user-login-migration/controller/dto/index.ts +++ b/apps/server/src/modules/user-login-migration/controller/dto/index.ts @@ -1,5 +1,3 @@ export * from './request/user-login-migration-search.params'; -export * from './request/page-type.query.param'; export * from './response/user-login-migration.response'; export * from './response/user-login-migration-search-list.response'; -export * from './response/page-content.response'; diff --git a/apps/server/src/modules/user-login-migration/index.ts b/apps/server/src/modules/user-login-migration/index.ts index c357ceaf98e..c3841ebf249 100644 --- a/apps/server/src/modules/user-login-migration/index.ts +++ b/apps/server/src/modules/user-login-migration/index.ts @@ -1,3 +1,2 @@ export * from './user-login-migration.module'; export * from './service'; -export * from './error'; diff --git a/apps/server/src/modules/user-login-migration/mapper/index.ts b/apps/server/src/modules/user-login-migration/mapper/index.ts index 03b0a12e9be..7cf4b79d56c 100644 --- a/apps/server/src/modules/user-login-migration/mapper/index.ts +++ b/apps/server/src/modules/user-login-migration/mapper/index.ts @@ -1,2 +1 @@ -export * from './page-content.mapper'; export * from './user-login-migration.mapper'; diff --git a/apps/server/src/modules/user-login-migration/mapper/page-content.mapper.spec.ts b/apps/server/src/modules/user-login-migration/mapper/page-content.mapper.spec.ts deleted file mode 100644 index e3b1e538c47..00000000000 --- a/apps/server/src/modules/user-login-migration/mapper/page-content.mapper.spec.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { Test, TestingModule } from '@nestjs/testing'; -import { PageContentMapper } from './page-content.mapper'; -import { PageContentDto } from '../service/dto/page-content.dto'; - -describe('PageContentMapper', () => { - let module: TestingModule; - let mapper: PageContentMapper; - - beforeAll(async () => { - module = await Test.createTestingModule({ - providers: [PageContentMapper], - }).compile(); - mapper = module.get(PageContentMapper); - }); - - afterAll(async () => { - await module.close(); - }); - - const setup = () => { - const dto: PageContentDto = { - proceedButtonUrl: 'proceed', - cancelButtonUrl: 'cancel', - }; - return { dto }; - }; - - describe('mapDtoToResponse is called', () => { - describe('when it maps from dto to response', () => { - it('should map the dto to a response', () => { - const { dto } = setup(); - const response = mapper.mapDtoToResponse(dto); - expect(response.proceedButtonUrl).toEqual(dto.proceedButtonUrl); - expect(response.cancelButtonUrl).toEqual(dto.cancelButtonUrl); - }); - }); - }); -}); diff --git a/apps/server/src/modules/user-login-migration/mapper/page-content.mapper.ts b/apps/server/src/modules/user-login-migration/mapper/page-content.mapper.ts deleted file mode 100644 index 7abec9c2208..00000000000 --- a/apps/server/src/modules/user-login-migration/mapper/page-content.mapper.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { Injectable } from '@nestjs/common'; -import { PageContentDto } from '../service/dto/page-content.dto'; -import { PageContentResponse } from '../controller/dto'; - -@Injectable() -export class PageContentMapper { - mapDtoToResponse(dto: PageContentDto): PageContentResponse { - const response: PageContentResponse = new PageContentResponse({ - proceedButtonUrl: dto.proceedButtonUrl, - cancelButtonUrl: dto.cancelButtonUrl, - }); - - return response; - } -} diff --git a/apps/server/src/modules/user-login-migration/service/school-migration.service.spec.ts b/apps/server/src/modules/user-login-migration/service/school-migration.service.spec.ts index e0c8de420f1..9a8adfb7fd5 100644 --- a/apps/server/src/modules/user-login-migration/service/school-migration.service.spec.ts +++ b/apps/server/src/modules/user-login-migration/service/school-migration.service.spec.ts @@ -2,7 +2,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import { LegacySchoolService } from '@modules/legacy-school'; import { UserService } from '@modules/user'; -import { UnprocessableEntityException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { LegacySchoolDo, Page, UserDO, UserLoginMigrationDO } from '@shared/domain'; import { UserLoginMigrationRepo } from '@shared/repo/userloginmigration/user-login-migration.repo'; @@ -296,18 +295,18 @@ describe(SchoolMigrationService.name, () => { const users: UserDO[] = userDoFactory.buildListWithId(3, { outdatedSince: undefined }); - userLoginMigrationRepo.findBySchoolId.mockResolvedValue(userLoginMigration); userService.findUsers.mockResolvedValue(new Page(users, users.length)); return { closedAt, + userLoginMigration, }; }; it('should save migrated user with removed outdatedSince entry', async () => { - const { closedAt } = setup(); + const { closedAt, userLoginMigration } = setup(); - await service.markUnmigratedUsersAsOutdated('schoolId'); + await service.markUnmigratedUsersAsOutdated(userLoginMigration); expect(userService.saveAll).toHaveBeenCalledWith([ expect.objectContaining>({ outdatedSince: closedAt }), @@ -316,20 +315,6 @@ describe(SchoolMigrationService.name, () => { ]); }); }); - - describe('when the school has no migration', () => { - const setup = () => { - userLoginMigrationRepo.findBySchoolId.mockResolvedValue(null); - }; - - it('should throw an UnprocessableEntityException', async () => { - setup(); - - const func = async () => service.markUnmigratedUsersAsOutdated('schoolId'); - - await expect(func).rejects.toThrow(UnprocessableEntityException); - }); - }); }); describe('unmarkOutdatedUsers', () => { @@ -345,14 +330,17 @@ describe(SchoolMigrationService.name, () => { const users: UserDO[] = userDoFactory.buildListWithId(3, { outdatedSince: new Date('2023-05-02') }); - userLoginMigrationRepo.findBySchoolId.mockResolvedValue(userLoginMigration); userService.findUsers.mockResolvedValue(new Page(users, users.length)); + + return { + userLoginMigration, + }; }; it('should save migrated user with removed outdatedSince entry', async () => { - setup(); + const { userLoginMigration } = setup(); - await service.unmarkOutdatedUsers('schoolId'); + await service.unmarkOutdatedUsers(userLoginMigration); expect(userService.saveAll).toHaveBeenCalledWith([ expect.objectContaining>({ outdatedSince: undefined }), @@ -361,20 +349,6 @@ describe(SchoolMigrationService.name, () => { ]); }); }); - - describe('when the school has no migration', () => { - const setup = () => { - userLoginMigrationRepo.findBySchoolId.mockResolvedValue(null); - }; - - it('should throw an UnprocessableEntityException', async () => { - setup(); - - const func = async () => service.unmarkOutdatedUsers('schoolId'); - - await expect(func).rejects.toThrow(UnprocessableEntityException); - }); - }); }); describe('hasSchoolMigratedUser', () => { diff --git a/apps/server/src/modules/user-login-migration/service/user-migration.service.spec.ts b/apps/server/src/modules/user-login-migration/service/user-migration.service.spec.ts index 3315dd0e5cf..efe9da83e66 100644 --- a/apps/server/src/modules/user-login-migration/service/user-migration.service.spec.ts +++ b/apps/server/src/modules/user-login-migration/service/user-migration.service.spec.ts @@ -46,7 +46,7 @@ describe(UserMigrationService.name, () => { jest.resetAllMocks(); }); - describe('migrateUser is called', () => { + describe('migrateUser', () => { const mockDate = new Date(2020, 1, 1); beforeAll(() => { @@ -85,8 +85,8 @@ describe(UserMigrationService.name, () => { systemId: sourceSystemId, }); - userService.findById.mockResolvedValueOnce(user); - accountService.findByUserIdOrFail.mockResolvedValueOnce(accountDto); + userService.findById.mockResolvedValueOnce({ ...user }); + accountService.findByUserIdOrFail.mockResolvedValueOnce({ ...accountDto }); return { user, @@ -173,8 +173,8 @@ describe(UserMigrationService.name, () => { const error = new Error('Cannot save'); - userService.findById.mockResolvedValueOnce(user); - accountService.findByUserIdOrFail.mockResolvedValueOnce(accountDto); + userService.findById.mockResolvedValueOnce({ ...user }); + accountService.findByUserIdOrFail.mockResolvedValueOnce({ ...accountDto }); userService.save.mockRejectedValueOnce(error); diff --git a/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.spec.ts b/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.spec.ts index 404458538a1..a3eead10096 100644 --- a/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.spec.ts +++ b/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.spec.ts @@ -94,19 +94,19 @@ describe(CloseUserLoginMigrationUc.name, () => { }); it('should close the migration', async () => { - const { user, schoolId } = setup(); + const { user, schoolId, userLoginMigration } = setup(); await uc.closeMigration(user.id, schoolId); - expect(userLoginMigrationService.closeMigration).toHaveBeenCalledWith(schoolId); + expect(userLoginMigrationService.closeMigration).toHaveBeenCalledWith(userLoginMigration); }); it('should mark all un-migrated users as outdated', async () => { - const { user, schoolId } = setup(); + const { user, schoolId, closedUserLoginMigration } = setup(); await uc.closeMigration(user.id, schoolId); - expect(schoolMigrationService.markUnmigratedUsersAsOutdated).toHaveBeenCalledWith(schoolId); + expect(schoolMigrationService.markUnmigratedUsersAsOutdated).toHaveBeenCalledWith(closedUserLoginMigration); }); it('should return the closed user login migration', async () => { diff --git a/apps/server/src/modules/user-login-migration/uc/user-login-migration.uc.spec.ts b/apps/server/src/modules/user-login-migration/uc/user-login-migration.uc.spec.ts index 39779247e90..c4965695b69 100644 --- a/apps/server/src/modules/user-login-migration/uc/user-login-migration.uc.spec.ts +++ b/apps/server/src/modules/user-login-migration/uc/user-login-migration.uc.spec.ts @@ -384,8 +384,8 @@ describe(UserLoginMigrationUc.name, () => { await uc.migrate('jwt', 'currentUserId', 'systemId', 'code', 'redirectUri'); expect(schoolMigrationService.migrateSchool).toHaveBeenCalledWith( - oauthData.externalSchool?.externalId, schoolDO, + oauthData.externalSchool?.externalId, 'systemId' ); }); diff --git a/apps/server/src/modules/user-login-migration/user-login-migration-api.module.ts b/apps/server/src/modules/user-login-migration/user-login-migration-api.module.ts index b30a3f40f4d..377689a4f18 100644 --- a/apps/server/src/modules/user-login-migration/user-login-migration-api.module.ts +++ b/apps/server/src/modules/user-login-migration/user-login-migration-api.module.ts @@ -1,13 +1,11 @@ -import { Module } from '@nestjs/common'; -import { LoggerModule } from '@src/core/logger'; import { AuthenticationModule } from '@modules/authentication/authentication.module'; import { AuthorizationModule } from '@modules/authorization'; +import { LegacySchoolModule } from '@modules/legacy-school'; import { OauthModule } from '@modules/oauth'; import { ProvisioningModule } from '@modules/provisioning'; -import { LegacySchoolModule } from '@modules/legacy-school'; +import { Module } from '@nestjs/common'; +import { LoggerModule } from '@src/core/logger'; import { UserLoginMigrationController } from './controller/user-login-migration.controller'; -import { UserMigrationController } from './controller/user-migration.controller'; -import { PageContentMapper } from './mapper'; import { CloseUserLoginMigrationUc, RestartUserLoginMigrationUc, @@ -33,8 +31,7 @@ import { UserLoginMigrationModule } from './user-login-migration.module'; RestartUserLoginMigrationUc, ToggleUserLoginMigrationUc, CloseUserLoginMigrationUc, - PageContentMapper, ], - controllers: [UserMigrationController, UserLoginMigrationController], + controllers: [UserLoginMigrationController], }) export class UserLoginMigrationApiModule {}