Skip to content

Commit

Permalink
Merge branch 'main' into SPSH-698
Browse files Browse the repository at this point in the history
  • Loading branch information
phaelcg committed Jul 8, 2024
2 parents 90c3949 + 89d5e49 commit 4e41bf2
Show file tree
Hide file tree
Showing 47 changed files with 1,272 additions and 84 deletions.
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
36 changes: 30 additions & 6 deletions src/modules/authentication/passport/jwt.strategy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,55 @@
import { JwtStrategy } from './jwt.strategy.js';
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { BaseClient, Client } from 'openid-client';
import { PersonRepository } from '../../person/persistence/person.repository.js';
import { KeycloakUserNotFoundError } from '../domain/keycloak-user-not-found.error.js';
import jwt from 'jsonwebtoken';
import { faker } from '@faker-js/faker';

describe('JWT Strategy', () => {
it('should extract a bearer token from the header and return it for session storage', () => {
it('should extract a bearer token from the header and return it for session storage', async () => {
const client: DeepMocked<BaseClient> = createMock<Client>({
issuer: { metadata: { jwks_uri: 'https://nowhere.example.com' } },
});
const sut: JwtStrategy = new JwtStrategy(client);
const personRepositoryMock: DeepMocked<PersonRepository> = createMock<PersonRepository>();
const sut: JwtStrategy = new JwtStrategy(client, personRepositoryMock);
const request: Request = createMock<Request & { headers: { authorization: string } }>({
headers: { authorization: 'Bearer 12345' },
});
const sessionContent: { access_token: string } = sut.validate(request, '');
const sessionContent: { access_token: string } = await sut.validate(request, '');

expect(sessionContent.access_token).toEqual('12345');
});

it('should return empty string if no accessToken can be extracted', () => {
it('should return empty string if no accessToken can be extracted', async () => {
const client: DeepMocked<BaseClient> = createMock<Client>({
issuer: { metadata: { jwks_uri: 'https://nowhere.example.com' } },
});
const sut: JwtStrategy = new JwtStrategy(client);
const personRepositoryMock: DeepMocked<PersonRepository> = createMock<PersonRepository>();
const sut: JwtStrategy = new JwtStrategy(client, personRepositoryMock);
const request: Request = createMock<Request & { headers: { authorization: string } }>({
headers: { authorization: '' },
});
const sessionContent: { access_token: string } = sut.validate(request, '');
const sessionContent: { access_token: string } = await sut.validate(request, '');

expect(sessionContent.access_token).toEqual('');
});

it('should throw KeycloakUserNotFoundError if the kc user does not exist', async () => {
const client: DeepMocked<BaseClient> = createMock<Client>({
issuer: { metadata: { jwks_uri: 'https://nowhere.example.com' } },
});
const personRepositoryMock: DeepMocked<PersonRepository> = createMock<PersonRepository>();
personRepositoryMock.findByKeycloakUserId.mockResolvedValueOnce(undefined);
const sut: JwtStrategy = new JwtStrategy(client, personRepositoryMock);

jest.spyOn(jwt, 'decode').mockReturnValue({
sub: faker.string.uuid().toString(),
});
const request: Request = createMock<Request & { headers: { authorization: string } }>({
headers: { authorization: 'Bearer 12345' },
});

await expect(sut.validate(request, '')).rejects.toThrow(KeycloakUserNotFoundError);
});
});
Loading

0 comments on commit 4e41bf2

Please sign in to comment.