N21-1273 calling oauth2 logoutEndpoint (#4467)

* adds externalIdToken to ICurrentUser to be able to trigger rp initiated logout, after successful login * adds migration script to remove logout endpoint for sanis * removes logoutUrl changes init script of moin.schule
hpi-schul-cloud · Oct 13, 2023 · e3afbd4 · e3afbd4
1 parent 43d02f8
commit e3afbd4
Show file tree

Hide file tree

Showing 37 changed files with 345 additions and 75 deletions.
diff --git a/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 b/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2
@@ -177,7 +177,6 @@ data:
               "redirectUri": "https://{{ NAMESPACE }}.cd.dbildungscloud.dev/api/v3/sso/oauth",
               "authEndpoint": "https://auth.stage.niedersachsen-login.schule/realms/SANIS/protocol/openid-connect/auth",
               "provider": "sanis",
-              "logoutEndpoint": "https://auth.stage.niedersachsen-login.schule/realms/SANIS/protocol/openid-connect/logout",
               "jwksEndpoint": "https://auth.stage.niedersachsen-login.schule/realms/SANIS/protocol/openid-connect/certs",
               "issuer": "https://auth.stage.niedersachsen-login.schule/realms/SANIS"
             }

diff --git a/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts b/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts
@@ -3,15 +3,15 @@ import { HttpStatus, INestApplication } from '@nestjs/common';
 import { Test, TestingModule } from '@nestjs/testing';
 import { Account, RoleName, SchoolEntity, SystemEntity, User } from '@shared/domain';
 import { accountFactory, roleFactory, schoolFactory, systemFactory, userFactory } from '@shared/testing';
-import { SSOErrorCode } from '@src/modules/oauth/error/sso-error-code.enum';
+import { SSOErrorCode } from '@src/modules/oauth/loggable';
 import { OauthTokenResponse } from '@src/modules/oauth/service/dto';
 import { ServerTestModule } from '@src/modules/server/server.module';
 import axios from 'axios';
 import MockAdapter from 'axios-mock-adapter';
 import crypto, { KeyPairKeyObjectResult } from 'crypto';
 import jwt from 'jsonwebtoken';
 import request, { Response } from 'supertest';
-import { LdapAuthorizationBodyParams, LocalAuthorizationBodyParams } from '../dto';
+import { LdapAuthorizationBodyParams, LocalAuthorizationBodyParams, OauthLoginResponse } from '../dto';
 
 const ldapAccountUserName = 'ldapAccountUserName';
 const mockUserLdapDN = 'mockUserLdapDN';
@@ -129,6 +129,7 @@ describe('Login Controller (api)', () => {
 				expect(decodedToken).toHaveProperty('accountId');
 				expect(decodedToken).toHaveProperty('schoolId');
 				expect(decodedToken).toHaveProperty('roles');
+				expect(decodedToken).not.toHaveProperty('externalIdToken');
 			});
 		});
 
@@ -193,6 +194,7 @@ describe('Login Controller (api)', () => {
 				expect(decodedToken).toHaveProperty('accountId');
 				expect(decodedToken).toHaveProperty('schoolId');
 				expect(decodedToken).toHaveProperty('roles');
+				expect(decodedToken).not.toHaveProperty('externalIdToken');
 			});
 		});
 
@@ -253,10 +255,30 @@ describe('Login Controller (api)', () => {
 
 				return {
 					system,
+					idToken,
 				};
 			};
 
-			it('should return jwt', async () => {
+			it('should return oauth login response', async () => {
+				const { system, idToken } = await setup();
+
+				const response: Response = await request(app.getHttpServer())
+					.post(`${basePath}/oauth2`)
+					.send({
+						redirectUri: 'redirectUri',
+						code: 'code',
+						systemId: system.id,
+					})
+					.expect(HttpStatus.OK);
+
+				// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+				expect(response.body).toEqual<OauthLoginResponse>({
+					accessToken: expect.any(String),
+					externalIdToken: idToken,
+				});
+			});
+
+			it('should return a valid jwt as access token', async () => {
 				const { system } = await setup();
 
 				const response: Response = await request(app.getHttpServer())
@@ -268,8 +290,15 @@ describe('Login Controller (api)', () => {
 					})
 					.expect(HttpStatus.OK);
 
+				// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-argument
+				const decodedToken = jwt.decode(response.body.accessToken);
 				// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
 				expect(response.body.accessToken).toBeDefined();
+				expect(decodedToken).toHaveProperty('userId');
+				expect(decodedToken).toHaveProperty('accountId');
+				expect(decodedToken).toHaveProperty('schoolId');
+				expect(decodedToken).toHaveProperty('roles');
+				expect(decodedToken).not.toHaveProperty('externalIdToken');
 			});
 		});
 

diff --git a/apps/server/src/modules/authentication/controllers/dto/index.ts b/apps/server/src/modules/authentication/controllers/dto/index.ts
@@ -2,3 +2,4 @@ export * from './oauth2-authorization.body.params';
 export * from './login.response';
 export * from './ldap-authorization.body.params';
 export * from './local-authorization.body.params';
+export * from './oauth-login.response';
diff --git a/apps/server/src/modules/authentication/controllers/dto/oauth-login.response.ts b/apps/server/src/modules/authentication/controllers/dto/oauth-login.response.ts
@@ -0,0 +1,15 @@
+import { ApiPropertyOptional } from '@nestjs/swagger';
+import { LoginResponse } from './login.response';
+
+export class OauthLoginResponse extends LoginResponse {
+	@ApiPropertyOptional({
+		description:
+			'The external id token which is from the external oauth system and set when scope openid is available.',
+	})
+	externalIdToken?: string;
+
+	constructor(props: OauthLoginResponse) {
+		super(props);
+		this.externalIdToken = props.externalIdToken;
+	}
+}
diff --git a/apps/server/src/modules/authentication/controllers/login.controller.ts b/apps/server/src/modules/authentication/controllers/login.controller.ts
@@ -3,14 +3,15 @@ import { AuthGuard } from '@nestjs/passport';
 import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger';
 import { ForbiddenOperationError, ValidationError } from '@shared/common';
 import { CurrentUser } from '../decorator/auth.decorator';
-import type { ICurrentUser } from '../interface';
+import type { ICurrentUser, OauthCurrentUser } from '../interface';
 import { LoginDto } from '../uc/dto';
 import { LoginUc } from '../uc/login.uc';
 import {
 	LdapAuthorizationBodyParams,
 	LocalAuthorizationBodyParams,
 	LoginResponse,
 	Oauth2AuthorizationBodyParams,
+	OauthLoginResponse,
 } from './dto';
 import { LoginResponseMapper } from './mapper/login-response.mapper';
 
@@ -30,7 +31,7 @@ export class LoginController {
 	async loginLdap(@CurrentUser() user: ICurrentUser, @Body() _: LdapAuthorizationBodyParams): Promise<LoginResponse> {
 		const loginDto: LoginDto = await this.loginUc.getLoginData(user);
 
-		const mapped: LoginResponse = LoginResponseMapper.mapLoginDtoToResponse(loginDto);
+		const mapped: LoginResponse = LoginResponseMapper.mapToLoginResponse(loginDto);
 
 		return mapped;
 	}
@@ -46,7 +47,7 @@ export class LoginController {
 	async loginLocal(@CurrentUser() user: ICurrentUser, @Body() _: LocalAuthorizationBodyParams): Promise<LoginResponse> {
 		const loginDto: LoginDto = await this.loginUc.getLoginData(user);
 
-		const mapped: LoginResponse = LoginResponseMapper.mapLoginDtoToResponse(loginDto);
+		const mapped: LoginResponse = LoginResponseMapper.mapToLoginResponse(loginDto);
 
 		return mapped;
 	}
@@ -59,13 +60,13 @@ export class LoginController {
 	@ApiResponse({ status: 400, type: ValidationError, description: 'Request data has invalid format.' })
 	@ApiResponse({ status: 403, type: ForbiddenOperationError, description: 'Invalid user credentials.' })
 	async loginOauth2(
-		@CurrentUser() user: ICurrentUser,
+		@CurrentUser() user: OauthCurrentUser,
 		// eslint-disable-next-line @typescript-eslint/no-unused-vars
 		@Body() _: Oauth2AuthorizationBodyParams
-	): Promise<LoginResponse> {
+	): Promise<OauthLoginResponse> {
 		const loginDto: LoginDto = await this.loginUc.getLoginData(user);
 
-		const mapped: LoginResponse = LoginResponseMapper.mapLoginDtoToResponse(loginDto);
+		const mapped: OauthLoginResponse = LoginResponseMapper.mapToOauthLoginResponse(loginDto, user.externalIdToken);
 
 		return mapped;
 	}

diff --git a/apps/server/src/modules/authentication/controllers/mapper/login-response.mapper.ts b/apps/server/src/modules/authentication/controllers/mapper/login-response.mapper.ts
@@ -1,9 +1,20 @@
-import { LoginResponse } from '../dto';
 import { LoginDto } from '../../uc/dto';
+import { LoginResponse, OauthLoginResponse } from '../dto';
 
 export class LoginResponseMapper {
-	static mapLoginDtoToResponse(loginDto: LoginDto): LoginResponse {
-		const response: LoginResponse = new LoginResponse({ accessToken: loginDto.accessToken });
+	static mapToLoginResponse(loginDto: LoginDto): LoginResponse {
+		const response: LoginResponse = new LoginResponse({
+			accessToken: loginDto.accessToken,
+		});
+
+		return response;
+	}
+
+	static mapToOauthLoginResponse(loginDto: LoginDto, externalIdToken?: string): OauthLoginResponse {
+		const response: OauthLoginResponse = new OauthLoginResponse({
+			accessToken: loginDto.accessToken,
+			externalIdToken,
+		});
 
 		return response;
 	}

diff --git a/apps/server/src/modules/authentication/interface/user.ts b/apps/server/src/modules/authentication/interface/user.ts
@@ -40,3 +40,8 @@ export interface ICurrentUser {
 	/** True if a support member impersonates the user */
 	impersonated?: boolean;
 }
+
+export interface OauthCurrentUser extends ICurrentUser {
+	/** Contains the idToken of the external idp. Will be set during oAuth2 login and used for rp initiated logout  */
+	externalIdToken?: string;
+}
diff --git a/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts b/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts
@@ -2,8 +2,8 @@ import { ValidationError } from '@shared/common';
 import { Permission, RoleName } from '@shared/domain';
 import { UserDO } from '@shared/domain/domainobject/user.do';
 import { roleFactory, schoolFactory, setupEntities, userDoFactory, userFactory } from '@shared/testing';
-import { ICurrentUser } from '../interface';
-import { JwtPayload } from '../interface/jwt-payload';
+import { ICurrentUser, OauthCurrentUser } from '../interface';
+import { CreateJwtPayload, JwtPayload } from '../interface/jwt-payload';
 import { CurrentUserMapper } from './current-user.mapper';
 
 describe('CurrentUserMapper', () => {
@@ -56,40 +56,89 @@ describe('CurrentUserMapper', () => {
 		});
 	});
 
-	describe('userDoToICurrentUser', () => {
-		const userId = 'mockUserId';
+	describe('OauthCurrentUser', () => {
+		const userIdMock = 'mockUserId';
 		describe('when userDO has no ID', () => {
 			it('should throw error', () => {
 				const user: UserDO = userDoFactory.build({ createdAt: new Date(), updatedAt: new Date() });
-				expect(() => CurrentUserMapper.userDoToICurrentUser(accountId, user)).toThrow(ValidationError);
+				expect(() => CurrentUserMapper.mapToOauthCurrentUser(accountId, user, undefined, 'idToken')).toThrow(
+					ValidationError
+				);
 			});
 		});
 
 		describe('when userDO is valid', () => {
-			it('should return valid ICurrentUser instance', () => {
-				const user: UserDO = userDoFactory.buildWithId({ id: userId, createdAt: new Date(), updatedAt: new Date() });
-				const currentUser = CurrentUserMapper.userDoToICurrentUser(accountId, user);
-				expect(currentUser).toMatchObject({
+			const setup = () => {
+				const user: UserDO = userDoFactory.buildWithId({
+					id: userIdMock,
+					createdAt: new Date(),
+					updatedAt: new Date(),
+				});
+				const idToken = 'idToken';
+
+				return {
+					user,
+					userId: user.id as string,
+					idToken,
+				};
+			};
+
+			it('should return valid oauth current user instance', () => {
+				const { user, userId, idToken } = setup();
+
+				const currentUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser(
+					accountId,
+					user,
+					undefined,
+					idToken
+				);
+
+				expect(currentUser).toMatchObject<OauthCurrentUser>({
 					accountId,
 					systemId: undefined,
 					roles: [],
 					schoolId: user.schoolId,
-					userId: user.id,
+					userId,
+					externalIdToken: idToken,
 				});
 			});
 		});
 
 		describe('when userDO is valid and a systemId is provided', () => {
-			it('should return valid ICurrentUser instance with systemId', () => {
-				const user: UserDO = userDoFactory.buildWithId({ id: userId, createdAt: new Date(), updatedAt: new Date() });
+			const setup = () => {
+				const user: UserDO = userDoFactory.buildWithId({
+					id: userIdMock,
+					createdAt: new Date(),
+					updatedAt: new Date(),
+				});
 				const systemId = 'mockSystemId';
-				const currentUser = CurrentUserMapper.userDoToICurrentUser(accountId, user, systemId);
-				expect(currentUser).toMatchObject({
+				const idToken = 'idToken';
+
+				return {
+					user,
+					userId: user.id as string,
+					idToken,
+					systemId,
+				};
+			};
+
+			it('should return valid ICurrentUser instance with systemId', () => {
+				const { user, userId, systemId, idToken } = setup();
+
+				const currentUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser(
+					accountId,
+					user,
+					systemId,
+					idToken
+				);
+
+				expect(currentUser).toMatchObject<OauthCurrentUser>({
 					accountId,
 					systemId,
 					roles: [],
 					schoolId: user.schoolId,
-					userId: user.id,
+					userId,
+					externalIdToken: idToken,
 				});
 			});
 		});
@@ -104,7 +153,7 @@ describe('CurrentUserMapper', () => {
 						},
 					])
 					.buildWithId({
-						id: userId,
+						id: userIdMock,
 						createdAt: new Date(),
 						updatedAt: new Date(),
 					});
@@ -117,7 +166,7 @@ describe('CurrentUserMapper', () => {
 			it('should return valid ICurrentUser instance without systemId', () => {
 				const { user } = setup();
 
-				const currentUser = CurrentUserMapper.userDoToICurrentUser(accountId, user);
+				const currentUser = CurrentUserMapper.mapToOauthCurrentUser(accountId, user, undefined, 'idToken');
 
 				expect(currentUser).toMatchObject({
 					accountId,
@@ -158,6 +207,7 @@ describe('CurrentUserMapper', () => {
 				});
 			});
 		});
+
 		describe('when JWT is provided without optional claims', () => {
 			it('should return current user', () => {
 				const jwtPayload: JwtPayload = {
@@ -182,4 +232,28 @@ describe('CurrentUserMapper', () => {
 			});
 		});
 	});
+
+	describe('mapCurrentUserToCreateJwtPayload', () => {
+		it('should map current user to create jwt payload', () => {
+			const currentUser: ICurrentUser = {
+				accountId: 'dummyAccountId',
+				systemId: 'dummySystemId',
+				roles: ['mockRoleId'],
+				schoolId: 'dummySchoolId',
+				userId: 'dummyUserId',
+				impersonated: true,
+			};
+
+			const createJwtPayload: CreateJwtPayload = CurrentUserMapper.mapCurrentUserToCreateJwtPayload(currentUser);
+
+			expect(createJwtPayload).toMatchObject<CreateJwtPayload>({
+				accountId: currentUser.accountId,
+				systemId: currentUser.systemId,
+				roles: currentUser.roles,
+				schoolId: currentUser.schoolId,
+				userId: currentUser.userId,
+				support: currentUser.impersonated,
+			});
+		});
+	});
 });