From 43470c41f1d3e56a24846af9a4b60d8582d859bf Mon Sep 17 00:00:00 2001 From: Charles Zhao Date: Wed, 8 Mar 2023 15:21:59 +0800 Subject: [PATCH] fix(core,schemas): add new verification status table (#3312) --- .vscode/settings.json | 1 + .../core/src/libraries/verification-status.ts | 50 +++++++++++++++ .../core/src/queries/verification-status.ts | 41 +++++++++++++ packages/core/src/routes-me/user.ts | 32 +++------- packages/core/src/routes/consts.ts | 3 +- packages/core/src/tenants/Libraries.ts | 2 + packages/core/src/tenants/Queries.ts | 2 + ...678199795-add-verification-status-table.ts | 61 +++++++++++++++++++ packages/schemas/src/types/user.ts | 10 --- .../schemas/tables/verification_statuses.sql | 16 +++++ 10 files changed, 182 insertions(+), 36 deletions(-) create mode 100644 packages/core/src/libraries/verification-status.ts create mode 100644 packages/core/src/queries/verification-status.ts create mode 100644 packages/schemas/alterations/next-1678199795-add-verification-status-table.ts create mode 100644 packages/schemas/tables/verification_statuses.sql diff --git a/.vscode/settings.json b/.vscode/settings.json index 3f1126a4d..dfc202bfe 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -37,6 +37,7 @@ "silverhand", "slonik", "stylelint", + "timestamptz", "topbar", "withtyped" ] diff --git a/packages/core/src/libraries/verification-status.ts b/packages/core/src/libraries/verification-status.ts new file mode 100644 index 000000000..b6e932f28 --- /dev/null +++ b/packages/core/src/libraries/verification-status.ts @@ -0,0 +1,50 @@ +import { generateStandardId } from '@logto/core-kit'; + +import RequestError from '#src/errors/RequestError/index.js'; +import { verificationTimeout } from '#src/routes/consts.js'; +import type Queries from '#src/tenants/Queries.js'; +import assertThat from '#src/utils/assert-that.js'; + +export type VerificationStatusLibrary = ReturnType; + +export const createVerificationStatusLibrary = (queries: Queries) => { + const { + findVerificationStatusByUserIdAndSessionId, + insertVerificationStatus, + deleteVerificationStatusesByUserIdAndSessionId, + } = queries.verificationStatuses; + + const createVerificationStatus = async (userId: string, sessionId: string) => { + // Remove existing verification statuses for current user in current session. + await deleteVerificationStatusesByUserIdAndSessionId(userId, sessionId); + + // When creating new verification record, we use session ID to identify the client device. + // The session ID is a cookie value, which is unique for each client. + // This prevents the user from proceeding after being verified on another device. + return insertVerificationStatus({ + id: generateStandardId(), + sessionId, + userId, + }); + }; + + const checkVerificationStatus = async (userId: string, sessionId: string): Promise => { + const verificationStatus = await findVerificationStatusByUserIdAndSessionId(userId, sessionId); + + assertThat(verificationStatus, 'session.verification_session_not_found'); + + const { sessionId: storedSessionId, createdAt } = verificationStatus; + + // The user verification status is considered valid if: + // 1. The user is verified within 10 minutes. + // 2. The user is verified with the same client session (cookie). + const isValid = + Date.now() - createdAt < verificationTimeout && + Boolean(sessionId) && + storedSessionId === sessionId; + + assertThat(isValid, new RequestError({ code: 'session.verification_failed', status: 422 })); + }; + + return { createVerificationStatus, checkVerificationStatus }; +}; diff --git a/packages/core/src/queries/verification-status.ts b/packages/core/src/queries/verification-status.ts new file mode 100644 index 000000000..56beb3525 --- /dev/null +++ b/packages/core/src/queries/verification-status.ts @@ -0,0 +1,41 @@ +import type { CreateVerificationStatus, VerificationStatus } from '@logto/schemas'; +import { VerificationStatuses } from '@logto/schemas'; +import { convertToIdentifiers } from '@logto/shared'; +import type { CommonQueryMethods } from 'slonik'; +import { sql } from 'slonik'; + +import { buildInsertIntoWithPool } from '#src/database/insert-into.js'; + +const { table, fields } = convertToIdentifiers(VerificationStatuses); + +export const createVerificationStatusQueries = (pool: CommonQueryMethods) => { + const findVerificationStatusByUserIdAndSessionId = async (userId: string, sessionId: string) => + pool.maybeOne(sql` + select ${sql.join(Object.values(fields), sql`, `)} + from ${table} + where ${fields.sessionId}=${sessionId} and ${fields.userId}=${userId} + `); + + const insertVerificationStatus = buildInsertIntoWithPool(pool)< + CreateVerificationStatus, + VerificationStatus + >(VerificationStatuses, { + returning: true, + }); + + const deleteVerificationStatusesByUserIdAndSessionId = async ( + userId: string, + sessionId: string + ) => { + await pool.query(sql` + delete from ${table} + where ${fields.sessionId}=${sessionId} and ${fields.userId}=${userId} + `); + }; + + return { + findVerificationStatusByUserIdAndSessionId, + insertVerificationStatus, + deleteVerificationStatusesByUserIdAndSessionId, + }; +}; diff --git a/packages/core/src/routes-me/user.ts b/packages/core/src/routes-me/user.ts index e7a9dd81c..cd022fa4d 100644 --- a/packages/core/src/routes-me/user.ts +++ b/packages/core/src/routes-me/user.ts @@ -1,10 +1,5 @@ import { emailRegEx, passwordRegEx, usernameRegEx } from '@logto/core-kit'; -import type { PasswordVerificationData } from '@logto/schemas'; -import { - userInfoSelectFields, - passwordVerificationGuard, - arbitraryObjectGuard, -} from '@logto/schemas'; +import { userInfoSelectFields, arbitraryObjectGuard } from '@logto/schemas'; import { pick } from '@silverhand/essentials'; import { literal, object, string } from 'zod'; @@ -26,6 +21,7 @@ export default function userRoutes( }, libraries: { users: { checkIdentifierCollision }, + verificationStatuses: { createVerificationStatus, checkVerificationStatus }, }, } = tenant; @@ -114,19 +110,14 @@ export default function userRoutes( const cookieMap = convertCookieToMap(ctx.request.headers.cookie); const sessionId = cookieMap.get('_session'); - assertThat(Boolean(sessionId), new RequestError({ code: 'session.not_found', status: 401 })); + assertThat(sessionId, new RequestError({ code: 'session.not_found', status: 401 })); const user = await findUserById(userId); assertThat(!user.isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); await verifyUserPassword(user, password); - const customData: PasswordVerificationData = { - passwordVerifiedAt: Date.now(), - passwordVerifiedWithSessionId: sessionId, - }; - - await updateUserById(userId, { customData }); + await createVerificationStatus(userId, sessionId); ctx.status = 204; @@ -141,23 +132,16 @@ export default function userRoutes( const { id: userId } = ctx.auth; const { password } = ctx.guard.body; - const { customData, isSuspended } = await findUserById(userId); + const { isSuspended } = await findUserById(userId); + assertThat(!isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); const cookieMap = convertCookieToMap(ctx.request.headers.cookie); const sessionId = cookieMap.get('_session'); - const parsed = passwordVerificationGuard.safeParse(customData); - // The password verification status is considered valid if: - // 1. The password is verified within 10 minutes. - // 2. The password is verified with the same session. - const isValid = - parsed.success && - Date.now() - parsed.data.passwordVerifiedAt < 1000 * 60 * 10 && - Boolean(sessionId) && - parsed.data.passwordVerifiedWithSessionId === sessionId; + assertThat(sessionId, new RequestError({ code: 'session.not_found', status: 401 })); - assertThat(isValid, new RequestError({ code: 'session.verification_failed', status: 401 })); + await checkVerificationStatus(userId, sessionId); const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password); await updateUserById(userId, { passwordEncrypted, passwordEncryptionMethod }); diff --git a/packages/core/src/routes/consts.ts b/packages/core/src/routes/consts.ts index b8de39015..8dc86cc4f 100644 --- a/packages/core/src/routes/consts.ts +++ b/packages/core/src/routes/consts.ts @@ -9,5 +9,4 @@ export const routes = Object.freeze({ signUp, } as const); -export const verificationTimeout = 10 * 60; // 10 mins. -export const continueSignInTimeout = 10 * 60; // 10 mins. +export const verificationTimeout = 10 * 60 * 1000; // 10 mins. diff --git a/packages/core/src/tenants/Libraries.ts b/packages/core/src/tenants/Libraries.ts index eb68d75f2..acee41282 100644 --- a/packages/core/src/tenants/Libraries.ts +++ b/packages/core/src/tenants/Libraries.ts @@ -7,6 +7,7 @@ import { createResourceLibrary } from '#src/libraries/resource.js'; import { createSignInExperienceLibrary } from '#src/libraries/sign-in-experience/index.js'; import { createSocialLibrary } from '#src/libraries/social.js'; import { createUserLibrary } from '#src/libraries/user.js'; +import { createVerificationStatusLibrary } from '#src/libraries/verification-status.js'; import type { ModelRouters } from '#src/model-routers/index.js'; import type Queries from './Queries.js'; @@ -21,6 +22,7 @@ export default class Libraries { socials = createSocialLibrary(this.queries, this.connectors); passcodes = createPasscodeLibrary(this.queries, this.connectors); applications = createApplicationLibrary(this.queries); + verificationStatuses = createVerificationStatusLibrary(this.queries); constructor(private readonly queries: Queries, private readonly modelRouters: ModelRouters) {} } diff --git a/packages/core/src/tenants/Queries.ts b/packages/core/src/tenants/Queries.ts index 95507f858..732bc52a9 100644 --- a/packages/core/src/tenants/Queries.ts +++ b/packages/core/src/tenants/Queries.ts @@ -15,6 +15,7 @@ import { createScopeQueries } from '#src/queries/scope.js'; import { createSignInExperienceQueries } from '#src/queries/sign-in-experience.js'; import { createUserQueries } from '#src/queries/user.js'; import { createUsersRolesQueries } from '#src/queries/users-roles.js'; +import { createVerificationStatusQueries } from '#src/queries/verification-status.js'; export default class Queries { applications = createApplicationQueries(this.pool); @@ -32,6 +33,7 @@ export default class Queries { users = createUserQueries(this.pool); usersRoles = createUsersRolesQueries(this.pool); applicationsRoles = createApplicationsRolesQueries(this.pool); + verificationStatuses = createVerificationStatusQueries(this.pool); constructor(public readonly pool: CommonQueryMethods) {} } diff --git a/packages/schemas/alterations/next-1678199795-add-verification-status-table.ts b/packages/schemas/alterations/next-1678199795-add-verification-status-table.ts new file mode 100644 index 000000000..bc6b46f01 --- /dev/null +++ b/packages/schemas/alterations/next-1678199795-add-verification-status-table.ts @@ -0,0 +1,61 @@ +import type { CommonQueryMethods } from 'slonik'; +import { sql } from 'slonik'; + +import type { AlterationScript } from '../lib/types/alteration.js'; + +const getId = (value: string) => sql.identifier([value]); + +const getDatabaseName = async (pool: CommonQueryMethods) => { + const { currentDatabase } = await pool.one<{ currentDatabase: string }>(sql` + select current_database(); + `); + + return currentDatabase.replaceAll('-', '_'); +}; + +const alteration: AlterationScript = { + up: async (pool) => { + const database = await getDatabaseName(pool); + const baseRoleId = getId(`logto_tenant_${database}`); + + await pool.query(sql` + create table verification_statuses ( + tenant_id varchar(21) not null + references tenants (id) on update cascade on delete cascade, + id varchar(21) not null, + user_id varchar(21) not null + references users (id) on update cascade on delete cascade, + session_id varchar(128) not null, + created_at timestamptz not null default(now()), + primary key (id) + ); + + create index verification_statuses__id + on verification_statuses (tenant_id, id); + + create index verification_statuses__user_id__session_id + on verification_statuses (tenant_id, user_id, session_id); + + create trigger set_tenant_id before insert on verification_statuses + for each row execute procedure set_tenant_id(); + + alter table verification_statuses enable row level security; + + create policy verification_statuses_tenant_id on verification_statuses to ${baseRoleId} + using (tenant_id = (select id from tenants where db_user = current_user)); + + grant select, insert, update, delete on verification_statuses to ${baseRoleId}; + `); + }, + down: async (pool) => { + await pool.query(sql` + drop policy verification_statuses_tenant_id on verification_statuses; + + alter table verification_statuses disable row level security; + + drop table verification_statuses; + `); + }, +}; + +export default alteration; diff --git a/packages/schemas/src/types/user.ts b/packages/schemas/src/types/user.ts index 5dd9a1500..a416c4be0 100644 --- a/packages/schemas/src/types/user.ts +++ b/packages/schemas/src/types/user.ts @@ -1,6 +1,3 @@ -import type { z } from 'zod'; -import { number, object, string } from 'zod'; - import type { CreateUser } from '../db-entries/index.js'; export const userInfoSelectFields = Object.freeze([ @@ -34,10 +31,3 @@ export enum UserRole { export enum PredefinedScope { All = 'all', } - -export const passwordVerificationGuard = object({ - passwordVerifiedAt: number(), - passwordVerifiedWithSessionId: string().optional(), -}); - -export type PasswordVerificationData = z.infer; diff --git a/packages/schemas/tables/verification_statuses.sql b/packages/schemas/tables/verification_statuses.sql new file mode 100644 index 000000000..1e76a24a6 --- /dev/null +++ b/packages/schemas/tables/verification_statuses.sql @@ -0,0 +1,16 @@ +create table verification_statuses ( + tenant_id varchar(21) not null + references tenants (id) on update cascade on delete cascade, + id varchar(21) not null, + user_id varchar(21) not null + references users (id) on update cascade on delete cascade, + session_id varchar(128) not null, + created_at timestamptz not null default(now()), + primary key (id) +); + +create index verification_statuses__id + on verification_statuses (tenant_id, id); + +create index verification_statuses__user_id__session_id + on verification_statuses (tenant_id, user_id, session_id);