diff --git a/src/passport-saml/saml.ts b/src/passport-saml/saml.ts index 93afc66b..988ec80a 100644 --- a/src/passport-saml/saml.ts +++ b/src/passport-saml/saml.ts @@ -198,6 +198,8 @@ class SAML { options.RACComparison = "exact"; } + options.authnRequestBinding = options.authnRequestBinding || "HTTP-Redirect"; + return options as SAMLOptions; } diff --git a/src/passport-saml/strategy.ts b/src/passport-saml/strategy.ts index 93793870..8e42287b 100644 --- a/src/passport-saml/strategy.ts +++ b/src/passport-saml/strategy.ts @@ -16,7 +16,6 @@ class Strategy extends PassportStrategy { _verify: VerifyWithRequest | VerifyWithoutRequest; _saml: saml.SAML; _passReqToCallback?: boolean; - _authnRequestBinding?: string; constructor(options: SamlConfig, verify: VerifyWithRequest | VerifyWithoutRequest) { super(); @@ -40,7 +39,6 @@ class Strategy extends PassportStrategy { this._verify = verify; this._saml = new saml.SAML(options); this._passReqToCallback = !!options.passReqToCallback; - this._authnRequestBinding = options.authnRequestBinding || "HTTP-Redirect"; } authenticate(req: RequestWithUser, options: AuthenticateOptions & AuthorizeOptions): void { @@ -101,7 +99,7 @@ class Strategy extends PassportStrategy { } else { const requestHandler = { "login-request": () => { - if (this._authnRequestBinding === "HTTP-POST") { + if (this._saml.options.authnRequestBinding === "HTTP-POST") { this._saml.getAuthorizeForm(req, (err: Error | null, data?: any) => { if (err) { this.error(err); diff --git a/src/passport-saml/types.ts b/src/passport-saml/types.ts index 854d3ed3..f9e0ceb0 100644 --- a/src/passport-saml/types.ts +++ b/src/passport-saml/types.ts @@ -39,6 +39,7 @@ export interface SAMLOptions { authnContext: string | string[]; forceAuthn: boolean; skipRequestCompression: boolean; + authnRequestBinding?: string; RACComparison: "exact" | "minimum" | "maximum" | "better"; providerName: string; passive: boolean; @@ -67,7 +68,6 @@ export type SamlConfig = Partial & StrategyOptions; interface StrategyOptions { name?: string; passReqToCallback?: boolean; - authnRequestBinding?: string; } export interface SamlScopingConfig { diff --git a/test/multiSamlStrategy.js b/test/multiSamlStrategy.js index eb5896b1..b1e7a235 100644 --- a/test/multiSamlStrategy.js +++ b/test/multiSamlStrategy.js @@ -2,12 +2,13 @@ var sinon = require("sinon"); var should = require("should"); +var saml = require("../lib/passport-saml/saml.js"); var SamlStrategy = require("../lib/passport-saml/index.js").Strategy; -var MultiSamlStrategy = require("../lib/passport-saml/multiSamlStrategy"); +var MultiSamlStrategy = require("../lib/passport-saml/multiSamlStrategy.js"); function verify() {} -describe("Strategy()", function () { +describe("MultiSamlStrategy()", function () { it("extends passport Strategy", function () { function getSamlOptions() { return {}; @@ -24,7 +25,7 @@ describe("Strategy()", function () { }); }); -describe("strategy#authenticate", function () { +describe("MultiSamlStrategy#authenticate", function () { beforeEach(function () { this.superAuthenticateStub = sinon.stub(SamlStrategy.prototype, "authenticate"); }); @@ -57,12 +58,10 @@ describe("strategy#authenticate", function () { it("passes options on to saml strategy", function (done) { var passportOptions = { passReqToCallback: true, - authnRequestBinding: "HTTP-POST", getSamlOptions: function (req, fn) { try { fn(); strategy._passReqToCallback.should.eql(true); - strategy._authnRequestBinding.should.eql("HTTP-POST"); done(); } catch (err2) { done(err2); @@ -78,6 +77,7 @@ describe("strategy#authenticate", function () { var superAuthenticateStub = this.superAuthenticateStub; var samlOptions = { issuer: "http://foo.issuer", + authnRequestBinding: "HTTP-POST", callbackUrl: "http://foo.callback", cert: "deadbeef", host: "lvh", @@ -109,7 +109,37 @@ describe("strategy#authenticate", function () { }); }); -describe("strategy#logout", function () { +describe("MultiSamlStrategy#authorize", function () { + beforeEach(function () { + this.getAuthorizeFormStub = sinon.stub(saml.SAML.prototype, "getAuthorizeForm"); + this.getAuthorizeUrlStub = sinon.stub(saml.SAML.prototype, "getAuthorizeUrl"); + }); + + afterEach(function () { + this.getAuthorizeFormStub.restore(); + this.getAuthorizeUrlStub.restore(); + }); + + it("calls getAuthorizeForm when authnRequestBinding is HTTP-POST", function () { + function getSamlOptions(req, fn) { + fn(null, { authnRequestBinding: "HTTP-POST" }); + } + var strategy = new MultiSamlStrategy({ getSamlOptions }, verify); + strategy.authenticate({}, {}); + sinon.assert.calledOnce(this.getAuthorizeFormStub); + }); + + it("calls getAuthorizeUrl when authnRequestBinding is not HTTP-POST", function () { + function getSamlOptions(req, fn) { + fn(null, {}); + } + var strategy = new MultiSamlStrategy({ getSamlOptions }, verify); + strategy.authenticate({}, {}); + sinon.assert.calledOnce(this.getAuthorizeUrlStub); + }); +}); + +describe("MultiSamlStrategy#logout", function () { beforeEach(function () { this.superLogoutMock = sinon.stub(SamlStrategy.prototype, "logout"); }); @@ -137,12 +167,10 @@ describe("strategy#logout", function () { it("passes options on to saml strategy", function (done) { var passportOptions = { passReqToCallback: true, - authnRequestBinding: "HTTP-POST", getSamlOptions: function (req, fn) { try { fn(); strategy._passReqToCallback.should.eql(true); - strategy._authnRequestBinding.should.eql("HTTP-POST"); done(); } catch (err2) { done(err2); @@ -159,6 +187,7 @@ describe("strategy#logout", function () { var samlOptions = { issuer: "http://foo.issuer", callbackUrl: "http://foo.callback", + authnRequestBinding: "HTTP-POST", cert: "deadbeef", host: "lvh", acceptedClockSkewMs: -1, @@ -184,7 +213,7 @@ describe("strategy#logout", function () { }); }); -describe("strategy#generateServiceProviderMetadata", function () { +describe("MultiSamlStrategy#generateServiceProviderMetadata", function () { beforeEach(function () { this.superGenerateServiceProviderMetadata = sinon .stub(SamlStrategy.prototype, "generateServiceProviderMetadata") @@ -216,12 +245,10 @@ describe("strategy#generateServiceProviderMetadata", function () { it("passes options on to saml strategy", function (done) { var passportOptions = { passReqToCallback: true, - authnRequestBinding: "HTTP-POST", getSamlOptions: function (req, fn) { try { fn(); strategy._passReqToCallback.should.eql(true); - strategy._authnRequestBinding.should.eql("HTTP-POST"); done(); } catch (err2) { done(err2); diff --git a/test/strategy.js b/test/strategy.js new file mode 100644 index 00000000..06f9c4eb --- /dev/null +++ b/test/strategy.js @@ -0,0 +1,36 @@ +"use strict"; + +var sinon = require("sinon"); +var saml = require("../lib/passport-saml/saml.js"); +var SamlStrategy = require("../lib/passport-saml/index.js").Strategy; + +function verify() {} + +describe("strategy#authorize", function () { + beforeEach(function () { + this.getAuthorizeFormStub = sinon.stub(saml.SAML.prototype, "getAuthorizeForm"); + this.getAuthorizeUrlStub = sinon.stub(saml.SAML.prototype, "getAuthorizeUrl"); + }); + + afterEach(function () { + this.getAuthorizeFormStub.restore(); + this.getAuthorizeUrlStub.restore(); + }); + + it("calls getAuthorizeForm when authnRequestBinding is HTTP-POST", function () { + var strategy = new SamlStrategy( + { + authnRequestBinding: "HTTP-POST", + }, + verify + ); + strategy.authenticate({}, {}); + sinon.assert.calledOnce(this.getAuthorizeFormStub); + }); + + it("calls getAuthorizeUrl when authnRequestBinding is not HTTP-POST", function () { + var strategy = new SamlStrategy({}, verify); + strategy.authenticate({}, {}); + sinon.assert.calledOnce(this.getAuthorizeUrlStub); + }); +});