From 746077ec7966d83ba8d5a80669dc534a9e7b6493 Mon Sep 17 00:00:00 2001 From: Darcy Ye Date: Thu, 24 Nov 2022 12:32:36 +0800 Subject: [PATCH] feat: update PATCH /connectors/:id (#2475) --- packages/core/src/routes/connector.ts | 11 +- .../core/src/routes/connector.update.test.ts | 54 +++++++ .../core/src/routes/session/social.test.ts | 142 +++++++++--------- packages/core/src/routes/session/social.ts | 11 ++ 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 + 11 files changed, 155 insertions(+), 70 deletions(-) diff --git a/packages/core/src/routes/connector.ts b/packages/core/src/routes/connector.ts index 00935ad5a..8847d3d88 100644 --- a/packages/core/src/routes/connector.ts +++ b/packages/core/src/routes/connector.ts @@ -190,7 +190,9 @@ export default function connectorRoutes(router: T) { '/connectors/:id', koaGuard({ params: object({ id: string().min(1) }), - body: Connectors.createGuard.pick({ config: true, metadata: true }).partial(), + body: Connectors.createGuard + .pick({ config: true, metadata: true, syncProfile: true }) + .partial(), }), async (ctx, next) => { @@ -202,6 +204,13 @@ export default function connectorRoutes(router: T) { const { metadata, type, validateConfig } = await getLogtoConnectorById(id); + if (body.syncProfile) { + assertThat( + type === ConnectorType.Social, + new RequestError({ code: 'connector.invalid_type_for_syncing_profile', status: 422 }) + ); + } + if (config) { validateConfig(config); } diff --git a/packages/core/src/routes/connector.update.test.ts b/packages/core/src/routes/connector.update.test.ts index a805464a8..9d8e264ff 100644 --- a/packages/core/src/routes/connector.update.test.ts +++ b/packages/core/src/routes/connector.update.test.ts @@ -317,5 +317,59 @@ describe('connector PATCH routes', () => { ); expect(response).toHaveProperty('statusCode', 200); }); + + it('throws when set syncProfile to `true` and with non-social connector', async () => { + getLogtoConnectorsPlaceholder.mockResolvedValueOnce([ + { + dbEntry: mockConnector, + metadata: mockMetadata, + type: ConnectorType.Sms, + ...mockLogtoConnector, + }, + ]); + const response = await connectorRequest.patch('/connectors/id').send({ syncProfile: true }); + expect(response).toHaveProperty('statusCode', 422); + expect(updateConnector).toHaveBeenCalledTimes(0); + }); + + it('successfully set syncProfile to `true` and with social connector', async () => { + getLogtoConnectorsPlaceholder.mockResolvedValueOnce([ + { + dbEntry: { ...mockConnector, syncProfile: false }, + metadata: mockMetadata, + type: ConnectorType.Social, + ...mockLogtoConnector, + }, + ]); + const response = await connectorRequest.patch('/connectors/id').send({ syncProfile: true }); + expect(updateConnector).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: 'id' }, + set: { syncProfile: true }, + jsonbMode: 'replace', + }) + ); + expect(response).toHaveProperty('statusCode', 200); + }); + + it('successfully set syncProfile to `false`', async () => { + getLogtoConnectorsPlaceholder.mockResolvedValueOnce([ + { + dbEntry: { ...mockConnector, syncProfile: false }, + metadata: mockMetadata, + type: ConnectorType.Social, + ...mockLogtoConnector, + }, + ]); + const response = await connectorRequest.patch('/connectors/id').send({ syncProfile: false }); + expect(updateConnector).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: 'id' }, + set: { syncProfile: false }, + jsonbMode: 'replace', + }) + ); + expect(response).toHaveProperty('statusCode', 200); + }); }); }); diff --git a/packages/core/src/routes/session/social.test.ts b/packages/core/src/routes/session/social.test.ts index 1a357cf83..fd4e9b1c9 100644 --- a/packages/core/src/routes/session/social.test.ts +++ b/packages/core/src/routes/session/social.test.ts @@ -5,19 +5,19 @@ import { Provider } from 'oidc-provider'; import { mockLogtoConnectorList, mockSignInExperience, mockUser } from '#src/__mocks__/index.js'; import { getLogtoConnectorById } from '#src/connectors/index.js'; +import type { SocialUserInfo } from '#src/connectors/types.js'; import RequestError from '#src/errors/RequestError/index.js'; import { createRequester } from '#src/utils/test-utils.js'; -import socialRoutes, { registerRoute, signInRoute } from './social.js'; +import socialRoutes, { signInRoute } from './social.js'; const findSocialRelatedUser = jest.fn(async () => [ 'phone', { id: 'user1', identities: {}, isSuspended: false }, ]); -jest.mock('#src/lib/social.js', () => ({ - ...jest.requireActual('#src/lib/social.js'), - findSocialRelatedUser: async () => findSocialRelatedUser(), - async getUserInfoByAuthCode(connectorId: string, data: { code: string }) { + +const getUserInfoByAuthCode = jest.fn( + async (connectorId: string, data: { code: string }): Promise => { if (connectorId === '_connectorId') { throw new RequestError({ code: 'session.invalid_connector_id', @@ -33,7 +33,14 @@ jest.mock('#src/lib/social.js', () => ({ // 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(' '); - }, + } +); + +jest.mock('#src/lib/social.js', () => ({ + ...jest.requireActual('#src/lib/social.js'), + findSocialRelatedUser: async () => findSocialRelatedUser(), + getUserInfoByAuthCode: async (connectorId: string, data: { code: string }) => + getUserInfoByAuthCode(connectorId, data), })); const insertUser = jest.fn(async (..._args: unknown[]) => mockUser); const findUserById = jest.fn(async (): Promise => mockUser); @@ -184,10 +191,14 @@ describe('session -> socialRoutes', () => { describe('POST /session/sign-in/social/auth', () => { const connectorTarget = 'connectorTarget'; + afterEach(() => { + jest.clearAllMocks(); + }); it('throw error when auth code is wrong', async () => { (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ metadata: { target: connectorTarget }, + dbEntry: { syncProfile: false }, }); const response = await sessionRequest.post(`${signInRoute}/auth`).send({ connectorId: 'connectorId', @@ -201,6 +212,7 @@ describe('session -> socialRoutes', () => { it('throw error when code is provided but connector can not be found', async () => { (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ metadata: { target: connectorTarget }, + dbEntry: { syncProfile: false }, }); const response = await sessionRequest.post(`${signInRoute}/auth`).send({ connectorId: '_connectorId', @@ -214,6 +226,7 @@ describe('session -> socialRoutes', () => { it('get and add user info with auth code, as well as assign result and redirect', async () => { (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ metadata: { target: connectorTarget }, + dbEntry: { syncProfile: false }, }); const response = await sessionRequest.post(`${signInRoute}/auth`).send({ connectorId: 'connectorId', @@ -244,6 +257,7 @@ describe('session -> socialRoutes', () => { it('throw error when user is suspended', async () => { (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ metadata: { target: connectorTarget }, + dbEntry: { syncProfile: false }, }); findUserByIdentity.mockResolvedValueOnce({ ...mockUser, @@ -264,6 +278,7 @@ describe('session -> socialRoutes', () => { const wrongConnectorTarget = 'wrongConnectorTarget'; (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ metadata: { target: wrongConnectorTarget }, + dbEntry: { syncProfile: false }, }); const response = await sessionRequest.post(`${signInRoute}/auth`).send({ connectorId: '_connectorId_', @@ -283,6 +298,58 @@ describe('session -> socialRoutes', () => { ); expect(response.statusCode).toEqual(422); }); + + it('should update `name` and `avatar` if exists when `syncProfile` is set to be true', async () => { + (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ + metadata: { target: connectorTarget }, + dbEntry: { syncProfile: true }, + }); + findUserByIdentity.mockResolvedValueOnce(mockUser); + getUserInfoByAuthCode.mockResolvedValueOnce({ + ...mockUser, + name: 'new_name', + avatar: 'new_avatar', + }); + await sessionRequest.post(`${signInRoute}/auth`).send({ + connectorId: 'connectorId', + data: { + state: 'state', + redirectUri: 'https://logto.dev', + code: '123456', + }, + }); + expect(updateUserById).toHaveBeenCalledWith( + mockUser.id, + expect.objectContaining({ name: 'new_name', avatar: 'new_avatar' }) + ); + }); + + it('should not update `name` and `avatar` if exists when `syncProfile` is set to be false', async () => { + (getLogtoConnectorById as jest.Mock).mockResolvedValueOnce({ + metadata: { target: connectorTarget }, + dbEntry: { syncProfile: true }, + }); + findUserByIdentity.mockResolvedValueOnce(mockUser); + getUserInfoByAuthCode.mockResolvedValueOnce({ + ...mockUser, + name: 'new_name', + avatar: 'new_avatar', + }); + await sessionRequest.post(`${signInRoute}/auth`).send({ + connectorId: 'connectorId', + data: { + state: 'state', + redirectUri: 'https://logto.dev', + code: '123456', + }, + }); + expect(updateUserById).not.toHaveBeenCalledWith(mockUser.id, { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + identities: expect.anything(), + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + lastSignInAt: expect.anything(), + }); + }); }); describe('POST /session/sign-in/bind-social-related-user', () => { @@ -365,67 +432,4 @@ describe('session -> socialRoutes', () => { ); }); }); - - 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.ts b/packages/core/src/routes/session/social.ts index 662bec104..566fdcd1e 100644 --- a/packages/core/src/routes/session/social.ts +++ b/packages/core/src/routes/session/social.ts @@ -1,5 +1,6 @@ import { validateRedirectUrl } from '@logto/core-kit'; import { ConnectorType, userInfoSelectFields } from '@logto/schemas'; +import { conditional } from '@silverhand/essentials'; import pick from 'lodash.pick'; import type { Provider } from 'oidc-provider'; import { object, string, unknown } from 'zod'; @@ -70,6 +71,7 @@ export default function socialRoutes(router: T, provi ctx.log(type, { connectorId, data }); const { metadata: { target }, + dbEntry: { syncProfile }, } = await getLogtoConnectorById(connectorId); const userInfo = await getUserInfoByAuthCode(connectorId, data); @@ -98,10 +100,19 @@ export default function socialRoutes(router: T, provi assertThat(!isSuspended, new RequestError({ code: 'user.suspended', status: 401 })); ctx.log(type, { userId: id }); + const { name, avatar } = userInfo; + const profileUpdate = Object.fromEntries( + Object.entries({ + name: conditional(syncProfile && name), + avatar: conditional(syncProfile && avatar), + }).filter(([_key, value]) => value !== undefined) + ); + // Update social connector's user info await updateUserById(id, { identities: { ...identities, [target]: { userId: userInfo.id, details: userInfo } }, lastSignInAt: Date.now(), + ...profileUpdate, }); const signInExperience = await getSignInExperienceForApplication( diff --git a/packages/phrases/src/locales/de/errors.ts b/packages/phrases/src/locales/de/errors.ts index 2924cdc81..3a78609ab 100644 --- a/packages/phrases/src/locales/de/errors.ts +++ b/packages/phrases/src/locales/de/errors.ts @@ -102,6 +102,7 @@ const errors = { not_found_with_connector_id: 'Can not find connector with given standard connector id.', multiple_instances_not_supported: 'Can not create multiple instance with picked standard connector.', + invalid_type_for_syncing_profile: 'You can only sync user profile with social connectors.', }, passcode: { phone_email_empty: 'Telefonnummer oder E-Mail darf nicht leer sein.', diff --git a/packages/phrases/src/locales/en/errors.ts b/packages/phrases/src/locales/en/errors.ts index e0623a626..197d2b60d 100644 --- a/packages/phrases/src/locales/en/errors.ts +++ b/packages/phrases/src/locales/en/errors.ts @@ -101,6 +101,7 @@ const errors = { not_found_with_connector_id: 'Can not find connector with given standard connector id.', multiple_instances_not_supported: 'Can not create multiple instance with picked standard connector.', + invalid_type_for_syncing_profile: 'You can only sync user profile with social connectors.', }, passcode: { phone_email_empty: 'Both phone and email are empty.', diff --git a/packages/phrases/src/locales/fr/errors.ts b/packages/phrases/src/locales/fr/errors.ts index b23cb9681..f9c5aaf55 100644 --- a/packages/phrases/src/locales/fr/errors.ts +++ b/packages/phrases/src/locales/fr/errors.ts @@ -108,6 +108,7 @@ const errors = { not_found_with_connector_id: 'Can not find connector with given standard connector id.', // UNTRANSLATED multiple_instances_not_supported: 'Can not create multiple instance with picked standard connector.', // UNTRANSLATED + invalid_type_for_syncing_profile: 'You can only sync user profile with social connectors.', // UNTRANSLATED }, passcode: { phone_email_empty: "Le téléphone et l'email sont vides.", diff --git a/packages/phrases/src/locales/ko/errors.ts b/packages/phrases/src/locales/ko/errors.ts index 0f0dee0a7..2062eeb37 100644 --- a/packages/phrases/src/locales/ko/errors.ts +++ b/packages/phrases/src/locales/ko/errors.ts @@ -100,6 +100,7 @@ const errors = { not_found_with_connector_id: 'Can not find connector with given standard connector id.', // UNTRANSLATED multiple_instances_not_supported: 'Can not create multiple instance with picked standard connector.', // UNTRANSLATED + invalid_type_for_syncing_profile: 'You can only sync user profile with social connectors.', // UNTRANSLATED }, passcode: { phone_email_empty: '휴대전화번호 그리고 이메일이 비어있어요.', diff --git a/packages/phrases/src/locales/pt-pt/errors.ts b/packages/phrases/src/locales/pt-pt/errors.ts index 3b35a9d5b..3c017cef6 100644 --- a/packages/phrases/src/locales/pt-pt/errors.ts +++ b/packages/phrases/src/locales/pt-pt/errors.ts @@ -103,6 +103,7 @@ const errors = { not_found_with_connector_id: 'Can not find connector with given standard connector id.', // UNTRANSLATED multiple_instances_not_supported: 'Can not create multiple instance with picked standard connector.', // UNTRANSLATED + invalid_type_for_syncing_profile: 'You can only sync user profile with social connectors.', // UNTRANSLATED }, passcode: { phone_email_empty: 'O campos telefone e email estão vazios.', diff --git a/packages/phrases/src/locales/tr-tr/errors.ts b/packages/phrases/src/locales/tr-tr/errors.ts index 8f2d0b225..17603dea0 100644 --- a/packages/phrases/src/locales/tr-tr/errors.ts +++ b/packages/phrases/src/locales/tr-tr/errors.ts @@ -102,6 +102,7 @@ const errors = { not_found_with_connector_id: 'Can not find connector with given standard connector id.', // UNTRANSLATED multiple_instances_not_supported: 'Can not create multiple instance with picked standard connector.', // UNTRANSLATED + invalid_type_for_syncing_profile: 'You can only sync user profile with social connectors.', // UNTRANSLATED }, passcode: { phone_email_empty: 'Hem telefon hem de e-posta adresi yok.', diff --git a/packages/phrases/src/locales/zh-cn/errors.ts b/packages/phrases/src/locales/zh-cn/errors.ts index a139fd8de..79497bcc2 100644 --- a/packages/phrases/src/locales/zh-cn/errors.ts +++ b/packages/phrases/src/locales/zh-cn/errors.ts @@ -97,6 +97,7 @@ const errors = { db_connector_type_mismatch: '数据库中存在一个类型不匹配的连接。', not_found_with_connector_id: '找不到所给 connector id 对应的连接器', multiple_instances_not_supported: '你选择的连接器不支持创建多实例。', + invalid_type_for_syncing_profile: '只有社交连接器可以开启用户档案同步。', }, passcode: { phone_email_empty: '手机号与邮箱地址均为空',