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.

Sign up for GitHub

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

Jump to bottom

Improve the typing of the Strategy class hierarchy. #554

Merged
merged 1 commit into from Apr 5, 2021
Merged

Improve the typing of the Strategy class hierarchy. #554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/passport-saml/index.ts
View file
Open in desktop
@@ -1,7 +1,8 @@
import type { CacheItem, CacheProvider } from "../node-saml/inmemory-cache-provider";
import { SAML } from "../node-saml";
import Strategy = require("./strategy");
import MultiSamlStrategy = require("./multiSamlStrategy");
import { Strategy, AbstractStrategy } from "./strategy";
import { MultiSamlStrategy } from "./multiSamlStrategy";

import type {
Profile,
SamlConfig,
Expand All @@ -12,6 +13,7 @@ import type {

export {
SAML,
AbstractStrategy,
Strategy,
MultiSamlStrategy,
CacheItem,
Expand Down
11 changes: 3 additions & 8 deletions src/passport-saml/multiSamlStrategy.ts
View file
Open in desktop
@@ -1,6 +1,5 @@
import * as util from "util";
import { SAML } from "../node-saml";
import SamlStrategy = require("./strategy");
import { AbstractStrategy } from "./strategy";
import type { Request } from "express";
import {
AuthenticateOptions,
Expand All @@ -11,9 +10,8 @@ import {
VerifyWithRequest,
} from "./types";

class MultiSamlStrategy extends SamlStrategy {
export class MultiSamlStrategy extends AbstractStrategy {
static readonly newSamlProviderOnConstruct = false;

_options: SamlConfig & MultiSamlConfig;

constructor(options: MultiSamlConfig, verify: VerifyWithRequest);
Expand Down Expand Up @@ -63,7 +61,6 @@ class MultiSamlStrategy extends SamlStrategy {
});
}

/** @ts-expect-error typescript disallows changing method signature in a subclass */
generateServiceProviderMetadata(
req: Request,
decryptionCert: string | null,
Expand All @@ -84,7 +81,7 @@ class MultiSamlStrategy extends SamlStrategy {
Object.setPrototypeOf(strategy, this);
return callback(
null,
super.generateServiceProviderMetadata.call(strategy, decryptionCert, signingCert)
this._generateServiceProviderMetadata.call(strategy, decryptionCert, signingCert)
);
});
}
Expand All @@ -94,5 +91,3 @@ class MultiSamlStrategy extends SamlStrategy {
super.error(err);
}
}

export = MultiSamlStrategy;
17 changes: 13 additions & 4 deletions src/passport-saml/strategy.ts
View file
Open in desktop
Expand Up @@ -10,8 +10,8 @@ import {
} from "./types";
import { Profile } from "./types";

class Strategy extends PassportStrategy {
static readonly newSamlProviderOnConstruct = true;
export abstract class AbstractStrategy extends PassportStrategy {
static readonly newSamlProviderOnConstruct: boolean;

name: string;
_verify: VerifyWithRequest | VerifyWithoutRequest;
Expand Down Expand Up @@ -178,7 +178,7 @@ class Strategy extends PassportStrategy {
.catch((err) => callback(err));
}

generateServiceProviderMetadata(
protected _generateServiceProviderMetadata(
decryptionCert: string | null,
signingCert?: string | null
): string {
Expand All @@ -195,4 +195,13 @@ class Strategy extends PassportStrategy {
}
}

export = Strategy;
export class Strategy extends AbstractStrategy {
static readonly newSamlProviderOnConstruct = true;

generateServiceProviderMetadata(
decryptionCert: string | null,
signingCert?: string | null
): string {
return this._generateServiceProviderMetadata(decryptionCert, signingCert);
}
}
12 changes: 7 additions & 5 deletions test/passport-saml/multiSamlStrategy.spec.ts
View file
Open in desktop
@@ -1,8 +1,9 @@
"use strict";
import * as express from "express";
import { Strategy } from "passport-strategy";
import * as sinon from "sinon";
import * as should from "should";
import { Strategy as SamlStrategy, MultiSamlStrategy, SAML } from "../../src/passport-saml";
import { MultiSamlStrategy, SAML, AbstractStrategy } from "../../src/passport-saml";
import {
MultiSamlConfig,
SamlOptionsCallback,
Expand All @@ -20,7 +21,8 @@ describe("MultiSamlStrategy()", function () {
return { cert: FAKE_CERT };
}
const strategy = new MultiSamlStrategy({ getSamlOptions }, noop);
strategy.should.be.an.instanceOf(SamlStrategy);
strategy.should.be.an.instanceOf(AbstractStrategy);
strategy.should.be.an.instanceOf(Strategy);
});

it("throws if wrong finder is provided", function () {
Expand All @@ -33,7 +35,7 @@ describe("MultiSamlStrategy()", function () {

describe("MultiSamlStrategy#authenticate", function () {
beforeEach(function () {
this.superAuthenticateStub = sinon.stub(SamlStrategy.prototype, "authenticate");
this.superAuthenticateStub = sinon.stub(AbstractStrategy.prototype, "authenticate");
});

afterEach(function () {
Expand Down Expand Up @@ -154,7 +156,7 @@ describe("MultiSamlStrategy#authorize", function () {

describe("MultiSamlStrategy#logout", function () {
beforeEach(function () {
this.superLogoutMock = sinon.stub(SamlStrategy.prototype, "logout");
this.superLogoutMock = sinon.stub(AbstractStrategy.prototype, "logout");
});

afterEach(function () {
Expand Down Expand Up @@ -229,7 +231,7 @@ describe("MultiSamlStrategy#logout", function () {
describe("MultiSamlStrategy#generateServiceProviderMetadata", function () {
beforeEach(function () {
this.superGenerateServiceProviderMetadata = sinon
.stub(SamlStrategy.prototype, "generateServiceProviderMetadata")
.stub(SAML.prototype, "generateServiceProviderMetadata")
.returns("My Metadata Result");
});

Expand Down