From 09bc7299e52ba496fd93a8ff90e8c3ae242ee9bd Mon Sep 17 00:00:00 2001 From: Alexander Bieber Date: Fri, 14 Oct 2022 13:37:12 +0200 Subject: [PATCH 1/4] SAML: allowing idpOptions sso_login_url and sso_logout_url to be parameterized with parameters that were passed to the authorization request. --- src/auth/src/common/generic-router.ts | 10 +++++-- src/auth/src/common/types.ts | 4 ++- src/auth/src/providers/saml.ts | 39 ++++++++++++++++++--------- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/auth/src/common/generic-router.ts b/src/auth/src/common/generic-router.ts index d0f2212a..6a8b2d56 100644 --- a/src/auth/src/common/generic-router.ts +++ b/src/auth/src/common/generic-router.ts @@ -452,6 +452,13 @@ export class GenericOAuth2Router { // Support prefilled username authRequest.prefill_username = givenPrefillUsername; + authRequest.options = {}; + Object.keys(req.query).forEach(key => { + if (key.startsWith("x_")) { + authRequest.options[key.substring(2)] = req.query[key]; + } + }) + // Validate parameters first now (TODO: This is pbly feasible centrally, // it will be the same for all Auth Methods). let subscriptionInfo: WickedSubscriptionInfo; @@ -992,8 +999,7 @@ export class GenericOAuth2Router { const scopeRequest: PassthroughScopeRequest = { scope: scope, auth_method: this.authMethodId, - profile: profile, - data: data + profile: profile }; debug(JSON.stringify(scopeRequest)); async.retry({ diff --git a/src/auth/src/common/types.ts b/src/auth/src/common/types.ts index 881024d6..e4ff8dba 100644 --- a/src/auth/src/common/types.ts +++ b/src/auth/src/common/types.ts @@ -54,7 +54,9 @@ export interface AuthRequest extends OAuth2Request { prefill_username?: string, validNamespaces?: string[], // Used in the SAML case - requestId?: string + requestId?: string, + // additional parameters passed to the authorization request + options?: any } export interface AuthRequestCallback { diff --git a/src/auth/src/providers/saml.ts b/src/auth/src/providers/saml.ts index d6b7f9d1..3fa38015 100644 --- a/src/auth/src/providers/saml.ts +++ b/src/auth/src/providers/saml.ts @@ -24,7 +24,7 @@ export class SamlIdP implements IdentityProvider { private authMethodConfig: SamlIdpConfig; private serviceProvider: any; - private identityProvider: any; + private identityProviders: any; constructor(basePath: string, authMethodId: string, authMethodConfig: any, options: IdpOptions) { debug(`constructor(${basePath}, ${authMethodId},...)`); @@ -54,7 +54,7 @@ export class SamlIdP implements IdentityProvider { this.authMethodConfig.spOptions.entity_id = entityUrl; this.serviceProvider = new saml2.ServiceProvider(authMethodConfig.spOptions); - this.identityProvider = new saml2.IdentityProvider(authMethodConfig.idpOptions); + this.identityProviders = {}; this.genericFlow.initIdP(this); } @@ -73,6 +73,20 @@ export class SamlIdP implements IdentityProvider { return this.genericFlow.getRouter(); } + private getIdentityProvider(req): any { + const authRequest = utils.getAuthRequest(req, this.authMethodId); + const key = authRequest.options ? JSON.stringify(authRequest.options) : ""; + if (!this.identityProviders[key]) { + const clonedIdpOptions = Object.assign({}, this.authMethodConfig.idpOptions); + clonedIdpOptions.sso_login_url = mustache.render(clonedIdpOptions.sso_login_url, authRequest); + if (clonedIdpOptions.sso_logout_url) { + clonedIdpOptions.sso_logout_url = mustache.render(clonedIdpOptions.sso_logout_url, authRequest); + } + this.identityProviders[key] = new saml2.IdentityProvider(clonedIdpOptions) + } + return this.identityProviders[key]; + } + /** * In case the user isn't already authenticated, this method will * be called from the generic flow implementation. It is assumed to @@ -108,7 +122,7 @@ export class SamlIdP implements IdentityProvider { } options.is_passive = true; } - this.serviceProvider.create_login_request_url(this.identityProvider, options, function (err, loginUrl, requestId) { + this.serviceProvider.create_login_request_url(this.getIdentityProvider(req), options, function (err, loginUrl, requestId) { if (err) return failError(500, err, next); // Remember the request ID @@ -134,13 +148,14 @@ export class SamlIdP implements IdentityProvider { const instance = this; try { const authResponse = utils.getAuthResponse(req, instance.authMethodId) as SamlAuthResponse; + const identityProvider = instance.getIdentityProvider(req); const options: any = { name_id: authResponse.name_id, session_index: authResponse.session_index }; // Check that the identityProvider is correctly configured - if (!instance.identityProvider.sso_logout_url) { + if (!identityProvider.sso_logout_url) { next(makeError('The SAML configuration does not contain an sso_logout_url.', 500)); return true; } @@ -151,7 +166,7 @@ export class SamlIdP implements IdentityProvider { if (redirect_uri) options.relay_state = Buffer.from(redirect_uri).toString('base64'); instance.serviceProvider.create_logout_request_url( - instance.identityProvider, + identityProvider, options, function (err, logoutUrl) { if (err) { @@ -273,7 +288,7 @@ export class SamlIdP implements IdentityProvider { request_body: req.query }; const relay_state = req.query.RelayState; - instance.serviceProvider.redirect_assert(instance.identityProvider, options, function (err, response) { + instance.serviceProvider.redirect_assert(instance.getIdentityProvider(req), options, function (err, response) { if (err) return next(err); debug(response); @@ -281,7 +296,7 @@ export class SamlIdP implements IdentityProvider { // IdP initiated logout debug('SAML: logout_request'); const in_response_to = response && response.response_header ? response.response_header.in_response_to : null; - instance.getLogoutResponseUrl(in_response_to, relay_state, function (err, redirectUrl) { + instance.getLogoutResponseUrl(req, in_response_to, relay_state, function (err, redirectUrl) { if (err) return next(err); info(redirectUrl); @@ -349,14 +364,14 @@ export class SamlIdP implements IdentityProvider { }); }; - private getLogoutResponseUrl(inResponseTo, relayState, callback) { + private getLogoutResponseUrl(req, inResponseTo, relayState, callback) { debug('getLogoutResponseUrl'); const instance = this; - if (!instance.identityProvider.sso_logout_url) { + const identityProvider = instance.getIdentityProvider(req); + if (!identityProvider.sso_logout_url) { return callback(makeError('The SAML configuration (identityProvider) does not contain an sso_logout_url.', 500)); } - this.serviceProvider.create_logout_response_url( - instance.identityProvider, + this.serviceProvider.create_logout_response_url(identityProvider, { in_response_to: inResponseTo, relay_state: relayState }, function (err, logoutResponseUrl) { if (err) { @@ -375,7 +390,7 @@ export class SamlIdP implements IdentityProvider { return callback(new Error('assert needs a requestId to verify the SAML assertion.')); const options = { request_body: req.body }; - this.serviceProvider.post_assert(this.identityProvider, options, function (err, samlResponse) { + this.serviceProvider.post_assert(this.getIdentityProvider(req), options, function (err, samlResponse) { if (err) { error('post_assert failed.'); return callback(err); From 05cc84ddc10be6fcf8b34a9ce1b76e17ab55bc14 Mon Sep 17 00:00:00 2001 From: Alexander Bieber Date: Fri, 14 Oct 2022 14:21:24 +0200 Subject: [PATCH 2/4] Added validation/allowed format for additional authorization request parameters. --- src/auth/src/common/generic-router.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/auth/src/common/generic-router.ts b/src/auth/src/common/generic-router.ts index 6a8b2d56..2b091c62 100644 --- a/src/auth/src/common/generic-router.ts +++ b/src/auth/src/common/generic-router.ts @@ -24,6 +24,7 @@ import { GrantManager } from './grant-manager'; const ERROR_TIMEOUT = 500; // ms const EXTERNAL_URL_INTERVAL = 500; const EXTERNAL_URL_RETRIES = 10; +const ADDITIONAL_AUTHORIZE_OPTION_VALUES_REGEX = /^([a-zA-Z0-9][a-zA-Z0-9-_.]+)$/; export class GenericOAuth2Router { @@ -454,8 +455,9 @@ export class GenericOAuth2Router { authRequest.options = {}; Object.keys(req.query).forEach(key => { - if (key.startsWith("x_")) { - authRequest.options[key.substring(2)] = req.query[key]; + let value = req.query[key]; + if (key.startsWith("x_") && ADDITIONAL_AUTHORIZE_OPTION_VALUES_REGEX.test(value)) { + authRequest.options[key.substring(2)] = value; } }) From d3ab5cd865b48ae16c6cbb44656dcf6d055a87f2 Mon Sep 17 00:00:00 2001 From: Alexander Bieber Date: Fri, 14 Oct 2022 14:35:36 +0200 Subject: [PATCH 3/4] SAML: parameterizable sso urls better keys for intrnal IdentityProvider map. --- src/auth/src/providers/saml.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth/src/providers/saml.ts b/src/auth/src/providers/saml.ts index 3fa38015..ef949880 100644 --- a/src/auth/src/providers/saml.ts +++ b/src/auth/src/providers/saml.ts @@ -75,7 +75,7 @@ export class SamlIdP implements IdentityProvider { private getIdentityProvider(req): any { const authRequest = utils.getAuthRequest(req, this.authMethodId); - const key = authRequest.options ? JSON.stringify(authRequest.options) : ""; + const key = (authRequest.options && Object.keys(authRequest.options).length > 0) ? JSON.stringify(authRequest.options) : "global"; if (!this.identityProviders[key]) { const clonedIdpOptions = Object.assign({}, this.authMethodConfig.idpOptions); clonedIdpOptions.sso_login_url = mustache.render(clonedIdpOptions.sso_login_url, authRequest); From cb2f3ccdf702359fc40ffd808866e36d6df1f8ae Mon Sep 17 00:00:00 2001 From: Alexander Bieber Date: Fri, 14 Oct 2022 14:40:01 +0200 Subject: [PATCH 4/4] SAML: Restored accidentally removed code --- src/auth/src/common/generic-router.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/auth/src/common/generic-router.ts b/src/auth/src/common/generic-router.ts index 2b091c62..9cf011aa 100644 --- a/src/auth/src/common/generic-router.ts +++ b/src/auth/src/common/generic-router.ts @@ -1001,7 +1001,8 @@ export class GenericOAuth2Router { const scopeRequest: PassthroughScopeRequest = { scope: scope, auth_method: this.authMethodId, - profile: profile + profile: profile, + data: data }; debug(JSON.stringify(scopeRequest)); async.retry({