From f75d5e605eb4b68370d70ce5339c9a42f586c2f5 Mon Sep 17 00:00:00 2001 From: wangsijie Date: Thu, 10 Nov 2022 15:49:26 +0800 Subject: [PATCH] feat(core): block suspended user from sign in (#2366) --- .../session/middleware/passwordless-action.ts | 6 +- .../src/routes/session/passwordless.test.ts | 39 +++- .../routes/session/social.bind-social.test.ts | 193 ++++++++++++++++++ .../core/src/routes/session/social.test.ts | 103 +++++----- packages/core/src/routes/session/social.ts | 6 +- .../core/src/routes/session/utils.test.ts | 16 ++ packages/core/src/routes/session/utils.ts | 3 +- packages/phrases/src/locales/de/errors.ts | 1 + packages/phrases/src/locales/en/errors.ts | 1 + packages/phrases/src/locales/fr/errors.ts | 1 + packages/phrases/src/locales/ko/errors.ts | 1 + packages/phrases/src/locales/pt-pt/errors.ts | 1 + packages/phrases/src/locales/tr-tr/errors.ts | 1 + packages/phrases/src/locales/zh-cn/errors.ts | 1 + 14 files changed, 312 insertions(+), 61 deletions(-) create mode 100644 packages/core/src/routes/session/social.bind-social.test.ts diff --git a/packages/core/src/routes/session/middleware/passwordless-action.ts b/packages/core/src/routes/session/middleware/passwordless-action.ts index 82c517bef..d485ac937 100644 --- a/packages/core/src/routes/session/middleware/passwordless-action.ts +++ b/packages/core/src/routes/session/middleware/passwordless-action.ts @@ -59,7 +59,8 @@ export const smsSignInAction = mockUser); const findUserById = jest.fn(async (): Promise => mockUser); const findUserByEmail = jest.fn(async (): Promise => mockUser); +const findUserByPhone = jest.fn(async (): Promise => mockUser); const updateUserById = jest.fn(async (..._args: unknown[]) => mockUser); const findDefaultSignInExperience = jest.fn(async () => ({ ...mockSignInExperience, @@ -34,7 +35,7 @@ jest.mock('@/lib/user', () => ({ jest.mock('@/queries/user', () => ({ findUserById: async () => findUserById(), - findUserByPhone: async () => mockUser, + findUserByPhone: async () => findUserByPhone(), findUserByEmail: async () => findUserByEmail(), updateUserById: async (...args: unknown[]) => updateUserById(...args), hasUser: async (username: string) => username === 'username1', @@ -503,6 +504,24 @@ describe('session -> passwordlessRoutes', () => { expect(response.statusCode).toEqual(404); }); + it('throw when user is suspended', async () => { + findUserByPhone.mockResolvedValueOnce({ + ...mockUser, + isSuspended: true, + }); + interactionDetails.mockResolvedValueOnce({ + result: { + verification: { + phone: '13000000000', + flow: PasscodeType.SignIn, + expiresAt: getTomorrowIsoString(), + }, + }, + }); + const response = await sessionRequest.post(`${signInRoute}/sms`); + expect(response.statusCode).toEqual(401); + }); + it('throw error if sign in method is not enabled', async () => { findDefaultSignInExperience.mockResolvedValueOnce({ ...mockSignInExperience, @@ -639,6 +658,24 @@ describe('session -> passwordlessRoutes', () => { expect(response.statusCode).toEqual(404); }); + it('throw when user is suspended', async () => { + findUserByEmail.mockResolvedValueOnce({ + ...mockUser, + isSuspended: true, + }); + interactionDetails.mockResolvedValueOnce({ + result: { + verification: { + email: 'a@a.com', + flow: PasscodeType.SignIn, + expiresAt: getTomorrowIsoString(), + }, + }, + }); + const response = await sessionRequest.post(`${signInRoute}/email`); + expect(response.statusCode).toEqual(401); + }); + it('throw error if sign in method is not enabled', async () => { findDefaultSignInExperience.mockResolvedValueOnce({ ...mockSignInExperience, diff --git a/packages/core/src/routes/session/social.bind-social.test.ts b/packages/core/src/routes/session/social.bind-social.test.ts new file mode 100644 index 000000000..646218d20 --- /dev/null +++ b/packages/core/src/routes/session/social.bind-social.test.ts @@ -0,0 +1,193 @@ +import { ConnectorType } from '@logto/connector-kit'; +import type { User } from '@logto/schemas'; +import { SignUpIdentifier } from '@logto/schemas'; +import { Provider } from 'oidc-provider'; + +import { mockLogtoConnectorList, mockSignInExperience, mockUser } from '@/__mocks__'; +import { getLogtoConnectorById } from '@/connectors'; +import RequestError from '@/errors/RequestError'; +import { createRequester } from '@/utils/test-utils'; + +import socialRoutes, { registerRoute } from './social'; + +const findSocialRelatedUser = jest.fn(async () => [ + 'phone', + { id: 'user1', identities: {}, isSuspended: false }, +]); +jest.mock('@/lib/social', () => ({ + ...jest.requireActual('@/lib/social'), + findSocialRelatedUser: async () => findSocialRelatedUser(), + async getUserInfoByAuthCode(connectorId: string, data: { code: string }) { + if (connectorId === '_connectorId') { + throw new RequestError({ + code: 'session.invalid_connector_id', + status: 422, + connectorId, + }); + } + + if (data.code === '123456') { + return { id: mockUser.id }; + } + + // This mocks the case that can not get userInfo with access token and auth code + // (most likely third-party social connectors' problem). + throw new Error(' '); + }, +})); +const insertUser = jest.fn(async (..._args: unknown[]) => mockUser); +const findUserById = jest.fn(async (): Promise => mockUser); +const updateUserById = jest.fn(async (..._args: unknown[]) => mockUser); +const findUserByIdentity = jest.fn(async () => mockUser); + +jest.mock('@/queries/user', () => ({ + findUserById: async () => findUserById(), + findUserByIdentity: async () => findUserByIdentity(), + updateUserById: async (...args: unknown[]) => updateUserById(...args), + hasUserWithIdentity: async (target: string, userId: string) => + target === 'connectorTarget' && userId === mockUser.id, +})); + +jest.mock('@/lib/user', () => ({ + generateUserId: () => 'user1', + insertUser: async (...args: unknown[]) => insertUser(...args), +})); + +jest.mock('@/queries/sign-in-experience', () => ({ + findDefaultSignInExperience: async () => ({ + ...mockSignInExperience, + signUp: { + ...mockSignInExperience.signUp, + identifier: SignUpIdentifier.None, + }, + }), +})); + +const getLogtoConnectorByIdHelper = jest.fn(async (connectorId: string) => { + const database = { + enabled: connectorId === 'social_enabled', + }; + const metadata = { + id: + connectorId === 'social_enabled' + ? 'social_enabled' + : connectorId === 'social_disabled' + ? 'social_disabled' + : 'others', + }; + + return { + dbEntry: database, + metadata, + type: connectorId.startsWith('social') ? ConnectorType.Social : ConnectorType.Sms, + getAuthorizationUri: jest.fn(async () => ''), + }; +}); + +jest.mock('@/connectors', () => ({ + getLogtoConnectors: jest.fn(async () => mockLogtoConnectorList), + getLogtoConnectorById: jest.fn(async (connectorId: string) => { + const connector = await getLogtoConnectorByIdHelper(connectorId); + + if (connector.type !== ConnectorType.Social) { + throw new RequestError({ + code: 'entity.not_found', + status: 404, + }); + } + + return connector; + }), +})); + +const interactionResult = jest.fn(async () => 'redirectTo'); +const interactionDetails: jest.MockedFunction<() => Promise> = jest.fn(async () => ({})); + +jest.mock('oidc-provider', () => ({ + Provider: jest.fn(() => ({ + interactionDetails, + interactionResult, + })), +})); + +afterEach(() => { + interactionResult.mockClear(); +}); + +describe('session -> socialRoutes', () => { + const sessionRequest = createRequester({ + anonymousRoutes: socialRoutes, + provider: new Provider(''), + middlewares: [ + async (ctx, next) => { + ctx.addLogContext = jest.fn(); + ctx.log = jest.fn(); + + return next(); + }, + ], + }); + + describe('POST /session/register/social', () => { + beforeEach(() => { + const mockGetLogtoConnectorById = getLogtoConnectorById as jest.Mock; + mockGetLogtoConnectorById.mockResolvedValueOnce({ + metadata: { target: 'connectorTarget' }, + }); + }); + + it('register with social, assign result and redirect', async () => { + interactionDetails.mockResolvedValueOnce({ + jti: 'jti', + result: { + socialUserInfo: { connectorId: 'connectorId', userInfo: { id: 'user1' } }, + }, + }); + const response = await sessionRequest + .post(`${registerRoute}`) + .send({ connectorId: 'connectorId' }); + expect(insertUser).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'user1', + identities: { connectorTarget: { userId: 'user1', details: { id: 'user1' } } }, + }) + ); + expect(response.body).toHaveProperty('redirectTo'); + expect(interactionResult).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ login: { accountId: 'user1' } }), + expect.anything() + ); + }); + + it('throw error if no result can be found in interactionResults', async () => { + interactionDetails.mockResolvedValueOnce({}); + const response = await sessionRequest + .post(`${registerRoute}`) + .send({ connectorId: 'connectorId' }); + expect(response.statusCode).toEqual(400); + }); + + it('throw error if result parsing fails', async () => { + interactionDetails.mockResolvedValueOnce({ result: { login: { accountId: mockUser.id } } }); + const response = await sessionRequest + .post(`${registerRoute}`) + .send({ connectorId: 'connectorId' }); + expect(response.statusCode).toEqual(400); + }); + + it('throw error when user with identity exists', async () => { + interactionDetails.mockResolvedValueOnce({ + result: { + login: { accountId: 'user1' }, + socialUserInfo: { connectorId: 'connectorId', userInfo: { id: mockUser.id } }, + }, + }); + const response = await sessionRequest + .post(`${registerRoute}`) + .send({ connectorId: 'connectorId' }); + expect(response.statusCode).toEqual(400); + }); + }); +}); diff --git a/packages/core/src/routes/session/social.test.ts b/packages/core/src/routes/session/social.test.ts index e00a6a8cd..1b4aec636 100644 --- a/packages/core/src/routes/session/social.test.ts +++ b/packages/core/src/routes/session/social.test.ts @@ -10,11 +10,13 @@ import { createRequester } from '@/utils/test-utils'; import socialRoutes, { registerRoute, signInRoute } from './social'; +const findSocialRelatedUser = jest.fn(async () => [ + 'phone', + { id: 'user1', identities: {}, isSuspended: false }, +]); jest.mock('@/lib/social', () => ({ ...jest.requireActual('@/lib/social'), - async findSocialRelatedUser() { - return ['phone', { id: 'user1', identities: {} }]; - }, + findSocialRelatedUser: async () => findSocialRelatedUser(), async getUserInfoByAuthCode(connectorId: string, data: { code: string }) { if (connectorId === '_connectorId') { throw new RequestError({ @@ -36,10 +38,11 @@ jest.mock('@/lib/social', () => ({ const insertUser = jest.fn(async (..._args: unknown[]) => mockUser); const findUserById = jest.fn(async (): Promise => mockUser); const updateUserById = jest.fn(async (..._args: unknown[]) => mockUser); +const findUserByIdentity = jest.fn(async () => mockUser); jest.mock('@/queries/user', () => ({ findUserById: async () => findUserById(), - findUserByIdentity: async () => mockUser, + findUserByIdentity: async () => findUserByIdentity(), updateUserById: async (...args: unknown[]) => updateUserById(...args), hasUserWithIdentity: async (target: string, userId: string) => target === 'connectorTarget' && userId === mockUser.id, @@ -238,6 +241,25 @@ describe('session -> socialRoutes', () => { ); }); + it('throw error when user is suspended', async () => { + (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ + metadata: { target: connectorTarget }, + }); + findUserByIdentity.mockResolvedValueOnce({ + ...mockUser, + isSuspended: true, + }); + const response = await sessionRequest.post(`${signInRoute}/auth`).send({ + connectorId: 'connectorId', + data: { + state: 'state', + redirectUri: 'https://logto.dev', + code: '123456', + }, + }); + expect(response.statusCode).toEqual(401); + }); + it('throw error when identity exists', async () => { const wrongConnectorTarget = 'wrongConnectorTarget'; (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ @@ -287,6 +309,28 @@ describe('session -> socialRoutes', () => { .send({ connectorId: 'connectorId' }) ).resolves.toHaveProperty('statusCode', 400); }); + it('throw error when user is suspended', async () => { + interactionDetails.mockResolvedValueOnce({ + result: { + login: { accountId: 'user1' }, + socialUserInfo: { + connectorId: 'connectorId', + userInfo: { id: 'connectorUser', phone: 'phone' }, + }, + }, + }); + findSocialRelatedUser.mockResolvedValueOnce([ + 'phone', + { + ...mockUser, + isSuspended: true, + }, + ]); + const response = await sessionRequest.post('/session/sign-in/bind-social-related-user').send({ + connectorId: 'connectorId', + }); + expect(response.statusCode).toEqual(401); + }); it('updates user identities and sign in', async () => { interactionDetails.mockResolvedValueOnce({ result: { @@ -384,55 +428,4 @@ describe('session -> socialRoutes', () => { expect(response.statusCode).toEqual(400); }); }); - - describe('POST /session/bind-social', () => { - beforeEach(() => { - const mockGetLogtoConnectorById = getLogtoConnectorById as jest.Mock; - mockGetLogtoConnectorById.mockResolvedValueOnce({ - metadata: { target: 'connectorTarget' }, - }); - }); - it('throw if session is not authorized', async () => { - interactionDetails.mockResolvedValueOnce({}); - await expect( - sessionRequest.post('/session/bind-social').send({ connectorId: 'connectorId' }) - ).resolves.toHaveProperty('statusCode', 400); - }); - it('throw if no social info in session', async () => { - interactionDetails.mockResolvedValueOnce({ - result: { login: { accountId: 'user1' } }, - }); - await expect( - sessionRequest.post('/session/bind-social').send({ connectorId: 'connectorId' }) - ).resolves.toHaveProperty('statusCode', 400); - }); - it('updates user identities', async () => { - interactionDetails.mockResolvedValueOnce({ - jti: 'jti', - result: { - login: { accountId: 'user1' }, - socialUserInfo: { - connectorId: 'connectorId', - userInfo: { id: 'connectorUser', phone: 'phone' }, - }, - }, - }); - const response = await sessionRequest.post('/session/bind-social').send({ - connectorId: 'connectorId', - }); - expect(response.statusCode).toEqual(200); - expect(updateUserById).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - identities: { - connectorTarget: { - details: { id: 'connectorUser', phone: 'phone' }, - userId: 'connectorUser', - }, - connector1: { userId: 'connector1', details: {} }, - }, - }) - ); - }); - }); }); diff --git a/packages/core/src/routes/session/social.ts b/packages/core/src/routes/session/social.ts index 9b65daf98..e0db07e30 100644 --- a/packages/core/src/routes/session/social.ts +++ b/packages/core/src/routes/session/social.ts @@ -94,7 +94,8 @@ export default function socialRoutes(router: T, provi } const user = await findUserByIdentity(target, userInfo.id); - const { id, identities } = user; + const { id, identities, isSuspended } = user; + assertThat(!isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); ctx.log(type, { userId: id }); // Update social connector's user info @@ -133,7 +134,8 @@ export default function socialRoutes(router: T, provi const relatedInfo = await findSocialRelatedUser(userInfo); assertThat(relatedInfo, 'session.connector_session_not_found'); - const { id, identities } = relatedInfo[1]; + const { id, identities, isSuspended } = relatedInfo[1]; + assertThat(!isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); ctx.log(type, { userId: id }); const user = await updateUserById(id, { diff --git a/packages/core/src/routes/session/utils.test.ts b/packages/core/src/routes/session/utils.test.ts index 3bceb3391..6e6679d74 100644 --- a/packages/core/src/routes/session/utils.test.ts +++ b/packages/core/src/routes/session/utils.test.ts @@ -152,6 +152,22 @@ describe('signInWithPassword()', () => { ).rejects.toThrowError(new RequestError('session.invalid_credentials')); }); + it('throw if user is suspended', async () => { + interactionDetails.mockResolvedValueOnce({ params: {} }); + await expect( + signInWithPassword(createContext(), createProvider(), { + identifier: SignInIdentifier.Username, + password: 'password', + findUser: jest.fn(async () => ({ + ...mockUser, + isSuspended: true, + })), + logType: 'SignInUsernamePassword', + logPayload: { username: 'username' }, + }) + ).rejects.toThrowError(new RequestError('user.suspended')); + }); + it('throw if sign in method is not enabled', async () => { findDefaultSignInExperience.mockResolvedValueOnce({ ...mockSignInExperience, diff --git a/packages/core/src/routes/session/utils.ts b/packages/core/src/routes/session/utils.ts index 0a0273f66..f21d60701 100644 --- a/packages/core/src/routes/session/utils.ts +++ b/packages/core/src/routes/session/utils.ts @@ -219,7 +219,8 @@ export const signInWithPassword = async ( ctx.log(logType, logPayload); const user = await findUser(); - const { id } = await verifyUserPassword(user, password); + const { id, isSuspended } = await verifyUserPassword(user, password); + assertThat(!isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); ctx.log(logType, { userId: id }); await updateUserById(id, { lastSignInAt: Date.now() }); diff --git a/packages/phrases/src/locales/de/errors.ts b/packages/phrases/src/locales/de/errors.ts index 48c9e4ec6..9ce620802 100644 --- a/packages/phrases/src/locales/de/errors.ts +++ b/packages/phrases/src/locales/de/errors.ts @@ -54,6 +54,7 @@ const errors = { require_sms: 'You need to set a phone before sign in.', // UNTRANSLATED sms_exists: 'Your phone has been set.', // UNTRANSLATED require_email_or_sms: 'You need to set a phone or email before sign in.', // UNTRANSLATED + suspended: 'This account is suspended.', // UNTRANSLATED }, password: { unsupported_encryption_method: 'Die Verschlüsselungsmethode {{name}} wird nicht unterstützt.', diff --git a/packages/phrases/src/locales/en/errors.ts b/packages/phrases/src/locales/en/errors.ts index 66450f599..b366f90df 100644 --- a/packages/phrases/src/locales/en/errors.ts +++ b/packages/phrases/src/locales/en/errors.ts @@ -54,6 +54,7 @@ const errors = { require_sms: 'You need to set a phone before sign in.', sms_exists: 'Your phone has been set.', require_email_or_sms: 'You need to set a phone or email before sign in.', + suspended: 'This account is suspended.', }, password: { unsupported_encryption_method: 'The encryption method {{name}} is not supported.', diff --git a/packages/phrases/src/locales/fr/errors.ts b/packages/phrases/src/locales/fr/errors.ts index 3e81986cc..7dbbbcb17 100644 --- a/packages/phrases/src/locales/fr/errors.ts +++ b/packages/phrases/src/locales/fr/errors.ts @@ -55,6 +55,7 @@ const errors = { require_sms: 'You need to set a phone before sign in.', // UNTRANSLATED sms_exists: 'Your phone has been set.', // UNTRANSLATED require_email_or_sms: 'You need to set a phone or email before sign in.', // UNTRANSLATED + suspended: 'This account is suspended.', // UNTRANSLATED }, password: { unsupported_encryption_method: "La méthode de cryptage {{name}} n'est pas prise en charge.", diff --git a/packages/phrases/src/locales/ko/errors.ts b/packages/phrases/src/locales/ko/errors.ts index 0ea74901d..32e954213 100644 --- a/packages/phrases/src/locales/ko/errors.ts +++ b/packages/phrases/src/locales/ko/errors.ts @@ -53,6 +53,7 @@ const errors = { require_sms: 'You need to set a phone before sign in.', // UNTRANSLATED sms_exists: 'Your phone has been set.', // UNTRANSLATED require_email_or_sms: 'You need to set a phone or email before sign in.', // UNTRANSLATED + suspended: 'This account is suspended.', // UNTRANSLATED }, password: { unsupported_encryption_method: '{{name}} 암호화 방법을 지원하지 않아요.', diff --git a/packages/phrases/src/locales/pt-pt/errors.ts b/packages/phrases/src/locales/pt-pt/errors.ts index 224728440..7db036690 100644 --- a/packages/phrases/src/locales/pt-pt/errors.ts +++ b/packages/phrases/src/locales/pt-pt/errors.ts @@ -53,6 +53,7 @@ const errors = { require_sms: 'You need to set a phone before sign in.', // UNTRANSLATED sms_exists: 'Your phone has been set.', // UNTRANSLATED require_email_or_sms: 'You need to set a phone or email before sign in.', // UNTRANSLATED + suspended: 'This account is suspended.', // UNTRANSLATED }, password: { unsupported_encryption_method: 'O método de enncriptação {{name}} não é suportado.', diff --git a/packages/phrases/src/locales/tr-tr/errors.ts b/packages/phrases/src/locales/tr-tr/errors.ts index 7234df25a..06bbd32cd 100644 --- a/packages/phrases/src/locales/tr-tr/errors.ts +++ b/packages/phrases/src/locales/tr-tr/errors.ts @@ -54,6 +54,7 @@ const errors = { require_sms: 'You need to set a phone before sign in.', // UNTRANSLATED sms_exists: 'Your phone has been set.', // UNTRANSLATED require_email_or_sms: 'You need to set a phone or email before sign in.', // UNTRANSLATED + suspended: 'This account is suspended.', // UNTRANSLATED }, password: { unsupported_encryption_method: '{{name}} şifreleme metodu desteklenmiyor.', diff --git a/packages/phrases/src/locales/zh-cn/errors.ts b/packages/phrases/src/locales/zh-cn/errors.ts index 3cdf5263d..256c3b351 100644 --- a/packages/phrases/src/locales/zh-cn/errors.ts +++ b/packages/phrases/src/locales/zh-cn/errors.ts @@ -53,6 +53,7 @@ const errors = { require_sms: '请绑定手机号码', sms_exists: '已绑定手机号码', require_email_or_sms: '请绑定邮箱地址或手机号码', + suspended: '账号已被禁用', }, password: { unsupported_encryption_method: '不支持的加密方法 {{name}}',