Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Integrate API, Client and backoffice auth #130

Merged
merged 2 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"reflect-metadata": "catalog:",
"rxjs": "^7.8.1",
"typeorm": "catalog:",
"uid-safe": "^2.1.5",
"xlsx": "^0.18.5",
"zod": "catalog:"
},
Expand All @@ -61,6 +62,7 @@
"@types/passport-jwt": "^4.0.1",
"@types/passport-local": "^1.0.38",
"@types/supertest": "^6.0.0",
"@types/uid-safe": "^2.1.5",
"@typescript-eslint/eslint-plugin": "^7.0.0",
"@typescript-eslint/parser": "^7.0.0",
"eslint": "^8.42.0",
Expand Down
2 changes: 2 additions & 0 deletions api/src/modules/auth/auth.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { AuthenticationModule } from '@api/modules/auth/authentication.module';
import { RequestPasswordRecoveryCommandHandler } from '@api/modules/auth/commands/request-password-recovery-command.handler';
import { NewUserEventHandler } from '@api/modules/admin/events/handlers/new-user-event.handler';
import { PasswordRecoveryRequestedEventHandler } from '@api/modules/auth/events/handlers/password-recovery-requested.handler';
import { BackofficeService } from './backoffice.service';

@Module({
imports: [AuthenticationModule, NotificationsModule],
Expand All @@ -15,6 +16,7 @@ import { PasswordRecoveryRequestedEventHandler } from '@api/modules/auth/events/
RequestPasswordRecoveryCommandHandler,
NewUserEventHandler,
PasswordRecoveryRequestedEventHandler,
BackofficeService,
],
exports: [AuthenticationModule, AuthMailer],
})
Expand Down
28 changes: 26 additions & 2 deletions api/src/modules/auth/authentication.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
UseInterceptors,
ClassSerializerInterceptor,
HttpStatus,
Res,
} from '@nestjs/common';
import { User } from '@shared/entities/users/user.entity';
import { LocalAuthGuard } from '@api/modules/auth/guards/local-auth.guard';
Expand All @@ -21,13 +22,18 @@ import { CommandBus } from '@nestjs/cqrs';
import { RequestPasswordRecoveryCommand } from '@api/modules/auth/commands/request-password-recovery.command';
import { EmailConfirmation } from '@api/modules/auth/strategies/email-update.strategy';
import { ROLES } from '@shared/entities/users/roles.enum';
import { Response } from 'express';
import { ApiConfigService } from '../config/app-config.service';
import { BackofficeService } from './backoffice.service';

@Controller()
@UseInterceptors(ClassSerializerInterceptor)
export class AuthenticationController {
constructor(
private authService: AuthenticationService,
private readonly backofficeService: BackofficeService,
private readonly commandBus: CommandBus,
private readonly configService: ApiConfigService,
) {}

@Public()
Expand All @@ -48,9 +54,27 @@ export class AuthenticationController {
@Public()
@UseGuards(LocalAuthGuard)
@TsRestHandler(authContract.login)
async login(@GetUser() user: User): Promise<ControllerResponse> {
async login(
@GetUser() user: User,
@Res({ passthrough: true }) res: Response,
alepefe marked this conversation as resolved.
Show resolved Hide resolved
): Promise<ControllerResponse> {
return tsRestHandler(authContract.login, async () => {
const userWithAccessToken = await this.authService.logIn(user);
const [userWithAccessToken, backofficeSession] =
await this.authService.logIn(user);
if (backofficeSession !== undefined) {
const cookieName = this.configService.get(
'BACKOFFICE_SESSION_COOKIE_NAME',
);
const cookieValue =
this.backofficeService.generateCookieFromBackofficeSession(
backofficeSession,
);
res.cookie(cookieName, cookieValue, {
...backofficeSession.sess.cookie,
sameSite: 'lax',
});
}

return {
body: userWithAccessToken,
status: 201,
Expand Down
3 changes: 3 additions & 0 deletions api/src/modules/auth/authentication.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ import { JwtManager } from '@api/modules/auth/services/jwt.manager';
import { ConfirmAccountStrategy } from '@api/modules/auth/strategies/confirm-account.strategy';
import { PasswordManager } from '@api/modules/auth/services/password.manager';
import { EmailConfirmationJwtStrategy } from '@api/modules/auth/strategies/email-update.strategy';
import { BackOfficeSession } from '@shared/entities/users/backoffice-session';
import { TypeOrmModule } from '@nestjs/typeorm';

@Module({
imports: [
TypeOrmModule.forFeature([BackOfficeSession]),
PassportModule.register({ defaultStrategy: 'jwt' }),
JwtModule.registerAsync({
imports: [ApiConfigModule],
Expand Down
73 changes: 71 additions & 2 deletions api/src/modules/auth/authentication.service.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// Does not work without * as uid
import * as uid from 'uid-safe';
import {
ConflictException,
Injectable,
Expand All @@ -21,6 +23,13 @@ import { RequestEmailUpdateDto } from '@shared/dtos/users/request-email-update.d
import { SendEmailConfirmationEmailCommand } from '@api/modules/notifications/email/commands/send-email-confirmation-email.command';
import { PasswordManager } from '@api/modules/auth/services/password.manager';
import { API_EVENT_TYPES } from '@api/modules/api-events/events.enum';
import { Repository } from 'typeorm';
import {
BACKOFFICE_SESSIONS_TABLE,
BackOfficeSession,
} from '@shared/entities/users/backoffice-session';
import { ROLES } from '@shared/entities/users/roles.enum';
import { InjectRepository } from '@nestjs/typeorm';

@Injectable()
export class AuthenticationService {
Expand All @@ -31,6 +40,8 @@ export class AuthenticationService {
private readonly commandBus: CommandBus,
private readonly eventBus: EventBus,
private readonly passwordManager: PasswordManager,
@InjectRepository(BackOfficeSession)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor detail:

We might inject the back office session repo only in the back office service, and move this service around. it might be more clean and would also allow us to encapsulate potential helper methods that we might need in the future, coupling back office related logic and data retrieval in a single point

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do it when we need to create the back office service and move forward now.

private readonly backOfficeSessionRepository: Repository<BackOfficeSession>,
) {}
async validateUser(email: string, password: string): Promise<User> {
const user = await this.usersService.findByEmail(email);
Expand Down Expand Up @@ -81,9 +92,67 @@ export class AuthenticationService {
};
}

async logIn(user: User): Promise<UserWithAccessToken> {
private async createBackOfficeSession(
user: User,
accessToken: string,
): Promise<BackOfficeSession> {
// We replicate what adminjs does by default using postgres as session storage (the default in memory session storage is not production ready)
// This implementation is not compatible with many devices per user
await this.backOfficeSessionRepository
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Maybe it would be nice to move this to some private method like:
clearExistingSessionAdminSession or smth like that.

It also feels weird to not declaratively know what would happen if the query does not target any row but I guess we can live with that as long as we know how the underlying query works

.createQueryBuilder()
.delete()
.from(BACKOFFICE_SESSIONS_TABLE)
.where(`sess -> 'adminUser' ->> 'id' = :id`, { id: user.id })
.execute();

const currentDate = new Date();
const sessionExpirationDate = new Date(
Date.UTC(
currentDate.getUTCFullYear() + 1,
currentDate.getUTCMonth(),
currentDate.getUTCDate(),
currentDate.getUTCHours(),
currentDate.getUTCMinutes(),
currentDate.getUTCSeconds(),
),
);
const backofficeSession: BackOfficeSession = {
sid: uid.sync(24),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason you consider pg's gen_random_uuid() is not safe enough crypto-wise?
According to the docs it should be, unless the instance runs out of some resource and it falls back to not-very-oosp method to generate it

If no specific reason, we might offload this here, and also avoid using a lib that was last released 7 years ago

sess: {
cookie: {
secure: false,
httpOnly: true,
path: '/',
},
adminUser: {
id: user.id,
email: user.email,
name: user.name,
partnerName: user.partnerName,
isActive: true,
role: user.role,
createdAt: user.createdAt,
accessToken,
},
},
expire: sessionExpirationDate,
};
await this.backOfficeSessionRepository.insert(backofficeSession);
return backofficeSession;
}

async logIn(user: User): Promise<[UserWithAccessToken, BackOfficeSession?]> {
const { accessToken } = await this.jwtManager.signAccessToken(user.id);
return { user, accessToken };
if (user.role !== ROLES.ADMIN) {
return [{ user, accessToken }];
}

// An adminjs session needs to be created for the admin user
const backofficeSession = await this.createBackOfficeSession(
user,
accessToken,
);
return [{ user, accessToken }, backofficeSession];
}

async signUp(user: User, signUpDto: SignUpDto): Promise<void> {
Expand Down
25 changes: 25 additions & 0 deletions api/src/modules/auth/backoffice.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { BackOfficeSession } from '@shared/entities/users/backoffice-session';
import * as crypto from 'crypto';
import { ApiConfigService } from '../config/app-config.service';
import { Inject } from '@nestjs/common';

export class BackofficeService {
constructor(
@Inject(ApiConfigService)
private readonly configService: ApiConfigService,
) {}

public generateCookieFromBackofficeSession(
backofficeSession: BackOfficeSession,
): string {
const cookieSecret = this.configService.get(
'BACKOFFICE_SESSION_COOKIE_SECRET',
);
const hmac = crypto
.createHmac('sha256', cookieSecret)
.update(backofficeSession.sid)
.digest('base64')
.replace(/=+$/, '');
return `s:${backofficeSession.sid}.${hmac}`;
}
}
20 changes: 20 additions & 0 deletions api/src/modules/config/app-config.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,26 @@ import { JwtConfigHandler } from '@api/modules/config/auth-config.handler';
resolveConfigPath(`shared/config/.env.${process.env.NODE_ENV}`),
resolveConfigPath(`shared/config/.env`),
],
validate(config) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

const expectedVariables = [
'BACKOFFICE_SESSION_COOKIE_NAME',
'BACKOFFICE_SESSION_COOKIE_SECRET',
];

const missingVariables = [];
for (const expectedVariable of expectedVariables) {
if (config[expectedVariable] === undefined) {
missingVariables.push(expectedVariable);
}
}

if (missingVariables.length > 0) {
throw new Error(
`Missing required environment variables: ${missingVariables.join(', ')}`,
);
}
return config;
},
}),
DatabaseModule,
],
Expand Down
42 changes: 42 additions & 0 deletions api/test/integration/auth/delete-account.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { TestManager } from '../../utils/test-manager';
import { HttpStatus } from '@nestjs/common';
import { usersContract } from '@shared/contracts/users.contract';
import { ROLES } from '@shared/entities/users/roles.enum';

describe('Delete Account', () => {
let testManager: TestManager;

beforeAll(async () => {
testManager = await TestManager.createTestManager();
});

afterEach(async () => {
await testManager.clearDatabase();
});

afterAll(async () => {
await testManager.close();
});

test("An existing user should be able to delete it's account", async () => {
// Given a user exists with valid credentials
const user = await testManager.mocks().createUser({
role: ROLES.PARTNER,
email: '[email protected]',
isActive: true,
password: '12345678',
});

const { jwtToken } = await testManager.logUserIn(user);

// And the user tries to sign in with valid credentials
const response = await testManager
.request()
.delete(usersContract.deleteMe.path)
.set('Authorization', `Bearer ${jwtToken}`)
.send();

// We should get back OK response and an access token
expect(response.status).toBe(HttpStatus.OK);
});
});
28 changes: 28 additions & 0 deletions api/test/integration/auth/sign-in.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,34 @@ describe('Sign In', () => {
expect(response.body.accessToken).toBeDefined();
});

test('Should return 201 an access token and set a backoffice cookie when an admin user successfully signs in', async () => {
// Given a user exists with valid credentials
const user = await testManager.mocks().createUser({
role: ROLES.ADMIN,
email: '[email protected]',
isActive: true,
password: '12345678',
});

// And the user tries to sign in with valid credentials
const response = await testManager
.request()
.post(authContract.login.path)
.send({
email: '[email protected]',
password: '12345678',
});

// We should get back OK response and an access token
expect(response.status).toBe(HttpStatus.CREATED);
expect(response.body.accessToken).toBeDefined();
const setCookieHeader = response.headers['set-cookie'];
expect(setCookieHeader).toHaveLength(1);
expect(decodeURIComponent(setCookieHeader[0])).toMatch(
/^backoffice=s:[^\s]+\.[^\s]+;/,
);
});

test('Should return UNAUTHORIZED when trying to sign in with an inactive account', async () => {
// Given a user exists with valid credentials
const user = await testManager.mocks().createUser({
Expand Down
File renamed without changes.
File renamed without changes.
8 changes: 7 additions & 1 deletion admin/datasource.ts → backoffice/datasource.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import "reflect-metadata";
import dotenv from "dotenv";
dotenv.config({ path: `../shared/config/.env` });
import { DataSource } from "typeorm";
import { User } from "@shared/entities/users/user.entity.js";
import { ApiEventsEntity } from "@api/modules/api-events/api-events.entity.js";
Expand Down Expand Up @@ -32,6 +35,7 @@ import { ModelAssumptions } from "@shared/entities/model-assumptions.entity.js";
import { UserUploadCostInputs } from "@shared/entities/users/user-upload-cost-inputs.entity.js";
import { UserUploadRestorationInputs } from "@shared/entities/users/user-upload-restoration-inputs.entity.js";
import { UserUploadConservationInputs } from "@shared/entities/users/user-upload-conservation-inputs.entity.js";
import { BackOfficeSession } from "@shared/entities/users/backoffice-session.js";
import { CustomProject } from "@shared/entities/custom-project.entity.js";

// TODO: If we import the COMMON_DATABASE_ENTITIES from shared, we get an error where DataSouce is not set for a given entity
Expand Down Expand Up @@ -70,6 +74,8 @@ export const ADMINJS_ENTITIES = [
BaseSize,
BaseIncrease,
ModelAssumptions,
CustomProject,
BackOfficeSession
];

export const dataSource = new DataSource({
Expand All @@ -86,4 +92,4 @@ export const dataSource = new DataSource({
? { rejectUnauthorized: false }
: false,
logging: false,
});
});
Loading
Loading