feat(core): update social sign-in connector IDs only when social sign-in is enabled (#488)

packages/core/src/lib/sign-in-experience.test.ts

@@ -3,6 +3,7 @@ import { BrandingStyle, SignInMethodState, ConnectorType } from '@logto/schemas'
 import { ConnectorInstance } from '@/connectors/types';
 import RequestError from '@/errors/RequestError';
 import {
+  isEnabled,
   validateBranding,
   validateSignInMethods,
   validateTermsOfUse,
@@ -58,6 +59,20 @@ describe('validate terms of use', () => {
   });
 });
 
+describe('check whether the social sign in method state is enabled', () => {
+  it('should be truthy when sign-in method state is primary', () => {
+    expect(isEnabled(SignInMethodState.primary)).toBeTruthy();
+  });
+
+  it('should be truthy when sign-in method state is secondary', () => {
+    expect(isEnabled(SignInMethodState.secondary)).toBeTruthy();
+  });
+
+  it('should be falsy when sign-in method state is disabled', () => {
+    expect(isEnabled(SignInMethodState.disabled)).toBeFalsy();
+  });
+});
+
 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', () => {

packages/core/src/lib/sign-in-experience.ts

@@ -23,7 +23,7 @@ export const validateTermsOfUse = (termsOfUse: TermsOfUse) => {
   );
 };
 
-const isEnabled = (state: SignInMethodState) => state !== SignInMethodState.disabled;
+export const isEnabled = (state: SignInMethodState) => state !== SignInMethodState.disabled;
 
 export const validateSignInMethods = (
   signInMethods: SignInMethods,

packages/core/src/routes/sign-in-experience.test.ts

@@ -1,4 +1,9 @@
-import { SignInExperience, CreateSignInExperience, TermsOfUse } from '@logto/schemas';
+import {
+  SignInExperience,
+  CreateSignInExperience,
+  TermsOfUse,
+  SignInMethodState,
+} from '@logto/schemas';
 
 import * as signInExpLib from '@/lib/sign-in-experience';
 import {
@@ -39,18 +44,67 @@ jest.mock('@/queries/sign-in-experience', () => ({
   ),
 }));
 
-describe('signInExperiences routes', () => {
+const signInExperienceRequester = createRequester({ authedRoutes: signInExperiencesRoutes });
-  const signInExperienceRequester = createRequester({ authedRoutes: signInExperiencesRoutes });
 
-  it('GET /sign-in-exp', async () => {
+it('GET /sign-in-exp', async () => {
-    const response = await signInExperienceRequester.get('/sign-in-exp');
+  const response = await signInExperienceRequester.get('/sign-in-exp');
+  expect(response.status).toEqual(200);
+  expect(response.body).toEqual(mockSignInExperience);
+});
+
+describe('PATCH /sign-in-exp', () => {
+  it('should not update social connector ids when social sign-in is disabled', async () => {
+    const signInMethods = { ...mockSignInMethods, social: SignInMethodState.disabled };
+    const response = await signInExperienceRequester.patch('/sign-in-exp').send({
+      signInMethods,
+      socialSignInConnectorIds: ['facebook'],
+    });
     expect(response).toMatchObject({
       status: 200,
-      body: mockSignInExperience,
+      body: {
+        ...mockSignInExperience,
+        signInMethods,
+      },
     });
   });
 
-  it('PATCH /sign-in-exp', async () => {
+  it('should update enabled social connector IDs only when social sign-in is enabled', async () => {
+    const signInMethods = { ...mockSignInMethods, social: SignInMethodState.secondary };
+    const socialSignInConnectorIds = ['facebook'];
+    const signInExperience = {
+      signInMethods,
+      socialSignInConnectorIds,
+    };
+    const response = await signInExperienceRequester.patch('/sign-in-exp').send(signInExperience);
+    expect(response).toMatchObject({
+      status: 200,
+      body: {
+        ...mockSignInExperience,
+        signInMethods,
+        socialSignInConnectorIds,
+      },
+    });
+  });
+
+  it('should update social connector IDs in correct sorting order', async () => {
+    const signInMethods = { ...mockSignInMethods, social: SignInMethodState.secondary };
+    const socialSignInConnectorIds = ['github', 'facebook'];
+    const signInExperience = {
+      signInMethods,
+      socialSignInConnectorIds,
+    };
+    const response = await signInExperienceRequester.patch('/sign-in-exp').send(signInExperience);
+    expect(response).toMatchObject({
+      status: 200,
+      body: {
+        ...mockSignInExperience,
+        signInMethods,
+        socialSignInConnectorIds,
+      },
+    });
+  });
+
+  it('should succeed to update when the input is valid', async () => {
     const termsOfUse: TermsOfUse = { enabled: false };
     const socialSignInConnectorIds = ['github', 'facebook'];
 
@@ -86,7 +140,7 @@ describe('signInExperiences routes', () => {
     });
   });
 
-  it('PATCH /sign-in-exp should throw with invalid inputs', async () => {
+  it('should throw when the type of social connector IDs is wrong', async () => {
     const socialSignInConnectorIds = [123, 456];
 
     const response = await signInExperienceRequester.patch('/sign-in-exp').send({
@@ -94,4 +148,6 @@ describe('signInExperiences routes', () => {
     });
     expect(response.status).toEqual(400);
   });
+
+  // TODO: test other Zod guards of sign-in experiences
 });

packages/core/src/routes/sign-in-experience.ts

@@ -5,6 +5,7 @@ import {
   validateBranding,
   validateTermsOfUse,
   validateSignInMethods,
+  isEnabled,
 } from '@/lib/sign-in-experience';
 import koaGuard from '@/middleware/koa-guard';
 import {
@@ -46,7 +47,8 @@ export default function signInExperiencesRoutes<T extends AuthedRouter>(router:
       body: SignInExperiences.createGuard.omit({ id: true }).partial(),
     }),
     async (ctx, next) => {
-      const { branding, termsOfUse, signInMethods, socialSignInConnectorIds } = ctx.guard.body;
+      const { socialSignInConnectorIds, ...rest } = ctx.guard.body;
+      const { branding, termsOfUse, signInMethods } = rest;
 
       if (branding) {
         validateBranding(branding);
@@ -66,10 +68,11 @@ export default function signInExperiencesRoutes<T extends AuthedRouter>(router:
         validateSignInMethods(signInMethods, socialSignInConnectorIds, enabledConnectorInstances);
       }
 
-      // TODO: Only update socialSignInConnectorIds when social sign-in is enabled.
+      // Update socialSignInConnectorIds only when social sign-in is enabled.
-      ctx.body = await updateDefaultSignInExperience({
+      const signInExperience =
-        ...ctx.guard.body,
+        signInMethods && isEnabled(signInMethods.social) ? ctx.guard.body : rest;
-      });
+
+      ctx.body = await updateDefaultSignInExperience(signInExperience);
 
       return next();
     }