diff options
| author | Fuwn <[email protected]> | 2026-06-01 15:11:27 +0000 |
|---|---|---|
| committer | Fuwn <[email protected]> | 2026-06-01 15:11:27 +0000 |
| commit | 7c49986a5646e892a115725abb43cf97b5d7c1d2 (patch) | |
| tree | ed842f6009cdd07f0ff890bfde3be90c49759970 | |
| parent | fix(badges): drop content-visibility that clipped tooltip and tilt (diff) | |
| download | due.moe-7c49986a5646e892a115725abb43cf97b5d7c1d2.tar.xz due.moe-7c49986a5646e892a115725abb43cf97b5d7c1d2.zip | |
PUT /api/badges?shadowHide=<userId> called setShadowHidden on an
arbitrary user_id with no ownership/privilege check, so any logged-in
user could flip shadow_hidden on another user's badges (e.g. un-hide
moderator-hidden ones). The GraphQL path already guarded this; the REST
twin didn't. Extract the owner-or-privileged check into a shared
isOwnerOrPrivileged helper, use it in both the REST endpoint and the
GraphQL resolver, and add a regression test.
| -rw-r--r-- | src/graphql/user/resolvers.ts | 3 | ||||
| -rw-r--r-- | src/lib/Utility/authorisation.test.ts | 20 | ||||
| -rw-r--r-- | src/lib/Utility/authorisation.ts | 9 | ||||
| -rw-r--r-- | src/routes/api/badges/+server.ts | 14 |
4 files changed, 40 insertions, 6 deletions
diff --git a/src/graphql/user/resolvers.ts b/src/graphql/user/resolvers.ts index 905a2b4f..a90c6c4c 100644 --- a/src/graphql/user/resolvers.ts +++ b/src/graphql/user/resolvers.ts @@ -25,6 +25,7 @@ import { type UserPreferences, } from "$lib/Database/SB/User/preferences"; import { decodeAuthCookieOrNull } from "$lib/Effect/authCookie"; +import { isOwnerOrPrivileged } from "$lib/Utility/authorisation"; import privilegedUser from "$lib/Utility/privilegedUser"; import type { Badge, Resolvers as RootResolvers, WithIndex } from "../$types"; @@ -110,7 +111,7 @@ const ensureOwnerOrPrivileged = ( authorised: boolean, targetUserId: number, ) => { - if (!authorised && identity.id !== targetUserId) + if (!isOwnerOrPrivileged(identity.id, targetUserId, authorised)) throw new Error("Unauthorized"); }; diff --git a/src/lib/Utility/authorisation.test.ts b/src/lib/Utility/authorisation.test.ts new file mode 100644 index 00000000..0027782b --- /dev/null +++ b/src/lib/Utility/authorisation.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from "vitest"; +import { isOwnerOrPrivileged } from "./authorisation"; + +describe("isOwnerOrPrivileged", () => { + it("allows the owner to act on their own resources", () => { + expect(isOwnerOrPrivileged(7, 7, false)).toBe(true); + }); + + it("allows a privileged user to act on anyone", () => { + expect(isOwnerOrPrivileged(7, 999, true)).toBe(true); + }); + + it("blocks a non-privileged user acting on someone else (the IDOR case)", () => { + expect(isOwnerOrPrivileged(7, 999, false)).toBe(false); + }); + + it("allows a privileged owner (both conditions)", () => { + expect(isOwnerOrPrivileged(7, 7, true)).toBe(true); + }); +}); diff --git a/src/lib/Utility/authorisation.ts b/src/lib/Utility/authorisation.ts new file mode 100644 index 00000000..c6b64414 --- /dev/null +++ b/src/lib/Utility/authorisation.ts @@ -0,0 +1,9 @@ +/** + * Whether a caller may act on resources belonging to `targetUserId`: either the + * caller owns them, or the caller is a privileged (allow-listed) user. + */ +export const isOwnerOrPrivileged = ( + callerUserId: number, + targetUserId: number, + privileged: boolean, +) => privileged || callerUserId === targetUserId; diff --git a/src/routes/api/badges/+server.ts b/src/routes/api/badges/+server.ts index 2673273c..10b63125 100644 --- a/src/routes/api/badges/+server.ts +++ b/src/routes/api/badges/+server.ts @@ -16,6 +16,7 @@ import { import { decodeAuthCookieOrNull } from "$lib/Effect/authCookie"; import { decodeRequestJsonOrThrow } from "$lib/Effect/requestBody"; import { appOrigin, appOriginHeaders } from "$lib/Utility/appOrigin"; +import { isOwnerOrPrivileged } from "$lib/Utility/authorisation"; import privilegedUser from "$lib/Utility/privilegedUser"; const unauthorised = () => new Response("Unauthorised", { status: 401 }); @@ -76,11 +77,14 @@ export const PUT = async ({ cookies, url, request }) => { if (!identity) return unauthorised(); const authorised = privilegedUser(identity.id); - if (url.searchParams.get("shadowHide")) - await setShadowHidden( - Number(url.searchParams.get("shadowHide")), - authorised, - ); + if (url.searchParams.get("shadowHide")) { + const targetUserId = Number(url.searchParams.get("shadowHide")); + + if (!isOwnerOrPrivileged(identity.id, targetUserId, authorised)) + return unauthorised(); + + await setShadowHidden(targetUserId, authorised); + } if (url.searchParams.get("import") || undefined) { const importedBadges = await decodeRequestJsonOrThrow( |