feat: Integrate API, Client and backoffice auth #130
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
alepefe
force-pushed
the
feat/adminjs-integration
branch
from
d80db11 to
a53359a
Compare
alepefe
force-pushed
the
feat/adminjs-integration
branch
from
a53359a to
764214c
Compare
alepefe
force-pushed
the
feat/adminjs-integration
branch
2 times, most recently
from
5266b78 to
e3e6790
Compare
alepefe
requested review from
alexeh and
andresgnlez
and removed request for
alexeh
…d delete-account.spec.ts tests in the backend
alepefe
force-pushed
the
feat/adminjs-integration
branch
from
e3e6790 to
86c1f77
Compare
| ): 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 |
There was a problem hiding this comment.
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
| @@ -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) { | |||
| @@ -26,7 +26,7 @@ export class CustomProject { | |||
| @PrimaryGeneratedColumn("uuid") | |||
| id?: string; | |||
|
|
|||
| @Column({ type: "varchar", name: "project_name" }) | |||
| @Column({ name: "project_name", type: "varchar" }) | |||
| ), | ||
| ); | ||
| const backofficeSession: BackOfficeSession = { | ||
| sid: uid.sync(24), |
There was a problem hiding this comment.
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
| @@ -31,6 +40,8 @@ export class AuthenticationService { | |||
| private readonly commandBus: CommandBus, | |||
| private readonly eventBus: EventBus, | |||
| private readonly passwordManager: PasswordManager, | |||
| @InjectRepository(BackOfficeSession) | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
We can do it when we need to create the back office service and move forward now.
| override async handleLogout({res}: {res: Response}) { | ||
| // Remove auth cookies | ||
| res.setHeader('Set-Cookie', [ | ||
| `backoffice=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly`, |
There was a problem hiding this comment.
Wouldn't work if we just remove the cookie/cookies altogether instead of rewriting this value?
There was a problem hiding this comment.
Amazing job @alepefe thanks.
I have left some minor details, but not strong opinions about. Feel free to consider or ignore them altogether.
I am just particularly curious about this tho:
https://github.com/Vizzuality/tnc-blue-carbon-cost-tool/pull/130/files#r1867154414
I haven't had the time to properly run and test this to be honest, let's just iterate if somehting goes south. Let me know what your plan is before pushing this through!
This pull request includes significant changes to add backoffice session management and improve authentication functionality. The most important changes include adding new dependencies, modifying authentication logic to handle backoffice sessions, and renaming the
adminmodule tobackoffice.Backoffice Session Management:
api/src/modules/auth/authentication.controller.ts: Updated theloginmethod to create and set a backoffice session cookie for admin users.api/src/modules/auth/authentication.service.ts: Added methods to create backoffice sessions and updated thelogInmethod to handle admin users differently.api/src/modules/auth/backoffice.service.ts: Added a new service to generate cookies from backoffice sessions.Dependency Additions:
api/package.json: Addeduid-safeand its types as dependencies. [1] [2]backoffice/package.json: Added dependencies for session management, includingconnect-pg-simple,cookie, andcookie-parser. [1] [2]Authentication Enhancements:
api/src/modules/auth/auth.module.ts: ImportedBackofficeServiceand added it to the providers list. [1] [2]api/src/modules/auth/authentication.service.ts: InjectedBackOfficeSessionrepository and updatedlogInmethod to handle backoffice sessions.Module Renaming:
adminmodule tobackofficeand updated related files and imports. [1] [2] [3]Configuration Updates:
api/src/modules/config/app-config.module.ts: Added validation for new environment variables related to backoffice sessions.