From c3b2b9cb8a9a1d24cfc264e471f4aebeec76c6b6 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Thu, 11 Jul 2024 14:44:48 +0800 Subject: [PATCH] refactor: fix third-party app experience branding --- .../src/components/ImageInputs/index.tsx | 5 +- .../src/libraries/sign-in-experience/index.ts | 12 ++++- .../queries/application-sign-in-experience.ts | 26 +++++++++-- .../src/tests/experience/overrides.test.ts | 46 +++++++++++++++++-- 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/packages/console/src/components/ImageInputs/index.tsx b/packages/console/src/components/ImageInputs/index.tsx index bce230e7c2b..a9f8d6f5e90 100644 --- a/packages/console/src/components/ImageInputs/index.tsx +++ b/packages/console/src/components/ImageInputs/index.tsx @@ -45,7 +45,10 @@ export type ImageField = { type Props = { /** The condensed title when user assets service is available. */ readonly uploadTitle: React.ComponentProps['title']; - /** The tooltip to show for all the fields. */ + /** + * When user assets service is available, the tip will be displayed for the `uploadTitle`; + * otherwise, it will be displayed for each text input. + */ readonly tip?: React.ComponentProps['tip']; readonly control: Control; readonly register: UseFormRegister; diff --git a/packages/core/src/libraries/sign-in-experience/index.ts b/packages/core/src/libraries/sign-in-experience/index.ts index 5c7672f3b8f..a98f80c6305 100644 --- a/packages/core/src/libraries/sign-in-experience/index.ts +++ b/packages/core/src/libraries/sign-in-experience/index.ts @@ -147,7 +147,7 @@ export const createSignInExperienceLibrary = ( return; } - return pick(found, 'branding', 'color'); + return pick(found, 'branding', 'color', 'type', 'isThirdParty'); }; const getFullSignInExperience = async ({ @@ -223,9 +223,17 @@ export const createSignInExperienceLibrary = ( }; }; + /** Get the branding and color from the app sign-in experience if it is not a third-party app. */ + const getAppSignInExperience = () => { + if (!appSignInExperience || appSignInExperience.isThirdParty) { + return {}; + } + return pick(appSignInExperience, 'branding', 'color'); + }; + return { ...deepmerge( - deepmerge(signInExperience, appSignInExperience ?? {}), + deepmerge(signInExperience, getAppSignInExperience()), organizationOverride ?? {} ), socialConnectors, diff --git a/packages/core/src/queries/application-sign-in-experience.ts b/packages/core/src/queries/application-sign-in-experience.ts index 3a9e0f157ae..84a891085c9 100644 --- a/packages/core/src/queries/application-sign-in-experience.ts +++ b/packages/core/src/queries/application-sign-in-experience.ts @@ -1,4 +1,10 @@ -import { ApplicationSignInExperiences, type ApplicationSignInExperience } from '@logto/schemas'; +import { + type Application, + ApplicationSignInExperiences, + Applications, + type ApplicationSignInExperience, +} from '@logto/schemas'; +import { type Nullable } from '@silverhand/essentials'; import { sql, type CommonQueryMethods } from '@silverhand/slonik'; import { buildInsertIntoWithPool } from '#src/database/insert-into.js'; @@ -10,12 +16,22 @@ const createApplicationSignInExperienceQueries = (pool: CommonQueryMethods) => { returning: true, }); - const safeFindSignInExperienceByApplicationId = async (applicationId: string) => { - const { table, fields } = convertToIdentifiers(ApplicationSignInExperiences); + type ApplicationSignInExperienceReturn = ApplicationSignInExperience & + Pick; - return pool.maybeOne(sql` - select ${sql.join(Object.values(fields), sql`, `)} + const safeFindSignInExperienceByApplicationId = async ( + applicationId: string + ): Promise> => { + const applications = convertToIdentifiers(Applications, true); + const { table, fields } = convertToIdentifiers(ApplicationSignInExperiences, true); + + return pool.maybeOne(sql` + select + ${sql.join(Object.values(fields), sql`, `)}, + ${applications.fields.type}, + ${applications.fields.isThirdParty} from ${table} + join ${applications.table} on ${fields.applicationId}=${applications.fields.id} where ${fields.applicationId}=${applicationId} `); }; diff --git a/packages/integration-tests/src/tests/experience/overrides.test.ts b/packages/integration-tests/src/tests/experience/overrides.test.ts index 67d4790b465..1ad22b0bd96 100644 --- a/packages/integration-tests/src/tests/experience/overrides.test.ts +++ b/packages/integration-tests/src/tests/experience/overrides.test.ts @@ -3,13 +3,20 @@ */ import { ConnectorType } from '@logto/connector-kit'; -import { ApplicationType, type Branding, type Color, SignInIdentifier } from '@logto/schemas'; -import { pick } from '@silverhand/essentials'; - +import { + ApplicationType, + type Branding, + type Color, + SignInIdentifier, + type FullSignInExperience, +} from '@logto/schemas'; +import { appendPath, pick } from '@silverhand/essentials'; + +import api from '#src/api/api.js'; import { setApplicationSignInExperience } from '#src/api/application-sign-in-experience.js'; import { createApplication, deleteApplication } from '#src/api/application.js'; import { updateSignInExperience } from '#src/api/sign-in-experience.js'; -import { demoAppRedirectUri, demoAppUrl } from '#src/constants.js'; +import { demoAppRedirectUri, demoAppUrl, logtoUrl } from '#src/constants.js'; import { clearConnectorsByTypes } from '#src/helpers/connector.js'; import { OrganizationApiTest } from '#src/helpers/organization.js'; import ExpectExperience from '#src/ui-helpers/expect-experience.js'; @@ -194,9 +201,39 @@ describe('overrides', () => { await expectMatchBranding('light', organizationBranding.logoUrl, 'rgb(0, 0, 255)'); await expectMatchBranding('dark', organizationBranding.darkLogoUrl, 'rgb(255, 0, 255)'); + await deleteApplication(application.id); await experience.page.close(); }); + it('should not use app-level branding when the app is an third-party app', async () => { + const application = await createApplication( + 'Sign-in experience override', + ApplicationType.Traditional, + { + isThirdParty: true, + oidcClientMetadata: { + redirectUris: [demoAppRedirectUri], + postLogoutRedirectUris: [demoAppRedirectUri], + }, + } + ); + + await setApplicationSignInExperience(application.id, { + color: appColor, + branding: appBranding, + }); + + // It's hard to simulate third-party apps because their type is "Traditional" while our demo + // app is an SPA. Only test the API response here. + const experience = await api + .get(appendPath(new URL(logtoUrl), 'api/.well-known/sign-in-exp')) + .json(); + + expect(experience.branding).toEqual(omniBranding); + + await deleteApplication(application.id); + }); + describe('override fallback', () => { beforeAll(async () => { await updateSignInExperience({ @@ -261,6 +298,7 @@ describe('overrides', () => { expect(faviconElement).toBe(appBranding.favicon); expect(appleFavicon).toBe(appBranding.favicon); + await deleteApplication(application.id); await experience.page.close(); }); });