Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe('SAML Login Flow', () => {

await controller.initSsoGet(req, res);

expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/');
expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/', undefined, undefined);
});

test('validates redirect URL that is passed in via referrer header', async () => {
Expand All @@ -216,7 +216,7 @@ describe('SAML Login Flow', () => {

await controller.initSsoGet(req, res);

expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/');
expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/', undefined, undefined);
});

const hostWithoutRedirect = 'http://localhost:5678/';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,16 @@ describe('SamlService', () => {
});

describe('getIdentityProviderInstance', () => {
test('does throw `InvalidSamlMetadataError` when metadata is empty', async () => {
await samlService.loadPreferencesWithoutValidation({
metadata: '',
});
await samlService.loadSamlify();
expect(() => samlService.getIdentityProviderInstance(true)).toThrowError(
InvalidSamlMetadataError,
);
});

test('does throw `InvalidSamlMetadataError` when a metadata does not contain redirect binding', async () => {
await samlService.loadPreferencesWithoutValidation({
metadata: SamlMetadataWithoutRedirectBinding,
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/modules/sso-saml/saml-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class SamlValidator {
this.xmllint = await import('xmllint-wasm');
}

validateIdentiyProvider(idp: IdentityProviderInstance) {
validateIdentityProvider(idp: IdentityProviderInstance) {
const binding = idp.entityMeta.getSingleSignOnService(
this.samlify.Constants.wording.binding.redirect,
);
Expand All @@ -45,7 +45,7 @@ export class SamlValidator {
const idp = this.samlify.IdentityProvider({
metadata,
});
this.validateIdentiyProvider(idp);
this.validateIdentityProvider(idp);
}

return validXML;
Expand Down
26 changes: 20 additions & 6 deletions packages/cli/src/modules/sso-saml/saml.controller.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,30 @@ export class SamlController {

/**
* Test SAML config
* This endpoint is available if SAML is licensed and the requestor is an instance owner
* Accepts metadata from the request body so testing works without saving first.
* This endpoint is available if SAML is licensed and the requestor is an instance owner.
*/
@Get('/config/test', { middlewares: [samlLicensedMiddleware] })
@Post('/config/test', { middlewares: [samlLicensedMiddleware] })
@GlobalScope('saml:manage')
async configTestGet(_: AuthenticatedRequest, res: Response) {
return await this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl());
async configTestPost(_req: AuthenticatedRequest, res: Response, @Body payload: SamlPreferences) {
return await this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl(), payload);
}

private async handleInitSSO(res: Response, relayState?: string) {
const result = await this.samlService.getLoginRequestUrl(relayState);
private async handleInitSSO(res: Response, relayState?: string, config?: SamlPreferences) {
let metadata: string | undefined;
if (config) {
metadata = config.metadata;
if (!metadata && config.metadataUrl) {
metadata =
(await this.samlService.fetchMetadataFromUrl(config.metadataUrl, config.ignoreSSL)) ?? '';
}
}

const result = await this.samlService.getLoginRequestUrl(
relayState,
config?.loginBinding,
metadata,
);
if (result?.binding === 'redirect') {
return result.context.context;
} else if (result?.binding === 'post') {
Expand Down
83 changes: 45 additions & 38 deletions packages/cli/src/modules/sso-saml/saml.service.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,18 @@ export class SamlService {
if (this.samlify === undefined) {
throw new UnexpectedError('Samlify is not initialized');
}
if (!this._samlPreferences.metadata) {
throw new InvalidSamlMetadataError(
'No IdP metadata configured. Please provide valid identity provider metadata.',
);
}
if (this.identityProviderInstance === undefined || forceRecreate) {
this.identityProviderInstance = this.samlify.IdentityProvider({
metadata: this._samlPreferences.metadata,
});
}

this.validator.validateIdentiyProvider(this.identityProviderInstance);
this.validator.validateIdentityProvider(this.identityProviderInstance);

return this.identityProviderInstance;
}
Expand All @@ -156,43 +161,44 @@ export class SamlService {
return getServiceProviderInstance(this._samlPreferences, this.samlify);
}

/**
* Generate a login request URL.
* When `metadata` is provided, creates a temporary IdP from it (for testing without saving).
* Otherwise uses the cached IdP from persisted preferences.
*/
async getLoginRequestUrl(
relayState?: string,
binding?: SamlLoginBinding,
metadata?: string,
): Promise<{
binding: SamlLoginBinding;
context: BindingContext | PostBindingContext;
}> {
await this.loadSamlify();
if (binding === undefined) binding = this._samlPreferences.loginBinding ?? 'redirect';
if (binding === 'post') {
return {
binding,
context: this.getPostLoginRequestUrl(relayState),
};
} else {
return {
binding,
context: this.getRedirectLoginRequestUrl(relayState),
};
if (this.samlify === undefined) {
throw new UnexpectedError('Samlify is not initialized');
}
}

private getRedirectLoginRequestUrl(relayState?: string): BindingContext {
const sp = this.getServiceProviderInstance();
sp.entitySetting.relayState = relayState ?? this.urlService.getInstanceBaseUrl();
const loginRequest = sp.createLoginRequest(this.getIdentityProviderInstance(), 'redirect');
return loginRequest;
}
let idp: IdentityProviderInstance;
if (metadata) {
const validationResult = await this.validator.validateMetadata(metadata);
if (!validationResult) {
throw new InvalidSamlMetadataError();
}
idp = this.samlify.IdentityProvider({ metadata });
this.validator.validateIdentityProvider(idp);
} else {
idp = this.getIdentityProviderInstance();
}

private getPostLoginRequestUrl(relayState?: string): PostBindingContext {
binding ??= this._samlPreferences.loginBinding ?? 'redirect';
const sp = this.getServiceProviderInstance();
sp.entitySetting.relayState = relayState ?? this.urlService.getInstanceBaseUrl();
const loginRequest = sp.createLoginRequest(
this.getIdentityProviderInstance(),
'post',
) as PostBindingContext;
return loginRequest;
const loginRequest = sp.createLoginRequest(idp, binding);
return {
binding,
context: binding === 'post' ? (loginRequest as PostBindingContext) : loginRequest,
};
}

async handleSamlLogin(
Expand Down Expand Up @@ -446,40 +452,41 @@ export class SamlService {
return;
}

async fetchMetadataFromUrl(): Promise<string | undefined> {
async fetchMetadataFromUrl(
metadataUrl?: string,
ignoreSSL?: boolean,
): Promise<string | undefined> {
await this.loadSamlify();
if (!this._samlPreferences.metadataUrl)
throw new BadRequestError('Error fetching SAML Metadata, no Metadata URL set');
const url = metadataUrl ?? this._samlPreferences.metadataUrl;
const shouldIgnoreSSL = ignoreSSL ?? this._samlPreferences.ignoreSSL;
if (!url) throw new BadRequestError('Error fetching SAML Metadata, no Metadata URL set');
try {
// Create a proxy-aware HTTPS agent that respects HTTP_PROXY, HTTPS_PROXY, and NO_PROXY
// environment variables while also supporting SSL certificate validation options
const httpsAgent = createHttpsProxyAgent(
null, // Uses proxy from environment variables
this._samlPreferences.metadataUrl,
url,
{
rejectUnauthorized: !this._samlPreferences.ignoreSSL,
rejectUnauthorized: !shouldIgnoreSSL,
},
);
const httpAgent = createHttpProxyAgent(null, this._samlPreferences.metadataUrl);
const httpAgent = createHttpProxyAgent(null, url);

const response = await axios.get(this._samlPreferences.metadataUrl, {
const response = await axios.get(url, {
httpsAgent,
httpAgent,
});
if (response.status === 200 && response.data) {
const xml = (await response.data) as string;
const validationResult = await this.validator.validateMetadata(xml);
if (!validationResult) {
throw new BadRequestError(
`Data received from ${this._samlPreferences.metadataUrl} is not valid SAML metadata.`,
);
throw new BadRequestError(`Data received from ${url} is not valid SAML metadata.`);
}
return xml;
}
} catch (error) {
throw new BadRequestError(
`Error fetching SAML Metadata from ${this._samlPreferences.metadataUrl}: ${error}`,
);
if (error instanceof BadRequestError) throw error;
throw new BadRequestError(`Error fetching SAML Metadata from ${url}: ${error}`);
}
return;
}
Expand Down
15 changes: 9 additions & 6 deletions packages/cli/test/integration/saml/saml.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,11 @@ describe('Check endpoint permissions', () => {
await authOwnerAgent.get('/sso/saml/initsso').expect(200);
});

test('should be able to access GET /sso/saml/config/test', async () => {
await authOwnerAgent.get('/sso/saml/config/test').expect(200);
test('should be able to access POST /sso/saml/config/test', async () => {
await authOwnerAgent
.post('/sso/saml/config/test')
.send({ metadata: sampleConfig.metadata })
.expect(200);
});
});

Expand Down Expand Up @@ -241,8 +244,8 @@ describe('Check endpoint permissions', () => {
await authMemberAgent.get('/sso/saml/initsso').expect(200);
});

test('should NOT be able to access GET /sso/saml/config/test', async () => {
await authMemberAgent.get('/sso/saml/config/test').expect(403);
test('should NOT be able to access POST /sso/saml/config/test', async () => {
await authMemberAgent.post('/sso/saml/config/test').expect(403);
});
});
describe('Non-Authenticated User', () => {
Expand Down Expand Up @@ -280,8 +283,8 @@ describe('Check endpoint permissions', () => {
await testServer.authlessAgent.get('/sso/saml/initsso').expect(200);
});

test('should NOT be able to access GET /sso/saml/config/test', async () => {
await testServer.authlessAgent.get('/sso/saml/config/test').expect(401);
test('should NOT be able to access POST /sso/saml/config/test', async () => {
await testServer.authlessAgent.post('/sso/saml/config/test').expect(401);
});
});
});
Expand Down
4 changes: 4 additions & 0 deletions packages/frontend/@n8n/i18n/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -4082,6 +4082,10 @@
"settings.sso.settings.userRoleProvisioning.option.instanceAndProjectRoles.label": "Instance and project roles",
"settings.sso.settings.test": "Test settings",
"settings.sso.settings.save": "Save settings",
"settings.sso.settings.unsavedChanges.title": "Unsaved changes",
"settings.sso.settings.unsavedChanges.message": "You have unsaved changes. They'll be lost if you leave.",
"settings.sso.settings.unsavedChanges.saveAndLeave": "Save and leave",
"settings.sso.settings.unsavedChanges.leaveWithoutSaving": "Leave without saving",
"settings.sso.settings.save.testConnection.title": "Test and activate SAML SSO",
"settings.sso.settings.save.testConnection.message": "You are about to activate SSO via SAML. Test your SAML SSO settings first before proceeding.",
"settings.sso.settings.save.testConnection.test": "Test settings",
Expand Down
7 changes: 5 additions & 2 deletions packages/frontend/@n8n/rest-api-client/src/api/sso.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ export const toggleSamlConfig = async (
return await makeRestApiRequest(context, 'POST', '/sso/saml/config/toggle', data);
};

export const testSamlConfig = async (context: IRestApiContext): Promise<string> => {
return await makeRestApiRequest(context, 'GET', '/sso/saml/config/test');
export const testSamlConfig = async (
context: IRestApiContext,
data: Partial<SamlPreferences>,
): Promise<string> => {
return await makeRestApiRequest(context, 'POST', '/sso/saml/config/test', data);
};

export const getOidcConfig = async (context: IRestApiContext): Promise<OidcConfigDto> => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ function sendTrackingEvent(config: OidcConfigDto) {
telemetry.track('User updated single sign on settings', trackingMetadata);
}

const hasUnsavedChanges = computed(() => !cannotSaveOidcSettings.value && !savingForm.value);

defineExpose({ hasUnsavedChanges, onSave: onOidcSettingsSave });

onMounted(async () => {
await loadOidcConfig();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,11 @@ const onSave = async (provisioningChangesConfirmed: boolean = false) => {

const onTest = async () => {
try {
const url = await ssoStore.testSamlConfig();
const metaDataConfig: Partial<SamlPreferences> =
ipsType.value === IdentityProviderSettingsType.URL
? { metadataUrl: metadataUrl.value }
: { metadata: metadata.value };
const url = await ssoStore.testSamlConfig(metaDataConfig);

if (typeof window !== 'undefined') {
window.open(url, '_blank');
Expand Down Expand Up @@ -270,6 +274,10 @@ const validateSamlInput = () => {
}
};

const hasUnsavedChanges = computed(() => isSaveEnabled.value);

defineExpose({ hasUnsavedChanges, onSave });

onMounted(async () => {
await loadSamlConfig();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ export const useSSOStore = defineStore('sso', () => {
const saveSamlConfig = async (config: Partial<SamlPreferences>) =>
await ssoApi.saveSamlConfig(rootStore.restApiContext, config);

const testSamlConfig = async () => await ssoApi.testSamlConfig(rootStore.restApiContext);
const testSamlConfig = async (config: Partial<SamlPreferences>) =>
await ssoApi.testSamlConfig(rootStore.restApiContext, config);

/**
* OIDC
Expand Down
Loading