From dd46070b9cd30154cc37a6846183746ac1390710 Mon Sep 17 00:00:00 2001 From: Josh Gachnang Date: Sun, 12 Dec 2021 00:03:30 -0600 Subject: [PATCH] Fix JWT token not going through (#19) The options for signing when saving the user were incorrect --- package.json | 1 - src/example.ts | 5 +- src/expressServer.ts | 11 ++- src/mongooseRestFramework.test.ts | 76 +++++++++++++++++--- src/mongooseRestFramework.ts | 111 ++++++++++++++++-------------- 5 files changed, 130 insertions(+), 74 deletions(-) diff --git a/package.json b/package.json index b58f03c..8b185a9 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,6 @@ "mongoose-rest-framework": "^0.1.1", "on-finished": "^2.3.0", "passport-firebase-jwt": "^1.2.1", - "bcrypt": "^5.0.1", "express": "^4.17.1", "express-session": "^1.17.2", "jsonwebtoken": "^8.5.1", diff --git a/src/example.ts b/src/example.ts index 901baf8..b033473 100644 --- a/src/example.ts +++ b/src/example.ts @@ -63,10 +63,7 @@ function getBaseServer() { } }); app.use(express.json()); - setupAuth(app, UserModel as any, { - sessionSecret: "cats", - jwtIssuer: "example.com", - }); + setupAuth(app, UserModel as any); app.use( "/food", gooseRestRouter(FoodModel, { diff --git a/src/expressServer.ts b/src/expressServer.ts index 76bdd64..945c96d 100644 --- a/src/expressServer.ts +++ b/src/expressServer.ts @@ -3,14 +3,14 @@ import axios from "axios"; import cron from "cron"; import express, {Router} from "express"; import cloneDeep from "lodash/cloneDeep"; -import {setupAuth, UserModel} from "./mongooseRestFramework"; +import {Env, setupAuth, UserModel} from "./mongooseRestFramework"; import onFinished from "on-finished"; import passport from "passport"; const SLOW_READ_MAX = 200; const SLOW_WRITE_MAX = 500; -const dsn = (process.env as any).SENTRY_DSN; +const dsn = (process.env as Env).SENTRY_DSN; if (process.env.NODE_ENV === "production") { if (!dsn) { throw new Error("You must set SENTRY_DSN in the environment."); @@ -125,10 +125,7 @@ function initializeRoutes(UserModel: UserModel, addRoutes: AddRoutes) { app.use(logRequests); - setupAuth(app as any, UserModel as any, { - sessionSecret: process.env.SESSION_SECRET || "pumpkin", - jwtIssuer: process.env.JWT_ISSUER || "example.com", - }); + setupAuth(app as any, UserModel as any); // Adds all the user addRoutes(app); @@ -196,7 +193,7 @@ export function cronjob(name: string, schedule: "hourly" | string, callback: () // Convenience method to send data to a Slack webhook. export async function sendToSlack(text: string, channel = "bots") { - const slackWebhookUrl = (process.env as any).SLACK_WEBHOOK; + const slackWebhookUrl = (process.env as Env).SLACK_WEBHOOK; if (!slackWebhookUrl) { throw new Error("You must set SLACK_WEBHOOK in the environment."); } diff --git a/src/mongooseRestFramework.test.ts b/src/mongooseRestFramework.test.ts index 12892f7..248c169 100644 --- a/src/mongooseRestFramework.test.ts +++ b/src/mongooseRestFramework.test.ts @@ -13,11 +13,7 @@ import { } from "./mongooseRestFramework"; const assert = chai.assert; -const JWTOptions = { - sessionSecret: "cats", - jwtSecret: "secret", - jwtIssuer: "example.com", -}; + mongoose.connect("mongodb://localhost:27017/mrf"); interface User { @@ -85,6 +81,9 @@ describe("mongoose rest framework", () => { // jest.resetModules(); // Most important - it clears the cache process.env = {...OLD_ENV}; // Make a copy process.env.TOKEN_SECRET = "secret"; + process.env.TOKEN_EXPIRES_IN = "30m"; + process.env.TOKEN_ISSUER = "example.com"; + process.env.SESSION_SECRET = "session"; }); afterEach(function() { @@ -119,7 +118,7 @@ describe("mongoose rest framework", () => { }), ]); app = getBaseServer(); - setupAuth(app, UserModel as any, JWTOptions); + setupAuth(app, UserModel as any); app.use( "/food", gooseRestRouter(FoodModel, { @@ -334,7 +333,7 @@ describe("mongoose rest framework", () => { }), ]); app = getBaseServer(); - setupAuth(app, UserModel as any, JWTOptions); + setupAuth(app, UserModel as any); app.use( "/food", gooseRestRouter(FoodModel, { @@ -613,7 +612,7 @@ describe("mongoose rest framework", () => { }), ]); app = getBaseServer(); - setupAuth(app, UserModel as any, JWTOptions); + setupAuth(app, UserModel as any); app.use( "/food", gooseRestRouter(FoodModel, { @@ -686,6 +685,9 @@ describe("test token auth", function() { // jest.resetModules(); // Most important - it clears the cache process.env = {...OLD_ENV}; // Make a copy process.env.TOKEN_SECRET = "secret"; + process.env.TOKEN_EXPIRES_IN = "30m"; + process.env.TOKEN_ISSUER = "example.com"; + process.env.SESSION_SECRET = "session"; }); afterEach(function() { @@ -728,7 +730,7 @@ describe("test token auth", function() { }), ]); app = getBaseServer(); - setupAuth(app, UserModel as any, JWTOptions); + setupAuth(app, UserModel as any); app.use( "/food", gooseRestRouter(FoodModel, { @@ -785,6 +787,34 @@ describe("test token auth", function() { ownerId: userId, }); + const meRes = await server + .get("/auth/me") + .set("authorization", `Bearer ${token}`) + .expect(200); + console.log("ME RES", meRes.body.data); + assert.isDefined(meRes.body.data._id); + assert.isDefined(meRes.body.data.id); + assert.isUndefined(meRes.body.data.hash); + assert.equal(meRes.body.data.email, "new@example.com"); + assert.isDefined(meRes.body.data.token); + assert.isDefined(meRes.body.data.updated); + assert.isDefined(meRes.body.data.created); + assert.isFalse(meRes.body.data.admin); + + const mePatchRes = await server + .patch("/auth/me") + .send({email: "new2@example.com"}) + .set("authorization", `Bearer ${token}`) + .expect(200); + assert.isDefined(mePatchRes.body.data._id); + assert.isDefined(mePatchRes.body.data.id); + assert.isUndefined(mePatchRes.body.data.hash); + assert.equal(mePatchRes.body.data.email, "new2@example.com"); + assert.isDefined(mePatchRes.body.data.token); + assert.isDefined(mePatchRes.body.data.updated); + assert.isDefined(mePatchRes.body.data.created); + assert.isFalse(mePatchRes.body.data.admin); + // Use token to see 2 foods + the one we just created const getRes = await server .get("/food") @@ -811,6 +841,34 @@ describe("test token auth", function() { assert.isDefined(userId); assert.isDefined(token); + const meRes = await server + .get("/auth/me") + .set("authorization", `Bearer ${token}`) + .expect(200); + console.log("ME RES", meRes.body.data); + assert.isDefined(meRes.body.data._id); + assert.isDefined(meRes.body.data.id); + assert.isUndefined(meRes.body.data.hash); + assert.equal(meRes.body.data.email, "admin@example.com"); + assert.isDefined(meRes.body.data.token); + assert.isDefined(meRes.body.data.updated); + assert.isDefined(meRes.body.data.created); + assert.isTrue(meRes.body.data.admin); + + const mePatchRes = await server + .patch("/auth/me") + .send({email: "admin2@example.com"}) + .set("authorization", `Bearer ${token}`) + .expect(200); + assert.isDefined(mePatchRes.body.data._id); + assert.isDefined(mePatchRes.body.data.id); + assert.isUndefined(mePatchRes.body.data.hash); + assert.equal(mePatchRes.body.data.email, "admin2@example.com"); + assert.isDefined(mePatchRes.body.data.token); + assert.isDefined(mePatchRes.body.data.updated); + assert.isDefined(mePatchRes.body.data.created); + assert.isTrue(mePatchRes.body.data.admin); + // Use token to see admin foods const getRes = await server .get("/food") diff --git a/src/mongooseRestFramework.ts b/src/mongooseRestFramework.ts index 763dc85..6ab92ea 100644 --- a/src/mongooseRestFramework.ts +++ b/src/mongooseRestFramework.ts @@ -4,10 +4,22 @@ import session from "express-session"; import jwt from "jsonwebtoken"; import mongoose, {Document, Model, ObjectId, Schema} from "mongoose"; import passport from "passport"; -import {Strategy as JwtStrategy, ExtractJwt} from "passport-jwt"; +import {Strategy as JwtStrategy} from "passport-jwt"; import {Strategy as AnonymousStrategy} from "passport-anonymous"; import {Strategy as LocalStrategy} from "passport-local"; -import bcrypt from "bcrypt"; + +export interface Env { + NODE_ENV?: string; + PORT?: string; + SENTRY_DSN?: string; + SLACK_WEBHOOK?: string; + // JWT + TOKEN_SECRET?: string; + TOKEN_EXPIRES_IN?: string; + TOKEN_ISSUER?: string; + // AUTH + SESSION_SECRET?: string; +} // TODOS: // Support bulk actions @@ -157,13 +169,18 @@ export function tokenPlugin(schema: Schema, options: {expiresIn?: number} = {}) const tokenOptions: any = { expiresIn: "10h", }; - if ((process.env as any).TOKEN_EXPIRES_IN) { - tokenOptions.issuer = (process.env as any).TOKEN_EXPIRES_IN; + if ((process.env as Env).TOKEN_EXPIRES_IN) { + tokenOptions.expiresIn = (process.env as Env).TOKEN_EXPIRES_IN; } - if ((process.env as any).TOKEN_ISSUER) { - tokenOptions.issuer = (process.env as any).TOKEN_ISSUER; + if ((process.env as Env).TOKEN_ISSUER) { + tokenOptions.issuer = (process.env as Env).TOKEN_ISSUER; + } + + const secretOrKey = (process.env as Env).TOKEN_SECRET; + if (!secretOrKey) { + throw new Error(`TOKEN_SECRET must be set in env.`); } - this.token = jwt.sign({id: this._id.toString()}, (process.env as any).TOKEN_SECRET, options); + this.token = jwt.sign({id: this._id.toString()}, secretOrKey, tokenOptions); } // On any save, update updated. this.updated = new Date(); @@ -227,20 +244,16 @@ export function firebaseJWTPlugin(schema: Schema) { schema.add({firebaseId: {type: String, index: true}}); } -export function authenticateMiddleware() { - return passport.authenticate(["jwt", "anonymous"], {session: false}); +export function authenticateMiddleware(anonymous = false) { + const strategies = ["jwt"]; + if (anonymous) { + strategies.push("anonymous"); + } + return passport.authenticate(strategies, {session: false}); } // TODO allow customization -export function setupAuth( - app: express.Application, - userModel: UserModel, - options: { - disableBasicAuth?: boolean; - sessionSecret: string; - jwtIssuer?: string; - } -) { +export function setupAuth(app: express.Application, userModel: UserModel) { passport.use(new AnonymousStrategy()); passport.use( "signup", @@ -259,7 +272,7 @@ export function setupAuth( } return done(null, user); } catch (error) { - done(error); + return done(error); } } ) @@ -307,17 +320,14 @@ export function setupAuth( throw new Error("setupAuth userModel must have .deserializeUser()"); } - if (!options.disableBasicAuth) { - passport.use(userModel.createStrategy()); - } // use static serialize and deserialize of model for passport session support passport.serializeUser(userModel.serializeUser()); passport.deserializeUser(userModel.deserializeUser()); - if ((process.env as any).TOKEN_SECRET && options.jwtIssuer) { + if ((process.env as Env).TOKEN_SECRET) { console.debug("Setting up JWT Authentication"); - const customExtractor = function(req: any) { + const customExtractor = function(req: express.Request) { let token = null; if (req?.cookies?.jwt) { token = req.cookies.jwt; @@ -326,13 +336,18 @@ export function setupAuth( } return token; }; + const secretOrKey = (process.env as Env).TOKEN_SECRET; + if (!secretOrKey) { + throw new Error(`TOKEN_SECRET must be set in env.`); + } const jwtOpts = { // jwtFromRequest: ExtractJwt.fromAuthHeaderWithScheme("Bearer"), jwtFromRequest: customExtractor, - secretOrKey: (process.env as any).TOKEN_SECRET, - issuer: (process.env as any).TOKEN_ISSUER, + secretOrKey, + issuer: (process.env as Env).TOKEN_ISSUER, }; passport.use( + "jwt", new JwtStrategy(jwtOpts, async function( payload: {id: string; iat: number; exp: number}, done: any @@ -344,15 +359,18 @@ export function setupAuth( try { user = await userModel.findById((payload as any).id); } catch (e) { + console.warn("[jwt] Error finding user from id", e); return done(e, false); } if (user) { return done(null, user); } else { if (userModel.createAnonymousUser) { + console.log("[jwt] Creating anonymous user"); user = await userModel.createAnonymousUser(); return done(null, user); } else { + console.log("[jwt] No user found from token"); return done(null, false); } } @@ -409,27 +427,13 @@ export function setupAuth( } }); - router.patch("/me", passport.authenticate("firebase-jwt", {session: false}), async (req, res) => { - if (!req.user?.id) { - return res.status(401).send(); - } - // TODO support limited updates for profile. - // try { - // body = transform(req.body, "update", req.user); - // } catch (e) { - // return res.status(403).send({message: (e as any).message}); - // } - try { - const data = await userModel.findOneAndUpdate({_id: req.user.id}, req.body, {new: true}); - - (data as any).id = data._id; - return res.json({data}); - } catch (e) { - return res.status(403).send({message: (e as any).message}); - } - }); - - app.use(session({secret: options.sessionSecret, resave: true, saveUninitialized: true}) as any); + app.use( + session({ + secret: (process.env as Env).SESSION_SECRET as string, + resave: true, + saveUninitialized: true, + }) as any + ); app.use(bodyParser.urlencoded({extended: false}) as any); app.use(passport.initialize() as any); app.use(passport.session()); @@ -552,7 +556,8 @@ export function gooseRestRouter( options.endpoints(router); } - router.post("/", authenticateMiddleware(), async (req, res) => { + // TODO Enable/disable anonymous auth middleware based on settings for route. + router.post("/", authenticateMiddleware(true), async (req, res) => { if (!checkPermissions("create", options.permissions.create, req.user)) { return res.status(405).send(); } @@ -567,7 +572,7 @@ export function gooseRestRouter( return res.json({data: serialize(data, req.user)}); }); - router.get("/", authenticateMiddleware(), async (req, res) => { + router.get("/", authenticateMiddleware(true), async (req, res) => { if (!checkPermissions("list", options.permissions.list, req.user)) { return res.status(403).send(); } @@ -652,7 +657,7 @@ export function gooseRestRouter( } }); - router.get("/:id", authenticateMiddleware(), async (req, res) => { + router.get("/:id", authenticateMiddleware(true), async (req, res) => { if (!checkPermissions("read", options.permissions.read, req.user)) { return res.status(405).send(); } @@ -670,12 +675,12 @@ export function gooseRestRouter( return res.json({data: serialize(data, req.user)}); }); - router.put("/:id", authenticateMiddleware(), async (req, res) => { + router.put("/:id", authenticateMiddleware(true), async (req, res) => { // Patch is what we want 90% of the time return res.status(500); }); - router.patch("/:id", authenticateMiddleware(), async (req, res) => { + router.patch("/:id", authenticateMiddleware(true), async (req, res) => { if (!checkPermissions("update", options.permissions.update, req.user)) { console.debug(`Patch not allowed for user ${req.user?.id} on collection`); return res.status(405).send(); @@ -703,7 +708,7 @@ export function gooseRestRouter( return res.json({data: serialize(doc, req.user)}); }); - router.delete("/:id", authenticateMiddleware(), async (req, res) => { + router.delete("/:id", authenticateMiddleware(true), async (req, res) => { if (!checkPermissions("delete", options.permissions.delete, req.user)) { return res.status(405).send(); }