Skip to content

Commit

Permalink
Clean up types (node-saml#813)
Browse files Browse the repository at this point in the history
  • Loading branch information
@cjbarth
cjbarth authored Jun 13, 2023
1 parent a14f25a commit 930082a
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 58 deletions.
2 changes: 2 additions & 0 deletions src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
VerifyWithRequest,
VerifyWithoutRequest,
MultiStrategyConfig,
PassportSamlConfig,
} from "./types";

export * from "@node-saml/node-saml";
Expand All @@ -18,4 +19,5 @@ export {
VerifyWithRequest,
VerifyWithoutRequest,
MultiStrategyConfig,
PassportSamlConfig,
};
7 changes: 4 additions & 3 deletions src/multiSamlStrategy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import {
AuthenticateOptions,
MultiStrategyConfig,
RequestWithUser,
PassportSamlConfig,
VerifyWithoutRequest,
VerifyWithRequest,
} from "./types";
import { SAML, SamlConfig } from "@node-saml/node-saml";
import { SAML } from ".";

export class MultiSamlStrategy extends AbstractStrategy {
static readonly newSamlProviderOnConstruct = false;
_options: SamlConfig & MultiStrategyConfig;
_options: PassportSamlConfig & MultiStrategyConfig;

constructor(
options: MultiStrategyConfig,
Expand All @@ -33,7 +34,7 @@ export class MultiSamlStrategy extends AbstractStrategy {
// and there are defaults for all `strategy`-required options.
const samlConfig = {
...options,
} as SamlConfig & MultiStrategyConfig;
} as PassportSamlConfig & MultiStrategyConfig;

super(samlConfig, signonVerify, logoutVerify);
this._options = samlConfig;
Expand Down
9 changes: 5 additions & 4 deletions src/strategy.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Strategy as PassportStrategy } from "passport-strategy";
import { strict as assert } from "assert";
import * as url from "url";
import { Profile, SAML, SamlConfig } from ".";
import { Profile, SAML } from ".";
import { PassportSamlConfig } from "./types";
import {
AuthenticateOptions,
RequestWithUser,
Expand All @@ -22,16 +23,16 @@ export abstract class AbstractStrategy extends PassportStrategy {
_passReqToCallback?: boolean;

constructor(
options: SamlConfig,
options: PassportSamlConfig,
signonVerify: VerifyWithRequest,
logoutVerify: VerifyWithRequest
);
constructor(
options: SamlConfig,
options: PassportSamlConfig,
signonVerify: VerifyWithoutRequest,
logoutVerify: VerifyWithoutRequest
);
constructor(options: SamlConfig, signonVerify: never, logoutVerify: never) {
constructor(options: PassportSamlConfig, signonVerify: never, logoutVerify: never) {
super();
if (typeof options === "function") {
throw new Error("Mandatory SAML options missing");
Expand Down
10 changes: 7 additions & 3 deletions src/types.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type * as express from "express";
import * as passport from "passport";
import { Profile, SamlConfig } from "@node-saml/node-saml";
import { Profile, SamlConfig } from ".";

export interface AuthenticateOptions extends passport.AuthenticateOptions {
samlFallback?: "login-request" | "logout-request";
Expand Down Expand Up @@ -37,13 +37,17 @@ export type VerifyWithRequest = (

export type VerifyWithoutRequest = (profile: Profile | null, done: VerifiedCallback) => void;

export type StrategyOptionsCallback = (err: Error | null, samlOptions?: SamlConfig) => void;
export type PassportSamlConfig = SamlConfig & StrategyOptions;

export type StrategyOptionsCallback = (err: Error | null, samlOptions?: PassportSamlConfig) => void;

interface BaseMultiStrategyConfig {
getSamlOptions(req: express.Request, callback: StrategyOptionsCallback): void;
}

export type MultiStrategyConfig = Partial<SamlConfig> & StrategyOptions & BaseMultiStrategyConfig;
export type MultiStrategyConfig = Partial<PassportSamlConfig> &
StrategyOptions &
BaseMultiStrategyConfig;

export class ErrorWithXmlStatus extends Error {
constructor(message: string, public readonly xmlStatus: string) {
Expand Down
73 changes: 42 additions & 31 deletions test/multiSamlStrategy.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@ import * as express from "express";
import { Strategy } from "passport-strategy";
import * as sinon from "sinon";
import { expect } from "chai";
import { MultiSamlStrategy, SAML, AbstractStrategy, SamlConfig } from "../src";
import { MultiStrategyConfig, RequestWithUser, StrategyOptionsCallback } from "../src/types";
import assert = require("assert");
import { MultiSamlStrategy, SAML, AbstractStrategy } from "../src";
import {
MultiStrategyConfig,
RequestWithUser,
StrategyOptionsCallback,
PassportSamlConfig,
} from "../src/types";
import * as assert from "assert";
import { FAKE_CERT } from "./types";

const noop = () => undefined;

describe("MultiSamlStrategy()", function () {
it("extends passport Strategy", function () {
function getSamlOptions(): SamlConfig {
function getSamlOptions(): PassportSamlConfig {
return { cert: FAKE_CERT, issuer: "onesaml_login" };
}
const strategy = new MultiSamlStrategy({ getSamlOptions }, noop, noop);
Expand Down Expand Up @@ -55,7 +60,8 @@ describe("MultiSamlStrategy()", function () {
noop,
noop
);
strategy.authenticate("random" as any, "random" as any);
// @ts-expect-error
strategy.authenticate("random", "random");
});

it("passes options on to saml strategy", function (done) {
Expand All @@ -64,7 +70,7 @@ describe("MultiSamlStrategy()", function () {
getSamlOptions: function (req: express.Request, fn: StrategyOptionsCallback) {
try {
fn(null, { cert: FAKE_CERT, issuer: "onesaml_login" });
expect(strategy._passReqToCallback!).to.equal(true);
expect(strategy._passReqToCallback).to.equal(true);
done();
} catch (err2) {
done(err2);
Expand All @@ -73,12 +79,13 @@ describe("MultiSamlStrategy()", function () {
};

const strategy = new MultiSamlStrategy(passportOptions, noop, noop);
strategy.authenticate("random" as any, "random" as any);
// @ts-expect-error
strategy.authenticate("random", "random");
});

it("uses given options to setup internal saml provider", function (done) {
const superAuthenticateStub = this.superAuthenticateStub;
const samlOptions: SamlConfig = {
const samlOptions: PassportSamlConfig = {
issuer: "http://foo.issuer",
callbackUrl: "http://foo.callback",
cert: "deadbeef",
Expand All @@ -104,11 +111,13 @@ describe("MultiSamlStrategy()", function () {
}

const strategy = new MultiSamlStrategy(
{ getSamlOptions, cacheProvider: "mock cache provider" as any },
// @ts-expect-error
{ getSamlOptions, cacheProvider: "mock cache provider" },
noop,
noop
);
strategy.authenticate("random" as any, "random" as any);
// @ts-expect-error
strategy.authenticate("random", "random");
});
});

Expand Down Expand Up @@ -172,7 +181,8 @@ describe("MultiSamlStrategy()", function () {
}

const strategy = new MultiSamlStrategy({ getSamlOptions }, noop, noop);
strategy.logout("random" as any, "random" as any);
// @ts-expect-error
strategy.logout("random", "random");
});

it("passes options on to saml strategy", function (done) {
Expand All @@ -181,7 +191,7 @@ describe("MultiSamlStrategy()", function () {
getSamlOptions: function (req: express.Request, fn: StrategyOptionsCallback) {
try {
fn(null, { cert: FAKE_CERT, issuer: "onesaml_login" });
expect(strategy._passReqToCallback!).to.equal(true);
expect(strategy._passReqToCallback).to.equal(true);
done();
} catch (err2) {
done(err2);
Expand All @@ -190,12 +200,13 @@ describe("MultiSamlStrategy()", function () {
};

const strategy = new MultiSamlStrategy(passportOptions, noop, noop);
strategy.logout("random" as any, "random" as any);
// @ts-expect-error
strategy.logout("random", "random");
});

it("uses given options to setup internal saml provider", function (done) {
const superLogoutMock = this.superLogoutMock;
const samlOptions: SamlConfig = {
const samlOptions: PassportSamlConfig = {
issuer: "http://foo.issuer",
callbackUrl: "http://foo.callback",
authnRequestBinding: "HTTP-POST",
Expand All @@ -220,7 +231,8 @@ describe("MultiSamlStrategy()", function () {
}

const strategy = new MultiSamlStrategy({ getSamlOptions }, noop, noop);
strategy.logout("random" as any, sinon.spy());
// @ts-expect-error
strategy.logout("random", sinon.spy());
});
});

Expand Down Expand Up @@ -250,7 +262,8 @@ describe("MultiSamlStrategy()", function () {
}

const strategy = new MultiSamlStrategy({ getSamlOptions }, noop, noop);
strategy.generateServiceProviderMetadata("foo" as any, "bar", "baz", noop);
// @ts-expect-error
strategy.generateServiceProviderMetadata("foo", "bar", "baz", noop);
});

it("passes options on to saml strategy", function (done) {
Expand All @@ -260,7 +273,7 @@ describe("MultiSamlStrategy()", function () {
getSamlOptions: function (req: express.Request, fn: StrategyOptionsCallback) {
try {
fn(null, { cert: FAKE_CERT, issuer: "onesaml_login" });
expect(strategy._passReqToCallback!).to.equal(true);
expect(strategy._passReqToCallback).to.equal(true);
done();
} catch (err2) {
done(err2);
Expand All @@ -269,7 +282,8 @@ describe("MultiSamlStrategy()", function () {
};

const strategy = new MultiSamlStrategy(passportOptions, noop, noop);
strategy.generateServiceProviderMetadata("foo" as any, "bar", "baz", noop);
// @ts-expect-error
strategy.generateServiceProviderMetadata("foo", "bar", "baz", noop);
});

it("should pass error to callback function", function (done) {
Expand All @@ -280,19 +294,15 @@ describe("MultiSamlStrategy()", function () {
};

const strategy = new MultiSamlStrategy(passportOptions, noop, noop);
strategy.generateServiceProviderMetadata(
"foo" as any,
"bar",
"baz",
function (error, result) {
try {
expect(error?.message).to.equal("My error");
done();
} catch (err2) {
done(err2);
}
// @ts-expect-error
strategy.generateServiceProviderMetadata("foo", "bar", "baz", function (error) {
try {
expect(error?.message).to.equal("My error");
done();
} catch (err2) {
done(err2);
}
);
});
});

it("should pass result to callback function", function (done) {
Expand All @@ -304,7 +314,8 @@ describe("MultiSamlStrategy()", function () {

const strategy = new MultiSamlStrategy(passportOptions, noop, noop);
strategy.generateServiceProviderMetadata(
"foo" as any,
// @ts-expect-error
"foo",
"bar",
"baz",
function (error, result) {
Expand Down
32 changes: 17 additions & 15 deletions test/strategy.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
import type * as express from "express";
import { expect } from "chai";
import * as sinon from "sinon";
import { Profile, SAML, SamlConfig, Strategy as SamlStrategy } from "../src";
import { RequestWithUser, VerifiedCallback, VerifyWithoutRequest } from "../src/types";
import { Profile, SAML, Strategy as SamlStrategy } from "../src";
import {
RequestWithUser,
VerifiedCallback,
VerifyWithoutRequest,
PassportSamlConfig,
} from "../src/types";
import { FAKE_CERT } from "./types";

const noop = () => undefined;
Expand Down Expand Up @@ -304,10 +309,13 @@ describe("Strategy()", function () {
});

it("should call through to get logout URL", function () {
// @ts-ignore
new SamlStrategy({ cert: FAKE_CERT, issuer: "onesaml_login" }, noop, noop).logout({
query: "",
});
new SamlStrategy({ cert: FAKE_CERT, issuer: "onesaml_login" }, noop, noop).logout(
{
// @ts-expect-error
query: "",
},
noop
);
sinon.assert.calledOnce(getLogoutUrlAsyncStub);
});
});
Expand All @@ -327,18 +335,12 @@ describe("Strategy()", function () {
});

it("should call through to generate metadata", function () {
const samlConfig: SamlConfig = { cert: FAKE_CERT, issuer: "onesaml_login" };
const signonVerify: VerifyWithoutRequest = function (
_profile: Profile | null,
cb: VerifiedCallback
): void {
const samlConfig: PassportSamlConfig = { cert: FAKE_CERT, issuer: "onesaml_login" };
const signonVerify: VerifyWithoutRequest = function (): void {
throw Error("This shouldn't be called to generate metadata");
};

const logoutVerify: VerifyWithoutRequest = function (
_profile: Profile | null,
cb: VerifiedCallback
): void {
const logoutVerify: VerifyWithoutRequest = function (): void {
throw Error("This shouldn't be called to generate metadata");
};
new SamlStrategy(samlConfig, signonVerify, logoutVerify).generateServiceProviderMetadata("");
Expand Down
2 changes: 0 additions & 2 deletions test/types.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { SamlConfig } from "../src";

// a certificate which is re-used by several tests
export const TEST_CERT =
"MIIEFzCCAv+gAwIBAgIUFJsUjPM7AmWvNtEvULSHlTTMiLQwDQYJKoZIhvcNAQEFBQAwWDELMAkGA1UEBhMCVVMxETAPBgNVBAoMCFN1YnNwYWNlMRUwEwYDVQQLDAxPbmVMb2dpbiBJZFAxHzAdBgNVBAMMFk9uZUxvZ2luIEFjY291bnQgNDIzNDkwHhcNMTQwNTEzMTgwNjEyWhcNMTkwNTE0MTgwNjEyWjBYMQswCQYDVQQGEwJVUzERMA8GA1UECgwIU3Vic3BhY2UxFTATBgNVBAsMDE9uZUxvZ2luIElkUDEfMB0GA1UEAwwWT25lTG9naW4gQWNjb3VudCA0MjM0OTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKrAzJdY9FzFLt5blArJfPzgi87EnFGlTfcV5T1TUDwLBlDkY/0ZGKnMOpf3D7ie2C4pPFOImOogcM5kpDDL7qxTXZ1ewXVyjBdMu29NG2C6NzWeQTUMUji01EcHkC8o+Pts8ANiNOYcjxEeyhEyzJKgEizblYzMMKzdrOET6QuqWo3C83K+5+5dsjDn1ooKGRwj3HvgsYcFrQl9NojgQFjoobwsiE/7A+OJhLpBcy/nSVgnoJaMfrO+JsnukZPztbntLvOl56+Vra0N8n5NAYhaSayPiv/ayhjVgjfXd1tjMVTOiDknUOwizZuJ1Y3QH94vUtBgp0WBpBSs/xMyTs8CAwEAAaOB2DCB1TAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBRQO4WpM5fWwxib49WTuJkfYDbxODCBlQYDVR0jBIGNMIGKgBRQO4WpM5fWwxib49WTuJkfYDbxOKFcpFowWDELMAkGA1UEBhMCVVMxETAPBgNVBAoMCFN1YnNwYWNlMRUwEwYDVQQLDAxPbmVMb2dpbiBJZFAxHzAdBgNVBAMMFk9uZUxvZ2luIEFjY291bnQgNDIzNDmCFBSbFIzzOwJlrzbRL1C0h5U0zIi0MA4GA1UdDwEB/wQEAwIHgDANBgkqhkiG9w0BAQUFAAOCAQEACdDAAoaZFCEY5pmfwbKuKrXtO5iE8lWtiCPjCZEUuT6bXRNcqrdnuV/EAfX9WQoXjalPi0eM78zKmbvRGSTUHwWw49RHjFfeJUKvHNeNnFgTXDjEPNhMvh69kHm453lFRmB+kk6yjtXRZaQEwS8Uuo2Ot+krgNbl6oTBZJ0AHH1MtZECDloms1Km7zsK8wAi5i8TVIKkVr5b2VlhrLgFMvzZ5ViAxIMGB6w47yY4QGQB/5Q8ya9hBs9vkn+wubA+yr4j14JXZ7blVKDSTYva65Ea+PqHyrp+Wnmnbw2ObS7iWexiTy1jD3G0R2avDBFjM8Fj5DbfufsE1b0U10RTtg==";
Expand Down

0 comments on commit 930082a

Please sign in to comment.