diff --git a/app/(main)/profile/events/Analytics/[eventId]/page.jsx b/app/(main)/profile/events/Analytics/[eventId]/page.jsx index cf8b4e9..be32803 100644 --- a/app/(main)/profile/events/Analytics/[eventId]/page.jsx +++ b/app/(main)/profile/events/Analytics/[eventId]/page.jsx @@ -6,14 +6,20 @@ // was written and exported from the modals barrel, but nothing ever mounted it // — so the button 404'd and the component was dead code. // -// Gated to ADMIN on the server, matching /profile/attendance: `proxy.ts` only -// checks for a valid session, not a role, so without this any signed-in -// participant who typed the path would see every registrant's email and -// payment screenshot. +// Gated on the server, because `proxy.ts` only checks for a valid session, not +// a role — without this any signed-in participant who typed the path would see +// every registrant's email and payment screenshot. +// +// The gate is `canViewFormAnalytics`, the same rule the endpoint this page +// calls enforces. It was `isAdmin` at first, which was narrower than both the +// API and the Express-era route (`access !== "USER"` in App.jsx) — so a +// president or director was shown the Analytics button by EventsView, and then +// bounced back to /profile by this line when they clicked it. import { redirect } from "next/navigation"; -import { getCurrentUser, isAdmin } from "@/lib/auth/access"; +import { getCurrentUser } from "@/lib/auth/access"; +import { canViewFormAnalytics } from "@/lib/auth/permissions"; import EventStats from "@/src/features/Modals/Event/EventStats/EventStats"; export default async function Page({ params }) { @@ -21,7 +27,7 @@ export default async function Page({ params }) { const user = await getCurrentUser(); if (!user) redirect(`/Login?next=/profile/events/Analytics/${eventId}`); - if (!isAdmin(user)) redirect("/profile"); + if (!canViewFormAnalytics(user)) redirect("/profile"); return ; } diff --git a/app/api/form/getFormAnalytics/[id]/route.ts b/app/api/form/getFormAnalytics/[id]/route.ts index 0f2070d..f8d3318 100644 --- a/app/api/form/getFormAnalytics/[id]/route.ts +++ b/app/api/form/getFormAnalytics/[id]/route.ts @@ -1,8 +1,7 @@ import { prisma } from "@/lib/db"; import { expressError, handle, json } from "@/lib/api/express"; import { getCurrentUser } from "@/lib/auth/access"; -import { can } from "@/lib/auth/permissions"; -import { envList, getEnv } from "@/lib/env"; +import { canViewFormAnalytics } from "@/lib/auth/permissions"; /** * GET /api/form/getFormAnalytics/:id @@ -21,18 +20,10 @@ export async function GET( const user = await getCurrentUser(); if (!user) return expressError(401, "Token is required"); - // Role list lives in `lib/auth/permissions.ts`; the address allowlist that - // sits beside it is `FORM_ANALYTICS_ALLOWED_EMAILS` in the environment, - // both previously hardcoded here. - const allowedEmails = envList(getEnv().FORM_ANALYTICS_ALLOWED_EMAILS).map( - (entry) => entry.toLowerCase(), - ); - + // Role list and address allowlist both live in `lib/auth/permissions.ts`, + // shared with the page that renders EventStats so the two cannot disagree. // 401 rather than 403, matching `ApiError(status.UNAUTHORIZED, ...)`. - if ( - !can(user, "FORM_ANALYTICS_VIEW") && - !allowedEmails.includes(user.email.toLowerCase()) - ) { + if (!canViewFormAnalytics(user)) { return expressError(401, "Access Denied"); } diff --git a/lib/auth/permissions.ts b/lib/auth/permissions.ts index da7f91e..3b36219 100644 --- a/lib/auth/permissions.ts +++ b/lib/auth/permissions.ts @@ -1,6 +1,8 @@ import "server-only"; import type { SafeUser } from "@/lib/auth/access"; +import { FORM_ANALYTICS_ROLES } from "@/lib/auth/roles"; +import { envList, getEnv } from "@/lib/env"; /** * Who may do what, declared once. @@ -15,18 +17,7 @@ import type { SafeUser } from "@/lib/auth/access"; */ export const PERMISSIONS = { /** Read a form's registration analytics (EventStats). */ - FORM_ANALYTICS_VIEW: new Set([ - "PRESIDENT", - "VICEPRESIDENT", - "DIRECTOR_CREATIVE", - "DIRECTOR_TECHNICAL", - "DIRECTOR_MARKETING", - "DIRECTOR_OPERATIONS", - // Not a member of the AccessTypes enum, so it can never match. Carried over - // from the Express controller, where it was equally dead; kept so the list - // stays a faithful copy rather than a silent narrowing. - "DIRECTOR_SPONSORSHIP", - ]), + FORM_ANALYTICS_VIEW: new Set(FORM_ANALYTICS_ROLES), } satisfies Record>; export type Permission = keyof typeof PERMISSIONS; @@ -43,3 +34,26 @@ export function can( if (user.access === "ADMIN") return true; return PERMISSIONS[permission].has(user.access); } + +/** + * The whole of the Express analytics rule, in one place: + * + * if (!allowedUsers.includes(req.user.access) && req.user.email != "srex@…") + * + * The address list moved to `FORM_ANALYTICS_ALLOWED_EMAILS` in the environment. + * + * Both the API route and the page that renders EventStats call this, so the + * page can no longer refuse someone the endpoint would have served — which is + * exactly what happened when the page checked `isAdmin` on its own. + */ +export function canViewFormAnalytics( + user: Pick | null | undefined, +): boolean { + if (!user) return false; + if (can(user, "FORM_ANALYTICS_VIEW")) return true; + + const allowed = envList(getEnv().FORM_ANALYTICS_ALLOWED_EMAILS).map((entry) => + entry.toLowerCase(), + ); + return allowed.includes(user.email.toLowerCase()); +} diff --git a/lib/auth/roles.ts b/lib/auth/roles.ts new file mode 100644 index 0000000..9b1c318 --- /dev/null +++ b/lib/auth/roles.ts @@ -0,0 +1,39 @@ +/** + * Role lists that both the server and the browser need. + * + * Deliberately free of `server-only` and of any Node import: the analytics + * button in `EventsView` has to ask the same question the API asks, and it asks + * it in the browser. When the two lists were written out separately they + * disagreed — the route would have served a president that the table never + * offered a button to. + */ + +/** + * May read a form's registration analytics. + * + * Copied from the Express controller's `allowedUsers` + * (FED-Backend/controllers/forms/analytics.js). ADMIN is not listed because + * `can()` grants it everything, matching `checkAccess`, where ADMIN always + * passed — but the browser-side list needs it spelled out, so it is added in + * `FORM_ANALYTICS_ROLES_CLIENT` below. + */ +export const FORM_ANALYTICS_ROLES = [ + "PRESIDENT", + "VICEPRESIDENT", + "DIRECTOR_CREATIVE", + "DIRECTOR_TECHNICAL", + "DIRECTOR_MARKETING", + "DIRECTOR_OPERATIONS", + // Not a member of the AccessTypes enum, so it can never match. Carried over + // from the Express controller, where it was equally dead; kept so the list + // stays a faithful copy rather than a silent narrowing. Note that this means + // DIRECTOR_PR_AND_FINANCE and DIRECTOR_HUMAN_RESOURCE have never had + // analytics access, on this stack or the previous one. + "DIRECTOR_SPONSORSHIP", +] as const; + +/** The same list with ADMIN spelled out, for client-side `includes()` checks. */ +export const FORM_ANALYTICS_ROLES_CLIENT: readonly string[] = [ + ...FORM_ANALYTICS_ROLES, + "ADMIN", +]; diff --git a/src/authentication/SignUp/CompleteProfile.jsx b/src/authentication/SignUp/CompleteProfile.jsx index 179bc30..0c33bc2 100644 --- a/src/authentication/SignUp/CompleteProfile.jsx +++ b/src/authentication/SignUp/CompleteProfile.jsx @@ -110,10 +110,15 @@ function CompleteProfile() { userObject.github, userObject.linkedin, userObject.designation, - userObject.regForm, - userObject.access, - userObject.editProfileCount, + // `access` is the 12th argument, not the 13th. These four were off + // by one against the signature, so `regForm` landed in `access` — + // leaving every account created here with a role that matches no + // entry in any role list, which is how a member could lose a + // permission their database record still granted them. "USER", + userObject.editProfileCount, + userObject.regForm, + userObject.blurhash || "", "someToken", SESSION_TTL_MS ); diff --git a/src/sections/Profile/General/EventsView/EventsView.jsx b/src/sections/Profile/General/EventsView/EventsView.jsx index 43c0f81..39e3904 100644 --- a/src/sections/Profile/General/EventsView/EventsView.jsx +++ b/src/sections/Profile/General/EventsView/EventsView.jsx @@ -6,6 +6,7 @@ import AuthContext from "../../../../context/AuthContext"; import { api } from "../../../../services"; import { ComponentLoading, MicroLoading } from "../../../../microInteraction"; +import { FORM_ANALYTICS_ROLES_CLIENT } from "@/lib/auth/roles"; import Link from "next/link"; const Events = () => { @@ -22,16 +23,10 @@ const Events = () => { const viewPath = "/Events"; const analyticsPath = "/profile/events/Analytics"; - const analyticsAccessRoles = [ - "PRESIDENT", - "VICEPRESIDENT", - "DIRECTOR_CREATIVE", - "DIRECTOR_TECHNICAL", - "DIRECTOR_MARKETING", - "DIRECTOR_OPERATIONS", - "DIRECTOR_SPONSORSHIP", - "ADMIN", - ]; + // Shared with the API route and the Analytics page, so the button cannot be + // offered to someone the server will turn away — or withheld from someone it + // would have served. + const analyticsAccessRoles = FORM_ANALYTICS_ROLES_CLIENT; useEffect(() => { const fetchEventsData = async () => {