Skip to content

Commit

Permalink
merge SPSH-746 into SPSH-751
Browse files Browse the repository at this point in the history
  • Loading branch information
DPDS93CT committed Jul 10, 2024
2 parents 81aa181 + 2b0ea9d commit 93a4d1f
Show file tree
Hide file tree
Showing 64 changed files with 1,763 additions and 359 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
branches:
- "**"
schedule:
- cron: '0 2 * * *'
- cron: '0 2 * * *'
delete:

concurrency:
Expand Down Expand Up @@ -133,11 +133,12 @@ jobs:
- create_branch_identifier
- release_helm
- build_image_on_push
uses: dBildungsplattform/spsh-app-deploy/.github/workflows/deploy.yml@4
uses: dBildungsplattform/spsh-app-deploy/.github/workflows/deploy.yml@5
with:
dbildungs_iam_server_branch: ${{ needs.branch_meta.outputs.ticket }}
schulportal_client_branch: ${{ needs.branch_meta.outputs.ticket }}
dbildungs_iam_keycloak_branch: ${{ needs.branch_meta.outputs.ticket }}
dbildungs_iam_ldap_branch: ${{ needs.branch_meta.outputs.ticket }}
namespace: ${{ needs.create_branch_identifier.outputs.namespace_from_branch }}
secrets: inherit

Expand Down
3 changes: 2 additions & 1 deletion charts/dbildungs-iam-server/config/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"SESSION_TTL_MS": 3600000,
"BACKEND_ADDRESS": "http://dbildungs-iam-server-backend:80",
"DEFAULT_AUTH_REDIRECT": "/",
"TRUST_PROXY": 1
"TRUST_PROXY": 1,
"ERROR_PAGE_REDIRECT": "/login-error"
},
"DB": {
"USE_SSL": true
Expand Down
3 changes: 2 additions & 1 deletion config/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"BACKEND_ADDRESS": "http://localhost:9090",
"OIDC_CALLBACK_URL": "https://localhost:8099/api/auth/login",
"DEFAULT_LOGIN_REDIRECT": "https://localhost:8099/",
"LOGOUT_REDIRECT": "https://localhost:8099/"
"LOGOUT_REDIRECT": "https://localhost:8099/",
"ERROR_PAGE_REDIRECT": "https://localhost:8099/login-error"
},
"DB": {
"CLIENT_URL": "postgres://admin:[email protected]:5432",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { ArgumentsHost } from '@nestjs/common';
import { DeepMocked, createMock } from '@golevelup/ts-jest';
import { Response } from 'express';
import { HttpArgumentsHost } from '@nestjs/common/interfaces/index.js';
import { AuthenticationDomainError } from '../domain/authentication-domain.error.js';
import { AuthenticationExceptionFilter } from './authentication-exception-filter.js';
import { AuthenticationErrorI18nTypes, DbiamAuthenticationError } from './dbiam-authentication.error.js';

describe('AuthenticationExceptionFilter', () => {
let filter: AuthenticationExceptionFilter;
const statusCode: number = 403;
let responseMock: DeepMocked<Response>;
let argumentsHost: DeepMocked<ArgumentsHost>;

const generalBadRequestError: DbiamAuthenticationError = new DbiamAuthenticationError({
code: 403,
i18nKey: AuthenticationErrorI18nTypes.AUTHENTICATION_ERROR,
});

beforeEach(() => {
filter = new AuthenticationExceptionFilter();
responseMock = createMock<Response>();
argumentsHost = createMock<ArgumentsHost>({
switchToHttp: () =>
createMock<HttpArgumentsHost>({
getResponse: () => responseMock,
}),
});
});

describe('catch', () => {
describe('when filter catches undefined error', () => {
it('should throw a general AuthenticationError', () => {
const error: AuthenticationDomainError = new AuthenticationDomainError('error', undefined);

filter.catch(error, argumentsHost);

expect(responseMock.json).toHaveBeenCalled();
expect(responseMock.status).toHaveBeenCalledWith(statusCode);
expect(responseMock.json).toHaveBeenCalledWith(generalBadRequestError);
});
});
});
});
40 changes: 40 additions & 0 deletions src/modules/authentication/api/authentication-exception-filter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { ArgumentsHost, Catch, ExceptionFilter } from '@nestjs/common';
import { HttpArgumentsHost } from '@nestjs/common/interfaces/index.js';
import { Response } from 'express';
import { AuthenticationDomainError } from '../domain/authentication-domain.error.js';
import { KeycloakUserNotFoundError } from '../domain/keycloak-user-not-found.error.js';
import { AuthenticationErrorI18nTypes, DbiamAuthenticationError } from './dbiam-authentication.error.js';

@Catch(AuthenticationDomainError)
export class AuthenticationExceptionFilter implements ExceptionFilter<AuthenticationDomainError> {
private ERROR_MAPPINGS: Map<string, DbiamAuthenticationError> = new Map([
[
KeycloakUserNotFoundError.name,
new DbiamAuthenticationError({
code: 403,
i18nKey: AuthenticationErrorI18nTypes.KEYCLOAK_USER_NOT_FOUND,
}),
],
]);

public catch(exception: AuthenticationDomainError, host: ArgumentsHost): void {
const ctx: HttpArgumentsHost = host.switchToHttp();
const response: Response = ctx.getResponse<Response>();
const status: number = 403; //all errors regarding organisation specifications are InternalServerErrors at the moment

const dbiamAuthenticationError: DbiamAuthenticationError = this.mapDomainErrorToDbiamError(exception);

response.status(status);
response.json(dbiamAuthenticationError);
}

private mapDomainErrorToDbiamError(error: AuthenticationDomainError): DbiamAuthenticationError {
return (
this.ERROR_MAPPINGS.get(error.constructor.name) ??
new DbiamAuthenticationError({
code: 403,
i18nKey: AuthenticationErrorI18nTypes.AUTHENTICATION_ERROR,
})
);
}
}
4 changes: 3 additions & 1 deletion src/modules/authentication/api/authentication.controller.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Controller, Get, Inject, Req, Res, Session, UseGuards } from '@nestjs/common';
import { Controller, Get, Inject, Req, Res, Session, UseFilters, UseGuards } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import {
ApiBearerAuth,
Expand Down Expand Up @@ -28,6 +28,8 @@ import { PersonenkontextRolleFields } from '../domain/person-permissions.js';
import { RolleID } from '../../../shared/types/index.js';
import { PersonenkontextRolleFieldsResponse } from './personen-kontext-rolle-fields.response.js';
import { RollenSystemRechtServiceProviderIDResponse } from './rolle-systemrechte-serviceproviderid.response.js';
import { AuthenticationExceptionFilter } from './authentication-exception-filter.js';
@UseFilters(new AuthenticationExceptionFilter())
@ApiTags('auth')
@Controller({ path: 'auth' })
export class AuthenticationController {
Expand Down
21 changes: 21 additions & 0 deletions src/modules/authentication/api/dbiam-authentication.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { ApiProperty } from '@nestjs/swagger';
import { DbiamError, DbiamErrorProps } from '../../../shared/error/dbiam.error.js';

export enum AuthenticationErrorI18nTypes {
AUTHENTICATION_ERROR = 'AUTHENTICATION_ERROR',
KEYCLOAK_USER_NOT_FOUND = 'KEYCLOAK_USER_NOT_FOUND',
}

export type DbiamAuthenticationErrorProps = DbiamErrorProps & {
i18nKey: AuthenticationErrorI18nTypes;
};

export class DbiamAuthenticationError extends DbiamError {
@ApiProperty({ enum: AuthenticationErrorI18nTypes })
public override readonly i18nKey: string;

public constructor(props: DbiamAuthenticationErrorProps) {
super(props);
this.i18nKey = props.i18nKey;
}
}
37 changes: 36 additions & 1 deletion src/modules/authentication/api/login.guard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ import { Request } from 'express';

import { LoginGuard } from './login.guard.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';
import { ConfigService } from '@nestjs/config';
import { ServerConfig } from '../../../shared/config/server.config.js';
import { KeycloakUserNotFoundError } from '../domain/keycloak-user-not-found.error.js';
import { HttpFoundException } from '../../../shared/error/http.found.exception.js';
import { AuthenticationErrorI18nTypes } from './dbiam-authentication.error.js';

const canActivateSpy: jest.SpyInstance = jest.spyOn(AuthGuard(['jwt', 'oidc']).prototype as IAuthGuard, 'canActivate');
const logInSpy: jest.SpyInstance = jest.spyOn(AuthGuard(['jwt', 'oidc']).prototype as IAuthGuard, 'logIn');
Expand All @@ -17,7 +22,17 @@ describe('LoginGuard', () => {

beforeAll(async () => {
module = await Test.createTestingModule({
providers: [LoginGuard, { provide: ClassLogger, useValue: createMock<ClassLogger>() }],
providers: [
LoginGuard,
{
provide: ClassLogger,
useValue: createMock<ClassLogger>(),
},
{
provide: ConfigService<ServerConfig>,
useValue: createMock<ConfigService<ServerConfig>>(),
},
],
}).compile();

sut = module.get(LoginGuard);
Expand All @@ -27,6 +42,10 @@ describe('LoginGuard', () => {
await module.close();
}, 30 * 1_000);

afterEach(() => {
jest.resetAllMocks();
});

it('should be defined', () => {
expect(sut).toBeDefined();
});
Expand Down Expand Up @@ -100,5 +119,21 @@ describe('LoginGuard', () => {

await expect(sut.canActivate(contextMock)).resolves.toBe(true);
});

it('should throw HttpFoundException exception if KeycloakUser does not exist', async () => {
canActivateSpy.mockRejectedValueOnce(new KeycloakUserNotFoundError());
logInSpy.mockResolvedValueOnce(undefined);

const contextMock: DeepMocked<ExecutionContext> = createMock();
contextMock.switchToHttp().getRequest<DeepMocked<Request>>().isAuthenticated.mockReturnValue(false);
await expect(sut.canActivate(contextMock)).rejects.toThrow(
new HttpFoundException({
DbiamAuthenticationError: {
code: 403,
i18nKey: AuthenticationErrorI18nTypes.KEYCLOAK_USER_NOT_FOUND,
},
}),
);
});
});
});
29 changes: 27 additions & 2 deletions src/modules/authentication/api/login.guard.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
import { ExecutionContext, Injectable } from '@nestjs/common';
import { AuthGuard } from '@nestjs/passport';
import { Request } from 'express';
import { Request, Response } from 'express';
import { ClassLogger } from '../../../core/logging/class-logger.js';
import { KeycloakUserNotFoundError } from '../domain/keycloak-user-not-found.error.js';
import { HttpFoundException } from '../../../shared/error/http.found.exception.js';
import { ConfigService } from '@nestjs/config';
import { ServerConfig } from '../../../shared/config/server.config.js';
import { FrontendConfig } from '../../../shared/config/frontend.config.js';
import { AuthenticationErrorI18nTypes } from './dbiam-authentication.error.js';

@Injectable()
export class LoginGuard extends AuthGuard(['jwt', 'oidc']) {
public constructor(private logger: ClassLogger) {
public constructor(
private logger: ClassLogger,
private configService: ConfigService<ServerConfig>,
) {
super();
}

public override async canActivate(context: ExecutionContext): Promise<boolean> {
const request: Request = context.switchToHttp().getRequest<Request>();
const res: Response = context.switchToHttp().getResponse<Response>();

if (request.query['redirectUrl']) {
request.session.redirectUrl = request.query['redirectUrl'] as string;
}
Expand All @@ -25,6 +36,20 @@ export class LoginGuard extends AuthGuard(['jwt', 'oidc']) {
await super.logIn(request);
} catch (err) {
this.logger.info(JSON.stringify(err));

if (err instanceof KeycloakUserNotFoundError) {
//Redirect to error page
const frontendConfig: FrontendConfig = this.configService.getOrThrow<FrontendConfig>('FRONTEND');
res.setHeader('location', frontendConfig.ERROR_PAGE_REDIRECT).status(403);
const msg: Record<string, unknown> = {
DbiamAuthenticationError: {
code: 403,
i18nKey: AuthenticationErrorI18nTypes.KEYCLOAK_USER_NOT_FOUND,
},
};
throw new HttpFoundException(msg);
}

return false;
}

Expand Down
11 changes: 11 additions & 0 deletions src/modules/authentication/domain/authentication-domain.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { DomainError } from '../../../shared/error/index.js';

export class AuthenticationDomainError extends DomainError {
public constructor(
public override readonly message: string,
public readonly entityId: string | undefined,
details?: unknown[] | Record<string, undefined>,
) {
super(message, 'USER_COULD_NOT_BE_AUTHENTICATED', details);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { KeycloakUserNotFoundError } from './keycloak-user-not-found.error.js';

describe('KeycloakUserNotFoundError', () => {
describe('constructor', () => {
describe('when calling the constructor', () => {
it('should set message and code', () => {
const error: KeycloakUserNotFoundError = new KeycloakUserNotFoundError({});
expect(error.message).toBe('The Keycloak User does not exist.');
expect(error.code).toBe('USER_COULD_NOT_BE_AUTHENTICATED');
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { AuthenticationDomainError } from './authentication-domain.error.js';

export class KeycloakUserNotFoundError extends AuthenticationDomainError {
public constructor(details?: unknown[] | Record<string, undefined>) {
super('The Keycloak User does not exist.', undefined, details);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { Person } from '../../person/domain/person.js';
import { PersonPermissions } from './person-permissions.js';
import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js';
import { UnauthorizedException } from '@nestjs/common';
import { RolleRepo } from '../../rolle/repo/rolle.repo.js';
import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js';
import { KeycloakUserNotFoundError } from './keycloak-user-not-found.error.js';

describe('PersonPermissionRepo', () => {
let module: TestingModule;
Expand Down Expand Up @@ -82,7 +82,7 @@ describe('PersonPermissionRepo', () => {
describe('when person cannot be found', () => {
it('should throw exception', async () => {
personRepositoryMock.findByKeycloakUserId.mockResolvedValueOnce(undefined);
await expect(sut.loadPersonPermissions(faker.string.uuid())).rejects.toThrow(UnauthorizedException);
await expect(sut.loadPersonPermissions(faker.string.uuid())).rejects.toThrow(KeycloakUserNotFoundError);
});
});
});
Expand Down
5 changes: 3 additions & 2 deletions src/modules/authentication/domain/person-permission.repo.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Injectable, UnauthorizedException } from '@nestjs/common';
import { Injectable } from '@nestjs/common';
import { PersonPermissions } from './person-permissions.js';
import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js';
import { PersonRepository } from '../../person/persistence/person.repository.js';
import { Person } from '../../person/domain/person.js';
import { RolleRepo } from '../../rolle/repo/rolle.repo.js';
import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js';
import { KeycloakUserNotFoundError } from './keycloak-user-not-found.error.js';

@Injectable()
export class PersonPermissionsRepo {
Expand All @@ -18,7 +19,7 @@ export class PersonPermissionsRepo {
public async loadPersonPermissions(keycloakUserId: string): Promise<PersonPermissions> {
const person: Option<Person<true>> = await this.personRepo.findByKeycloakUserId(keycloakUserId);
if (!person) {
throw new UnauthorizedException();
throw new KeycloakUserNotFoundError();
}

return new PersonPermissions(this.personenkontextRepo, this.organisationRepo, this.rollenRepo, person);
Expand Down
Loading

0 comments on commit 93a4d1f

Please sign in to comment.