From 0c9e8cba0deca6d5b4e9c89bb753f41bb1cd3d28 Mon Sep 17 00:00:00 2001 From: Charles Zhao Date: Mon, 5 Dec 2022 14:14:21 +0800 Subject: [PATCH] refactor(core): promote profile apis from session routes to its own api routes (#2583) --- packages/core/src/lib/user.ts | 26 ++++++++++- packages/core/src/routes/admin-user.test.ts | 1 + packages/core/src/routes/admin-user.ts | 10 +++-- packages/core/src/routes/consts.ts | 3 ++ packages/core/src/routes/init.ts | 45 ++++++++++--------- .../src/routes/{session => }/profile.test.ts | 0 .../core/src/routes/{session => }/profile.ts | 14 +++--- packages/core/src/routes/session/consts.ts | 2 - packages/core/src/routes/session/index.ts | 2 - .../src/routes/session/passwordless.test.ts | 2 +- packages/core/src/routes/session/utils.ts | 27 +---------- 11 files changed, 70 insertions(+), 62 deletions(-) rename packages/core/src/routes/{session => }/profile.test.ts (100%) rename packages/core/src/routes/{session => }/profile.ts (94%) delete mode 100644 packages/core/src/routes/session/consts.ts diff --git a/packages/core/src/lib/user.ts b/packages/core/src/lib/user.ts index 5aac5639b..a1110e1bd 100644 --- a/packages/core/src/lib/user.ts +++ b/packages/core/src/lib/user.ts @@ -7,8 +7,9 @@ import pRetry from 'p-retry'; import { buildInsertInto } from '#src/database/insert-into.js'; import envSet from '#src/env-set/index.js'; +import RequestError from '#src/errors/RequestError/index.js'; import { findRolesByRoleNames, insertRoles } from '#src/queries/roles.js'; -import { hasUserWithId } from '#src/queries/user.js'; +import { hasUser, hasUserWithEmail, hasUserWithId, hasUserWithPhone } from '#src/queries/user.js'; import assertThat from '#src/utils/assert-that.js'; import { encryptPassword } from '#src/utils/password.js'; @@ -88,3 +89,26 @@ export const insertUser: typeof insertUserQuery = async ({ roleNames, ...rest }) return insertUserQuery({ roleNames: computedRoleNames, ...rest }); }; + +export const checkIdentifierCollision = async ( + identifiers: { + username?: Nullable; + primaryEmail?: Nullable; + primaryPhone?: Nullable; + }, + excludeUserId?: string +) => { + const { username, primaryEmail, primaryPhone } = identifiers; + + if (username && (await hasUser(username, excludeUserId))) { + throw new RequestError({ code: 'user.username_exists', status: 422 }); + } + + if (primaryEmail && (await hasUserWithEmail(primaryEmail, excludeUserId))) { + throw new RequestError({ code: 'user.email_exists', status: 422 }); + } + + if (primaryPhone && (await hasUserWithPhone(primaryPhone, excludeUserId))) { + throw new RequestError({ code: 'user.sms_exists', status: 422 }); + } +}; diff --git a/packages/core/src/routes/admin-user.test.ts b/packages/core/src/routes/admin-user.test.ts index 62499cf67..601ecaef8 100644 --- a/packages/core/src/routes/admin-user.test.ts +++ b/packages/core/src/routes/admin-user.test.ts @@ -66,6 +66,7 @@ jest.mock('#src/queries/user.js', () => ({ })); jest.mock('#src/lib/user.js', () => ({ + ...jest.requireActual('#src/lib/user.js'), generateUserId: jest.fn(() => 'fooId'), encryptUserPassword: jest.fn(() => ({ passwordEncrypted: 'password', diff --git a/packages/core/src/routes/admin-user.ts b/packages/core/src/routes/admin-user.ts index adc3ffd15..7e1499631 100644 --- a/packages/core/src/routes/admin-user.ts +++ b/packages/core/src/routes/admin-user.ts @@ -6,7 +6,12 @@ import { boolean, literal, object, string } from 'zod'; import { isTrue } from '#src/env-set/parameters.js'; import RequestError from '#src/errors/RequestError/index.js'; -import { encryptUserPassword, generateUserId, insertUser } from '#src/lib/user.js'; +import { + checkIdentifierCollision, + encryptUserPassword, + generateUserId, + insertUser, +} from '#src/lib/user.js'; import koaGuard from '#src/middleware/koa-guard.js'; import koaPagination from '#src/middleware/koa-pagination.js'; import { revokeInstanceByUserId } from '#src/queries/oidc-model-instance.js'; @@ -23,7 +28,6 @@ import { } from '#src/queries/user.js'; import assertThat from '#src/utils/assert-that.js'; -import { checkSignUpIdentifierCollision } from './session/utils.js'; import type { AuthedRouter } from './types.js'; export default function adminUserRoutes(router: T) { @@ -187,7 +191,7 @@ export default function adminUserRoutes(router: T) { } = ctx.guard; await findUserById(userId); - await checkSignUpIdentifierCollision(body, userId); + await checkIdentifierCollision(body, userId); // Temp solution to validate the existence of input roleNames if (body.roleNames?.length) { diff --git a/packages/core/src/routes/consts.ts b/packages/core/src/routes/consts.ts index 5daf0f88d..d1099ad24 100644 --- a/packages/core/src/routes/consts.ts +++ b/packages/core/src/routes/consts.ts @@ -6,3 +6,6 @@ export const routes = Object.freeze({ consent: signIn + '/consent', }, }); + +export const verificationTimeout = 10 * 60; // 10 mins. +export const continueSignInTimeout = 10 * 60; // 10 mins. diff --git a/packages/core/src/routes/init.ts b/packages/core/src/routes/init.ts index 2edf869a0..b0d093845 100644 --- a/packages/core/src/routes/init.ts +++ b/packages/core/src/routes/init.ts @@ -4,26 +4,26 @@ import mount from 'koa-mount'; import Router from 'koa-router'; import type { Provider } from 'oidc-provider'; -import koaAuth from '#src/middleware/koa-auth.js'; -import koaLogSession from '#src/middleware/koa-log-session.js'; -import adminUserRoutes from '#src/routes/admin-user.js'; -import applicationRoutes from '#src/routes/application.js'; -import authnRoutes from '#src/routes/authn.js'; -import connectorRoutes from '#src/routes/connector.js'; -import customPhraseRoutes from '#src/routes/custom-phrase.js'; -import dashboardRoutes from '#src/routes/dashboard.js'; -import logRoutes from '#src/routes/log.js'; -import phraseRoutes from '#src/routes/phrase.js'; -import resourceRoutes from '#src/routes/resource.js'; -import roleRoutes from '#src/routes/role.js'; -import sessionRoutes from '#src/routes/session/index.js'; -import settingRoutes from '#src/routes/setting.js'; -import signInExperiencesRoutes from '#src/routes/sign-in-experience.js'; -import statusRoutes from '#src/routes/status.js'; -import swaggerRoutes from '#src/routes/swagger.js'; -import wellKnownRoutes from '#src/routes/well-known.js'; - +import koaAuth from '../middleware/koa-auth.js'; +import koaLogSession from '../middleware/koa-log-session.js'; +import adminUserRoutes from './admin-user.js'; +import applicationRoutes from './application.js'; +import authnRoutes from './authn.js'; +import connectorRoutes from './connector.js'; +import customPhraseRoutes from './custom-phrase.js'; +import dashboardRoutes from './dashboard.js'; +import logRoutes from './log.js'; +import phraseRoutes from './phrase.js'; +import profileRoutes from './profile.js'; +import resourceRoutes from './resource.js'; +import roleRoutes from './role.js'; +import sessionRoutes from './session/index.js'; +import settingRoutes from './setting.js'; +import signInExperiencesRoutes from './sign-in-experience.js'; +import statusRoutes from './status.js'; +import swaggerRoutes from './swagger.js'; import type { AnonymousRouter, AuthedRouter } from './types.js'; +import wellKnownRoutes from './well-known.js'; const createRouters = (provider: Provider) => { const sessionRouter: AnonymousRouter = new Router(); @@ -43,15 +43,18 @@ const createRouters = (provider: Provider) => { dashboardRoutes(managementRouter); customPhraseRoutes(managementRouter); + const profileRouter: AnonymousRouter = new Router(); + profileRoutes(profileRouter, provider); + const anonymousRouter: AnonymousRouter = new Router(); phraseRoutes(anonymousRouter, provider); wellKnownRoutes(anonymousRouter, provider); statusRoutes(anonymousRouter); authnRoutes(anonymousRouter); // The swagger.json should contain all API routers. - swaggerRoutes(anonymousRouter, [sessionRouter, managementRouter, anonymousRouter]); + swaggerRoutes(anonymousRouter, [sessionRouter, profileRouter, managementRouter, anonymousRouter]); - return [sessionRouter, managementRouter, anonymousRouter]; + return [sessionRouter, profileRouter, managementRouter, anonymousRouter]; }; export default function initRouter(app: Koa, provider: Provider) { diff --git a/packages/core/src/routes/session/profile.test.ts b/packages/core/src/routes/profile.test.ts similarity index 100% rename from packages/core/src/routes/session/profile.test.ts rename to packages/core/src/routes/profile.test.ts diff --git a/packages/core/src/routes/session/profile.ts b/packages/core/src/routes/profile.ts similarity index 94% rename from packages/core/src/routes/session/profile.ts rename to packages/core/src/routes/profile.ts index 70332ba6f..f8fdc458b 100644 --- a/packages/core/src/routes/session/profile.ts +++ b/packages/core/src/routes/profile.ts @@ -10,16 +10,15 @@ import { getLogtoConnectorById } from '#src/connectors/index.js'; import RequestError from '#src/errors/RequestError/index.js'; import { checkSessionHealth } from '#src/lib/session.js'; import { getUserInfoByAuthCode } from '#src/lib/social.js'; -import { encryptUserPassword } from '#src/lib/user.js'; +import { checkIdentifierCollision, encryptUserPassword } from '#src/lib/user.js'; import koaGuard from '#src/middleware/koa-guard.js'; import { deleteUserIdentity, findUserById, updateUserById } from '#src/queries/user.js'; import assertThat from '#src/utils/assert-that.js'; -import type { AnonymousRouter } from '../types.js'; import { verificationTimeout } from './consts.js'; -import { checkSignUpIdentifierCollision } from './utils.js'; +import type { AnonymousRouter } from './types.js'; -export const profileRoute = '/session/profile'; +export const profileRoute = '/profile'; export default function profileRoutes(router: T, provider: Provider) { router.get(profileRoute, async (ctx, next) => { @@ -30,6 +29,7 @@ export default function profileRoutes(router: T, prov const user = await findUserById(userId); ctx.body = pick(user, ...userInfoSelectFields); + ctx.status = 200; return next(); }); @@ -70,7 +70,7 @@ export default function profileRoutes(router: T, prov const { username } = ctx.guard.body; - await checkSignUpIdentifierCollision({ username }, userId); + await checkIdentifierCollision({ username }, userId); const user = await updateUserById(userId, { username }, 'replace'); @@ -120,7 +120,7 @@ export default function profileRoutes(router: T, prov const { primaryEmail } = ctx.guard.body; - await checkSignUpIdentifierCollision({ primaryEmail }); + await checkIdentifierCollision({ primaryEmail }); await updateUserById(userId, { primaryEmail }); ctx.status = 204; @@ -157,7 +157,7 @@ export default function profileRoutes(router: T, prov const { primaryPhone } = ctx.guard.body; - await checkSignUpIdentifierCollision({ primaryPhone }); + await checkIdentifierCollision({ primaryPhone }); await updateUserById(userId, { primaryPhone }); ctx.status = 204; diff --git a/packages/core/src/routes/session/consts.ts b/packages/core/src/routes/session/consts.ts deleted file mode 100644 index 65b7777a8..000000000 --- a/packages/core/src/routes/session/consts.ts +++ /dev/null @@ -1,2 +0,0 @@ -export const verificationTimeout = 10 * 60; // 10 mins. -export const continueSignInTimeout = 10 * 60; // 10 mins. diff --git a/packages/core/src/routes/session/index.ts b/packages/core/src/routes/session/index.ts index a79fa6d88..63607d43a 100644 --- a/packages/core/src/routes/session/index.ts +++ b/packages/core/src/routes/session/index.ts @@ -18,7 +18,6 @@ import forgotPasswordRoutes from './forgot-password.js'; import koaGuardSessionAction from './middleware/koa-guard-session-action.js'; import passwordRoutes from './password.js'; import passwordlessRoutes from './passwordless.js'; -import profileRoutes from './profile.js'; import socialRoutes from './social.js'; import { getRoutePrefix } from './utils.js'; @@ -106,5 +105,4 @@ export default function sessionRoutes(router: T, prov socialRoutes(router, provider); continueRoutes(router, provider); forgotPasswordRoutes(router, provider); - profileRoutes(router, provider); } diff --git a/packages/core/src/routes/session/passwordless.test.ts b/packages/core/src/routes/session/passwordless.test.ts index 0a759d142..e4ca9411e 100644 --- a/packages/core/src/routes/session/passwordless.test.ts +++ b/packages/core/src/routes/session/passwordless.test.ts @@ -9,7 +9,7 @@ import { mockSignInExperience, mockSignInMethod, mockUser } from '#src/__mocks__ import RequestError from '#src/errors/RequestError/index.js'; import { createRequester } from '#src/utils/test-utils.js'; -import { verificationTimeout } from './consts.js'; +import { verificationTimeout } from '../consts.js'; import * as passwordlessActions from './middleware/passwordless-action.js'; import passwordlessRoutes, { registerRoute, signInRoute } from './passwordless.js'; diff --git a/packages/core/src/routes/session/utils.ts b/packages/core/src/routes/session/utils.ts index 0b82180ba..f18c742b0 100644 --- a/packages/core/src/routes/session/utils.ts +++ b/packages/core/src/routes/session/utils.ts @@ -13,10 +13,10 @@ import { assignInteractionResults, getApplicationIdFromInteraction } from '#src/ import { getSignInExperienceForApplication } from '#src/lib/sign-in-experience/index.js'; import { verifyUserPassword } from '#src/lib/user.js'; import type { LogContext } from '#src/middleware/koa-log.js'; -import { hasUser, hasUserWithEmail, hasUserWithPhone, updateUserById } from '#src/queries/user.js'; +import { updateUserById } from '#src/queries/user.js'; import assertThat from '#src/utils/assert-that.js'; -import { continueSignInTimeout, verificationTimeout } from './consts.js'; +import { continueSignInTimeout, verificationTimeout } from '../consts.js'; import type { Method, Operation, VerificationResult, VerificationStorage } from './types.js'; import { continueSignInStorageGuard } from './types.js'; @@ -196,29 +196,6 @@ export const checkRequiredProfile = async ( }; /* eslint-enable complexity */ -export const checkSignUpIdentifierCollision = async ( - identifiers: { - username?: Nullable; - primaryEmail?: Nullable; - primaryPhone?: Nullable; - }, - excludeUserId?: string -) => { - const { username, primaryEmail, primaryPhone } = identifiers; - - if (username && (await hasUser(username, excludeUserId))) { - throw new RequestError({ code: 'user.username_exists', status: 422 }); - } - - if (primaryEmail && (await hasUserWithEmail(primaryEmail, excludeUserId))) { - throw new RequestError({ code: 'user.email_exists', status: 422 }); - } - - if (primaryPhone && (await hasUserWithPhone(primaryPhone, excludeUserId))) { - throw new RequestError({ code: 'user.sms_exists', status: 422 }); - } -}; - type SignInWithPasswordParameter = { identifier: SignInIdentifier; password: string;