Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up types #813

Merged
merged 6 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/index.ts
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
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
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
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
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
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
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