From 5a905a123995f5f6c79664cbcfb766a098704fc0 Mon Sep 17 00:00:00 2001 From: Maieul BOYER Date: Thu, 13 Nov 2025 16:00:05 +0100 Subject: [PATCH] feat(oauth2/db): reworked oauth2 database footprint - Removed `auth` table and merged its information inside the `user` table - Changed around some field names in the database - Changed Create*User functions to not be using overload but different functions --- src/@shared/src/database/index.ts | 4 +- src/@shared/src/database/init.dbml | 18 ++--- src/@shared/src/database/init.sql | 14 ++-- src/@shared/src/database/mixin/oauth2.ts | 88 ------------------------ src/@shared/src/database/mixin/user.ts | 87 +++++++++++++++++++---- src/@shared/src/utils/index.ts | 1 + src/auth/src/routes/guestLogin.ts | 10 +-- src/auth/src/routes/oauth2/callback.ts | 6 +- src/auth/src/routes/signin.ts | 2 +- src/user/src/routes/info.ts | 2 +- 10 files changed, 90 insertions(+), 142 deletions(-) delete mode 100644 src/@shared/src/database/mixin/oauth2.ts diff --git a/src/@shared/src/database/index.ts b/src/@shared/src/database/index.ts index 38853d4..597efac 100644 --- a/src/@shared/src/database/index.ts +++ b/src/@shared/src/database/index.ts @@ -3,14 +3,12 @@ import { FastifyInstance, FastifyPluginAsync } from 'fastify'; import { isNullish } from '@shared/utils'; import { Database as DbImpl } from './mixin/_base'; -import { IOauthDb, OauthImpl } from './mixin/oauth2'; import { IUserDb, UserImpl } from './mixin/user'; Object.assign(DbImpl.prototype, UserImpl); -Object.assign(DbImpl.prototype, OauthImpl); -export interface Database extends DbImpl, IUserDb, IOauthDb { } +export interface Database extends DbImpl, IUserDb { } // When using .decorate you have to specify added properties for Typescript declare module 'fastify' { diff --git a/src/@shared/src/database/init.dbml b/src/@shared/src/database/init.dbml index 6c4c160..1a663de 100644 --- a/src/@shared/src/database/init.dbml +++ b/src/@shared/src/database/init.dbml @@ -17,21 +17,11 @@ Project Transcendance { Table user { id text [PK, not null] - login_name text [unique] - display_name text [not null] - password text [null, Note: "If password is NULL, this means that the user is created through OAUTH2"] + login text [unique] + name text [not null] + password text [null, Note: "If password is NULL, this means that the user is created through OAUTH2 or guest login"] otp text [null, Note: "If otp is NULL, then the user didn't configure 2FA"] guest integer [not null, default: 0] - + oauth2 text [null, default: `NULL` , Note: "format: :; null if not logged via provider"] Note: "Represent a user" } - -Table auth { - id integer [PK, not null, increment] - provider text [not null] - user text [ref: > user.id, not null] - oauth2_user text [not null, unique, Note: ''' - This makes sure that an oauth2 login is the always the same `user` - Aka can't have two account bound to the same account - '''] -} diff --git a/src/@shared/src/database/init.sql b/src/@shared/src/database/init.sql index 959e8e4..b53675b 100644 --- a/src/@shared/src/database/init.sql +++ b/src/@shared/src/database/init.sql @@ -1,15 +1,9 @@ CREATE TABLE IF NOT EXISTS user ( id TEXT PRIMARY KEY NOT NULL, - login_name TEXT UNIQUE, - display_name TEXT NOT NULL, + login TEXT UNIQUE, + name TEXT NOT NULL, password TEXT, otp TEXT, - guest INTEGER NOT NULL DEFAULT 0 -); -CREATE TABLE IF NOT EXISTS auth ( - id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, - provider TEXT NOT NULL, - user TEXT NOT NULL, - oauth2_user TEXT NOT NULL UNIQUE, - FOREIGN KEY(user) REFERENCES user(id) + guest INTEGER NOT NULL DEFAULT 0, + oauth2 TEXT DEFAULT NULL ); diff --git a/src/@shared/src/database/mixin/oauth2.ts b/src/@shared/src/database/mixin/oauth2.ts deleted file mode 100644 index 6d03e9c..0000000 --- a/src/@shared/src/database/mixin/oauth2.ts +++ /dev/null @@ -1,88 +0,0 @@ -// import type { Database } from './_base'; -import { isNullish } from '@shared/utils'; -import { UserId, IUserDb, userFromRow, User } from './user'; - -// never use this directly - -export interface IOauthDb extends IUserDb { - getAllFromProvider(this: IOauthDb, provider: string): ProviderUser[], - getProviderUser(this: IOauthDb, provider: string, unique_id: string): ProviderUser | undefined, - getUserFromProviderUser(this: IOauthDb, provider: string, unique_id: string): User | undefined, - getUserFromProviderUserId(this: IOauthDb, id: ProviderUserId): User | undefined, - getProviderUserFromId(this: IOauthDb, id: ProviderUserId): ProviderUser | undefined, - - createUserWithProvider(this: IOauthDb, provider: string, unique_id: string, username: string): Promise, -}; - -export const OauthImpl: Omit = { - getAllFromProvider(this: IOauthDb, provider: string): ProviderUser[] { - return this.prepare('SELECT * FROM auth WHERE provider = @provider') - .all({ provider }) - .map(r => providerUserFromRow(r as Partial | undefined)) - .filter(v => !isNullish(v)) ?? []; - }, - - getProviderUser(this: IOauthDb, provider: string, unique_id: string): ProviderUser | undefined { - unique_id = `provider:${unique_id}`; - return providerUserFromRow(this.prepare('SELECT * FROM auth WHERE provider = @provider AND oauth2_user = @unique_id') - .get({ provider, unique_id }) as Partial | undefined); - }, - - getProviderUserFromId(this: IOauthDb, id: ProviderUserId): ProviderUser | undefined { - return providerUserFromRow(this.prepare('SELECT * FROM auth WHERE id = @id') - .get({ id }) as Partial | undefined); - }, - - getUserFromProviderUser(this: IOauthDb, provider: string, unique_id: string): User | undefined { - unique_id = `provider:${unique_id}`; - return userFromRow( - this.prepare('SELECT user.* from auth INNER JOIN user ON user.id = auth.user WHERE auth.provider = @provider AND auth.oauth2_user = @unique_id') - .get({ provider, unique_id }) as Partial | undefined, - ); - }, - - getUserFromProviderUserId(this: IOauthDb, id: ProviderUserId): User | undefined { - return userFromRow( - this.prepare('SELECT user.* from auth INNER JOIN user ON user.id = auth.user WHERE auth.id = @id') - .get({ id }) as Partial | undefined, - ); - }, - - async createUserWithProvider(this: IOauthDb, provider: string, unique_id: string, username: string): Promise { - unique_id = `provider:${unique_id}`; - const user = await this.createUser(null, username, undefined, false); - if (isNullish(user)) { return undefined; } - this.prepare('INSERT INTO auth (provider, user, oauth2_user) VALUES (@provider, @user_id, @unique_id)').run({ provider, user_id: user.id, unique_id }); - return user; - }, -}; - -export type ProviderUserId = number & { readonly __brand: unique symbol }; - -export type ProviderUser = { - readonly id: ProviderUserId, - readonly provider: string, - readonly user: UserId, - readonly oauth2_user: string, -}; - -/** - * Get a user from a row - * - * @param row The data from sqlite - * - * @returns The user if it exists, undefined otherwise - */ -function providerUserFromRow(row?: Partial): ProviderUser | undefined { - if (isNullish(row)) return undefined; - if (isNullish(row.id)) return undefined; - if (isNullish(row.provider)) return undefined; - if (isNullish(row.user)) return undefined; - if (isNullish(row.oauth2_user)) return undefined; - return { - id: row.id, - provider: row.provider, - user: row.user, - oauth2_user: row.oauth2_user, - }; -} diff --git a/src/@shared/src/database/mixin/user.ts b/src/@shared/src/database/mixin/user.ts index 5408efd..558bfee 100644 --- a/src/@shared/src/database/mixin/user.ts +++ b/src/@shared/src/database/mixin/user.ts @@ -9,12 +9,15 @@ import { UUID, newUUID } from '@shared/utils/uuid'; export interface IUserDb extends Database { getUserFromLoginName(name: string): User | undefined, getUser(id: string): User | undefined, + getOauth2User(provider: string, unique: string): User | undefined, getUserOtpSecret(id: UserId): string | undefined, - createUser(login_name: string | null, display_name: string, password: string | undefined, guest: boolean): Promise, - createUser(login_name: string | null, display_name: string, password: string | undefined): Promise, + createUser(login: string | null, name: string, password: string): Promise, + createGuestUser(name: string): Promise, + createOauth2User(name: string, provider: string, provider_unique: string): Promise, setUserPassword(id: UserId, password: string | undefined): Promise, ensureUserOtpSecret(id: UserId): string | undefined, deleteUserOtpSecret(id: UserId): void, + getAllUserFromProvider(provider: string): User[] | undefined, }; export const UserImpl: Omit = { @@ -28,7 +31,7 @@ export const UserImpl: Omit = { getUserFromLoginName(this: IUserDb, name: string): User | undefined { return userFromRow( this.prepare( - 'SELECT * FROM user WHERE login_name = @name LIMIT 1', + 'SELECT * FROM user WHERE login = @name LIMIT 1', ).get({ name }) as (Partial | undefined), ); }, @@ -56,13 +59,47 @@ export const UserImpl: Omit = { * * @returns The user struct */ - async createUser(this: IUserDb, login_name: string | null, display_name: string, password: string | undefined, guest: boolean = false): Promise { - password = await hashPassword(password); + async createUser(this: IUserDb, login: string, name: string, password: string): Promise { + const password_ = await hashPassword(password); const id = newUUID(); return userFromRow( this.prepare( - 'INSERT OR FAIL INTO user (id, login_name, display_name, password, guest) VALUES (@id, @login_name, @display_name, @password, @guest) RETURNING *', - ).get({ id, login_name, display_name, password, guest: guest ? 1 : 0 }) as (Partial | undefined), + 'INSERT OR FAIL INTO user (id, login, name, password, guest, oauth2) VALUES (@id, @login, @name, @password, @guest, @oauth2) RETURNING *', + ).get({ id, login, name, password: password_, guest: 0, oauth2: null }) as (Partial | undefined), + ); + }, + + /** + * Create a new user using password hash + * + * @param name the username for the new user (must be unique and sanitized) + * @param password the plaintext password of the new user (if any) + * + * @returns The user struct + */ + async createGuestUser(this: IUserDb, name: string): Promise { + const id = newUUID(); + return userFromRow( + this.prepare( + 'INSERT OR FAIL INTO user (id, login, name, password, guest, oauth2) VALUES (@id, @login, @name, @password, @guest, @oauth2) RETURNING *', + ).get({ id, login: null, name, password: null, guest: 1, oauth2: null }) as (Partial | undefined), + ); + }, + + /** + * Create a new user using password hash + * + * @param name the username for the new user (must be unique and sanitized) + * @param password the plaintext password of the new user (if any) + * + * @returns The user struct + */ + async createOauth2User(this: IUserDb, name: string, provider: string, provider_unique: string): Promise { + const id = newUUID(); + return userFromRow( + this.prepare( + 'INSERT OR FAIL INTO user (id, login, name, password, guest, oauth2) VALUES (@id, @login, @name, @password, @guest, @oauth2) RETURNING *', + ).get({ id, login: null, name, password: null, guest: 0, oauth2: `${provider}:${provider_unique}` }) as (Partial | undefined), ); }, @@ -102,17 +139,29 @@ export const UserImpl: Omit = { deleteUserOtpSecret(this: IUserDb, id: UserId): void { this.prepare('UPDATE OR IGNORE user SET otp = NULL WHERE id = @id').run({ id }); }, + + getAllUserFromProvider(this: IUserDb, provider: string): User[] | undefined { + const req = this.prepare('SELECT * FROM user WHERE oauth2 LIKE oauth2 = @oauth2 || \'%\''); + return (req.all({ oauth2: provider }) as Partial[]).map(userFromRow).filter(v => !isNullish(v)); + }, + getOauth2User(this: IUserDb, provider: string, unique: string): User | undefined { + const req = this.prepare('SELECT * FROM user WHERE oauth2 = @oauth2').get({ oauth2: `${provider}:${unique}` }) as Partial | undefined; + return userFromRow(req); + }, }; export type UserId = UUID; export type User = { readonly id: UserId; - readonly login_name?: string; - readonly display_name: string; + readonly login?: string; + readonly name: string; readonly password?: string; readonly otp?: string; readonly guest: boolean; + // will be split/merged from the `provider` column + readonly provider_name?: string; + readonly provider_unique?: string; }; export async function verifyUserPassword( @@ -147,17 +196,29 @@ async function hashPassword( * * @returns The user if it exists, undefined otherwise */ -export function userFromRow(row?: Partial): User | undefined { +export function userFromRow(row?: Partial & { provider?: string }>): User | undefined { if (isNullish(row)) return undefined; if (isNullish(row.id)) return undefined; - if (isNullish(row.display_name)) return undefined; + if (isNullish(row.name)) return undefined; if (isNullish(row.guest)) return undefined; + + let provider_name = undefined; + let provider_unique = undefined; + + if (row.provider) { + const splitted = row.provider.split(':', 1); + if (splitted.length != 2) { return undefined; } + provider_name = splitted[0]; + provider_unique = splitted[1]; + } + return { id: row.id, - login_name: row.login_name ?? undefined, - display_name: row.display_name, + login: row.login ?? undefined, + name: row.name, password: row.password ?? undefined, otp: row.otp ?? undefined, guest: !!(row.guest ?? true), + provider_name, provider_unique, }; } diff --git a/src/@shared/src/utils/index.ts b/src/@shared/src/utils/index.ts index 02ec07b..a79cbe9 100644 --- a/src/@shared/src/utils/index.ts +++ b/src/@shared/src/utils/index.ts @@ -59,6 +59,7 @@ function makeResponse( key: string, payload?: T, ): ReturnType { + this.log.info(`Sending response: ${status}; response = ${JSON.stringify({ kind, msg: key, payload })}`); return this.code(status).send({ kind, msg: key, payload }); } diff --git a/src/auth/src/routes/guestLogin.ts b/src/auth/src/routes/guestLogin.ts index 30f4d89..06f7376 100644 --- a/src/auth/src/routes/guestLogin.ts +++ b/src/auth/src/routes/guestLogin.ts @@ -31,15 +31,7 @@ const route: FastifyPluginAsync = async (fastify, _opts): Promise => { const adjective = getRandomFromList(fastify.words.adjectives); const noun = getRandomFromList(fastify.words.nouns); - const user = await this.db.createUser( - // no login_name => can't login - null, - `${adjective} ${noun}`, - // no password - undefined, - // is a guest - true, - ); + const user = await this.db.createGuestUser(`${adjective} ${noun}`); if (isNullish(user)) { return res.makeResponse(500, 'failed', 'guestLogin.failed.generic.unknown'); } diff --git a/src/auth/src/routes/oauth2/callback.ts b/src/auth/src/routes/oauth2/callback.ts index 7a82404..10bc72a 100644 --- a/src/auth/src/routes/oauth2/callback.ts +++ b/src/auth/src/routes/oauth2/callback.ts @@ -30,9 +30,9 @@ const route: FastifyPluginAsync = async (fastify, _opts): Promise => { const result = await creq.getCode(); const userinfo = await provider.getUserInfo(result); - let u = this.db.getUserFromProviderUser(provider.display_name, userinfo.unique_id); + let u = this.db.getOauth2User(provider.display_name, userinfo.unique_id); if (isNullish(u)) { - u = await this.db.createUserWithProvider(provider.display_name, userinfo.unique_id, userinfo.name); + u = await this.db.createOauth2User(userinfo.name, provider.display_name, userinfo.unique_id); } if (isNullish(u)) { return res.code(500).send('failed to fetch or create user...'); @@ -40,7 +40,7 @@ const route: FastifyPluginAsync = async (fastify, _opts): Promise => { const token = this.signJwt('auth', u.id); - return res.setCookie('token', token, { path: '/' }).redirect('/'); + return res.setCookie('token', token, { path: '/' }).redirect('/app/'); }, ); }; diff --git a/src/auth/src/routes/signin.ts b/src/auth/src/routes/signin.ts index d59dd6b..f8b304a 100644 --- a/src/auth/src/routes/signin.ts +++ b/src/auth/src/routes/signin.ts @@ -47,7 +47,7 @@ const route: FastifyPluginAsync = async (fastify, _opts): Promise => { // password is good too ! if (this.db.getUserFromLoginName(name) !== undefined) { return res.makeResponse(400, 'failed', 'signin.failed.username.existing'); } - const u = await this.db.createUser(name, name, password, false); + const u = await this.db.createUser(name, name, password); if (isNullish(u)) { return res.makeResponse(500, 'failed', 'signin.failed.generic'); } // every check has been passed, they are now logged in, using this token to say who they are... diff --git a/src/user/src/routes/info.ts b/src/user/src/routes/info.ts index e0242ff..edf7c00 100644 --- a/src/user/src/routes/info.ts +++ b/src/user/src/routes/info.ts @@ -46,7 +46,7 @@ const route: FastifyPluginAsync = async (fastify, _opts): Promise => { const payload = { - name: user.display_name, + name: user.name, id: user.id, // the !! converts a value from to either `true` or `false` // it uses the same convention from using in a if, meaning that