Skip to content

Commit

Permalink
Allow for authnRequestBinding in SAML options (#529)
Browse files Browse the repository at this point in the history 
Co-authored-by: Mike Hesler <mhesler@grapevine6.com>
Co-authored-by: Mike Hesler <mhesler@seismic.com>
Co-authored-by: Chris Barth <chrisjbarth@hotmail.com>
  • Loading branch information
@mhesler74 @cjbarth
4 people committed Feb 15, 2021
1 parent 54704de commit ed4be0c
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 15 deletions.
2 changes: 2 additions & 0 deletions src/passport-saml/saml.ts
View file
Expand Up @@ -198,6 +198,8 @@ class SAML {
options.RACComparison = "exact";
}

options.authnRequestBinding = options.authnRequestBinding || "HTTP-Redirect";

return options as SAMLOptions;
}

Expand Down
4 changes: 1 addition & 3 deletions src/passport-saml/strategy.ts
View file
Expand Up @@ -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();
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/passport-saml/types.ts
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -67,7 +68,6 @@ export type SamlConfig = Partial<SAMLOptions> & StrategyOptions;
interface StrategyOptions {
name?: string;
passReqToCallback?: boolean;
authnRequestBinding?: string;
}

export interface SamlScopingConfig {
Expand Down
49 changes: 38 additions & 11 deletions test/multiSamlStrategy.js
View file
Expand Up @@ -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 {};
Expand All @@ -24,7 +25,7 @@ describe("Strategy()", function () {
});
});

describe("strategy#authenticate", function () {
describe("MultiSamlStrategy#authenticate", function () {
beforeEach(function () {
this.superAuthenticateStub = sinon.stub(SamlStrategy.prototype, "authenticate");
});
Expand Down Expand Up @@ -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);
Expand All @@ -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",
Expand Down Expand Up @@ -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");
});
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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")
Expand Down Expand Up @@ -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);
Expand Down
36 changes: 36 additions & 0 deletions test/strategy.js
View file
@@ -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);
});
});

0 comments on commit ed4be0c

Please sign in to comment.