From 98963900652cd71400b532a37e4631030798009f Mon Sep 17 00:00:00 2001 From: Charles Zhao Date: Mon, 13 Mar 2023 11:20:52 +0800 Subject: [PATCH] fix(core,schemas): remove sessionId usage from verification status table (#3345) --- .../core/src/libraries/verification-status.ts | 30 ++++++------------- .../core/src/queries/verification-status.ts | 15 ++++------ packages/core/src/routes-me/user.ts | 14 ++------- .../core/src/routes-me/verification-code.ts | 8 +---- packages/core/src/utils/cookie.ts | 15 ---------- ...678199795-add-verification-status-table.ts | 5 ++-- .../schemas/tables/verification_statuses.sql | 5 ++-- 7 files changed, 22 insertions(+), 70 deletions(-) delete mode 100644 packages/core/src/utils/cookie.ts diff --git a/packages/core/src/libraries/verification-status.ts b/packages/core/src/libraries/verification-status.ts index b6e932f28..1ada8ade9 100644 --- a/packages/core/src/libraries/verification-status.ts +++ b/packages/core/src/libraries/verification-status.ts @@ -9,40 +9,28 @@ export type VerificationStatusLibrary = ReturnType { const { - findVerificationStatusByUserIdAndSessionId, + findVerificationStatusByUserId, insertVerificationStatus, - deleteVerificationStatusesByUserIdAndSessionId, + deleteVerificationStatusesByUserId, } = queries.verificationStatuses; - const createVerificationStatus = async (userId: string, sessionId: string) => { - // Remove existing verification statuses for current user in current session. - await deleteVerificationStatusesByUserIdAndSessionId(userId, sessionId); + const createVerificationStatus = async (userId: string) => { + // Remove existing verification statuses for current user. + await deleteVerificationStatusesByUserId(userId); - // 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); + const checkVerificationStatus = async (userId: string): Promise => { + const verificationStatus = await findVerificationStatusByUserId(userId); 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; - + // The user verification status is considered valid if the user is verified within 10 minutes. + const isValid = Date.now() - verificationStatus.createdAt < verificationTimeout; assertThat(isValid, new RequestError({ code: 'session.verification_failed', status: 422 })); }; diff --git a/packages/core/src/queries/verification-status.ts b/packages/core/src/queries/verification-status.ts index 56beb3525..7fd0ccc86 100644 --- a/packages/core/src/queries/verification-status.ts +++ b/packages/core/src/queries/verification-status.ts @@ -9,11 +9,11 @@ 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) => + const findVerificationStatusByUserId = async (userId: string) => pool.maybeOne(sql` select ${sql.join(Object.values(fields), sql`, `)} from ${table} - where ${fields.sessionId}=${sessionId} and ${fields.userId}=${userId} + where ${fields.userId}=${userId} `); const insertVerificationStatus = buildInsertIntoWithPool(pool)< @@ -23,19 +23,16 @@ export const createVerificationStatusQueries = (pool: CommonQueryMethods) => { returning: true, }); - const deleteVerificationStatusesByUserIdAndSessionId = async ( - userId: string, - sessionId: string - ) => { + const deleteVerificationStatusesByUserId = async (userId: string) => { await pool.query(sql` delete from ${table} - where ${fields.sessionId}=${sessionId} and ${fields.userId}=${userId} + where ${fields.userId}=${userId} `); }; return { - findVerificationStatusByUserIdAndSessionId, + findVerificationStatusByUserId, insertVerificationStatus, - deleteVerificationStatusesByUserIdAndSessionId, + deleteVerificationStatusesByUserId, }; }; diff --git a/packages/core/src/routes-me/user.ts b/packages/core/src/routes-me/user.ts index 86707c5a7..a3108f663 100644 --- a/packages/core/src/routes-me/user.ts +++ b/packages/core/src/routes-me/user.ts @@ -8,7 +8,6 @@ import RequestError from '#src/errors/RequestError/index.js'; import { encryptUserPassword, verifyUserPassword } from '#src/libraries/user.js'; import koaGuard from '#src/middleware/koa-guard.js'; import assertThat from '#src/utils/assert-that.js'; -import { convertCookieToMap } from '#src/utils/cookie.js'; import type { RouterInitArgs } from '../routes/types.js'; import type { AuthedMeRouter } from './types.js'; @@ -108,17 +107,13 @@ export default function userRoutes( async (ctx, next) => { const { id: userId } = ctx.auth; const { password } = ctx.guard.body; - const cookieMap = convertCookieToMap(ctx.request.headers.cookie); - const sessionId = cookieMap.get('_session'); - - 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); - await createVerificationStatus(userId, sessionId); + await createVerificationStatus(userId); ctx.status = 204; @@ -137,13 +132,8 @@ export default function userRoutes( assertThat(!isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); - const cookieMap = convertCookieToMap(ctx.request.headers.cookie); - const sessionId = cookieMap.get('_session'); - - assertThat(sessionId, new RequestError({ code: 'session.not_found', status: 401 })); - if (oldPasswordEncrypted) { - await checkVerificationStatus(userId, sessionId); + await checkVerificationStatus(userId); } const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password); diff --git a/packages/core/src/routes-me/verification-code.ts b/packages/core/src/routes-me/verification-code.ts index fbaa69797..8c2b9833e 100644 --- a/packages/core/src/routes-me/verification-code.ts +++ b/packages/core/src/routes-me/verification-code.ts @@ -6,7 +6,6 @@ import RequestError from '#src/errors/RequestError/index.js'; import koaGuard from '#src/middleware/koa-guard.js'; import type { RouterInitArgs } from '#src/routes/types.js'; import assertThat from '#src/utils/assert-that.js'; -import { convertCookieToMap } from '#src/utils/cookie.js'; import type { AuthedMeRouter } from './types.js'; @@ -55,15 +54,10 @@ export default function verificationCodeRoutes( if (action === 'changePassword') { // Store password verification status - const cookieMap = convertCookieToMap(ctx.request.headers.cookie); - const sessionId = cookieMap.get('_session'); - - 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 createVerificationStatus(userId, sessionId); + await createVerificationStatus(userId); } ctx.status = 204; diff --git a/packages/core/src/utils/cookie.ts b/packages/core/src/utils/cookie.ts deleted file mode 100644 index 5ac32bfc7..000000000 --- a/packages/core/src/utils/cookie.ts +++ /dev/null @@ -1,15 +0,0 @@ -import type { Optional } from '@silverhand/essentials'; - -export const convertCookieToMap = (cookie?: string): Map> => { - const map = new Map>(); - - for (const element of cookie?.split(';') ?? []) { - const [key, value] = element.trim().split('='); - - if (key) { - map.set(key, value); - } - } - - return map; -}; diff --git a/packages/schemas/alterations/next-1678199795-add-verification-status-table.ts b/packages/schemas/alterations/next-1678199795-add-verification-status-table.ts index bc6b46f01..38e4e0d94 100644 --- a/packages/schemas/alterations/next-1678199795-add-verification-status-table.ts +++ b/packages/schemas/alterations/next-1678199795-add-verification-status-table.ts @@ -25,7 +25,6 @@ const alteration: AlterationScript = { 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) ); @@ -33,8 +32,8 @@ const alteration: AlterationScript = { 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 index verification_statuses__user_id + on verification_statuses (tenant_id, user_id); create trigger set_tenant_id before insert on verification_statuses for each row execute procedure set_tenant_id(); diff --git a/packages/schemas/tables/verification_statuses.sql b/packages/schemas/tables/verification_statuses.sql index 1e76a24a6..1c9f60300 100644 --- a/packages/schemas/tables/verification_statuses.sql +++ b/packages/schemas/tables/verification_statuses.sql @@ -4,7 +4,6 @@ create table verification_statuses ( 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) ); @@ -12,5 +11,5 @@ create table verification_statuses ( 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 index verification_statuses__user_id + on verification_statuses (tenant_id, user_id);