From f413b74a1f51de8690417bb2c87b8bd1b75140cf Mon Sep 17 00:00:00 2001 From: Nolan Hellyer Date: Sun, 2 Feb 2025 18:52:25 -0800 Subject: [PATCH] add unit tests for auth related functions. --- src/hooks.server.ts | 43 +- src/lib/GameEvent.ts | 18 +- src/lib/Listing.ts | 9 +- src/lib/ServerResponse.ts | 4 + src/lib/server/auth.ts | 31 +- src/lib/server/cache.ts | 4 - src/lib/server/mongo.ts | 73 +-- src/lib/server/routeAuth.ts | 2 +- src/lib/server/test/auth.spec.ts | 456 ++++++++++++++++++ ...{Listing.spec.ts => modifyListing.spec.ts} | 2 +- src/lib/{server => }/test/GameData.spec.ts | 11 +- src/lib/{server => }/test/GameEvent.spec.ts | 84 +++- src/lib/test/Listing.spec.ts | 111 +++++ src/lib/test/Login.spec.ts | 62 +++ src/lib/test/ServerResponse.spec.ts | 47 ++ src/lib/{server => }/test/validation.spec.ts | 20 +- src/routes/api/games/+server.ts | 9 +- src/routes/api/games/[gameid]/+server.ts | 26 +- src/routes/api/token/+server.ts | 3 +- src/tests/hooks.server.spec.ts | 66 ++- src/tests/requests.http | 5 +- 21 files changed, 953 insertions(+), 133 deletions(-) delete mode 100644 src/lib/server/cache.ts create mode 100644 src/lib/server/test/auth.spec.ts rename src/lib/server/test/{Listing.spec.ts => modifyListing.spec.ts} (96%) rename src/lib/{server => }/test/GameData.spec.ts (85%) rename src/lib/{server => }/test/GameEvent.spec.ts (93%) create mode 100644 src/lib/test/Listing.spec.ts create mode 100644 src/lib/test/Login.spec.ts create mode 100644 src/lib/test/ServerResponse.spec.ts rename src/lib/{server => }/test/validation.spec.ts (91%) diff --git a/src/hooks.server.ts b/src/hooks.server.ts index 0d2f6c9..a124e2a 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -2,28 +2,33 @@ import * as auth from "$lib/server/auth"; import { forbiddenResponse, unauthorizedResponse } from "$lib/server/responseBodies"; import { routeAuth, type Method } from "$lib/server/routeAuth"; import { type Handle } from "@sveltejs/kit"; +import { JWT_SECRET } from "$env/static/private"; -export const handle: Handle = async ({ event, resolve }) => { - const creds = await auth.authenticate(event); +export const handle: Handle = getHandleFn(JWT_SECRET); - if (!creds) { - return unauthorizedResponse(); - } +export function getHandleFn(jwtSecret: string): Handle { + return async ({ event, resolve }) => { + const creds = await auth.authenticate(event, jwtSecret); - const authResult = auth.isAuthorized( - routeAuth, - event.request.method as Method, - event.url.pathname, - creds, - ); + if (!creds) { + return unauthorizedResponse(); + } - if (authResult === auth.AuthorizationResult.Denied) { - return forbiddenResponse(); - } else if (authResult === auth.AuthorizationResult.Unauthenticated) { - return unauthorizedResponse(); - } + const authResult = auth.isAuthorized( + routeAuth, + event.request.method as Method, + event.url.pathname, + creds, + ); - event.locals.user = creds; + if (authResult === auth.AuthorizationResult.Denied) { + return forbiddenResponse(); + } else if (authResult === auth.AuthorizationResult.Unauthenticated) { + return unauthorizedResponse(); + } - return await resolve(event); -}; + event.locals.user = creds; + + return await resolve(event); + }; +} diff --git a/src/lib/GameEvent.ts b/src/lib/GameEvent.ts index d3d5227..26e7c55 100644 --- a/src/lib/GameEvent.ts +++ b/src/lib/GameEvent.ts @@ -174,7 +174,9 @@ export class RollForFirst implements GameEvent { state.dieCount = 6; } else { // ...otherwise, setup for tie breaking rolls. - state.scores = scores.map((_, i) => (ties.has(i) ? FIRST_ROLL_PENDING : FIRST_ROLL_LOST)); + state.scores = scores.map((_, i) => + ties.has(i) ? FIRST_ROLL_PENDING : FIRST_ROLL_LOST, + ); } } } @@ -201,8 +203,6 @@ export class Roll implements GameEvent { } run(state: State) { - const scores = state.scores ?? []; - throwIfGameOver(state); throwIfWrongTurn(state, this.player); @@ -346,7 +346,11 @@ export class Score implements GameEvent { run(state: State) { const { dieCount, heldScore, scores } = state; - const playerCount = scores?.length ?? 0; + const playerCount = scores?.length; + + if (!playerCount) { + throw new Error("there are no players"); + } throwIfGameOver(state); throwIfWrongTurn(state, this.player); @@ -361,12 +365,14 @@ export class Score implements GameEvent { // It's safe to tell the compiler that the player is not undefined because of the // check above. - state.scores![this.player] += heldScore ?? 0; + state.scores![this.player] += heldScore!; // Increment the index of the active player, circling back to 1 if the player // who just scored was the last player in the array. state.playing = - playerCount - 1 === this.player ? (state.playing = 0) : (state.playing = this.player + 1); + playerCount - 1 === this.player + ? (state.playing = 0) + : (state.playing = this.player + 1); state.dieCount = 6; delete state.heldScore; diff --git a/src/lib/Listing.ts b/src/lib/Listing.ts index c19d755..454f819 100644 --- a/src/lib/Listing.ts +++ b/src/lib/Listing.ts @@ -1,17 +1,17 @@ -import { hasProperty } from "$lib/validation"; +import { hasOnlyKeys, hasProperty } from "$lib/validation"; import { isId, type Id } from "$lib/Id"; export interface Listing { id: Id; createdAt: string; - modifiedAt: string | null; deleted: boolean; + modifiedAt: string | null; data: T; } export function isListing( target: unknown, - dataGuard?: (target: unknown) => target is T + dataGuard?: (target: unknown) => target is T, ): target is Listing { if (!hasProperty(target, "id", isId)) return false; if (!hasProperty(target, "createdAt", "string")) return false; @@ -23,5 +23,8 @@ export function isListing( if (!hasProperty(target, "deleted", "boolean")) return false; if (!hasProperty(target, "data", "object")) return false; + if (!hasOnlyKeys(target, ["id", "createdAt", "modifiedAt", "deleted", "data"])) + return false; + return dataGuard?.((target as any)["data"]) ?? true; } diff --git a/src/lib/ServerResponse.ts b/src/lib/ServerResponse.ts index 8798627..89dfb2e 100644 --- a/src/lib/ServerResponse.ts +++ b/src/lib/ServerResponse.ts @@ -7,6 +7,10 @@ export type ServerResponse = | { error: string }; export function isServerResponse(target: unknown): target is ServerResponse { + const props = Object.getOwnPropertyNames(target); + + if (props.length !== 1) return false; + if (hasProperty(target, "item", "object")) return true; if (hasProperty(target, "items", "object[]")) return true; if (hasProperty(target, "error", "string")) return true; diff --git a/src/lib/server/auth.ts b/src/lib/server/auth.ts index cd2ceb6..cfe6bb4 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth.ts @@ -1,4 +1,3 @@ -import { JWT_SECRET } from "$env/static/private"; import type { RequestEvent } from "@sveltejs/kit"; import jwt from "jsonwebtoken"; import { type Method, type RouteAuthRule } from "./routeAuth"; @@ -17,10 +16,10 @@ export enum AuthorizationResult { Unauthenticated, } -export async function createToken(listing: Listing) { +export async function createToken(listing: Listing, secret: string) { return await jwt.sign( { sub: listing.id, username: listing.data.username, role: listing.data.role }, - JWT_SECRET, + secret, { expiresIn: "1d", }, @@ -29,21 +28,13 @@ export async function createToken(listing: Listing) { export async function authenticate( event: RequestEvent, + jwtSecret: string, ): Promise { - let path = event.url.pathname; + const authHeader = event.request.headers.get("authorization"); let tokenKind: "Basic" | "Bearer" | "None"; let tokenRole: string; let tokenDesc: LocalCredentials; - const parts = breakupPath(path); - - if (parts[0] !== "api") { - // not concerned about requests made to the frontend server - return { kind: "None", role: "default" }; - } - - const authHeader = event.request.headers.get("authorization"); - // get token kind and role from the header if (!authHeader) { // This is a stranger: they have no token and they will be assigned the default @@ -58,7 +49,7 @@ export async function authenticate( tokenKind = "Bearer"; // The role can be derived from the JWT token. - const payload = await jwt.verify(token, JWT_SECRET); + const payload = await jwt.verify(token, jwtSecret); if (typeof payload === "string") { // I do not assign, and don't know what to do with, these kinds of // tokens. Perhaps an error should be logged here, since this is a @@ -93,7 +84,7 @@ export async function authenticate( } export function isAuthorized( - roleRules: { [k: string]: RouteAuthRule[] }, + roleRules: Record, method: Method, path: string, creds: LocalCredentials, @@ -125,8 +116,9 @@ export function isAuthorized( } function breakupPath(path: string): string[] { - // remove a leading / + // remove leading and trailing / if (path[0] === "/") path = path.slice(1); + if (path[path.length - 1] === "/") path = path.slice(0, path.length - 1); return path.split("/"); } @@ -144,6 +136,9 @@ function matchesRequest( for (let i = 0; i < ruleParts.length; i++) { const rulePart = ruleParts[i]; const reqPart = requestParts[i]; + + if (!reqPart) return false; + if (rulePart[0] === "[" && rulePart[rulePart.length - 1] === "]") { // This part of the path represents an ID. continue; @@ -159,5 +154,7 @@ function matchesRequest( } } - return true; + // Finished comparing without seeing a wildcard, meaning the requested path + // and the path rules should also be the same length. + return requestParts.length === ruleParts.length; } diff --git a/src/lib/server/cache.ts b/src/lib/server/cache.ts deleted file mode 100644 index 0f7a443..0000000 --- a/src/lib/server/cache.ts +++ /dev/null @@ -1,4 +0,0 @@ -import type { GameData } from "$lib/GameData"; -import type { Listing } from "$lib/Listing"; - -export const games: Listing[] = []; diff --git a/src/lib/server/mongo.ts b/src/lib/server/mongo.ts index 933cd62..7a89f8a 100644 --- a/src/lib/server/mongo.ts +++ b/src/lib/server/mongo.ts @@ -1,20 +1,28 @@ import type { Listing } from "$lib/Listing"; -import { MongoClient, ObjectId, ServerApiVersion, type Document, type WithId } from "mongodb"; +import { + MongoClient, + ObjectId, + ServerApiVersion, + type Document, + type WithId, +} from "mongodb"; import type { Id } from "../Id"; type ListingFromMongo = Omit & { _id: ObjectId }; const uri = `mongodb://127.0.0.1:27017`; const DATABASE = "ten-thousand"; -let client: MongoClient | null = null; +let cachedClient: MongoClient | null = null; + +async function getClient() { + if (cachedClient !== null) return cachedClient; -async function init() { const c = new MongoClient(uri, { serverApi: { version: ServerApiVersion.v1, strict: true, - deprecationErrors: true - } + deprecationErrors: true, + }, }); await c.connect(); @@ -22,55 +30,60 @@ async function init() { } export async function writeListing(col: string, listing: Listing) { - if (client === null) { - client = await init(); - } + const client = await getClient(); await client.db(DATABASE).collection(col).insertOne(fixListingForMongo(listing)); } +export async function listCollection( + col: string, + dataGuard: (target: unknown) => target is Listing, +) { + const client = await getClient(); + const res = await client.db(DATABASE).collection(col).find(); + + return (await res.toArray()).map((doc) => fixListingFromMongo(doc, dataGuard)); +} + export async function readListingById( col: string, id: Id, - dataGuard: (target: unknown) => target is T -) {} + dataGuard: (target: unknown) => target is Listing, +) { + const client = await getClient(); + const res = await client.db(DATABASE).collection(col).findOne({ _id: id }); + + if (res === null) return null; + return fixListingFromMongo(res, dataGuard); +} export async function readListingByQuery( col: string, query: object, - dataGuard: (target: unknown) => target is Listing + dataGuard: (target: unknown) => target is Listing, ): Promise | null> { - if (client === null) { - client = await init(); - } - + const client = await getClient(); const res = await client.db(DATABASE).collection(col).findOne(query); - if (res === null) { - return null; - } + if (res === null) return null; return fixListingFromMongo(res, dataGuard); } function fixListingForMongo(listing: Listing): ListingFromMongo { - // TODO: These two statements are tricky without any. Perhaps id could be optional, - // but that's kind of stupid because it's not optional on the type I'm - // constructing. Figure out something better here. - const adjusted: any = { _id: listing.id, ...listing }; - delete adjusted.id; - return adjusted; + const { id, ...rest } = listing; + return { + _id: id, + ...rest, + }; } function fixListingFromMongo( target: WithId, - dataGuard: (target: unknown) => target is Listing + dataGuard: (target: unknown) => target is Listing, ): Listing { - // TODO: These two statements are tricky without any. Perhaps id could be optional, - // but that's kind of stupid because it's not optional on the type I'm - // constructing. Figure out something better here. - const adjusted: any = { id: target._id, ...target }; - delete adjusted._id; + const { _id, ...rest } = target; + const adjusted = { id: _id, ...rest }; if (!dataGuard(adjusted)) { throw new Error("the returned document does not conform to the provided type"); diff --git a/src/lib/server/routeAuth.ts b/src/lib/server/routeAuth.ts index 6d9130b..038f08b 100644 --- a/src/lib/server/routeAuth.ts +++ b/src/lib/server/routeAuth.ts @@ -7,7 +7,7 @@ export interface RouteAuthRule { tokenKind?: "None" | "Bearer" | "Basic"; // defaults to "Bearer" } -export const routeAuth: { [k: string]: RouteAuthRule[] } = { +export const routeAuth: Record = { // default is an unknown user. They are allowed to create a new user for themselves // without any token, and they are allowed to access the token endpoint to login with // a Basic token. Other than that, they cannot do anything! diff --git a/src/lib/server/test/auth.spec.ts b/src/lib/server/test/auth.spec.ts new file mode 100644 index 0000000..806fd45 --- /dev/null +++ b/src/lib/server/test/auth.spec.ts @@ -0,0 +1,456 @@ +import { describe, expect, it } from "vitest"; +import * as jwt from "jsonwebtoken"; +import type { Listing } from "$lib/Listing"; +import type { LoginData } from "$lib/Login"; +import { createId } from "$lib/Id"; +import { + authenticate, + AuthorizationResult, + createToken, + isAuthorized, + type LocalCredentials, +} from "$lib/server/auth"; +import type { Cookies, RequestEvent } from "@sveltejs/kit"; +import type { Method, RouteAuthRule } from "$lib/server/routeAuth"; + +type TestData = { + heading: string; + conditions: { + token: LocalCredentials; + method: Method; + path: string; + }; + expectations: { + result: AuthorizationResult; + }; +}; + +const JWT_SECRET = "server-secret"; + +const loginListing: Listing = { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString(), + deleted: false, + data: { + username: "Somebody Important", + password: "some-password-hash", + role: "test-role", + }, +}; + +function getEvent(overrides: Partial = {}): RequestEvent { + const event: RequestEvent = { + cookies: {} as Cookies, + fetch: async (): Promise => { + return new Response(); + }, + getClientAddress: () => "", + locals: { user: {} }, + params: {}, + platform: undefined, + request: new Request(new URL("https://localhost/api")), + route: { id: "" }, + setHeaders: () => {}, + url: new URL("https://localhost/api"), + isDataRequest: false, + isSubRequest: false, + }; + + return { ...event, ...overrides }; +} + +describe("auth", () => { + describe("createToken", () => { + it("should create a json web token", async () => { + const token = await createToken(loginListing, "some-server-secret"); + + expect(jwt.decode(token)).to.not.be.null; + }); + }); + + describe("authenticate", () => { + it("should return a default/'None' object when no auth header is present", async () => { + const event = getEvent(); + + expect(await authenticate(event, JWT_SECRET)).to.deep.equal({ + kind: "None", + role: "default", + }); + }); + + it("should return a 'Bearer' object with the correct user role when a valid jwt token is passed", async () => { + const token = await createToken(loginListing, JWT_SECRET); + const request = new Request("http://localhost/api", { + headers: [["Authorization", `Bearer ${token}`]], + }); + const event = getEvent({ request }); + const tokenDesc = await authenticate(event, JWT_SECRET); + + expect(tokenDesc).property("kind", "Bearer"); + const { payload, ...rest } = tokenDesc as { + kind: "Bearer"; + role: string; + payload: jwt.JwtPayload; + }; + + expect(payload).property("sub", loginListing.id.toString()); + expect(payload).property("username", loginListing.data.username); + expect(payload).property("role", loginListing.data.role); + expect(rest).to.deep.equal({ + kind: "Bearer", + role: loginListing.data.role, + }); + }); + + it("should return null if a JWT token payload is a string", async () => { + const token = await jwt.sign("I'm just a dumb string!", JWT_SECRET); + + const request = new Request("http://localhost/api", { + headers: [["Authorization", `Bearer ${token}`]], + }); + + const event = getEvent({ request }); + const tokenDesc = await authenticate(event, JWT_SECRET); + + expect(tokenDesc).to.be.null; + }); + + it("should return null if a JWT token payload is missing a role", async () => { + const token = await jwt.sign( + { + sub: loginListing.id, + username: loginListing.data.username, + }, + JWT_SECRET, + { + expiresIn: "1d", + }, + ); + + const request = new Request("http://localhost/api", { + headers: [["Authorization", `Bearer ${token}`]], + }); + + const event = getEvent({ request }); + const tokenDesc = await authenticate(event, JWT_SECRET); + + expect(tokenDesc).to.be.null; + }); + + it("should return a default/'Basic' object when a Basic header is provided", async () => { + const user = "someuser"; + const pass = "somepass"; + const token = Buffer.from(`${user}:${pass}`).toString("base64"); + const request = new Request("http://localhost/api", { + headers: [["Authorization", `Basic ${token}`]], + }); + + const event = getEvent({ request }); + const tokenDesc = await authenticate(event, JWT_SECRET); + + expect(tokenDesc).to.deep.equal({ + kind: "Basic", + payload: { + username: user, + password: pass, + }, + role: "default", + }); + }); + + it("should return null if the token type is nonesense", async () => { + const request = new Request("http://localhost/api", { + headers: [["Authorization", "Nonesense tokenbody"]], + }); + + const event = getEvent({ request }); + const tokenDesc = await authenticate(event, JWT_SECRET); + + expect(tokenDesc).to.be.null; + }); + }); + + describe("isAuthorized", () => { + const authRules: Record = { + alternativeRole: [ + { + action: "allow", + methods: ["*"], + endpoint: "/api/thing", + }, + ], + superUser: [ + { + action: "allow", + methods: ["GET", "POST", "DELETE", "PUT"], + endpoint: "*", + }, + ], + tokenBearer: [ + // user is explicitely not allowed to use PATCH + { action: "deny", methods: ["PATCH"], endpoint: "*" }, + + // user is allowed to POST to resources + { + action: "allow", + methods: ["POST"], + endpoint: "/api/resource", + tokenKind: "Bearer", + }, + + // user is allowed to use any method on a single resource + { action: "allow", methods: ["*"], endpoint: "/api/resource/[resourceid]" }, + + // user is allowed to GET or DELETE any resource nested under resource + { + action: "allow", + methods: ["GET", "DELETE"], + endpoint: "/api/resource/[resourceid]/*", + }, + ], + tokenNone: [ + // user is allowed to hit an endpoint with a "None" token + { action: "allow", methods: ["POST"], endpoint: "/api/user", tokenKind: "None" }, + ], + tokenBasic: [ + // user is allowed to hit an endpoint with a "Basic token" + { + action: "allow", + methods: ["POST"], + endpoint: "/api/token", + tokenKind: "Basic", + }, + ], + }; + + const tokenBearer = { kind: "Bearer", role: "tokenBearer", payload: {} } as const; + + const tests: TestData[] = [ + { + heading: "should return allowed when a user calls an explicitly allowed path", + conditions: { + token: tokenBearer, + method: "POST", + path: "/api/resource", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + { + heading: "should match paths based on the role provided in the token", + conditions: { + token: { kind: "Bearer", role: "alternativeRole", payload: {} }, + method: "POST", + path: "/api/thing", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + { + heading: "should ignore paths that only exist on other roles", + conditions: { + token: { kind: "Bearer", role: "alternativeRole", payload: {} }, + method: "POST", + path: "/api/resource", + }, + expectations: { + result: AuthorizationResult.Denied, + }, + }, + { + heading: "should return unauthenticated when a user has the wrong token type", + conditions: { + token: { + kind: "Basic", + role: "tokenBearer", + payload: { username: "Mr. Sneaky", password: "something-secret" }, + }, + method: "POST", + path: "/api/resource", + }, + expectations: { + result: AuthorizationResult.Unauthenticated, + }, + }, + { + heading: "should return allowed when a rule opens a path with a wildcard method", + conditions: { + token: tokenBearer, + method: "PUT", + path: "/api/resource/some-resource-id", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + { + heading: "should return denied when the endpoint is valid, but the method is not", + conditions: { + token: tokenBearer, + method: "GET", + path: "/api/resource", + }, + expectations: { + result: AuthorizationResult.Denied, + }, + }, + { + heading: "should return allowed when matching a wildcard endpoint", + conditions: { + token: { + kind: "Bearer", + role: "superUser", + payload: {}, + }, + method: "PUT", + path: "/api/magicresource", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + { + heading: + "should return denied wehen matching a wildcard endpoint with an invalid method", + conditions: { + token: { + kind: "Bearer", + role: "superUser", + payload: {}, + }, + method: "PATCH", + path: "/api/magicresource", + }, + expectations: { + result: AuthorizationResult.Denied, + }, + }, + { + heading: "should return allowed when matching a nested endpoint wildcard", + conditions: { + token: tokenBearer, + method: "GET", + path: "/api/resource/some-resource-id/nested-resource", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + { + heading: + "should return return allowed when matching a resource deeply " + + "nested under an endpoint wildcard", + conditions: { + token: tokenBearer, + method: "GET", + path: + "/api/resource/some-resource-id/nested-resource/" + + "more-nested-resource/more-nested-id", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + { + heading: + "should not match a path rule if the request path ends before the " + + "rule path's endpoint wildcard", + conditions: { + token: tokenBearer, + method: "GET", + path: "/api/resource", + }, + expectations: { + result: AuthorizationResult.Denied, + }, + }, + { + heading: + "should return denied when a 'denied' rule and an 'allowed' rule both match", + conditions: { + token: tokenBearer, + method: "PATCH", + path: "/api/resource/some-resource-id", + }, + expectations: { + result: AuthorizationResult.Denied, + }, + }, + { + heading: "should return denied when a user calls an unmentioned path", + conditions: { + token: tokenBearer, + method: "GET", + path: "/api/unmentioned", + }, + expectations: { + result: AuthorizationResult.Denied, + }, + }, + { + heading: + "should return unauthenticated when a user calls an unmentioned " + + "path with a 'None' token", + conditions: { + token: { kind: "None", role: "tokenBearer" }, + method: "GET", + path: "/api/unmentioned", + }, + expectations: { + result: AuthorizationResult.Unauthenticated, + }, + }, + { + heading: + "should return unauthenticated when a user calls an unmentioned " + + "path with a 'Basic' token", + conditions: { + token: { + kind: "Basic", + role: "tokenBearer", + payload: { username: "someuser", password: "super-secure" }, + }, + method: "GET", + path: "/api/unmentioned", + }, + expectations: { + result: AuthorizationResult.Unauthenticated, + }, + }, + { + heading: "correctly matches a route with a trailing /", + conditions: { + token: tokenBearer, + method: "POST", + path: "/api/resource/", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + { + heading: "corretly matches a route without a leading /", + conditions: { + token: tokenBearer, + method: "POST", + path: "api/resource", + }, + expectations: { + result: AuthorizationResult.Allowed, + }, + }, + ]; + + for (const { heading, conditions, expectations } of tests) { + it(heading, () => { + const { token, method, path } = conditions; + const { result } = expectations; + + expect(isAuthorized(authRules, method, path, token)).to.equal(result); + }); + } + }); +}); diff --git a/src/lib/server/test/Listing.spec.ts b/src/lib/server/test/modifyListing.spec.ts similarity index 96% rename from src/lib/server/test/Listing.spec.ts rename to src/lib/server/test/modifyListing.spec.ts index a574e6a..34339b5 100644 --- a/src/lib/server/test/Listing.spec.ts +++ b/src/lib/server/test/modifyListing.spec.ts @@ -4,7 +4,7 @@ import { Game } from "$lib/server/Game"; import { deepEqual, equal, ok } from "node:assert/strict"; import { isId } from "$lib/Id"; -describe("Listing", () => { +describe("modifyListing", () => { describe("createNewListing", () => { it("should create a new Listing with the provided data, and a new UUID", () => { const game = new Game(); diff --git a/src/lib/server/test/GameData.spec.ts b/src/lib/test/GameData.spec.ts similarity index 85% rename from src/lib/server/test/GameData.spec.ts rename to src/lib/test/GameData.spec.ts index 45bc7cf..9ef4ada 100644 --- a/src/lib/server/test/GameData.spec.ts +++ b/src/lib/test/GameData.spec.ts @@ -29,8 +29,17 @@ describe("GameData", () => { equal(isGameData(data), false); }); + it("rejects an object without a players property", () => { + const data: unknown = { + state: {}, + isStarted: false, + }; + + equal(isGameData(data), false); + }); + it("rejects an object with extra properties", () => { - const data: GameData & { extra: boolean } = { + const data: unknown = { players: [idFromString(idString)], isStarted: false, state: {}, diff --git a/src/lib/server/test/GameEvent.spec.ts b/src/lib/test/GameEvent.spec.ts similarity index 93% rename from src/lib/server/test/GameEvent.spec.ts rename to src/lib/test/GameEvent.spec.ts index ca5ac72..17c2c2b 100644 --- a/src/lib/server/test/GameEvent.spec.ts +++ b/src/lib/test/GameEvent.spec.ts @@ -9,11 +9,11 @@ import { RollForFirst, Score, SeatPlayers, -} from "../../GameEvent"; -import type { GameEventData } from "../../GameEvent"; -import type { GameData } from "../../GameData"; +} from "$lib/GameEvent"; +import type { GameEventData } from "$lib/GameEvent"; +import type { GameData } from "$lib/GameData"; import { describe, it } from "vitest"; -import type { State } from "../../State"; +import type { State } from "$lib/State"; import { doesNotThrow, deepStrictEqual, equal, ok, throws } from "assert"; import { createId, idFromString, stringFromId } from "$lib/Id"; @@ -220,7 +220,10 @@ describe("Game Events", () => { }); it("should throw if the value is not a number", () => { - throws(() => new RollForFirst({ kind: GameEventKind.RollForFirst, player: 0, value: [4] })); + throws( + () => + new RollForFirst({ kind: GameEventKind.RollForFirst, player: 0, value: [4] }), + ); }); }); @@ -262,6 +265,10 @@ describe("Game Events", () => { const state: State = { scores: [FIRST_ROLL_PENDING, FIRST_ROLL_PENDING] }; throws(() => ev.run(state)); + + // should treat missing scores as an empty array + delete state.scores; + throws(() => ev.run(state)); }); it("should throw if the player has already rolled", () => { @@ -290,7 +297,12 @@ describe("Game Events", () => { it("should reset the scores and set the winning player when everyone has rolled", () => { const state: State = { - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + ], }; let ev = new RollForFirst({ @@ -313,14 +325,24 @@ describe("Game Events", () => { deepStrictEqual(state, { dieCount: 6, - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + ], playing: 2, }); }); it("should reset tied players for tie breaker", () => { const state: State = { - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + ], }; let ev = new RollForFirst({ @@ -340,13 +362,23 @@ describe("Game Events", () => { ev.run(state); deepStrictEqual(state, { - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_LOST, FIRST_ROLL_LOST, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_LOST, + FIRST_ROLL_LOST, + FIRST_ROLL_PENDING, + ], }); }); it("should throw if a player whose lost tries to roll again", () => { const state: State = { - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_LOST, FIRST_ROLL_LOST, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_LOST, + FIRST_ROLL_LOST, + FIRST_ROLL_PENDING, + ], }; const ev = new RollForFirst({ @@ -359,7 +391,12 @@ describe("Game Events", () => { it("should allow tied players to keep rolling until somoene wins", () => { const state: State = { - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_LOST, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_LOST, + FIRST_ROLL_PENDING, + ], }; // simulate another 3-way tie @@ -379,7 +416,12 @@ describe("Game Events", () => { deepStrictEqual( state, { - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_PENDING, FIRST_ROLL_LOST, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_PENDING, + FIRST_ROLL_LOST, + FIRST_ROLL_PENDING, + ], }, "shouldn't change in a 3-way tie", ); @@ -397,7 +439,12 @@ describe("Game Events", () => { deepStrictEqual( state, { - scores: [FIRST_ROLL_PENDING, FIRST_ROLL_LOST, FIRST_ROLL_LOST, FIRST_ROLL_PENDING], + scores: [ + FIRST_ROLL_PENDING, + FIRST_ROLL_LOST, + FIRST_ROLL_LOST, + FIRST_ROLL_PENDING, + ], }, "should update for a smaller tie", ); @@ -813,6 +860,17 @@ describe("Game Events", () => { throws(() => ev.run(state)); }); + it("should throw if there are no players", () => { + const state: State = { + dieCount: 4, + heldScore: 250, + playing: 0, + }; + + const ev = new Score({ kind: GameEventKind.Score, player: 0, value: 250 }); + throws(() => ev.run(state)); + }); + it("should add the score to the players score and activate the next player", () => { let state: State = { dieCount: 4, diff --git a/src/lib/test/Listing.spec.ts b/src/lib/test/Listing.spec.ts new file mode 100644 index 0000000..c69e266 --- /dev/null +++ b/src/lib/test/Listing.spec.ts @@ -0,0 +1,111 @@ +import { createId } from "$lib/Id"; +import { isListing, type Listing } from "$lib/Listing"; +import { describe, expect, it } from "vitest"; + +let spiedValue: unknown; + +const passingDataGuard = (target: unknown): target is any => true; +const failingDataGuard = (target: unknown): target is any => false; +const spyingDataGuard = (target: unknown): target is any => { + spiedValue = target; + return true; +}; + +describe("Listing", () => { + describe("isListing", () => { + it("should return false if a required property is missing", () => { + const fullListing: Listing = { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString(), + deleted: false, + data: {}, + }; + + let listing: Partial = { ...fullListing }; + delete listing.id; + expect(isListing(listing)).to.be.false; + + listing = { ...fullListing }; + delete listing.createdAt; + expect(isListing(listing)).to.be.false; + + listing = { ...fullListing }; + delete listing.deleted; + expect(isListing(listing)).to.be.false; + + listing = { ...fullListing }; + delete listing.modifiedAt; + expect(isListing(listing)).to.be.false; + + listing = { ...fullListing }; + delete listing.data; + expect(isListing(listing)).to.be.false; + }); + + it("should return false if there is an extra property", () => { + const fullListing = { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString(), + deleted: false, + extra: "property", + data: {}, + }; + + expect(isListing(fullListing)).to.be.false; + }); + + it("should return true if all the required properties are present", () => { + const fullListing = { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString() as string | null, + deleted: false, + data: {}, + }; + + expect(isListing(fullListing)).to.be.true; + + fullListing.modifiedAt = null; + expect(isListing(fullListing)).to.be.true; + }); + + it("should return true if properties are present and the data guard returns true", () => { + const fullListing = { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString() as string | null, + deleted: false, + data: {}, + }; + + expect(isListing(fullListing, passingDataGuard)).to.be.true; + }); + + it("should return false if properties are present and the data guard returns false", () => { + const fullListing = { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString() as string | null, + deleted: false, + data: {}, + }; + + expect(isListing(fullListing, failingDataGuard)).to.be.false; + }); + + it("should pass the data object to the provided data guard", () => { + const fullListing = { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString() as string | null, + deleted: false, + data: { am: "the test object" }, + }; + + expect(isListing(fullListing, spyingDataGuard)).to.be.true; + expect(spiedValue).to.deep.equal(fullListing.data); + }); + }); +}); diff --git a/src/lib/test/Login.spec.ts b/src/lib/test/Login.spec.ts new file mode 100644 index 0000000..61f86d7 --- /dev/null +++ b/src/lib/test/Login.spec.ts @@ -0,0 +1,62 @@ +import { hashPassword, isLoginData, type LoginData } from "$lib/Login"; +import { describe, expect, it } from "vitest"; + +describe("Login", () => { + describe("isLoginData", () => { + it("should return false if a required property is missing", () => { + const fullLoginData: LoginData = { + password: "some-password", + username: "Sir User Userly III", + role: "default", + }; + + let loginData: Partial = { + ...fullLoginData, + }; + delete loginData.password; + expect(isLoginData(loginData)).to.be.false; + + loginData = { + ...fullLoginData, + }; + delete loginData.username; + expect(isLoginData(loginData)).to.be.false; + + loginData = { + ...fullLoginData, + }; + delete loginData.role; + expect(isLoginData(loginData)).to.be.false; + }); + + it("should return false if there are extra properties", () => { + const fullLoginData: LoginData & { extra: "property" } = { + password: "some-password", + username: "Sir User Userly III", + role: "default", + extra: "property", + }; + + expect(isLoginData(fullLoginData)).to.be.false; + }); + + it("should return true if the provided target is a Login", () => { + const fullLoginData: LoginData = { + password: "some-password", + username: "Sir User Userly III", + role: "default", + }; + + expect(isLoginData(fullLoginData)).to.be.true; + }); + }); + + describe("hashPassword", () => { + it("should hash a password", async () => { + const pass = "some-password"; + const hash = hashPassword(pass); + + expect(hash).to.not.equal(pass); + }); + }); +}); diff --git a/src/lib/test/ServerResponse.spec.ts b/src/lib/test/ServerResponse.spec.ts new file mode 100644 index 0000000..8b861a3 --- /dev/null +++ b/src/lib/test/ServerResponse.spec.ts @@ -0,0 +1,47 @@ +import { isServerResponse } from "$lib/ServerResponse"; +import { describe, expect, it } from "vitest"; + +describe("ServerResponse", () => { + describe("isServerResponse", () => { + it("should return false no optional property exists", () => { + const emptyResponse = {}; + + expect(isServerResponse(emptyResponse)).to.be.false; + }); + + it("should return false if more than one property is present", () => { + const overstuffedResponse = { + item: {}, + error: "something is wrong", + }; + + expect(isServerResponse(overstuffedResponse)).to.be.false; + }); + + it("should return false if there is an unkown property", () => { + const gibberishServerResponse = { + some: "gibberish", + }; + + expect(isServerResponse(gibberishServerResponse)).to.be.false; + }); + + it("should return true when target is a proper server response", () => { + const singleServerResponse = { + item: {}, + }; + + const listServerResponse = { + items: [], + }; + + const errorServerResponse = { + error: "something is wrong", + }; + + expect(isServerResponse(singleServerResponse)).to.be.true; + expect(isServerResponse(listServerResponse)).to.be.true; + expect(isServerResponse(errorServerResponse)).to.be.true; + }); + }); +}); diff --git a/src/lib/server/test/validation.spec.ts b/src/lib/test/validation.spec.ts similarity index 91% rename from src/lib/server/test/validation.spec.ts rename to src/lib/test/validation.spec.ts index 5b149f9..53d64b3 100644 --- a/src/lib/server/test/validation.spec.ts +++ b/src/lib/test/validation.spec.ts @@ -1,6 +1,6 @@ import { describe, it } from "vitest"; import { equal, ok } from "node:assert/strict"; -import { hasProperty, hasOnlyKeys } from "../../validation"; +import { hasProperty, hasOnlyKeys } from "$lib/validation"; describe("validation", () => { describe("hasProperty", () => { @@ -46,7 +46,7 @@ describe("validation", () => { third: false, fourth: null, fifth: { something: "important" }, - sixth: ["one", "two"] + sixth: ["one", "two"], }; ok(hasProperty(target, "first", "string")); @@ -59,7 +59,7 @@ describe("validation", () => { it("should return false if passed an array type and the property isn't an array", () => { const target = { - arr: "not array" + arr: "not array", }; equal(hasProperty(target, "arr", "string[]"), false); @@ -67,7 +67,7 @@ describe("validation", () => { it("should return false if the defined array contains a non-matching element", () => { const target = { - arr: ["I", "was", "born", "in", 1989] + arr: ["I", "was", "born", "in", 1989], }; equal(hasProperty(target, "arr", "string[]"), false); @@ -75,7 +75,7 @@ describe("validation", () => { it("should return true if all the elements in a defined array match", () => { const target = { - arr: ["I", "was", "born", "in", "1989"] + arr: ["I", "was", "born", "in", "1989"], }; ok(hasProperty(target, "arr", "string[]")); @@ -83,7 +83,7 @@ describe("validation", () => { it("should return true if all the elements in a defined array match one of multiple types", () => { const target = { - arr: ["I", "was", "born", "in", 1989] + arr: ["I", "was", "born", "in", 1989], }; ok(hasProperty(target, "arr", "(string|number)[]")); @@ -91,7 +91,7 @@ describe("validation", () => { it("should return true if type is null but property is nullable", () => { const target = { - nullable: null + nullable: null, }; ok(hasProperty(target, "nullable", "string", true)); @@ -107,7 +107,7 @@ describe("validation", () => { const target = { one: "one", two: "two", - three: "three" + three: "three", }; const keys = ["one", "two"]; @@ -119,7 +119,7 @@ describe("validation", () => { const target = { one: "one", two: "two", - three: "three" + three: "three", }; const keys = ["one", "two", "three"]; @@ -129,7 +129,7 @@ describe("validation", () => { it("should return true if the target has only a subset of the provided keys", () => { const target = { - one: "one" + one: "one", }; const keys = ["one", "two", "three"]; diff --git a/src/routes/api/games/+server.ts b/src/routes/api/games/+server.ts index 82a8af6..9c4e9c2 100644 --- a/src/routes/api/games/+server.ts +++ b/src/routes/api/games/+server.ts @@ -2,10 +2,12 @@ import type { RequestHandler } from "@sveltejs/kit"; import { listResponse, singleResponse } from "$lib/server/responseBodies"; import { createNewListing } from "$lib/server/modifyListing"; import { Game } from "$lib/server/Game"; -import { games } from "$lib/server/cache"; -import { writeListing } from "$lib/server/mongo"; +import { listCollection, writeListing } from "$lib/server/mongo"; +import { isListing } from "$lib/Listing"; +import type { GameData } from "$lib/GameData"; -export const GET: RequestHandler = (): Response => { +export const GET: RequestHandler = async (): Promise => { + const games = await listCollection("games", isListing); return listResponse(games); }; @@ -13,7 +15,6 @@ export const POST: RequestHandler = async (): Promise => { const newListing = createNewListing(new Game()); await writeListing("games", newListing); - games.push(newListing); return singleResponse(newListing.id); }; diff --git a/src/routes/api/games/[gameid]/+server.ts b/src/routes/api/games/[gameid]/+server.ts index 436ea74..d7f99f5 100644 --- a/src/routes/api/games/[gameid]/+server.ts +++ b/src/routes/api/games/[gameid]/+server.ts @@ -1,15 +1,29 @@ -import { games } from "$lib/server/cache"; -import { badRequestResponse, notFoundResponse, singleResponse } from "$lib/server/responseBodies"; +import type { GameData } from "$lib/GameData"; +import { idFromString, type Id } from "$lib/Id"; +import { isListing } from "$lib/Listing"; +import { readListingById } from "$lib/server/mongo"; +import { + badRequestResponse, + notFoundResponse, + singleResponse, +} from "$lib/server/responseBodies"; import type { RequestHandler } from "@sveltejs/kit"; -export const GET: RequestHandler = ({ params }): Response => { - const id = params["gameid"]; +export const GET: RequestHandler = async ({ params }): Promise => { + const idStr = params["gameid"]; - if (!id) { + if (!idStr) { return badRequestResponse("missing gameid parameter"); } - const game = games.find(({ id: gid }) => id === gid.toString()); + let id: Id; + try { + id = idFromString(idStr); + } catch (err) { + return notFoundResponse(); + } + + const game = await readListingById("games", id, isListing); if (!game) { return notFoundResponse(); diff --git a/src/routes/api/token/+server.ts b/src/routes/api/token/+server.ts index 9a2c8ce..9994e7f 100644 --- a/src/routes/api/token/+server.ts +++ b/src/routes/api/token/+server.ts @@ -10,6 +10,7 @@ import { import type { RequestHandler } from "@sveltejs/kit"; import { compare } from "bcrypt"; import { createToken } from "$lib/server/auth"; +import { JWT_SECRET } from "$env/static/private"; export const POST: RequestHandler = async ({ locals }): Promise => { try { @@ -33,7 +34,7 @@ export const POST: RequestHandler = async ({ locals }): Promise => { } if (await compare(password, listing.data.password)) { - const token = await createToken(listing); + const token = await createToken(listing, JWT_SECRET); return singleResponse(token); } diff --git a/src/tests/hooks.server.spec.ts b/src/tests/hooks.server.spec.ts index 93157f2..297f5e5 100644 --- a/src/tests/hooks.server.spec.ts +++ b/src/tests/hooks.server.spec.ts @@ -1,7 +1,7 @@ import type { Cookies, RequestEvent } from "@sveltejs/kit"; import { describe, it, expect, afterEach } from "vitest"; import * as auth from "../lib/server/auth"; -import { handle } from "../hooks.server"; +import { getHandleFn } from "../hooks.server"; import { createId } from "$lib/Id"; let events: RequestEvent[] = []; @@ -32,7 +32,7 @@ const resolve = async (event: RequestEvent): Promise => { return new Response(); }; -describe("handle", () => { +describe("getHandleFn", () => { afterEach(() => { event.locals.user = {}; event.request.headers.delete("authorization"); @@ -41,12 +41,12 @@ describe("handle", () => { it("returns unauthorized response if caller isn't properly authenticated", async () => { event.request.headers.set("authorization", "Nonesense Token"); - const res = await handle({ event, resolve }); + const res = await getHandleFn("server-secret")({ event, resolve }); expect(res.status).to.equal(401); }); it("returns unauthorized response if caller is missing required auth header", async () => { - const res = await handle({ event, resolve }); + const res = await getHandleFn("server-secret")({ event, resolve }); expect(res.status).to.equal(401); }); @@ -63,21 +63,57 @@ describe("handle", () => { request: new Request("https://localhost/api/some/secret/route"), }; - const token = await auth.createToken({ - id: createId(), - createdAt: new Date().toString(), - modifiedAt: new Date().toString(), - deleted: false, - data: { - password: "somethin' secret!", - username: "Mr. Man", - role: "default", + const token = await auth.createToken( + { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString(), + deleted: false, + data: { + password: "somethin' secret!", + username: "Mr. Man", + role: "default", + }, }, - }); + "server-secret", + ); ev.request.headers.set("authorization", `Bearer ${token}`); - const res = await handle({ event: ev, resolve }); + const res = await getHandleFn("server-secret")({ event: ev, resolve }); expect(res.status).to.equal(403); }); + + it("attaches information about the credentials to the event and resolves when the user is authorized", async () => { + const ev: RequestEvent = { + ...event, + url: new URL("https://localhost/api/some/secret/route"), + request: new Request("https://localhost/api/some/secret/route"), + }; + + const token = await auth.createToken( + { + id: createId(), + createdAt: new Date().toString(), + modifiedAt: new Date().toString(), + deleted: false, + data: { + password: "somethin' secret!", + username: "Mr. Man", + role: "player", + }, + }, + "server-secret", + ); + + ev.request.headers.set("authorization", `Bearer ${token}`); + + const res = await getHandleFn("server-secret")({ event: ev, resolve }); + expect(res.status).to.equal(200); + + const user: auth.LocalCredentials = events[0].locals.user; + + expect(user.role).to.equal("player"); + expect(user.kind).to.equal("Bearer"); + }); }); diff --git a/src/tests/requests.http b/src/tests/requests.http index 3afe69f..14e0fdc 100644 --- a/src/tests/requests.http +++ b/src/tests/requests.http @@ -1,7 +1,7 @@ @token=token - GET https://localhost:5173/api Accept: application/json +Authorization: Bearer {{token}} ### @@ -17,8 +17,9 @@ Authorization: Bearer {{token}} ### -GET https://localhost:5173/api/games/de4cdb8c-0346-4ac6-a7a8-b4135b2d79e3 +GET https://localhost:5173/api/games/6790386c4a41c3599d47d986 Accept: application/json +Authorization: Bearer {{token}} ###