From d596d555f5d4e0091423c78b3592f023bd6f701e Mon Sep 17 00:00:00 2001 From: Tamir Gershberg <47638346+tamirGer@users.noreply.github.com> Date: Sat, 20 Jan 2024 20:24:48 +0200 Subject: [PATCH] fix(oidc): store OIDC user info in local DB (#302) --- pg.sql | 6 ++-- src/auth/auth.controller.ts | 26 +++++++++----- src/auth/auth.guard.ts | 56 ++++++++++++++++-------------- src/keycloak/keycloak.service.ts | 3 +- src/model/user.entity.ts | 3 ++ src/users/api/CreateUserRequest.ts | 7 +++- src/users/api/UserDto.ts | 7 ++-- src/users/users.controller.ts | 44 ++++++++++++++++++----- src/users/users.service.ts | 3 +- 9 files changed, 103 insertions(+), 52 deletions(-) diff --git a/pg.sql b/pg.sql index a8473569..13435db5 100644 --- a/pg.sql +++ b/pg.sql @@ -1,7 +1,7 @@ set names 'utf8'; set session_replication_role = 'replica'; -create table "user" ("id" serial primary key, "created_at" timestamptz(0) not null, "updated_at" timestamptz(0) not null, "email" varchar(255) not null, "password" varchar(255) not null, "first_name" varchar(255) not null, "last_name" varchar(255) not null, "is_admin" bool not null, "photo" bytea null, "company" varchar(255) not null, "card_number" varchar(255) not null, "phone_number" varchar(255) not null); +create table "user" ("id" serial primary key, "created_at" timestamptz(0) not null, "updated_at" timestamptz(0) not null, "email" varchar(255) not null, "password" varchar(255) not null, "first_name" varchar(255) not null, "last_name" varchar(255) not null, "is_admin" bool not null, "photo" bytea null, "company" varchar(255) not null, "card_number" varchar(255) not null, "phone_number" varchar(255) not null, "is_basic" boolean not null); create table "testimonial" ("id" serial primary key, "created_at" timestamptz(0) not null, "updated_at" timestamptz(0) not null, "name" varchar(255) not null, "title" varchar(255) not null, "message" varchar(255) not null); @@ -9,8 +9,8 @@ create table "product" ("id" serial primary key, "created_at" timestamptz(0) not set session_replication_role = 'origin'; --password is admin -INSERT INTO "user" (created_at, updated_at, email, password, first_name, last_name, is_admin, photo, company, card_number, phone_number) VALUES (now(), now(), 'admin', '$2b$10$BBJjmVNNdyEgv7pV/zQR9u/ssIuwZsdDJbowW/Dgp28uws3GmO0Ky', 'admin', 'admin', true, null, 'Brightsec', '1234 5678 9012 3456', '+1 234 567 890'); -INSERT INTO "user" (created_at, updated_at, email, password, first_name, last_name, is_admin, photo, company, card_number, phone_number) VALUES (now(), now(), 'user', '$2b$10$edsq4aqzAHnrJu68t8GS2.v0Z7hJSstAo7wBBDmmbpjYGxMMTYpVi', 'user', 'user', false, null, 'Brightsec', '1234 5678 9012 3456', '+1 234 567 890'); +INSERT INTO "user" (created_at, updated_at, email, password, first_name, last_name, is_admin, photo, company, card_number, phone_number, is_basic) VALUES (now(), now(), 'admin', '$2b$10$BBJjmVNNdyEgv7pV/zQR9u/ssIuwZsdDJbowW/Dgp28uws3GmO0Ky', 'admin', 'admin', true, null, 'Brightsec', '1234 5678 9012 3456', '+1 234 567 890', true); +INSERT INTO "user" (created_at, updated_at, email, password, first_name, last_name, is_admin, photo, company, card_number, phone_number, is_basic) VALUES (now(), now(), 'user', '$2b$10$edsq4aqzAHnrJu68t8GS2.v0Z7hJSstAo7wBBDmmbpjYGxMMTYpVi', 'user', 'user', false, null, 'Brightsec', '1234 5678 9012 3456', '+1 234 567 890', true); --insert default products into the table INSERT INTO "product" ("category", "photo_url", "name", "description") VALUES ('Healing', '/api/file?path=config/products/crystals/amethyst.jpg&type=image/jpg', 'Amethyst', 'a violet variety of quartz'); diff --git a/src/auth/auth.controller.ts b/src/auth/auth.controller.ts index ef96048f..aec83b01 100644 --- a/src/auth/auth.controller.ts +++ b/src/auth/auth.controller.ts @@ -2,6 +2,7 @@ import { BadRequestException, Body, Controller, + ForbiddenException, Get, HttpStatus, InternalServerErrorException, @@ -131,7 +132,7 @@ export class AuthController { if (req.op === FormMode.OIDC) { loginData = await this.loginOidc(req); } else { - loginData = await this.login(req); + loginData = await this.loginBasic(req); } const { token, ...loginResponse } = loginData; @@ -264,7 +265,7 @@ export class AuthController { @Res({ passthrough: true }) res: FastifyReply, ): Promise { this.logger.debug('Call loginWithKIDSqlJwt'); - const profile = await this.login(req); + const profile = await this.loginBasic(req); res.header( 'authorization', @@ -324,7 +325,7 @@ export class AuthController { @Res({ passthrough: true }) res: FastifyReply, ): Promise { this.logger.debug('Call loginWithKIDSqlJwt'); - const profile = await this.login(req); + const profile = await this.loginBasic(req); res.header( 'authorization', @@ -384,7 +385,7 @@ export class AuthController { @Res({ passthrough: true }) res: FastifyReply, ): Promise { this.logger.debug('Call loginWithJKUJwt'); - const profile = await this.login(req); + const profile = await this.loginBasic(req); res.header( 'authorization', @@ -444,7 +445,7 @@ export class AuthController { @Res({ passthrough: true }) res: FastifyReply, ): Promise { this.logger.debug('Call loginWithJWKJwt'); - const profile = await this.login(req); + const profile = await this.loginBasic(req); res.header( 'authorization', @@ -504,7 +505,7 @@ export class AuthController { @Res({ passthrough: true }) res: FastifyReply, ): Promise { this.logger.debug('Call loginWithX5CJwt'); - const profile = await this.login(req); + const profile = await this.loginBasic(req); res.header( 'authorization', @@ -564,7 +565,7 @@ export class AuthController { @Res({ passthrough: true }) res: FastifyReply, ): Promise { this.logger.debug('Call loginWithX5UJwt'); - const profile = await this.login(req); + const profile = await this.loginBasic(req); res.header( 'Authorization', @@ -624,7 +625,7 @@ export class AuthController { @Res({ passthrough: true }) res: FastifyReply, ): Promise { this.logger.debug('Call loginWithHMACJwt'); - const profile = await this.login(req); + const profile = await this.loginBasic(req); res.header( 'authorization', @@ -690,7 +691,7 @@ export class AuthController { } } - private async login(req: LoginRequest): Promise { + private async loginBasic(req: LoginRequest): Promise { let user: User; try { @@ -709,6 +710,13 @@ export class AuthController { }); } + if (!user.isBasic) { + throw new ForbiddenException({ + error: 'Invalid authentication method for this user', + location: __filename, + }); + } + const token = await this.authService.createToken( { user: user.email, diff --git a/src/auth/auth.guard.ts b/src/auth/auth.guard.ts index a23ec0a3..de6f7071 100644 --- a/src/auth/auth.guard.ts +++ b/src/auth/auth.guard.ts @@ -14,6 +14,7 @@ import { GqlContextType, GqlExecutionContext } from '@nestjs/graphql'; @Injectable() export class AuthGuard implements CanActivate { private static readonly AUTH_HEADER = 'authorization'; + private static readonly BEARER_PREFIX = 'bearer'; private readonly logger = new Logger(AuthGuard.name); constructor( @@ -25,8 +26,13 @@ export class AuthGuard implements CanActivate { try { this.logger.debug('Called canActivate'); const request = this.getRequest(context); + const token = this.extractToken(request); - return !!(await this.verifyToken(request, context)); + if (!token) { + return false; + } + + return await this.verifyToken(token, context); } catch (err) { this.logger.debug(`Failed to validate token: ${err.message}`); throw new UnauthorizedException({ @@ -36,16 +42,7 @@ export class AuthGuard implements CanActivate { } } - private getRequest(context: ExecutionContext): FastifyRequest { - return context.getType() === 'graphql' - ? GqlExecutionContext.create(context).getContext().req - : context.switchToHttp().getRequest(); - } - - private async verifyToken( - request: FastifyRequest, - context: ExecutionContext, - ): Promise { + private extractToken(request: FastifyRequest): string | undefined { let token = request.headers[AuthGuard.AUTH_HEADER]; if (!token?.length) { @@ -53,31 +50,38 @@ export class AuthGuard implements CanActivate { } if (this.checkIsBearer(token)) { - token = token.substring(7); + token = token.substring(AuthGuard.BEARER_PREFIX.length).trim(); } - if (!token?.length) { - return false; - } + return token?.length ? token : undefined; + } + + private getRequest(context: ExecutionContext): FastifyRequest { + return context.getType() === 'graphql' + ? GqlExecutionContext.create(context).getContext().req + : context.switchToHttp().getRequest(); + } + private async verifyToken( + token: string, + context: ExecutionContext, + ): Promise { const processorType = this.reflector.get( JwTypeMetadataField, context.getHandler(), ); - return this.authService.validateToken( - token, - processorType ?? JwtProcessorType.BEARER, - ); + try { + return await this.authService.validateToken(token, processorType); + } catch (err) { + return this.authService.validateToken(token, JwtProcessorType.BEARER); + } } private checkIsBearer(bearer: string): boolean { - if (!bearer || bearer.length < 10) { - return false; - } - - const prefix = bearer.substring(0, 7).toLowerCase(); - - return prefix === 'bearer '; + return ( + !!bearer && + bearer.toLowerCase().startsWith(AuthGuard.BEARER_PREFIX.toLowerCase()) + ); } } diff --git a/src/keycloak/keycloak.service.ts b/src/keycloak/keycloak.service.ts index 77610625..a5df0fab 100644 --- a/src/keycloak/keycloak.service.ts +++ b/src/keycloak/keycloak.service.ts @@ -250,8 +250,9 @@ export class KeyCloakService implements OnModuleInit { ); } - return new Map( + const jwks = new Map( data.keys.map((key: JWK & { kid: string }) => [key.kid, jwkToPem(key)]), ); + return jwks; } } diff --git a/src/model/user.entity.ts b/src/model/user.entity.ts index 6fb035ad..5853f812 100644 --- a/src/model/user.entity.ts +++ b/src/model/user.entity.ts @@ -33,4 +33,7 @@ export class User extends Base { @Property() phoneNumber: string; + + @Property() + isBasic: boolean; } diff --git a/src/users/api/CreateUserRequest.ts b/src/users/api/CreateUserRequest.ts index 366d9bb0..d650493a 100644 --- a/src/users/api/CreateUserRequest.ts +++ b/src/users/api/CreateUserRequest.ts @@ -1,6 +1,11 @@ import { ApiProperty } from '@nestjs/swagger'; import { UserDto } from './UserDto'; +export enum SignupMode { + BASIC = 'basic', + OIDC = 'oidc', +} + export class CreateUserRequest extends UserDto { @ApiProperty() company: string; @@ -10,5 +15,5 @@ export class CreateUserRequest extends UserDto { phoneNumber: string; @ApiProperty() password: string; - op: string; + op: SignupMode; } diff --git a/src/users/api/UserDto.ts b/src/users/api/UserDto.ts index b786d8ed..398c75d5 100644 --- a/src/users/api/UserDto.ts +++ b/src/users/api/UserDto.ts @@ -51,9 +51,10 @@ export class UserDto { @Expose({ groups: [FULL_USER_INFO] }) createdAt: Date; - constructor(params: { - [P in keyof UserDto]: UserDto[P]; - }) { + @Exclude() + isBasic: boolean; + + constructor(params: UserDto) { Object.assign(this, params); } } diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 6bea6e75..4d3944ce 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -35,7 +35,7 @@ import { ApiTags, ApiUnauthorizedResponse, } from '@nestjs/swagger'; -import { CreateUserRequest } from './api/CreateUserRequest'; +import { CreateUserRequest, SignupMode } from './api/CreateUserRequest'; import { UserDto } from './api/UserDto'; import { LdapQueryHandler } from './ldap.query.handler'; import { UsersService } from './users.service'; @@ -346,17 +346,18 @@ export class UsersController { try { this.logger.debug(`Create a basic user: ${user}`); - const userExists = await this.usersService.findByEmail(user.email); + const userExists = await this.doesUserExist(user); if (userExists) { - throw new HttpException('User already exists', 409); + throw new HttpException('User already exists', HttpStatus.CONFLICT); } + + return new UserDto( + await this.usersService.createUser(user, user.op === SignupMode.BASIC), + ); } catch (err) { - if (err.status === 404) { - return new UserDto(await this.usersService.createUser(user)); - } throw new HttpException( err.message ?? 'Something went wrong', - err.status ?? 500, + err.status ?? HttpStatus.INTERNAL_SERVER_ERROR, ); } } @@ -381,7 +382,13 @@ export class UsersController { try { this.logger.debug(`Create a OIDC user: ${user}`); - return new UserDto( + const userExists = await this.doesUserExist(user); + + if (userExists) { + throw new HttpException('User already exists', HttpStatus.CONFLICT); + } + + const keycloakUser = new UserDto( await this.keyCloakService.registerUser({ email: user.email, firstName: user.firstName, @@ -389,6 +396,10 @@ export class UsersController { password: user.password, }), ); + + this.createUser(user); + + return keycloakUser; } catch (err) { throw new HttpException( err.response.data ?? 'Something went wrong', @@ -559,4 +570,21 @@ export class UsersController { ).toString(), ).user; } + + private async doesUserExist(user: UserDto): Promise { + try { + const userExists = await this.usersService.findByEmail(user.email); + if (userExists) { + return true; + } + } catch (err) { + if (err.status === HttpStatus.NOT_FOUND) { + return false; + } + throw new HttpException( + err.message ?? 'Something went wrong', + err.status ?? HttpStatus.INTERNAL_SERVER_ERROR, + ); + } + } } diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 0c15137d..62fbe581 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -23,7 +23,7 @@ export class UsersService { private readonly usersRepository: EntityRepository, ) {} - async createUser(user: UserDto): Promise { + async createUser(user: UserDto, isBasicUser: boolean = true): Promise { this.log.debug(`Called createUser`); const u = new User(); @@ -35,6 +35,7 @@ export class UsersService { u.cardNumber = user.cardNumber; u.phoneNumber = user.phoneNumber; u.password = await hashPassword(user.password); + u.isBasic = isBasicUser; await this.usersRepository.persistAndFlush(u); this.log.debug(`Saved new user`);