From 85af8d1bbf2266d862996f636b19a29805fcb2a6 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 21 Sep 2018 22:06:46 +0000 Subject: [PATCH] [next] Permit (#1899) * feat: replaced custom auth extraction logic with permit * fix: fixed tests --- package-lock.json | 5 ++ package.json | 1 + .../app/middleware/passport/jwt.spec.ts | 51 ++++--------------- .../server/app/middleware/passport/jwt.ts | 33 +++--------- src/types/permit.d.ts | 31 +++++++++++ 5 files changed, 53 insertions(+), 68 deletions(-) create mode 100644 src/types/permit.d.ts diff --git a/package-lock.json b/package-lock.json index 426a1dda3..b223ee4e8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18041,6 +18041,11 @@ "resolved": "https://registry.npmjs.org/performance-now/-/performance-now-2.1.0.tgz", "integrity": "sha1-Ywn04OX6kT7BxpMHrjZLSzd8nns=" }, + "permit": { + "version": "0.2.4", + "resolved": "https://registry.npmjs.org/permit/-/permit-0.2.4.tgz", + "integrity": "sha512-Mp2XTEMD3mPsZIWq3bp0claE4IxXKa4C6nhSDPZgGri8Q4CLjEjAQrP/xGKq2548a2KFENmA1V7W0Lob8kTuzw==" + }, "pify": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/pify/-/pify-3.0.0.tgz", diff --git a/package.json b/package.json index 355e5db4d..d7508b3da 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "passport-oauth2": "^1.4.0", "passport-strategy": "^1.0.0", "performance-now": "^2.1.0", + "permit": "^0.2.4", "subscriptions-transport-ws": "^0.9.12", "tlds": "^1.203.1", "uuid": "^3.3.2" diff --git a/src/core/server/app/middleware/passport/jwt.spec.ts b/src/core/server/app/middleware/passport/jwt.spec.ts index 3c25db7a4..6078b35f3 100644 --- a/src/core/server/app/middleware/passport/jwt.spec.ts +++ b/src/core/server/app/middleware/passport/jwt.spec.ts @@ -4,65 +4,34 @@ import { Config } from "talk-common/config"; import { createJWTSigningConfig, extractJWTFromRequest, - parseAuthHeader, } from "talk-server/app/middleware/passport/jwt"; import { Request } from "talk-server/types/express"; -describe("parseAuthHeader", () => { - it("parses valid headers", () => { - const parsed = { - scheme: "bearer", - value: "token", - }; - - expect(parseAuthHeader("Bearer token")).toEqual(parsed); - - expect(parseAuthHeader("bearer token")).toEqual(parsed); - - expect(parseAuthHeader("bearer token")).toEqual(parsed); - }); - - it("parses invalid headers", () => { - expect(parseAuthHeader("this-is-a-wrong-header")).toEqual(null); - expect(parseAuthHeader("bearerthis-is-a-wrong-header")).toEqual(null); - }); -}); - describe("extractJWTFromRequest", () => { it("extracts the token from header", () => { const req = { - get: sinon - .stub() - .withArgs("authorization") - .returns("Bearer token"), + headers: { + authorization: "Bearer token", + }, + url: "", }; expect(extractJWTFromRequest((req as any) as Request)).toEqual("token"); - expect(req.get.calledOnce).toBeTruthy(); - req.get.reset(); - req.get.returns(null); + delete req.headers.authorization; + expect(extractJWTFromRequest((req as any) as Request)).toEqual(null); - expect(req.get.calledOnce).toBeTruthy(); }); it("extracts the token from query string", () => { const req = { - get: sinon - .stub() - .withArgs("authorization") - .returns(null), - query: { access_token: "token" }, + url: "", }; + expect(extractJWTFromRequest((req as any) as Request)).toEqual(null); + + req.url = "https://talk.coralproject.net/api?access_token=token"; expect(extractJWTFromRequest((req as any) as Request)).toEqual("token"); - expect(req.get.calledOnce).toBeTruthy(); - - delete req.query.access_token; - - req.get.reset(); - expect(extractJWTFromRequest((req as any) as Request)).toEqual(null); - expect(req.get.calledOnce).toBeTruthy(); }); }); diff --git a/src/core/server/app/middleware/passport/jwt.ts b/src/core/server/app/middleware/passport/jwt.ts index 3736b6e14..0359ef450 100644 --- a/src/core/server/app/middleware/passport/jwt.ts +++ b/src/core/server/app/middleware/passport/jwt.ts @@ -2,41 +2,20 @@ import { Redis } from "ioredis"; import jwt, { SignOptions } from "jsonwebtoken"; import { Db } from "mongodb"; import { Strategy } from "passport-strategy"; +import { Bearer } from "permit"; import uuid from "uuid"; import { Config } from "talk-common/config"; import { retrieveUser, User } from "talk-server/models/user"; import { Request } from "talk-server/types/express"; -const authHeaderRegex = /(\S+)\s+(\S+)/; - -export function parseAuthHeader(header: string) { - const matches = header.match(authHeaderRegex); - if (!matches || matches.length < 3) { - return null; - } - - return { - scheme: matches[1].toLowerCase(), - value: matches[2], - }; -} - export function extractJWTFromRequest(req: Request) { - const header = req.get("authorization"); - if (header) { - const parts = parseAuthHeader(header); - if (parts && parts.scheme === "bearer") { - return parts.value; - } - } + const permit = new Bearer({ + basic: "password", + query: "access_token", + }); - const token: string | undefined | false = req.query && req.query.access_token; - if (token) { - return token; - } - - return null; + return permit.check(req) || null; } function generateJTIBlacklistKey(jti: string) { diff --git a/src/types/permit.d.ts b/src/types/permit.d.ts new file mode 100644 index 000000000..c24f13a6b --- /dev/null +++ b/src/types/permit.d.ts @@ -0,0 +1,31 @@ +// TODO: (wyattjoh) following https://github.com/DefinitelyTyped/DefinitelyTyped/pull/29061 to merge then replace this with @types/permit. +declare module "permit" { + import { IncomingMessage, ServerResponse } from "http"; + + export interface PermitOptions { + scheme?: string; + proxy?: string; + realm?: string; + } + + export interface BearerOptions extends PermitOptions { + basic?: string; + header?: string; + query?: string; + } + + export class Permit { + constructor(options: PermitOptions); + check(req: IncomingMessage): void; + fail(res: ServerResponse): void; + } + + export class Bearer extends Permit { + constructor(options: BearerOptions); + check(req: IncomingMessage): string; + } + + export class Basic extends Permit { + check(req: IncomingMessage): [string, string]; + } +} \ No newline at end of file