0
Fork 0
mirror of https://github.com/logto-io/logto.git synced 2025-01-20 21:32:31 -05:00

fix(core,schemas): remove sessionId usage from verification status table (#3345)

This commit is contained in:
Charles Zhao 2023-03-13 11:20:52 +08:00 committed by GitHub
parent f9c00f6dc5
commit 9896390065
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 22 additions and 70 deletions

View file

@ -9,40 +9,28 @@ export type VerificationStatusLibrary = ReturnType<typeof createVerificationStat
export const createVerificationStatusLibrary = (queries: Queries) => { export const createVerificationStatusLibrary = (queries: Queries) => {
const { const {
findVerificationStatusByUserIdAndSessionId, findVerificationStatusByUserId,
insertVerificationStatus, insertVerificationStatus,
deleteVerificationStatusesByUserIdAndSessionId, deleteVerificationStatusesByUserId,
} = queries.verificationStatuses; } = queries.verificationStatuses;
const createVerificationStatus = async (userId: string, sessionId: string) => { const createVerificationStatus = async (userId: string) => {
// Remove existing verification statuses for current user in current session. // Remove existing verification statuses for current user.
await deleteVerificationStatusesByUserIdAndSessionId(userId, sessionId); 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({ return insertVerificationStatus({
id: generateStandardId(), id: generateStandardId(),
sessionId,
userId, userId,
}); });
}; };
const checkVerificationStatus = async (userId: string, sessionId: string): Promise<void> => { const checkVerificationStatus = async (userId: string): Promise<void> => {
const verificationStatus = await findVerificationStatusByUserIdAndSessionId(userId, sessionId); const verificationStatus = await findVerificationStatusByUserId(userId);
assertThat(verificationStatus, 'session.verification_session_not_found'); assertThat(verificationStatus, 'session.verification_session_not_found');
const { sessionId: storedSessionId, createdAt } = verificationStatus; // The user verification status is considered valid if the user is verified within 10 minutes.
const isValid = Date.now() - verificationStatus.createdAt < verificationTimeout;
// 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 })); assertThat(isValid, new RequestError({ code: 'session.verification_failed', status: 422 }));
}; };

View file

@ -9,11 +9,11 @@ import { buildInsertIntoWithPool } from '#src/database/insert-into.js';
const { table, fields } = convertToIdentifiers(VerificationStatuses); const { table, fields } = convertToIdentifiers(VerificationStatuses);
export const createVerificationStatusQueries = (pool: CommonQueryMethods) => { export const createVerificationStatusQueries = (pool: CommonQueryMethods) => {
const findVerificationStatusByUserIdAndSessionId = async (userId: string, sessionId: string) => const findVerificationStatusByUserId = async (userId: string) =>
pool.maybeOne<VerificationStatus>(sql` pool.maybeOne<VerificationStatus>(sql`
select ${sql.join(Object.values(fields), sql`, `)} select ${sql.join(Object.values(fields), sql`, `)}
from ${table} from ${table}
where ${fields.sessionId}=${sessionId} and ${fields.userId}=${userId} where ${fields.userId}=${userId}
`); `);
const insertVerificationStatus = buildInsertIntoWithPool(pool)< const insertVerificationStatus = buildInsertIntoWithPool(pool)<
@ -23,19 +23,16 @@ export const createVerificationStatusQueries = (pool: CommonQueryMethods) => {
returning: true, returning: true,
}); });
const deleteVerificationStatusesByUserIdAndSessionId = async ( const deleteVerificationStatusesByUserId = async (userId: string) => {
userId: string,
sessionId: string
) => {
await pool.query(sql` await pool.query(sql`
delete from ${table} delete from ${table}
where ${fields.sessionId}=${sessionId} and ${fields.userId}=${userId} where ${fields.userId}=${userId}
`); `);
}; };
return { return {
findVerificationStatusByUserIdAndSessionId, findVerificationStatusByUserId,
insertVerificationStatus, insertVerificationStatus,
deleteVerificationStatusesByUserIdAndSessionId, deleteVerificationStatusesByUserId,
}; };
}; };

View file

@ -8,7 +8,6 @@ import RequestError from '#src/errors/RequestError/index.js';
import { encryptUserPassword, verifyUserPassword } from '#src/libraries/user.js'; import { encryptUserPassword, verifyUserPassword } from '#src/libraries/user.js';
import koaGuard from '#src/middleware/koa-guard.js'; import koaGuard from '#src/middleware/koa-guard.js';
import assertThat from '#src/utils/assert-that.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 { RouterInitArgs } from '../routes/types.js';
import type { AuthedMeRouter } from './types.js'; import type { AuthedMeRouter } from './types.js';
@ -108,17 +107,13 @@ export default function userRoutes<T extends AuthedMeRouter>(
async (ctx, next) => { async (ctx, next) => {
const { id: userId } = ctx.auth; const { id: userId } = ctx.auth;
const { password } = ctx.guard.body; 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); const user = await findUserById(userId);
assertThat(!user.isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); assertThat(!user.isSuspended, new RequestError({ code: 'user.suspended', status: 401 }));
await verifyUserPassword(user, password); await verifyUserPassword(user, password);
await createVerificationStatus(userId, sessionId); await createVerificationStatus(userId);
ctx.status = 204; ctx.status = 204;
@ -137,13 +132,8 @@ export default function userRoutes<T extends AuthedMeRouter>(
assertThat(!isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); 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) { if (oldPasswordEncrypted) {
await checkVerificationStatus(userId, sessionId); await checkVerificationStatus(userId);
} }
const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password); const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password);

View file

@ -6,7 +6,6 @@ import RequestError from '#src/errors/RequestError/index.js';
import koaGuard from '#src/middleware/koa-guard.js'; import koaGuard from '#src/middleware/koa-guard.js';
import type { RouterInitArgs } from '#src/routes/types.js'; import type { RouterInitArgs } from '#src/routes/types.js';
import assertThat from '#src/utils/assert-that.js'; import assertThat from '#src/utils/assert-that.js';
import { convertCookieToMap } from '#src/utils/cookie.js';
import type { AuthedMeRouter } from './types.js'; import type { AuthedMeRouter } from './types.js';
@ -55,15 +54,10 @@ export default function verificationCodeRoutes<T extends AuthedMeRouter>(
if (action === 'changePassword') { if (action === 'changePassword') {
// Store password verification status // 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); const user = await findUserById(userId);
assertThat(!user.isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); assertThat(!user.isSuspended, new RequestError({ code: 'user.suspended', status: 401 }));
await createVerificationStatus(userId, sessionId); await createVerificationStatus(userId);
} }
ctx.status = 204; ctx.status = 204;

View file

@ -1,15 +0,0 @@
import type { Optional } from '@silverhand/essentials';
export const convertCookieToMap = (cookie?: string): Map<string, Optional<string>> => {
const map = new Map<string, Optional<string>>();
for (const element of cookie?.split(';') ?? []) {
const [key, value] = element.trim().split('=');
if (key) {
map.set(key, value);
}
}
return map;
};

View file

@ -25,7 +25,6 @@ const alteration: AlterationScript = {
id varchar(21) not null, id varchar(21) not null,
user_id varchar(21) not null user_id varchar(21) not null
references users (id) on update cascade on delete cascade, references users (id) on update cascade on delete cascade,
session_id varchar(128) not null,
created_at timestamptz not null default(now()), created_at timestamptz not null default(now()),
primary key (id) primary key (id)
); );
@ -33,8 +32,8 @@ const alteration: AlterationScript = {
create index verification_statuses__id create index verification_statuses__id
on verification_statuses (tenant_id, id); on verification_statuses (tenant_id, id);
create index verification_statuses__user_id__session_id create index verification_statuses__user_id
on verification_statuses (tenant_id, user_id, session_id); on verification_statuses (tenant_id, user_id);
create trigger set_tenant_id before insert on verification_statuses create trigger set_tenant_id before insert on verification_statuses
for each row execute procedure set_tenant_id(); for each row execute procedure set_tenant_id();

View file

@ -4,7 +4,6 @@ create table verification_statuses (
id varchar(21) not null, id varchar(21) not null,
user_id varchar(21) not null user_id varchar(21) not null
references users (id) on update cascade on delete cascade, references users (id) on update cascade on delete cascade,
session_id varchar(128) not null,
created_at timestamptz not null default(now()), created_at timestamptz not null default(now()),
primary key (id) primary key (id)
); );
@ -12,5 +11,5 @@ create table verification_statuses (
create index verification_statuses__id create index verification_statuses__id
on verification_statuses (tenant_id, id); on verification_statuses (tenant_id, id);
create index verification_statuses__user_id__session_id create index verification_statuses__user_id
on verification_statuses (tenant_id, user_id, session_id); on verification_statuses (tenant_id, user_id);