From 3f84e724b9921872a354e728c3bd927502bd8b33 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Fri, 24 Nov 2023 14:21:59 +0800 Subject: [PATCH] refactor(core): remove SSO connector config partial update logic (#4958) * refactor(core): remove SSO connector config partial update logic remove SSO connector config partial update logic * fix(test): fix integration tests fix integration tests * refactor(core): refactor SAML SSO idpConfig type (#4960) refactor SAML SSO idepConfig type --- .../core/src/routes/sso-connector/index.ts | 65 +--------- .../core/src/routes/sso-connector/utils.ts | 10 +- packages/core/src/sso/SamlConnector/index.ts | 97 ++++++++------- packages/core/src/sso/SamlConnector/utils.ts | 32 ++--- .../src/sso/SamlSsoConnector/index.test.ts | 50 +++++--- .../core/src/sso/SamlSsoConnector/index.ts | 33 +++--- packages/core/src/sso/types/saml.ts | 23 ++-- .../src/api/sso-connector.ts | 13 -- .../single-sign-on/sad-path.test.ts | 4 - .../src/tests/api/sso-connectors.test.ts | 112 ++---------------- .../src/tests/api/well-known.test.ts | 2 +- packages/schemas/src/types/sso-connector.ts | 1 + 12 files changed, 153 insertions(+), 289 deletions(-) diff --git a/packages/core/src/routes/sso-connector/index.ts b/packages/core/src/routes/sso-connector/index.ts index 1c69c1e7d..152c3d83e 100644 --- a/packages/core/src/routes/sso-connector/index.ts +++ b/packages/core/src/routes/sso-connector/index.ts @@ -1,11 +1,11 @@ -import { SsoConnectors, jsonObjectGuard } from '@logto/schemas'; +import { SsoConnectors } from '@logto/schemas'; import { ssoConnectorFactoriesResponseGuard, type SsoConnectorFactoryDetail, ssoConnectorWithProviderConfigGuard, } from '@logto/schemas'; import { generateStandardShortId } from '@logto/shared'; -import { conditional, assert, yes } from '@silverhand/essentials'; +import { conditional, assert } from '@silverhand/essentials'; import { z } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; @@ -97,13 +97,10 @@ export default function singleSignOnRoutes(...args: Rout // Validate the connector domains if it's provided validateConnectorDomains(rest.domains); - /* - Validate the connector config if it's provided. - Allow partial config settings on create. - */ - const parsedConfig = config && parseConnectorConfig(providerName, config, true); - const connectorId = generateStandardShortId(); + // Validate the connector config if it's provided + const parsedConfig = config && parseConnectorConfig(providerName, config); + const connectorId = generateStandardShortId(); const connector = await ssoConnectors.insert({ id: connectorId, providerName, @@ -229,56 +226,4 @@ export default function singleSignOnRoutes(...args: Rout return next(); } ); - - /* Patch update a single sign on connector's config by id */ - router.patch( - `${pathname}/:id/config`, - koaGuard({ - params: z.object({ id: z.string().min(1) }), - /** - * Allow partially validate the connector config, on the guide page after - * the SSO connector is created, we allow users to save incomplete config without validating it. - */ - query: z.object({ partialValidateConfig: z.string().optional() }), - body: jsonObjectGuard, - response: ssoConnectorWithProviderConfigGuard, - status: [200, 404, 422], - }), - async (ctx, next) => { - const { - params: { id }, - body, - query: { partialValidateConfig }, - } = ctx.guard; - - const { providerName, config } = await getSsoConnectorById(id); - - // Merge with existing config and revalidate - const parsedConfig = parseConnectorConfig( - providerName, - { - ...config, - ...body, - }, - yes(partialValidateConfig) - ); - - const connector = await ssoConnectors.updateById(id, { - config: parsedConfig, - }); - - // Make the typescript happy - assert( - isSupportedSsoConnector(connector), - new RequestError({ code: 'connector.not_found', status: 404 }) - ); - - // Fetch provider details for the connector - const connectorWithProviderDetails = await fetchConnectorProviderDetails(connector, tenantId); - - ctx.body = connectorWithProviderDetails; - - return next(); - } - ); } diff --git a/packages/core/src/routes/sso-connector/utils.ts b/packages/core/src/routes/sso-connector/utils.ts index 09b7585e6..ca3ba4c2f 100644 --- a/packages/core/src/routes/sso-connector/utils.ts +++ b/packages/core/src/routes/sso-connector/utils.ts @@ -33,16 +33,10 @@ export const parseFactoryDetail = ( Throw error if the config is invalid. Partially validate the config if allowPartial is true. */ -export const parseConnectorConfig = ( - providerName: SsoProviderName, - config: JsonObject, - allowPartial?: boolean -) => { +export const parseConnectorConfig = (providerName: SsoProviderName, config: JsonObject) => { const factory = ssoConnectorFactories[providerName]; - const result = allowPartial - ? factory.configGuard.partial().safeParse(config) - : factory.configGuard.safeParse(config); + const result = factory.configGuard.safeParse(config); if (!result.success) { throw new RequestError({ diff --git a/packages/core/src/sso/SamlConnector/index.ts b/packages/core/src/sso/SamlConnector/index.ts index c9b3be42a..e061cd7b0 100644 --- a/packages/core/src/sso/SamlConnector/index.ts +++ b/packages/core/src/sso/SamlConnector/index.ts @@ -1,5 +1,5 @@ import { ConnectorError, ConnectorErrorCodes } from '@logto/connector-kit'; -import { trySafe, type Optional } from '@silverhand/essentials'; +import { type Optional } from '@silverhand/essentials'; import * as saml from 'samlify'; import { z } from 'zod'; @@ -9,14 +9,13 @@ import { type SamlConnectorConfig, type ExtendedSocialUserInfo, type SamlServiceProviderMetadata, - type SamlMetadata, type SamlIdentityProviderMetadata, samlIdentityProviderMetadataGuard, } from '../types/saml.js'; import { parseXmlMetadata, - getSamlMetadataXml, + fetchSamlMetadataXml, handleSamlAssertion, attributeMappingPostProcessor, getExtendedUserInfoFromRawUserProfile, @@ -31,11 +30,15 @@ import { * This class provides the basic functionality to connect with a SAML IdP. * All the SAML single sign-on connector should extend this class. * - * @property config The SAML connector config - * @property assertionConsumerServiceUrl The SAML connector's assertion consumer service URL {@link file://src/routes/authn.ts} - * @property _samlIdpMetadataXml The cached raw SAML metadata (in XML-format) from the raw SAML SSO connector config + * @property _idpConfig The input SAML connector config + * @property idpConfig The parsed SAML connector config, throws error if _idpConfig input is invalid + * @property serviceProviderMetadata The SAML service provider metadata + * @property serviceProviderMetadata.assertionConsumerServiceUrl The SAML connector's SP assertion consumer service URL {@link file://src/routes/authn.ts} + * @property serviceProviderMetadata.entityId spEntityId + * + * @property _samlIdpMetadata The parsed SAML IdP metadata cache from the SAML IdP instance + * @property _identityProvider The SAML identity provider instance cache * - * @method getSamlSpProperties Get the SAML service provider properties. * @method getSamlIdpMetadata Parse and return SAML config from the SAML connector config. Throws error if config is invalid. * @method parseSamlAssertion Parse and store the SAML assertion from IdP. * @method getSingleSignOnUrl Get the SAML SSO URL. @@ -43,51 +46,47 @@ import { * @method getIdpMetadataJson Get manually configured IdP SAML metadata from the raw SAML SSO connector config. */ class SamlConnector { - private readonly assertionConsumerServiceUrl: string; - private readonly spEntityId: string; - private readonly serviceProviderMetadata: SamlServiceProviderMetadata; - - private _samlIdpMetadataXml: Optional; + readonly serviceProviderMetadata: SamlServiceProviderMetadata; private _samlIdpMetadata: Optional; private _identityProvider: Optional; + // Allow _idpConfig input to be undefined when constructing the connector. constructor( - private readonly config: SamlConnectorConfig, tenantId: string, - ssoConnectorId: string + ssoConnectorId: string, + private readonly _idpConfig: SamlConnectorConfig | undefined ) { const tenantEndpoint = getTenantEndpoint(tenantId, EnvSet.values); - this.assertionConsumerServiceUrl = buildAssertionConsumerServiceUrl( + const assertionConsumerServiceUrl = buildAssertionConsumerServiceUrl( tenantEndpoint, ssoConnectorId ); - this.spEntityId = buildSpEntityId(tenantEndpoint, ssoConnectorId); + const spEntityId = buildSpEntityId(tenantEndpoint, ssoConnectorId); this.serviceProviderMetadata = { - entityId: this.spEntityId, - assertionConsumerServiceUrl: this.assertionConsumerServiceUrl, + entityId: spEntityId, + assertionConsumerServiceUrl, }; } /** - * @returns Properties of the SAML service provider. - */ - getSamlSpProperties() { - return this.serviceProviderMetadata; - } - - /** - * Return parsed SAML metadata. * - * @returns Parsed SAML metadata (contains both SP and IdP metadata). + * @remarks + * Since the SP config does not depend on the connector config, + * we allow the idpConfig to be undefined when constructing the connector. + * + * However, the connector config is required when getting the SAML IdP metadata. + * Therefore, we provide a getter to get the valid SAML connector config. */ - async getSamlConfig(): Promise { - const serviceProvider = this.serviceProviderMetadata; - const identityProvider = await trySafe(async () => this.getSamlIdpMetadata()); - return { serviceProvider, identityProvider }; + get idpConfig() { + if (!this._idpConfig) { + throw new ConnectorError(ConnectorErrorCodes.InvalidConfig, 'config not found'); + } + + return this._idpConfig; } /** @@ -98,15 +97,13 @@ class SamlConnector { * @returns The parsed SAML assertion from IdP (with attribute mapping applied). */ async parseSamlAssertion(body: Record): Promise { - const { x509Certificate } = await this.getSamlIdpMetadata(); - const profileMap = attributeMappingPostProcessor(this.config.attributeMapping); - const identityProvider = await this.getIdentityProvider(); + const { x509Certificate } = await this.getSamlIdpMetadata(); // HandleSamlAssertion takes a HTTPResponse-like object, need to wrap body in a object. const samlAssertionContent = await handleSamlAssertion({ body }, identityProvider, { x509Certificate, - entityId: this.spEntityId, + entityId: this.serviceProviderMetadata.entityId, }); const userProfileGuard = z.record(z.string().or(z.array(z.string()))); @@ -118,6 +115,7 @@ class SamlConnector { const rawUserProfile = rawProfileParseResult.data; + const profileMap = attributeMappingPostProcessor(this.idpConfig.attributeMapping); return getExtendedUserInfoFromRawUserProfile(rawUserProfile, profileMap); } @@ -128,19 +126,20 @@ class SamlConnector { * @returns The SSO URL. */ async getSingleSignOnUrl(relayState: string) { - const { x509Certificate } = await this.getSamlIdpMetadata(); const identityProvider = await this.getIdentityProvider(); + const { x509Certificate } = await this.getSamlIdpMetadata(); + const { entityId, assertionConsumerServiceUrl } = this.serviceProviderMetadata; try { // eslint-disable-next-line new-cap const serviceProvider = saml.ServiceProvider({ - entityID: this.spEntityId, + entityID: entityId, relayState, signingCert: x509Certificate, authnRequestsSigned: identityProvider.entityMeta.isWantAuthnRequestsSigned(), // Should align with IdP setting. assertionConsumerService: [ { - Location: this.assertionConsumerServiceUrl, + Location: assertionConsumerServiceUrl, Binding: saml.Constants.BindingNamespace.Post, }, ], @@ -161,13 +160,15 @@ class SamlConnector { * * @returns Parsed SAML config along with it's parsed metadata. */ - protected async getSamlIdpMetadata(): Promise { + async getSamlIdpMetadata(): Promise { if (this._samlIdpMetadata) { return this._samlIdpMetadata; } const identityProvider = await this.getIdentityProvider(); + this._samlIdpMetadata = parseXmlMetadata(identityProvider); + return this._samlIdpMetadata; } @@ -177,12 +178,13 @@ class SamlConnector { * @returns The raw SAML metadata in XML-format. */ private async getIdpMetadataXml() { - if (this._samlIdpMetadataXml) { - return this._samlIdpMetadataXml; + if ('metadataUrl' in this.idpConfig) { + return fetchSamlMetadataXml(this.idpConfig.metadataUrl); } - this._samlIdpMetadataXml = await getSamlMetadataXml(this.config); - return this._samlIdpMetadataXml; + if ('metadata' in this.idpConfig) { + return this.idpConfig.metadata; + } } /** @@ -192,7 +194,7 @@ class SamlConnector { */ private getIdpMetadataJson() { // Required fields of metadata should not be undefined. - const result = samlIdentityProviderMetadataGuard.safeParse(this.config); + const result = samlIdentityProviderMetadataGuard.safeParse(this.idpConfig); if (!result.success) { throw new ConnectorError(ConnectorErrorCodes.InvalidConfig, result.error); @@ -211,7 +213,9 @@ class SamlConnector { return this._identityProvider; } + // If `metadataUrl` or `metadata` is provided, we use it to construct the identity provider. const idpMetadataXml = await this.getIdpMetadataXml(); + if (idpMetadataXml) { // eslint-disable-next-line new-cap this._identityProvider = saml.IdentityProvider({ @@ -220,8 +224,9 @@ class SamlConnector { return this._identityProvider; } - const idpMetadataJson = this.getIdpMetadataJson(); - const { entityId: entityID, signInEndpoint, x509Certificate } = idpMetadataJson; + // If `metadataUrl` and `metadata` are not provided, we use get metadata from the idpConfig directly + const { entityId: entityID, signInEndpoint, x509Certificate } = this.getIdpMetadataJson(); + // eslint-disable-next-line new-cap this._identityProvider = saml.IdentityProvider({ entityID, diff --git a/packages/core/src/sso/SamlConnector/utils.ts b/packages/core/src/sso/SamlConnector/utils.ts index 617d339db..ec4fd60bc 100644 --- a/packages/core/src/sso/SamlConnector/utils.ts +++ b/packages/core/src/sso/SamlConnector/utils.ts @@ -8,7 +8,6 @@ import { z } from 'zod'; import { ssoPath } from '#src/routes/interaction/const.js'; import { - type SamlConnectorConfig, defaultAttributeMapping, type CustomizableAttributeMap, type AttributeMap, @@ -57,7 +56,7 @@ export const parseXmlMetadata = ( const result = samlIdentityProviderMetadataGuard.safeParse(rawSamlMetadata); if (!result.success) { - throw new ConnectorError(ConnectorErrorCodes.InvalidConfig, result.error); + throw new ConnectorError(ConnectorErrorCodes.InvalidMetadata, result.error); } return result.data; @@ -69,28 +68,21 @@ export const parseXmlMetadata = ( * @param config The raw SAML SSO connector config. * @returns The corresponding IdP's raw SAML metadata (in XML format). */ -export const getSamlMetadataXml = async ( - config: SamlConnectorConfig -): Promise> => { - const { metadata, metadataUrl } = config; - if (metadataUrl) { - try { - const { body } = await got.get(metadataUrl); +export const fetchSamlMetadataXml = async (metadataUrl: string): Promise> => { + try { + const { body } = await got.get(metadataUrl); - const result = z.string().safeParse(body); + const result = z.string().safeParse(body); - if (!result.success) { - throw new ConnectorError(ConnectorErrorCodes.InvalidConfig, result.error); - } - - return result.data; - } catch (error: unknown) { - // HTTP request error - throw new ConnectorError(ConnectorErrorCodes.General, error); + if (!result.success) { + throw new ConnectorError(ConnectorErrorCodes.InvalidConfig, result.error); } - } - return metadata; + return result.data; + } catch (error: unknown) { + // HTTP request error + throw new ConnectorError(ConnectorErrorCodes.General, error); + } }; /** diff --git a/packages/core/src/sso/SamlSsoConnector/index.test.ts b/packages/core/src/sso/SamlSsoConnector/index.test.ts index a021c7a55..7a92e26b9 100644 --- a/packages/core/src/sso/SamlSsoConnector/index.test.ts +++ b/packages/core/src/sso/SamlSsoConnector/index.test.ts @@ -3,16 +3,13 @@ import { SsoProviderName } from '@logto/schemas'; import { mockSsoConnector as _mockSsoConnector } from '#src/__mocks__/sso.js'; +import { type SamlConnectorConfig } from '../types/saml.js'; + import { samlSsoConnectorFactory } from './index.js'; const mockSsoConnector = { ..._mockSsoConnector, providerName: SsoProviderName.SAML }; describe('SamlSsoConnector', () => { - it('SamlSsoConnector should contains static properties', () => { - expect(samlSsoConnectorFactory.providerName).toEqual(SsoProviderName.SAML); - expect(samlSsoConnectorFactory.configGuard).toBeDefined(); - }); - it('constructor should work properly', () => { // eslint-disable-next-line unicorn/consistent-function-scoping const createSamlSsoConnector = () => @@ -21,20 +18,43 @@ describe('SamlSsoConnector', () => { expect(createSamlSsoConnector).not.toThrow(); }); - it('constructor should throw error if config is invalid', () => { + it('should get SP config properly without proper IdP config', async () => { const temporaryMockSsoConnector = { ...mockSsoConnector, config: { metadata: 123 } }; - const result = samlSsoConnectorFactory.configGuard.safeParse(temporaryMockSsoConnector.config); + const connector = new samlSsoConnectorFactory.constructor( + temporaryMockSsoConnector, + 'default_tenant' + ); - if (result.success) { - throw new Error('Invalid config'); - } + const { serviceProvider, identityProvider } = await connector.getConfig(); + expect(serviceProvider.entityId).not.toBeUndefined(); + expect(serviceProvider.assertionConsumerServiceUrl).not.toBeUndefined(); + expect(identityProvider).toBeUndefined(); + }); - const createSamlSsoConnector = () => { - return new samlSsoConnectorFactory.constructor(temporaryMockSsoConnector, 'default_tenant'); - }; + it('should throw error on calling getIdpMetadata, if the config is invalid', async () => { + const connector = new samlSsoConnectorFactory.constructor(mockSsoConnector, 'default_tenant'); - expect(createSamlSsoConnector).toThrow( - new ConnectorError(ConnectorErrorCodes.InvalidConfig, result.error) + await expect(async () => connector.getSamlIdpMetadata()).rejects.toThrow( + new ConnectorError(ConnectorErrorCodes.InvalidConfig, 'config not found') ); }); + + it.each([ + { metadata: 'metadata' }, + { metadataUrl: 'https://example.com' }, + { + entityId: '123', + signInEndpoint: 'https://example.com', + x509Certificate: 'mockCert', + }, + ])('should parse config with %p successfully', (config: SamlConnectorConfig) => { + const temporaryMockSsoConnector = { ...mockSsoConnector, config }; + + const connector = new samlSsoConnectorFactory.constructor( + temporaryMockSsoConnector, + 'default_tenant' + ); + + expect(connector.idpConfig).toEqual(config); + }); }); diff --git a/packages/core/src/sso/SamlSsoConnector/index.ts b/packages/core/src/sso/SamlSsoConnector/index.ts index 9075c0624..7ea097a6e 100644 --- a/packages/core/src/sso/SamlSsoConnector/index.ts +++ b/packages/core/src/sso/SamlSsoConnector/index.ts @@ -1,12 +1,12 @@ -import { ConnectorError, ConnectorErrorCodes } from '@logto/connector-kit'; import { type SsoConnector, SsoProviderName } from '@logto/schemas'; +import { conditional, trySafe } from '@silverhand/essentials'; import assertThat from '#src/utils/assert-that.js'; import SamlConnector from '../SamlConnector/index.js'; import { type SingleSignOnFactory } from '../index.js'; import { type SingleSignOn } from '../types/index.js'; -import { samlConnectorConfigGuard } from '../types/saml.js'; +import { samlConnectorConfigGuard, type SamlMetadata } from '../types/saml.js'; import { type SingleSignOnConnectorSession, type CreateSingleSignOnSession, @@ -18,10 +18,11 @@ import { * This class extends the basic SAML connector class and add some business related utils methods. * * @property data The SAML connector data from the database + * @property inValidConnectorConfig Whether the connector config is invalid * - * @method getProperties Get the SAML service provider properties. - * @method getConfig Get parsed SAML config along with it's metadata. Throws error if config is invalid. - * @method getUserInfo Get social user info. + * @method getConfig Get the SP and IdP metadata + * @method getUserInfo Get user info from the SAML assertion. + * @method getAuthorizationUrl Get the SAML SSO URL. */ export class SamlSsoConnector extends SamlConnector implements SingleSignOn { constructor( @@ -30,11 +31,8 @@ export class SamlSsoConnector extends SamlConnector implements SingleSignOn { ) { const parseConfigResult = samlConnectorConfigGuard.safeParse(data.config); - if (!parseConfigResult.success) { - throw new ConnectorError(ConnectorErrorCodes.InvalidConfig, parseConfigResult.error); - } - - super(parseConfigResult.data, tenantId, data.id); + // Fallback to undefined if config is invalid + super(tenantId, data.id, conditional(parseConfigResult.success && parseConfigResult.data)); } async getIssuer() { @@ -44,12 +42,17 @@ export class SamlSsoConnector extends SamlConnector implements SingleSignOn { } /** - * Get parsed SAML connector's config along with it's metadata. Throws error if config is invalid. - * - * @returns Parsed SAML connector config and it's metadata. + * ServiceProvider: SP metadata + * identityProvider: IdP metadata. Returns undefined if the idp config is invalid. */ - async getConfig() { - return this.getSamlConfig(); + async getConfig(): Promise { + const serviceProvider = this.serviceProviderMetadata; + const identityProvider = await trySafe(async () => this.getSamlIdpMetadata()); + + return { + serviceProvider, + identityProvider, + }; } /** diff --git a/packages/core/src/sso/types/saml.ts b/packages/core/src/sso/types/saml.ts index 9bf0e5791..da6963d40 100644 --- a/packages/core/src/sso/types/saml.ts +++ b/packages/core/src/sso/types/saml.ts @@ -32,17 +32,24 @@ export const samlIdentityProviderMetadataGuard = z.object({ signInEndpoint: z.string(), x509Certificate: z.string(), }); - export type SamlIdentityProviderMetadata = z.infer; -export const samlConnectorConfigGuard = samlIdentityProviderMetadataGuard - .extend({ - attributeMapping: customizableAttributeMappingGuard, - metadata: z.string(), +export const samlConnectorConfigGuard = z.union([ + // Config using Metadata URL + z.object({ metadataUrl: z.string(), - }) - .partial(); - + attributeMapping: customizableAttributeMappingGuard.optional(), + }), + // Config using Metadata XML + z.object({ + metadata: z.string(), + attributeMapping: customizableAttributeMappingGuard.optional(), + }), + // Config using Metadata detail + samlIdentityProviderMetadataGuard.extend({ + attributeMapping: customizableAttributeMappingGuard.optional(), + }), +]); export type SamlConnectorConfig = z.infer; const samlMetadataGuard = z.object({ diff --git a/packages/integration-tests/src/api/sso-connector.ts b/packages/integration-tests/src/api/sso-connector.ts index 231afdec0..4426e4634 100644 --- a/packages/integration-tests/src/api/sso-connector.ts +++ b/packages/integration-tests/src/api/sso-connector.ts @@ -1,5 +1,4 @@ import { type CreateSsoConnector, type SsoConnector } from '@logto/schemas'; -import { conditional } from '@silverhand/essentials'; import { authedAdminApi } from '#src/api/api.js'; @@ -44,15 +43,3 @@ export const patchSsoConnectorById = async (id: string, data: Partial(); - -export const patchSsoConnectorConfigById = async ( - id: string, - data: Record, - partialValidate = false -) => - authedAdminApi - .patch(`sso-connectors/${id}/config`, { - json: data, - ...conditional(partialValidate && { searchParams: { partialValidateConfig: 'true' } }), - }) - .json(); diff --git a/packages/integration-tests/src/tests/api/interaction/single-sign-on/sad-path.test.ts b/packages/integration-tests/src/tests/api/interaction/single-sign-on/sad-path.test.ts index 058975e44..9d25588d5 100644 --- a/packages/integration-tests/src/tests/api/interaction/single-sign-on/sad-path.test.ts +++ b/packages/integration-tests/src/tests/api/interaction/single-sign-on/sad-path.test.ts @@ -35,10 +35,6 @@ describe('Single Sign On Sad Path', () => { const { id } = await createSsoConnector({ providerName: SsoProviderName.OIDC, connectorName: 'test-oidc', - config: { - clientId: 'foo', - clientSecret: 'bar', - }, }); const client = await initClient(); diff --git a/packages/integration-tests/src/tests/api/sso-connectors.test.ts b/packages/integration-tests/src/tests/api/sso-connectors.test.ts index fc8e0d6f8..ba8307c62 100644 --- a/packages/integration-tests/src/tests/api/sso-connectors.test.ts +++ b/packages/integration-tests/src/tests/api/sso-connectors.test.ts @@ -1,5 +1,4 @@ import { SsoProviderName } from '@logto/schemas'; -import { conditional } from '@silverhand/essentials'; import { HTTPError } from 'got'; import { @@ -13,7 +12,6 @@ import { getSsoConnectorById, deleteSsoConnectorById, patchSsoConnectorById, - patchSsoConnectorConfigById, } from '#src/api/sso-connector.js'; describe('sso-connector library', () => { @@ -75,41 +73,29 @@ describe('post sso-connectors', () => { await deleteSsoConnectorById(response.id); }); - it.each(providerNames)('should throw if invalid config is provided', async (providerName) => { + it('OIDC connector should throw error if insufficient config is provided', async () => { await expect( createSsoConnector({ - providerName, + providerName: SsoProviderName.OIDC, connectorName: 'test', config: { - issuer: 23, - signInEndpoint: 123, + clientId: 'logto.io', }, }) ).rejects.toThrow(HTTPError); }); - it.each(partialConfigAndProviderNames)( - 'should create a new sso connector with partial configs', - async ({ providerName, config }) => { - const data = { - providerName, + it('SAML connector should throw error if invalid config is provided', async () => { + await expect( + createSsoConnector({ + providerName: SsoProviderName.SAML, connectorName: 'test', - config, - domains: ['test.com'], - }; - - const response = await createSsoConnector(data); - - expect(response).toHaveProperty('id'); - expect(response).toHaveProperty('providerName', providerName); - expect(response).toHaveProperty('connectorName', 'test'); - expect(response).toHaveProperty('config', data.config); - expect(response).toHaveProperty('domains', data.domains); - expect(response).toHaveProperty('syncProfile', false); - - await deleteSsoConnectorById(response.id); - } - ); + config: { + entityId: 123, + }, + }) + ).rejects.toThrow(HTTPError); + }); }); describe('get sso-connectors', () => { @@ -275,75 +261,3 @@ describe('patch sso-connector by id', () => { } ); }); - -describe('patch sso-connector config by id', () => { - it('should return 404 if connector is not found', async () => { - await expect(patchSsoConnectorConfigById('invalid-id', {})).rejects.toThrow(HTTPError); - }); - - it.each(providerNames)('should throw if invalid config is provided', async (providerName) => { - const { id } = await createSsoConnector({ - providerName, - connectorName: 'integration_test connector', - config: { - clientSecret: 'bar', - metadataType: 'URL', - }, - }); - - await expect( - patchSsoConnectorConfigById(id, { - issuer: 23, - signInEndpoint: 123, - }) - ).rejects.toThrow(HTTPError); - - await deleteSsoConnectorById(id); - }); - - it.each(providerNames)( - 'should successfully patch incomplete config if `partialValidateConfig` query parameter is specified', - async (providerName) => { - const { id } = await createSsoConnector({ - providerName, - connectorName: 'integration_test connector', - config: { - clientSecret: 'bar', - metadataType: 'URL', - }, - }); - - await expect( - patchSsoConnectorConfigById( - id, - { - ...conditional(providerName === SsoProviderName.OIDC && { issuer: 'issuer' }), - ...conditional(providerName === SsoProviderName.SAML && { entityId: 'entity_id' }), - }, - true - ) - ).resolves.not.toThrow(); - - await deleteSsoConnectorById(id); - } - ); - - it.each(partialConfigAndProviderNames)( - 'should patch sso connector config', - async ({ providerName, config }) => { - const { id } = await createSsoConnector({ - providerName, - connectorName: 'integration_test connector', - }); - - const connector = await patchSsoConnectorConfigById(id, config); - - expect(connector).toHaveProperty('id', id); - expect(connector).toHaveProperty('providerName', providerName); - expect(connector).toHaveProperty('connectorName', 'integration_test connector'); - expect(connector).toHaveProperty('config', config); - - await deleteSsoConnectorById(id); - } - ); -}); diff --git a/packages/integration-tests/src/tests/api/well-known.test.ts b/packages/integration-tests/src/tests/api/well-known.test.ts index c39c10bdc..86fca3be6 100644 --- a/packages/integration-tests/src/tests/api/well-known.test.ts +++ b/packages/integration-tests/src/tests/api/well-known.test.ts @@ -82,7 +82,7 @@ describe('.well-known api', () => { it('should filter out the sso connectors with invalid config', async () => { const { id } = await createSsoConnector({ ...newOidcSsoConnectorPayload, - config: {}, + config: undefined, }); const signInExperience = await api diff --git a/packages/schemas/src/types/sso-connector.ts b/packages/schemas/src/types/sso-connector.ts index cb77ced47..a5c5437f0 100644 --- a/packages/schemas/src/types/sso-connector.ts +++ b/packages/schemas/src/types/sso-connector.ts @@ -63,6 +63,7 @@ export const ssoConnectorFactoriesResponseGuard = z.object({ export type SsoConnectorFactoriesResponse = z.infer; +// API response guard for all the SSO connectors CRUD APIs export const ssoConnectorWithProviderConfigGuard = SsoConnectors.guard .omit({ providerName: true }) .merge(