From 58d03ae7158f74388826187042021c4ea016810d Mon Sep 17 00:00:00 2001 From: "IceHe.xyz" Date: Fri, 1 Apr 2022 17:57:29 +0800 Subject: [PATCH] feat(core,phrases): validate social sign-in method with social connector IDs (#485) --- .../core/src/lib/sign-in-experience.test.ts | 67 ++++++++++++++++--- packages/core/src/lib/sign-in-experience.ts | 19 +++++- .../src/routes/sign-in-experience.test.ts | 9 +-- .../core/src/routes/sign-in-experience.ts | 6 +- packages/phrases/src/locales/en.ts | 4 ++ packages/phrases/src/locales/zh-cn.ts | 4 ++ 6 files changed, 88 insertions(+), 21 deletions(-) diff --git a/packages/core/src/lib/sign-in-experience.test.ts b/packages/core/src/lib/sign-in-experience.test.ts index f84dfa19f..60a8b17bd 100644 --- a/packages/core/src/lib/sign-in-experience.test.ts +++ b/packages/core/src/lib/sign-in-experience.test.ts @@ -1,6 +1,6 @@ -import { BrandingStyle, SignInMethodState } from '@logto/schemas'; +import { BrandingStyle, SignInMethodState, ConnectorType } from '@logto/schemas'; -import { ConnectorType } from '@/connectors/types'; +import { ConnectorInstance } from '@/connectors/types'; import RequestError from '@/errors/RequestError'; import { validateBranding, @@ -62,7 +62,11 @@ describe('validate sign-in methods', () => { describe('There must be one and only one primary sign-in method.', () => { test('should throw when there is no primary sign-in method', () => { expect(() => { - validateSignInMethods({ ...mockSignInMethods, username: SignInMethodState.disabled }, []); + validateSignInMethods( + { ...mockSignInMethods, username: SignInMethodState.disabled }, + [], + [] + ); }).toMatchError( new RequestError('sign_in_experiences.not_one_and_only_one_primary_sign_in_method') ); @@ -70,7 +74,7 @@ describe('validate sign-in methods', () => { test('should throw when there are more than one primary sign-in methods', () => { expect(() => { - validateSignInMethods({ ...mockSignInMethods, social: SignInMethodState.primary }, []); + validateSignInMethods({ ...mockSignInMethods, social: SignInMethodState.primary }, [], []); }).toMatchError( new RequestError('sign_in_experiences.not_one_and_only_one_primary_sign_in_method') ); @@ -82,8 +86,8 @@ describe('validate sign-in methods', () => { expect(() => { validateSignInMethods( { ...mockSignInMethods, email: SignInMethodState.secondary }, - // @ts-expect-error-this-line - enabledConnectorInstances + [], + enabledConnectorInstances as ConnectorInstance[] ); }).toMatchError( new RequestError({ @@ -97,8 +101,8 @@ describe('validate sign-in methods', () => { expect(() => { validateSignInMethods( { ...mockSignInMethods, sms: SignInMethodState.secondary }, - // @ts-expect-error-this-line - enabledConnectorInstances + [], + enabledConnectorInstances as ConnectorInstance[] ); }).toMatchError( new RequestError({ @@ -110,10 +114,9 @@ describe('validate sign-in methods', () => { test('should throw when there is no enabled social connector and social sign-in method is enabled', () => { expect(() => { - validateSignInMethods({ ...mockSignInMethods, social: SignInMethodState.secondary }, [ - // @ts-expect-error-this-line + validateSignInMethods({ ...mockSignInMethods, social: SignInMethodState.secondary }, [], [ mockAliyunDmConnectorInstance, - ]); + ] as ConnectorInstance[]); }).toMatchError( new RequestError({ code: 'sign_in_experiences.enabled_connector_not_found', @@ -122,4 +125,46 @@ describe('validate sign-in methods', () => { ); }); }); + + test('should throw when the social connector IDs are empty and social sign-in method is enabled', () => { + expect(() => { + validateSignInMethods( + { ...mockSignInMethods, social: SignInMethodState.secondary }, + [], + enabledConnectorInstances as ConnectorInstance[] + ); + }).toMatchError(new RequestError('sign_in_experiences.empty_social_connectors')); + }); + + describe('Selected social connectors must be enabled only when social sign-in method is enabled.', () => { + test('should not validate selected social connectors when social sign-in method is disabled', () => { + expect(() => { + validateSignInMethods( + { ...mockSignInMethods, social: SignInMethodState.disabled }, + ['google', 'facebook'], + enabledConnectorInstances as ConnectorInstance[] + ); + }).not.toThrow(); + }); + + test('should throw when some selected social connector are disabled and social sign-in method is enabled', () => { + expect(() => { + validateSignInMethods( + { ...mockSignInMethods, social: SignInMethodState.secondary }, + ['google', 'facebook'], + enabledConnectorInstances as ConnectorInstance[] + ); + }).toMatchError(new RequestError('sign_in_experiences.invalid_social_connectors')); + }); + + test('should not throw when all selected social connectors are enabled and social sign-in method is enabled', () => { + expect(() => { + validateSignInMethods( + { ...mockSignInMethods, social: SignInMethodState.secondary }, + ['facebook', 'github'], + enabledConnectorInstances as ConnectorInstance[] + ); + }).not.toThrow(); + }); + }); }); diff --git a/packages/core/src/lib/sign-in-experience.ts b/packages/core/src/lib/sign-in-experience.ts index bc2c69053..7dbf1ed9c 100644 --- a/packages/core/src/lib/sign-in-experience.ts +++ b/packages/core/src/lib/sign-in-experience.ts @@ -5,6 +5,7 @@ import { SignInMethodState, TermsOfUse, } from '@logto/schemas'; +import { Optional } from '@silverhand/essentials'; import { ConnectorInstance, ConnectorType } from '@/connectors/types'; import assertThat from '@/utils/assert-that'; @@ -26,6 +27,7 @@ const isEnabled = (state: SignInMethodState) => state !== SignInMethodState.disa export const validateSignInMethods = ( signInMethods: SignInMethods, + socialSignInConnectorIds: Optional, enabledConnectorInstances: ConnectorInstance[] ) => { const signInMethodStates = Object.values(signInMethods); @@ -56,7 +58,20 @@ export const validateSignInMethods = ( 'sign_in_experiences.enabled_connector_not_found', { type: ConnectorType.Social } ); - // TODO: assertNonemptySocialConnectorIds - // TODO: assertEnabledSocialConnectorIds + + assertThat( + socialSignInConnectorIds && socialSignInConnectorIds.length > 0, + 'sign_in_experiences.empty_social_connectors' + ); + + const enabledSocialConnectorIds = new Set( + enabledConnectorInstances + .filter((instance) => instance.metadata.type === ConnectorType.Social) + .map((instance) => instance.connector.id) + ); + assertThat( + socialSignInConnectorIds.every((id) => enabledSocialConnectorIds.has(id)), + 'sign_in_experiences.invalid_social_connectors' + ); } }; diff --git a/packages/core/src/routes/sign-in-experience.test.ts b/packages/core/src/routes/sign-in-experience.test.ts index 6aa13a974..3d6d4dfe0 100644 --- a/packages/core/src/routes/sign-in-experience.test.ts +++ b/packages/core/src/routes/sign-in-experience.test.ts @@ -67,10 +67,11 @@ describe('signInExperiences routes', () => { expect(validateBranding).toHaveBeenCalledWith(mockBranding); expect(validateTermsOfUse).toHaveBeenCalledWith(termsOfUse); - expect(validateSignInMethods).toHaveBeenCalledWith(mockSignInMethods, [ - mockFacebookConnectorInstance, - mockGithubConnectorInstance, - ]); + expect(validateSignInMethods).toHaveBeenCalledWith( + mockSignInMethods, + socialSignInConnectorIds, + [mockFacebookConnectorInstance, mockGithubConnectorInstance] + ); // TODO: only update socialSignInConnectorIds when social sign-in is enabled. expect(response).toMatchObject({ diff --git a/packages/core/src/routes/sign-in-experience.ts b/packages/core/src/routes/sign-in-experience.ts index b5c0668fe..5b41dcaaa 100644 --- a/packages/core/src/routes/sign-in-experience.ts +++ b/packages/core/src/routes/sign-in-experience.ts @@ -46,7 +46,7 @@ export default function signInExperiencesRoutes(router: body: SignInExperiences.createGuard.omit({ id: true }).partial(), }), async (ctx, next) => { - const { branding, termsOfUse, signInMethods } = ctx.guard.body; + const { branding, termsOfUse, signInMethods, socialSignInConnectorIds } = ctx.guard.body; if (branding) { validateBranding(branding); @@ -63,11 +63,9 @@ export default function signInExperiencesRoutes(router: (instance) => instance.connector.enabled ); - validateSignInMethods(signInMethods, enabledConnectorInstances); + validateSignInMethods(signInMethods, socialSignInConnectorIds, enabledConnectorInstances); } - // TODO: validate socialConnectorIds - // TODO: Only update socialSignInConnectorIds when social sign-in is enabled. ctx.body = await updateDefaultSignInExperience({ ...ctx.guard.body, diff --git a/packages/phrases/src/locales/en.ts b/packages/phrases/src/locales/en.ts index d4352a11e..d9a0061ba 100644 --- a/packages/phrases/src/locales/en.ts +++ b/packages/phrases/src/locales/en.ts @@ -347,7 +347,11 @@ const errors = { 'Empty "Terms of use" content URL. Please add the content URL if "Terms of use" is enabled.', empty_slogan: 'Empty branding slogan. Please add a branding slogan if a UI style containing the slogan is selected.', + empty_social_connectors: + 'Empty social connectors. Please add enabled social connectors when the social sign-in method is enabled.', enabled_connector_not_found: 'Enabled {{type}} connector not found.', + invalid_social_connectors: + 'Invalid social connectors, Please confirm all your selected social connectors are enabled.', not_one_and_only_one_primary_sign_in_method: 'There must be one and only one primary sign-in method. Please check your input.', }, diff --git a/packages/phrases/src/locales/zh-cn.ts b/packages/phrases/src/locales/zh-cn.ts index 7fa7e073a..5236700f1 100644 --- a/packages/phrases/src/locales/zh-cn.ts +++ b/packages/phrases/src/locales/zh-cn.ts @@ -344,7 +344,11 @@ const errors = { empty_content_url_of_terms_of_use: '空的《用户协议》内容链接。当启用《用户协议》时，请添加其内容链接。', empty_slogan: '空的标语。当使用包含标语的 UI 风格时，请添加标语。', + empty_social_connectors: + '空的 social 连接器。当启用社交网络连接器的登录方式时，请添加可用的 social 连接器。', enabled_connector_not_found: '未找到可用的 {{type}} 类型的连接器。', + invalid_social_connectors: + '无效的 social 连接器。请确认你选择的所有连接器都可用，并且是 social 类型的。', not_one_and_only_one_primary_sign_in_method: '主要的登录方式必须有且仅有一个，请检查你的输入。', }, swagger: {