diff --git a/packages/core/src/__mocks__/sso.ts b/packages/core/src/__mocks__/sso.ts index c4fee57ee..5f5cc303b 100644 --- a/packages/core/src/__mocks__/sso.ts +++ b/packages/core/src/__mocks__/sso.ts @@ -27,3 +27,17 @@ export const wellConfiguredSsoConnector = { syncProfile: true, createdAt: Date.now(), } satisfies SsoConnector; + +export const mockSamlSsoConnector = { + id: 'mock-saml-sso-connector', + tenantId: 'mock-tenant', + providerName: SsoProviderName.SAML, + connectorName: 'mock-connector-name', + config: { + metadata: 'mock-metadata', + }, + domains: ['foo.com'], + branding: {}, + syncProfile: true, + createdAt: Date.now(), +} satisfies SsoConnector; diff --git a/packages/core/src/libraries/sso-connector.test.ts b/packages/core/src/libraries/sso-connector.test.ts index 763572641..3047323ac 100644 --- a/packages/core/src/libraries/sso-connector.test.ts +++ b/packages/core/src/libraries/sso-connector.test.ts @@ -1,4 +1,4 @@ -import { Prompt, QueryKey, withReservedScopes } from '@logto/js'; +import { Prompt, QueryKey, ReservedScope, UserScope } from '@logto/js'; import { ApplicationType, type SsoConnectorIdpInitiatedAuthConfig } from '@logto/schemas'; import { mockSsoConnector, wellConfiguredSsoConnector } from '#src/__mocks__/sso.js'; @@ -204,7 +204,7 @@ describe('SsoConnectorLibrary', () => { for (const [key, value] of Object.entries(defaultQueryParameters)) { expect(parameters.get(key)).toBe(value); } - expect(parameters.get(QueryKey.Scope)).toBe(withReservedScopes()); + expect(parameters.get(QueryKey.Scope)).toBe(`${ReservedScope.OpenId} ${UserScope.Profile}`); }); it('should use the provided redirectUri', async () => { @@ -233,7 +233,7 @@ describe('SsoConnectorLibrary', () => { }); it('should append extra scopes to the query parameters', async () => { - const scopes = ['scope1', 'scope2']; + const scopes = ['organization', 'email', 'profile']; const url = await getIdpInitiatedSamlSsoSignInUrl(issuer, { ...authConfig, @@ -243,7 +243,9 @@ describe('SsoConnectorLibrary', () => { }); const parameters = new URLSearchParams(url.search); - expect(parameters.get(QueryKey.Scope)).toBe(withReservedScopes(scopes)); + expect(parameters.get(QueryKey.Scope)).toBe( + `${ReservedScope.OpenId} ${UserScope.Profile} organization email` + ); }); it('should be able to append extra query parameters', async () => { diff --git a/packages/core/src/libraries/sso-connector.ts b/packages/core/src/libraries/sso-connector.ts index 6e35e12de..40aea5f10 100644 --- a/packages/core/src/libraries/sso-connector.ts +++ b/packages/core/src/libraries/sso-connector.ts @@ -1,4 +1,4 @@ -import { type DirectSignInOptions, Prompt, QueryKey, withReservedScopes } from '@logto/js'; +import { type DirectSignInOptions, Prompt, QueryKey, ReservedScope, UserScope } from '@logto/js'; import { ApplicationType, type SsoSamlAssertionContent, @@ -7,7 +7,7 @@ import { type SsoConnectorIdpInitiatedAuthConfig, } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; -import { assert, trySafe } from '@silverhand/essentials'; +import { assert, deduplicate, trySafe } from '@silverhand/essentials'; import { defaultIdPInitiatedSamlSsoSessionTtl } from '#src/constants/index.js'; import RequestError from '#src/errors/RequestError/index.js'; @@ -141,6 +141,9 @@ export const createSsoConnectorLibrary = (queries: Queries) => { * * @remarks * For IdP-initiated SAML SSO flow use only. Generate the sign-in URL for the user to sign in. + * Default scopes: openid, profile + * Default prompt: login + * Default response type: code * * @param issuer The oidc issuer endpoint of the current tenant. * @param authConfig The IdP-initiated SAML SSO authentication configuration. @@ -183,7 +186,11 @@ export const createSsoConnectorLibrary = (queries: Queries) => { ...extraParams, }); - queryParameters.append(QueryKey.Scope, withReservedScopes(scope?.split(' ') ?? [])); + queryParameters.append( + QueryKey.Scope, + // For security reasons, DO NOT include the offline_access scope for IdP-initiated SAML SSO by default + deduplicate([ReservedScope.OpenId, UserScope.Profile, ...(scope?.split(' ') ?? [])]).join(' ') + ); return new URL(`${issuer}/auth?${queryParameters.toString()}`); }; diff --git a/packages/core/src/queries/sso-connectors.ts b/packages/core/src/queries/sso-connectors.ts index 3edfb2808..babd4201e 100644 --- a/packages/core/src/queries/sso-connectors.ts +++ b/packages/core/src/queries/sso-connectors.ts @@ -6,6 +6,7 @@ import { type SsoConnectorKeys, SsoConnectors, IdpInitiatedSamlSsoSessions, + type IdpInitiatedSamlSsoSession, } from '@logto/schemas'; import { sql, type CommonQueryMethods } from '@silverhand/slonik'; @@ -92,4 +93,12 @@ export default class SsoConnectorQueries extends SchemaQueries< throw new DeletionError(SsoConnectorIdpInitiatedAuthConfigs.table); } } + + async findIdpInitiatedSamlSsoSessionById(id: string) { + const { table, fields } = convertToIdentifiers(IdpInitiatedSamlSsoSessions); + return this.pool.maybeOne(sql` + SELECT * FROM ${table} + WHERE ${fields.id}=${id} + `); + } } diff --git a/packages/core/src/routes/interaction/utils/single-sign-on.test.ts b/packages/core/src/routes/interaction/utils/single-sign-on.test.ts index 17d62e5e0..188c575a6 100644 --- a/packages/core/src/routes/interaction/utils/single-sign-on.test.ts +++ b/packages/core/src/routes/interaction/utils/single-sign-on.test.ts @@ -1,7 +1,14 @@ +/* eslint-disable max-lines */ import { createMockUtils } from '@logto/shared/esm'; import type Provider from 'oidc-provider'; +import Sinon from 'sinon'; -import { mockSsoConnector, wellConfiguredSsoConnector } from '#src/__mocks__/sso.js'; +import { + mockSsoConnector, + wellConfiguredSsoConnector, + mockSamlSsoConnector, +} from '#src/__mocks__/sso.js'; +import { EnvSet } from '#src/env-set/index.js'; import RequestError from '#src/errors/RequestError/index.js'; import { type WithLogContext } from '#src/middleware/koa-audit-log.js'; import { type WithInteractionDetailsContext } from '#src/middleware/koa-interaction-details.js'; @@ -13,6 +20,8 @@ import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { MockTenant } from '#src/test-utils/tenant.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; +import { idpInitiatedSamlSsoSessionCookieName } from '../../../constants/index.js'; +import { SamlSsoConnector } from '../../../sso/SamlSsoConnector/index.js'; import { type WithInteractionHooksContext } from '../middleware/koa-interaction-hooks.js'; const { jest } = import.meta; @@ -30,20 +39,31 @@ const insertUserMock = jest.fn().mockResolvedValue([{ id: 'foo' }, { organizatio const generateUserIdMock = jest.fn().mockResolvedValue('foo'); const getAvailableSsoConnectorsMock = jest.fn(); +const findIdpInitiatedSamlSsoSessionMock = jest.fn(); +const deleteIdpInitiatedSamlSsoSessionMock = jest.fn(); + class MockOidcSsoConnector extends OidcSsoConnector { override getAuthorizationUrl = getAuthorizationUrlMock; override getIssuer = getIssuerMock; override getUserInfo = getUserInfoMock; } +class MockSamlSsoConnector extends SamlSsoConnector { + override getAuthorizationUrl = getAuthorizationUrlMock; + override getIssuer = getIssuerMock; + override getUserInfo = getUserInfoMock; +} + mockEsm('./interaction.js', () => ({ storeInteractionResult: jest.fn(), })); const { + assignSingleSignOnSessionResult: assignSingleSignOnSessionResultMock, getSingleSignOnSessionResult: getSingleSignOnSessionResultMock, assignSingleSignOnAuthenticationResult: assignSingleSignOnAuthenticationResultMock, } = await mockEsmWithActual('./single-sign-on-session.js', () => ({ + assignSingleSignOnSessionResult: jest.fn(), getSingleSignOnSessionResult: jest.fn(), assignSingleSignOnAuthenticationResult: jest.fn(), })); @@ -51,6 +71,11 @@ const { jest .spyOn(ssoConnectorFactories.OIDC, 'constructor') .mockImplementation((data: SingleSignOnConnectorData) => new MockOidcSsoConnector(data)); +jest + .spyOn(ssoConnectorFactories.SAML, 'constructor') + .mockImplementation( + (data: SingleSignOnConnectorData) => new MockSamlSsoConnector(data, 'tenantId') + ); const { getSsoAuthorizationUrl, @@ -83,6 +108,10 @@ describe('Single sign on util methods tests', () => { updateUserById: updateUserMock, findUserByEmail: findUserByEmailMock, }, + ssoConnectors: { + findIdpInitiatedSamlSsoSessionById: findIdpInitiatedSamlSsoSessionMock, + deleteIdpInitiatedSamlSsoSessionById: deleteIdpInitiatedSamlSsoSessionMock, + }, }, undefined, { @@ -327,4 +356,171 @@ describe('Single sign on util methods tests', () => { }); }); }); + + describe('getSsoAuthorizationUrl tests with idp initiated sso session', () => { + const stub = Sinon.stub(EnvSet, 'values').value({ + ...EnvSet.values, + isDevFeaturesEnabled: true, + }); + + const payload = { + state: 'state', + redirectUri: 'https://example.com', + }; + + const samlSsoSessionId = 'samlSsoSessionId'; + const samlAuthorizationUrl = 'https://saml-connector/callback'; + + const mockContextWithIdpInitiatedSsoSession = { + ...mockContext, + ...createContextWithRouteParameters({ + cookies: { + [idpInitiatedSamlSsoSessionCookieName]: samlSsoSessionId, + }, + }), + }; + + afterAll(() => { + stub.restore(); + }); + + beforeEach(() => { + getAuthorizationUrlMock.mockResolvedValueOnce(samlAuthorizationUrl); + }); + + it('should not check idp initiated sso session if the connector is not SAML', async () => { + await expect( + getSsoAuthorizationUrl( + mockContextWithIdpInitiatedSsoSession, + tenant, + wellConfiguredSsoConnector, + payload + ) + ).resolves.toBe(samlAuthorizationUrl); + + expect(findIdpInitiatedSamlSsoSessionMock).not.toHaveBeenCalled(); + }); + + it('should not check idp initiated sso session if the session cookie is not found', async () => { + await expect( + getSsoAuthorizationUrl(mockContext, tenant, mockSamlSsoConnector, payload) + ).resolves.toBe(samlAuthorizationUrl); + + expect(findIdpInitiatedSamlSsoSessionMock).not.toHaveBeenCalled(); + expect(deleteIdpInitiatedSamlSsoSessionMock).not.toHaveBeenCalled(); + }); + + it('should redirect to the connector authorization uri if the idp initiated sso session is not found', async () => { + findIdpInitiatedSamlSsoSessionMock.mockResolvedValueOnce(null); + + await expect( + getSsoAuthorizationUrl( + mockContextWithIdpInitiatedSsoSession, + tenant, + mockSamlSsoConnector, + payload + ) + ).resolves.toBe(samlAuthorizationUrl); + + expect(findIdpInitiatedSamlSsoSessionMock).toBeCalledWith(samlSsoSessionId); + }); + + it('should redirect to the connector authorization uri if the idp initiated sso session connectorId mismatch', async () => { + findIdpInitiatedSamlSsoSessionMock.mockResolvedValueOnce({ + id: samlSsoSessionId, + connectorId: 'foo', + }); + + await expect( + getSsoAuthorizationUrl( + mockContextWithIdpInitiatedSsoSession, + tenant, + mockSamlSsoConnector, + payload + ) + ).resolves.toBe(samlAuthorizationUrl); + + expect(findIdpInitiatedSamlSsoSessionMock).toBeCalledWith(samlSsoSessionId); + expect(deleteIdpInitiatedSamlSsoSessionMock).not.toHaveBeenCalled(); + }); + + it('should redirect to the connector authorization uri if the idp initiated sso session is expired', async () => { + findIdpInitiatedSamlSsoSessionMock.mockResolvedValueOnce({ + id: samlSsoSessionId, + connectorId: mockSamlSsoConnector.id, + expiresAt: Date.now() - 1000 * 60 * 11, + }); + + await expect( + getSsoAuthorizationUrl( + mockContextWithIdpInitiatedSsoSession, + tenant, + mockSamlSsoConnector, + payload + ) + ).resolves.toBe(samlAuthorizationUrl); + + expect(findIdpInitiatedSamlSsoSessionMock).toBeCalledWith(samlSsoSessionId); + + // Should delete the idp initiated sso session + expect(mockContextWithIdpInitiatedSsoSession.cookies.set).toBeCalledWith( + idpInitiatedSamlSsoSessionCookieName, + '', + { + httpOnly: true, + expires: new Date(0), + } + ); + expect(deleteIdpInitiatedSamlSsoSessionMock).toBeCalledWith(samlSsoSessionId); + }); + + it('should assign the user info and redirect to the SSO callback uri if the idp initiated sso session is valid', async () => { + findIdpInitiatedSamlSsoSessionMock.mockResolvedValueOnce({ + id: samlSsoSessionId, + connectorId: mockSamlSsoConnector.id, + expiresAt: Date.now() + 1000 * 60 * 10, + assertionContent: { + nameID: mockSsoUserInfo.id, + attributes: { + email: mockSsoUserInfo.email, + name: mockSsoUserInfo.name, + avatar: mockSsoUserInfo.avatar, + }, + }, + }); + + await expect( + getSsoAuthorizationUrl( + mockContextWithIdpInitiatedSsoSession, + tenant, + mockSamlSsoConnector, + payload + ) + ).resolves.toBe(`${payload.redirectUri}/?state=${payload.state}`); + + expect(findIdpInitiatedSamlSsoSessionMock).toBeCalledWith(samlSsoSessionId); + + expect(assignSingleSignOnSessionResultMock).toBeCalledWith( + mockContextWithIdpInitiatedSsoSession, + mockProvider, + { + connectorId: mockSamlSsoConnector.id, + ...payload, + userInfo: mockSsoUserInfo, + } + ); + + // Should delete the idp initiated sso session + expect(mockContextWithIdpInitiatedSsoSession.cookies.set).toBeCalledWith( + idpInitiatedSamlSsoSessionCookieName, + '', + { + httpOnly: true, + expires: new Date(0), + } + ); + expect(deleteIdpInitiatedSamlSsoSessionMock).toBeCalledWith(samlSsoSessionId); + }); + }); }); +/* eslint-enable max-lines */ diff --git a/packages/core/src/routes/interaction/utils/single-sign-on.ts b/packages/core/src/routes/interaction/utils/single-sign-on.ts index 3ca526da2..07b480e1f 100644 --- a/packages/core/src/routes/interaction/utils/single-sign-on.ts +++ b/packages/core/src/routes/interaction/utils/single-sign-on.ts @@ -8,11 +8,14 @@ import { type UserSsoIdentity, } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; -import { conditional } from '@silverhand/essentials'; +import { conditional, trySafe } from '@silverhand/essentials'; import { z } from 'zod'; +import { idpInitiatedSamlSsoSessionCookieName } from '#src/constants/index.js'; +import { EnvSet } from '#src/env-set/index.js'; import RequestError from '#src/errors/RequestError/index.js'; import { type WithLogContext } from '#src/middleware/koa-audit-log.js'; +import SamlConnector from '#src/sso/SamlConnector/index.js'; import { ssoConnectorFactories, type SingleSignOnConnectorSession } from '#src/sso/index.js'; import type Queries from '#src/tenants/Queries.js'; import type TenantContext from '#src/tenants/TenantContext.js'; @@ -34,7 +37,7 @@ type AuthorizationUrlPayload = z.infer; export const getSsoAuthorizationUrl = async ( ctx: WithLogContext, - { provider, id: tenantId }: TenantContext, + { provider, id: tenantId, queries }: TenantContext, connectorData: SupportedSsoConnector, payload: AuthorizationUrlPayload ): Promise => { @@ -43,6 +46,7 @@ export const getSsoAuthorizationUrl = async ( const { createLog } = ctx; const log = createLog(`Interaction.SignIn.Identifier.SingleSignOn.Create`); + log.append({ connectorId, payload, @@ -59,6 +63,53 @@ export const getSsoAuthorizationUrl = async ( const { jti } = await provider.interactionDetails(ctx.req, ctx.res); + if ( + // TODO: Remove this check when IdP-initiated SSO is fully supported + EnvSet.values.isDevFeaturesEnabled && + connectorInstance instanceof SamlConnector + ) { + // Check if a IdP-initiated SSO session exists + const sessionId = ctx.cookies.get(idpInitiatedSamlSsoSessionCookieName); + + const idpInitiatedSamlSsoSession = + sessionId && (await queries.ssoConnectors.findIdpInitiatedSamlSsoSessionById(sessionId)); + + // Consume the session if it exists and the connector matches + if (idpInitiatedSamlSsoSession && idpInitiatedSamlSsoSession.connectorId === connectorId) { + log.append({ idpInitiatedSamlSsoSession }); + + // Clear the session cookie + ctx.cookies.set(idpInitiatedSamlSsoSessionCookieName, '', { + httpOnly: true, + expires: new Date(0), + }); + + // Safely clear the session record + void trySafe(async () => { + await queries.ssoConnectors.deleteIdpInitiatedSamlSsoSessionById(sessionId); + }); + + const { expiresAt, assertionContent } = idpInitiatedSamlSsoSession; + + // Validate the session expiry + // Directly assign the SAML assertion result to the interaction and redirect to the SSO callback URL + if (expiresAt > Date.now()) { + const userInfo = connectorInstance.getUserInfoFromSamlAssertion(assertionContent); + const { redirectUri, state } = payload; + await assignSingleSignOnSessionResult(ctx, provider, { + redirectUri, + state, + userInfo, + connectorId, + }); + // Redirect to the callback URL directly if the session is valid + const url = new URL(redirectUri); + url.searchParams.append('state', state); + return url.toString(); + } + } + } + return await connectorInstance.getAuthorizationUrl( { jti, ...payload, connectorId }, async (connectorSession: SingleSignOnConnectorSession) => diff --git a/packages/core/src/utils/test-utils.ts b/packages/core/src/utils/test-utils.ts index e334aa4ea..cc7c709b9 100644 --- a/packages/core/src/utils/test-utils.ts +++ b/packages/core/src/utils/test-utils.ts @@ -99,6 +99,7 @@ export const createContextWithRouteParameters = ( set: ctx.set, path: ctx.path, URL: ctx.URL, + cookies: ctx.cookies, params: {}, headers: {}, router: new Router(),