From c732566f63cb23183f4aa192f53ffd53b342bfd1 Mon Sep 17 00:00:00 2001 From: dswbx Date: Sat, 11 Jan 2025 10:52:31 +0100 Subject: [PATCH] Add integration tests for auth, improve auth middleware and cookies handling --- app/__test__/helper.ts | 2 +- .../integration/auth.integration.test.ts | 203 ++++++++++++++++++ app/build.ts | 7 +- app/src/auth/AppAuth.ts | 18 +- app/src/auth/authenticate/Authenticator.ts | 18 +- app/src/auth/middlewares.ts | 58 ++--- app/vite.dev.ts | 24 ++- 7 files changed, 276 insertions(+), 54 deletions(-) create mode 100644 app/__test__/integration/auth.integration.test.ts diff --git a/app/__test__/helper.ts b/app/__test__/helper.ts index f93f4e6..e11da33 100644 --- a/app/__test__/helper.ts +++ b/app/__test__/helper.ts @@ -40,7 +40,7 @@ const _oldConsoles = { error: console.error }; -export function disableConsoleLog(severities: ConsoleSeverity[] = ["log"]) { +export function disableConsoleLog(severities: ConsoleSeverity[] = ["log", "warn"]) { severities.forEach((severity) => { console[severity] = () => null; }); diff --git a/app/__test__/integration/auth.integration.test.ts b/app/__test__/integration/auth.integration.test.ts new file mode 100644 index 0000000..abd8ed5 --- /dev/null +++ b/app/__test__/integration/auth.integration.test.ts @@ -0,0 +1,203 @@ +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import { App, createApp } from "../../src"; +import type { AuthResponse } from "../../src/auth"; +import { randomString, secureRandomString } from "../../src/core/utils"; +import { disableConsoleLog, enableConsoleLog } from "../helper"; + +beforeAll(disableConsoleLog); +afterAll(enableConsoleLog); + +const roles = { + sloppy: { + guest: { + permissions: [ + "system.access.admin", + "system.schema.read", + "system.access.api", + "system.config.read", + "data.entity.read" + ], + is_default: true + }, + admin: { + is_default: true, + implicit_allow: true + } + }, + strict: { + guest: { + permissions: ["system.access.api", "system.config.read", "data.entity.read"], + is_default: true + }, + admin: { + is_default: true, + implicit_allow: true + } + } +}; +const configs = { + auth: { + enabled: true, + entity_name: "users", + jwt: { + secret: secureRandomString(20), + issuer: randomString(10) + }, + roles: roles.strict, + guard: { + enabled: true + } + }, + users: { + normal: { + email: "normal@bknd.io", + password: "12345678" + }, + admin: { + email: "admin@bknd.io", + password: "12345678", + role: "admin" + } + } +}; + +function createAuthApp() { + const app = createApp({ + initialConfig: { + auth: configs.auth + } + }); + + app.emgr.onEvent( + App.Events.AppFirstBoot, + async () => { + await app.createUser(configs.users.normal); + await app.createUser(configs.users.admin); + }, + "sync" + ); + + return app; +} + +function getCookie(r: Response, name: string) { + const cookies = r.headers.get("cookie") ?? r.headers.get("set-cookie"); + if (!cookies) return; + const cookie = cookies.split(";").find((c) => c.trim().startsWith(name)); + if (!cookie) return; + return cookie.split("=")[1]; +} + +const fns = (app: App, mode?: Mode) => { + function headers(token?: string, additional?: Record) { + if (mode === "cookie") { + return { + cookie: `auth=${token};`, + ...additional + }; + } + + return { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + ...additional + }; + } + function body(obj?: Record) { + if (mode === "cookie") { + const formData = new FormData(); + for (const key in obj) { + formData.append(key, obj[key]); + } + return formData; + } + + return JSON.stringify(obj); + } + + return { + login: async ( + user: any + ): Promise<{ res: Response; data: Mode extends "token" ? AuthResponse : string }> => { + const res = (await app.server.request("/api/auth/password/login", { + method: "POST", + headers: headers(), + body: body(user) + })) as Response; + + const data = mode === "cookie" ? getCookie(res, "auth") : await res.json(); + + return { res, data }; + }, + me: async (token?: string): Promise> => { + const res = (await app.server.request("/api/auth/me", { + method: "GET", + headers: headers(token) + })) as Response; + return await res.json(); + } + }; +}; + +describe("integration auth", () => { + it("should create users on boot", async () => { + const app = createAuthApp(); + await app.build(); + + const { data: users } = await app.em.repository("users").findMany(); + expect(users.length).toBe(2); + expect(users[0].email).toBe(configs.users.normal.email); + expect(users[1].email).toBe(configs.users.admin.email); + }); + + it("should log you in with API", async () => { + const app = createAuthApp(); + await app.build(); + const $fns = fns(app); + + // login api + const { data } = await $fns.login(configs.users.normal); + const me = await $fns.me(data.token); + + expect(data.user.email).toBe(me.user.email); + expect(me.user.email).toBe(configs.users.normal.email); + + // expect no user with no token + expect(await $fns.me()).toEqual({ user: null as any }); + + // expect no user with invalid token + expect(await $fns.me("invalid")).toEqual({ user: null as any }); + expect(await $fns.me()).toEqual({ user: null as any }); + }); + + it("should log you in with form and cookie", async () => { + const app = createAuthApp(); + await app.build(); + const $fns = fns(app, "cookie"); + + const { res, data: token } = await $fns.login(configs.users.normal); + expect(token).toBeDefined(); + expect(res.status).toBe(302); // because it redirects + + // test cookie jwt interchangability + { + // expect token to not work as-is for api endpoints + expect(await fns(app).me(token)).toEqual({ user: null as any }); + // hono adds an additional segment to cookies + const apified_token = token.split(".").slice(0, -1).join("."); + // now it should work + // @todo: maybe add a config to don't allow re-use? + expect((await fns(app).me(apified_token)).user.email).toBe(configs.users.normal.email); + } + + // test cookie with me endpoint + { + const me = await $fns.me(token); + expect(me.user.email).toBe(configs.users.normal.email); + + // check with invalid & empty + expect(await $fns.me("invalid")).toEqual({ user: null as any }); + expect(await $fns.me()).toEqual({ user: null as any }); + } + }); +}); diff --git a/app/build.ts b/app/build.ts index c349986..c1cd766 100644 --- a/app/build.ts +++ b/app/build.ts @@ -11,6 +11,9 @@ const types = args.includes("--types"); const sourcemap = args.includes("--sourcemap"); const clean = args.includes("--clean"); +// keep console logs if not minified +const debugging = minify; + if (clean) { console.log("Cleaning dist"); await $`rm -rf dist`; @@ -38,7 +41,7 @@ function buildTypes() { let watcher_timeout: any; function delayTypes() { - if (!watch) return; + if (!watch || !types) return; if (watcher_timeout) { clearTimeout(watcher_timeout); } @@ -63,7 +66,7 @@ const result = await esbuild.build({ bundle: true, splitting: true, metafile: true, - drop: ["console", "debugger"], + drop: debugging ? undefined : ["console", "debugger"], inject: ["src/ui/inject.js"], target: "es2022", format: "esm", diff --git a/app/src/auth/AppAuth.ts b/app/src/auth/AppAuth.ts index 0608063..e866414 100644 --- a/app/src/auth/AppAuth.ts +++ b/app/src/auth/AppAuth.ts @@ -114,12 +114,12 @@ export class AppAuth extends Module { identifier: string, profile: ProfileExchange ): Promise { - console.log("***** AppAuth:resolveUser", { + /*console.log("***** AppAuth:resolveUser", { action, strategy: strategy.getName(), identifier, profile - }); + });*/ if (!this.config.allow_register && action === "register") { throw new Exception("Registration is not allowed", 403); } @@ -140,12 +140,12 @@ export class AppAuth extends Module { } private filterUserData(user: any) { - console.log( + /*console.log( "--filterUserData", user, this.config.jwt.fields, pick(user, this.config.jwt.fields) - ); + );*/ return pick(user, this.config.jwt.fields); } @@ -171,18 +171,18 @@ export class AppAuth extends Module { if (!result.data) { throw new Exception("User not found", 404); } - console.log("---login data", result.data, result); + //console.log("---login data", result.data, result); // compare strategy and identifier - console.log("strategy comparison", result.data.strategy, strategy.getName()); + //console.log("strategy comparison", result.data.strategy, strategy.getName()); if (result.data.strategy !== strategy.getName()) { - console.log("!!! User registered with different strategy"); + //console.log("!!! User registered with different strategy"); throw new Exception("User registered with different strategy"); } - console.log("identifier comparison", result.data.strategy_value, identifier); + //console.log("identifier comparison", result.data.strategy_value, identifier); if (result.data.strategy_value !== identifier) { - console.log("!!! Invalid credentials"); + //console.log("!!! Invalid credentials"); throw new Exception("Invalid credentials"); } diff --git a/app/src/auth/authenticate/Authenticator.ts b/app/src/auth/authenticate/Authenticator.ts index 01355b5..c86244d 100644 --- a/app/src/auth/authenticate/Authenticator.ts +++ b/app/src/auth/authenticate/Authenticator.ts @@ -67,6 +67,9 @@ export const cookieConfig = Type.Partial( { default: {}, additionalProperties: false } ); +// @todo: maybe add a config to not allow cookie/api tokens to be used interchangably? +// see auth.integration test for further details + export const jwtConfig = Type.Object( { // @todo: autogenerate a secret if not present. But it must be persisted from AppAuth @@ -139,7 +142,7 @@ export class Authenticator = Record< } // @todo: determine what to do exactly - __setUserNull() { + resetUser() { this._user = undefined; } @@ -203,8 +206,8 @@ export class Authenticator = Record< this._user = omit(payload, ["iat", "exp", "iss"]) as SafeUser; return true; } catch (e) { - this._user = undefined; - console.error(e); + this.resetUser(); + //console.error(e); } return false; @@ -222,10 +225,8 @@ export class Authenticator = Record< private async getAuthCookie(c: Context): Promise { try { const secret = this.config.jwt.secret; - const token = await getSignedCookie(c, secret, "auth"); if (typeof token !== "string") { - await deleteCookie(c, "auth", this.cookieOptions); return undefined; } @@ -253,12 +254,17 @@ export class Authenticator = Record< await setSignedCookie(c, "auth", token, secret, this.cookieOptions); } + private async deleteAuthCookie(c: Context) { + await deleteCookie(c, "auth", this.cookieOptions); + } + async logout(c: Context) { const cookie = await this.getAuthCookie(c); if (cookie) { - await deleteCookie(c, "auth", this.cookieOptions); + await this.deleteAuthCookie(c); await addFlashMessage(c, "Signed out", "info"); } + this.resetUser(); } // @todo: move this to a server helper diff --git a/app/src/auth/middlewares.ts b/app/src/auth/middlewares.ts index 927d6c6..78a24cc 100644 --- a/app/src/auth/middlewares.ts +++ b/app/src/auth/middlewares.ts @@ -3,24 +3,6 @@ import type { Context } from "hono"; import { createMiddleware } from "hono/factory"; import type { ServerEnv } from "modules/Module"; -async function resolveAuth(app: ServerEnv["Variables"]["app"], c: Context) { - const resolved = c.get("auth_resolved") ?? false; - if (resolved) { - return; - } - if (!app.module.auth.enabled) { - return; - } - - const authenticator = app.module.auth.authenticator; - const guard = app.modules.ctx().guard; - - guard.setUserContext(await authenticator.resolveAuthFromRequest(c)); - - // renew cookie if applicable - authenticator.requestCookieRefresh(c); -} - export function shouldSkipAuth(req: Request) { const skip = new URL(req.url).pathname.startsWith(config.server.assets_path); if (skip) { @@ -30,22 +12,46 @@ export function shouldSkipAuth(req: Request) { } export const auth = createMiddleware(async (c, next) => { - if (!shouldSkipAuth(c.req.raw)) { - // make sure to only register once - if (c.get("auth_registered")) { - return; - } + // make sure to only register once + if (c.get("auth_registered")) { + throw new Error("auth middleware already registered"); + } + c.set("auth_registered", true); - await resolveAuth(c.get("app"), c); - c.set("auth_registered", true); + const skipped = shouldSkipAuth(c.req.raw); + const app = c.get("app"); + const guard = app.modules.ctx().guard; + const authenticator = app.module.auth.authenticator; + + if (!skipped) { + const resolved = c.get("auth_resolved"); + if (!resolved) { + if (!app.module.auth.enabled) { + guard.setUserContext(undefined); + } else { + guard.setUserContext(await authenticator.resolveAuthFromRequest(c)); + + // renew cookie if applicable + authenticator.requestCookieRefresh(c); + } + } } await next(); + + // release + guard.setUserContext(undefined); + authenticator.resetUser(); + c.set("auth_resolved", false); }); export const permission = (...permissions: Permission[]) => createMiddleware(async (c, next) => { - if (!shouldSkipAuth) { + if (!c.get("auth_registered")) { + throw new Error("auth middleware not registered, cannot check permissions"); + } + + if (!shouldSkipAuth(c.req.raw)) { const app = c.get("app"); if (app) { const p = Array.isArray(permissions) ? permissions : [permissions]; diff --git a/app/vite.dev.ts b/app/vite.dev.ts index 16e73cd..244b84d 100644 --- a/app/vite.dev.ts +++ b/app/vite.dev.ts @@ -33,18 +33,22 @@ if (run_example) { initialConfig = config; } +let app: App; +const recreate = true; export default { async fetch(request: Request) { - const app = App.create({ connection, initialConfig }); - app.emgr.onEvent( - App.Events.AppBuiltEvent, - async () => { - app.registerAdminController({ forceDev: true }); - app.module.server.client.get("/assets/*", serveStatic({ root: "./" })); - }, - "sync" - ); - await app.build(); + if (!app || recreate) { + app = App.create({ connection, initialConfig }); + app.emgr.onEvent( + App.Events.AppBuiltEvent, + async () => { + app.registerAdminController({ forceDev: true }); + app.module.server.client.get("/assets/*", serveStatic({ root: "./" })); + }, + "sync" + ); + await app.build(); + } return app.fetch(request); }