From f0b7687db0bf06ca8a6edcbb8c3c6928a68bf1b5 Mon Sep 17 00:00:00 2001 From: ganeshrajsekar <106777671+ganeshrajsekar@users.noreply.github.com> Date: Wed, 29 Jun 2022 14:11:33 -0400 Subject: [PATCH] [ULP-3649][ULP-3712] Fix: URL fragments in SAML Sign in URLs (#171) * fix: SAML request url construction with query and fragment * fix: Add tests for SAML request URL construction * fix: Change order of split for URL parsing and add tests Co-authored-by: Ganesh Rajasekar --- lib/passport-wsfed-saml2/samlp.js | 9 ++++++++- package.json | 2 +- test/samlp.tests.js | 32 +++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/passport-wsfed-saml2/samlp.js b/lib/passport-wsfed-saml2/samlp.js index 3683b2c..bdcd953 100644 --- a/lib/passport-wsfed-saml2/samlp.js +++ b/lib/passport-wsfed-saml2/samlp.js @@ -245,7 +245,10 @@ Samlp.prototype = { if (err) return callback(err); var parsedUrl = url.parse(options.identityProviderUrl, true); - var samlRequestUrl = options.identityProviderUrl.split('?')[0] + '?' + qs.encode(xtend(parsedUrl.query, params)); + var samlRequestUrl = stripQueryAndFragmentFromURL(options.identityProviderUrl) + '?' + qs.encode(xtend(parsedUrl.query, params)); + if (parsedUrl.hash !== null) { + samlRequestUrl += parsedUrl.hash; + } return callback(null, samlRequestUrl); }); }, @@ -501,3 +504,7 @@ function generateInstant() { var date = new Date(); return date.getUTCFullYear() + '-' + ('0' + (date.getUTCMonth()+1)).slice(-2) + '-' + ('0' + date.getUTCDate()).slice(-2) + 'T' + ('0' + date.getUTCHours()).slice(-2) + ":" + ('0' + date.getUTCMinutes()).slice(-2) + ":" + ('0' + date.getUTCSeconds()).slice(-2) + "Z"; } + +function stripQueryAndFragmentFromURL(url) { + return url.split("#")[0].split("?")[0]; +} diff --git a/package.json b/package.json index 6d0815a..226848a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "passport-wsfed-saml2", - "version": "4.6.1", + "version": "4.6.2", "description": "SAML2 Protocol and WS-Fed library", "scripts": { "test": "./node_modules/.bin/mocha --recursive", diff --git a/test/samlp.tests.js b/test/samlp.tests.js index 302a14a..87f5cf7 100644 --- a/test/samlp.tests.js +++ b/test/samlp.tests.js @@ -653,6 +653,38 @@ describe('samlp (unit tests)', function () { done(); }); }); + it('should return URL fragment at the end after parsing', function(done) { + var options = {identityProviderUrl: 'https://example.com/#Test'}; + this.samlp.getSamlRequestUrl(options, function(err, result) { + expect(err).to.not.exist; + expect(result).to.match(/^https:\/\/example.com\/\?SAMLRequest=.*&RelayState=.*\#Test/); + done(); + }); + }); + it('should return multiple URL fragments at the end after parsing', function(done) { + var options = {identityProviderUrl: 'https://example.com/#param1=value1¶m2=value'}; + this.samlp.getSamlRequestUrl(options, function(err, result) { + expect(err).to.not.exist; + expect(result).to.match(/^https:\/\/example.com\/\?SAMLRequest=.*&RelayState=.*\#param1=value1¶m2=value/); + done(); + }); + }); + it('should return URL query and fragments in correct order after parsing', function(done) { + var options = {identityProviderUrl: 'https://example.com/?ABC=123#Test'}; + this.samlp.getSamlRequestUrl(options, function(err, result) { + expect(err).to.not.exist; + expect(result).to.match(/^https:\/\/example.com\/\?ABC=123&SAMLRequest=.*&RelayState=.*\#Test/); + done(); + }); + }); + it('should return URL query and multiple fragments in correct order after parsing', function(done) { + var options = {identityProviderUrl: 'https://example.com/?ABC=123#param1=value1¶m2=value'}; + this.samlp.getSamlRequestUrl(options, function(err, result) { + expect(err).to.not.exist; + expect(result).to.match(/^https:\/\/example.com\/\?ABC=123&SAMLRequest=.*&RelayState=.*\#param1=value1¶m2=value/); + done(); + }); + }); }); describe('getSamlStatus', function(){