EW-649: Refactoring of authentication.

apps/server/src/modules/authentication/authentication-config.ts

@@ -1,5 +1,27 @@
 import { AccountConfig } from '@modules/account';
 import { XApiKeyConfig } from './config';
+import { jwtConstants } from './constants';
+
+// values copied from Algorithm definition. Type does not exist at runtime and can't be checked anymore otherwise
+const algorithms = [
+	'HS256',
+	'HS384',
+	'HS512',
+	'RS256',
+	'RS384',
+	'RS512',
+	'ES256',
+	'ES384',
+	'ES512',
+	'PS256',
+	'PS384',
+	'PS512',
+	'none',
+];
+
+if (!algorithms.includes(jwtConstants.jwtOptions.algorithm)) {
+	throw new Error(`${jwtConstants.jwtOptions.algorithm} is not a valid JWT signing algorithm`);
+}
 
 export interface AuthenticationConfig extends AccountConfig, XApiKeyConfig {
 	LOGIN_BLOCK_TIME: number;

apps/server/src/modules/authentication/authentication.module.ts

@@ -13,34 +13,14 @@ import { Algorithm, SignOptions } from 'jsonwebtoken';
 import { jwtConstants } from './constants';
 import { AuthenticationService } from './services/authentication.service';
 import { LdapService } from './services/ldap.service';
-import { JwtValidationAdapter } from './strategy/jwt-validation.adapter';
+import { JwtValidationAdapter } from './helper/jwt-validation.adapter';
 import { JwtStrategy } from './strategy/jwt.strategy';
 import { LdapStrategy } from './strategy/ldap.strategy';
 import { LocalStrategy } from './strategy/local.strategy';
 import { Oauth2Strategy } from './strategy/oauth2.strategy';
 import { XApiKeyStrategy } from './strategy/x-api-key.strategy';
 import { WsJwtStrategy } from './strategy/ws-jwt.strategy';
 
-// values copied from Algorithm definition. Type does not exist at runtime and can't be checked anymore otherwise
-const algorithms = [
-	'HS256',
-	'HS384',
-	'HS512',
-	'RS256',
-	'RS384',
-	'RS512',
-	'ES256',
-	'ES384',
-	'ES512',
-	'PS256',
-	'PS384',
-	'PS512',
-	'none',
-];
-
-if (!algorithms.includes(jwtConstants.jwtOptions.algorithm)) {
-	throw new Error(`${jwtConstants.jwtOptions.algorithm} is not a valid JWT signing algorithm`);
-}
 const signAlgorithm = jwtConstants.jwtOptions.algorithm as Algorithm;
 
 const signOptions: SignOptions = {

apps/server/src/modules/authentication/controllers/login.controller.ts

@@ -27,6 +27,7 @@ export class LoginController {
 	@ApiResponse({ status: 200, type: LoginResponse, description: 'Login was successful.' })
 	@ApiResponse({ status: 400, type: ValidationError, description: 'Request data has invalid format.' })
 	@ApiResponse({ status: 403, type: ForbiddenOperationError, description: 'Invalid user credentials.' })
+	// Body is not used, but validated and used in the strategy implementation
 	// eslint-disable-next-line @typescript-eslint/no-unused-vars
 	async loginLdap(@CurrentUser() user: ICurrentUser, @Body() _: LdapAuthorizationBodyParams): Promise<LoginResponse> {
 		const loginDto: LoginDto = await this.loginUc.getLoginData(user);
@@ -43,6 +44,7 @@ export class LoginController {
 	@ApiResponse({ status: 200, type: LoginResponse, description: 'Login was successful.' })
 	@ApiResponse({ status: 400, type: ValidationError, description: 'Request data has invalid format.' })
 	@ApiResponse({ status: 403, type: ForbiddenOperationError, description: 'Invalid user credentials.' })
+	// Body is not used, but validated and used in the strategy implementation
 	// eslint-disable-next-line @typescript-eslint/no-unused-vars
 	async loginLocal(@CurrentUser() user: ICurrentUser, @Body() _: LocalAuthorizationBodyParams): Promise<LoginResponse> {
 		const loginDto: LoginDto = await this.loginUc.getLoginData(user);
@@ -61,6 +63,7 @@ export class LoginController {
 	@ApiResponse({ status: 403, type: ForbiddenOperationError, description: 'Invalid user credentials.' })
 	async loginOauth2(
 		@CurrentUser() user: OauthCurrentUser,
+		// Body is not used, but validated and used in the strategy implementation
 		// eslint-disable-next-line @typescript-eslint/no-unused-vars
 		@Body() _: Oauth2AuthorizationBodyParams
 	): Promise<OauthLoginResponse> {

apps/server/src/modules/authentication/decorator/auth.decorator.ts

@@ -10,8 +10,8 @@ import { ApiBearerAuth } from '@nestjs/swagger';
 import { Request } from 'express';
 import { ExtractJwt } from 'passport-jwt';
 import { JwtAuthGuard } from '../guard/jwt-auth.guard';
-import { ICurrentUser } from '../interface/user';
-import { JwtExtractor } from '../strategy/jwt-extractor';
+import { ICurrentUser, isICurrentUser } from '../interface/user';
+import { JwtExtractor } from '../helper/jwt-extractor';
 
 const STRATEGIES = ['jwt'] as const;
 type Strategies = typeof STRATEGIES;
@@ -21,7 +21,6 @@ type Strategies = typeof STRATEGIES;
  * @param strategies accepted strategies
  * @returns
  */
-// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
 export const Authenticate = (...strategies: Strategies) => {
 	if (strategies.includes('jwt')) {
 		const decorators = [
@@ -39,22 +38,24 @@ export const Authenticate = (...strategies: Strategies) => {
  * Returns the current authenticated user.
  * @requires Authenticated
  */
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const CurrentUser = createParamDecorator<any, any, ICurrentUser>((data: unknown, ctx: ExecutionContext) => {
-	const { user }: Request = ctx.switchToHttp().getRequest();
-	if (!user)
+export const CurrentUser = createParamDecorator<never, never, ICurrentUser>((_, ctx: ExecutionContext) => {
+	const expressRequest = ctx.switchToHttp().getRequest<Request>();
+	const requestUser = expressRequest.user;
+
+	if (!requestUser || !isICurrentUser(requestUser)) {
 		throw new UnauthorizedException(
 			'CurrentUser missing in request context. This route requires jwt authentication guard enabled.'
 		);
-	return user as ICurrentUser;
+	}
+
+	return requestUser;
 });
 
 /**
  * Returns the current JWT.
  * @requires Authenticated
  */
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const JWT = createParamDecorator<any, any, string>((data: unknown, ctx: ExecutionContext) => {
+export const JWT = createParamDecorator<never, never, string>((_, ctx: ExecutionContext) => {
 	const getJWT = ExtractJwt.fromExtractors([ExtractJwt.fromAuthHeaderAsBearerToken(), JwtExtractor.fromCookie('jwt')]);
 	const req: Request = ctx.switchToHttp().getRequest();
 	const jwt = getJWT(req) || req.headers.authorization;

apps/server/src/modules/authentication/errors/brute-force.error.ts

@@ -6,7 +6,7 @@ export class BruteForceError extends BusinessError {
 
 	constructor(timeToWait: number, message: string) {
 		super(
-			{ type: 'ENTITY_NOT_FOUND', title: 'Entity Not Found', defaultMessage: message },
+			{ type: 'TOO_MANY_REQUESTS', title: 'Too many requests', defaultMessage: message },
 			HttpStatus.TOO_MANY_REQUESTS,
 			{
 				timeToWait,

apps/server/src/modules/authentication/strategy/jwt-validation.adapter.ts → apps/server/src/modules/authentication/helper/jwt-validation.adapter.ts

@@ -23,7 +23,6 @@ export class JwtValidationAdapter {
 	 * @param jti jwt id (here required to make jwt identifiers identical in redis)
 	 */
 	async isWhitelisted(accountId: string, jti: string): Promise<void> {
-		// eslint-disable-next-line @typescript-eslint/no-unsafe-call
 		await ensureTokenIsWhitelisted({ accountId, jti, privateDevice: false });
 	}
 

apps/server/src/modules/authentication/interface/jwt-payload.ts

@@ -12,17 +12,17 @@ export interface CreateJwtPayload {
 export interface JwtPayload extends CreateJwtPayload {
 	/** audience */
 	aud: string;
-	/** expiration in // TODO
-	 *
+	/**
+	 * expiration in
 	 */
 	exp: number;
 	iat: number;
 	/** issuer */
 	iss: string;
 	jti: string;
 
-	/** // TODO
-	 *
+	/**
+	 * subject
 	 */
 	sub: string;
 }

apps/server/src/modules/authentication/interface/user.ts

@@ -1,3 +1,7 @@
+/* eslint-disable @typescript-eslint/no-unsafe-member-access */
+/* eslint-disable @typescript-eslint/no-unsafe-call */
+/* eslint-disable @typescript-eslint/no-explicit-any */
+
 import { EntityId } from '@shared/domain/types';
 
 export interface ICurrentUser {
@@ -19,3 +23,13 @@ export interface ICurrentUser {
 	/** True if the user is an external user e.g. an oauth user or ldap user */
 	isExternalUser: boolean;
 }
+
+export function isICurrentUser(user: any): user is ICurrentUser {
+	return (
+		typeof user?.userId === 'string' &&
+		Array.isArray(user?.roles) &&
+		(user?.roles as any[]).every((id: any) => typeof id === 'string') &&
+		typeof user?.schoolId === 'string' &&
+		typeof user?.accountId === 'string'
+	);
+}

apps/server/src/modules/authentication/services/authentication.service.spec.ts

@@ -7,7 +7,7 @@ import { JwtService } from '@nestjs/jwt';
 import { Test, TestingModule } from '@nestjs/testing';
 import jwt from 'jsonwebtoken';
 import { BruteForceError } from '../errors/brute-force.error';
-import { JwtValidationAdapter } from '../strategy/jwt-validation.adapter';
+import { JwtValidationAdapter } from '../helper/jwt-validation.adapter';
 import { AuthenticationService } from './authentication.service';
 
 jest.mock('jsonwebtoken');

apps/server/src/modules/authentication/services/authentication.service.ts

@@ -8,7 +8,7 @@ import { randomUUID } from 'crypto';
 import jwt, { JwtPayload } from 'jsonwebtoken';
 import { BruteForceError, UnauthorizedLoggableException } from '../errors';
 import { CreateJwtPayload } from '../interface/jwt-payload';
-import { JwtValidationAdapter } from '../strategy/jwt-validation.adapter';
+import { JwtValidationAdapter } from '../helper/jwt-validation.adapter';
 import { LoginDto } from '../uc/dto';
 
 @Injectable()
@@ -55,11 +55,15 @@ export class AuthenticationService {
 	async removeJwtFromWhitelist(jwtToken: string): Promise<void> {
 		const decodedJwt: JwtPayload | null = jwt.decode(jwtToken, { json: true });
 
-		if (decodedJwt && decodedJwt.jti && decodedJwt.accountId && typeof decodedJwt.accountId === 'string') {
+		if (this.isValidJwt(decodedJwt)) {
 			await this.jwtValidationAdapter.removeFromWhitelist(decodedJwt.accountId, decodedJwt.jti);
 		}
 	}
 
+	private isValidJwt(decodedJwt: JwtPayload | null): decodedJwt is { accountId: string; jti: string } {
+		return typeof decodedJwt?.jti === 'string' && typeof decodedJwt?.accountId === 'string';
+	}
+
 	checkBrutForce(account: Account): void {
 		if (account.lasttriedFailedLogin) {
 			const timeDifference = (new Date().getTime() - account.lasttriedFailedLogin.getTime()) / 1000;

apps/server/src/modules/authentication/strategy/jwt.strategy.spec.ts

@@ -9,7 +9,7 @@ import { setupEntities } from '@shared/testing';
 import { jwtConstants } from '../constants';
 import { JwtPayload } from '../interface/jwt-payload';
 
-import { JwtValidationAdapter } from './jwt-validation.adapter';
+import { JwtValidationAdapter } from '../helper/jwt-validation.adapter';
 import { JwtStrategy } from './jwt.strategy';
 
 describe('jwt strategy', () => {

apps/server/src/modules/authentication/strategy/jwt.strategy.ts

@@ -5,8 +5,8 @@ import { jwtConstants } from '../constants';
 import { ICurrentUser } from '../interface';
 import { JwtPayload } from '../interface/jwt-payload';
 import { CurrentUserMapper } from '../mapper';
-import { JwtExtractor } from './jwt-extractor';
-import { JwtValidationAdapter } from './jwt-validation.adapter';
+import { JwtExtractor } from '../helper/jwt-extractor';
+import { JwtValidationAdapter } from '../helper/jwt-validation.adapter';
 
 @Injectable()
 export class JwtStrategy extends PassportStrategy(Strategy) {

apps/server/src/modules/authentication/strategy/ldap.strategy.ts

@@ -30,21 +30,18 @@ export class LdapStrategy extends PassportStrategy(Strategy, 'ldap') {
 		const { username, password, systemId, schoolId } = this.extractParamsFromRequest(request);
 
 		const system: SystemEntity = await this.systemRepo.findById(systemId);
-
 		const school: LegacySchoolDo = await this.schoolRepo.findById(schoolId);
 
 		if (!school.systems || !school.systems.includes(systemId)) {
 			throw new UnauthorizedException(`School ${schoolId} does not have the selected system ${systemId}`);
 		}
 
 		const account: Account = await this.loadAccount(username, system.id, school);
-
 		const userId: string = this.checkValue(account.userId);
 
 		this.authenticationService.checkBrutForce(account);
 
 		const user: User = await this.userRepo.findById(userId);
-
 		const ldapDn: string = this.checkValue(user.ldapDn);
 
 		await this.checkCredentials(account, system, ldapDn, password);

apps/server/src/modules/authentication/strategy/ws-jwt.strategy.spec.ts

@@ -7,7 +7,7 @@ import { WsException } from '@nestjs/websockets';
 import { setupEntities } from '@shared/testing';
 import { jwtConstants } from '../constants';
 import { JwtPayload } from '../interface/jwt-payload';
-import { JwtValidationAdapter } from './jwt-validation.adapter';
+import { JwtValidationAdapter } from '../helper/jwt-validation.adapter';
 import { WsJwtStrategy } from './ws-jwt.strategy';
 
 describe('jwt strategy', () => {

apps/server/src/modules/authentication/strategy/ws-jwt.strategy.ts

@@ -6,8 +6,8 @@ import { jwtConstants } from '../constants';
 import { ICurrentUser } from '../interface';
 import { JwtPayload } from '../interface/jwt-payload';
 import { CurrentUserMapper } from '../mapper';
-import { JwtExtractor } from './jwt-extractor';
-import { JwtValidationAdapter } from './jwt-validation.adapter';
+import { JwtExtractor } from '../helper/jwt-extractor';
+import { JwtValidationAdapter } from '../helper/jwt-validation.adapter';
 
 @Injectable()
 export class WsJwtStrategy extends PassportStrategy(Strategy, 'wsjwt') {