fix(security): scope profile-update override to global_admin only

Address PR review: admin/security_admin are per-tenant roles (granted via
the login_tenant join) but the JWT does not encode which tenant they
apply to, so honoring them as an override let an admin of one tenant edit
users in another tenant (cross-tenant privilege escalation). Restrict the
override to global_admin, which is a login-level, non-tenant-scoped flag
and safe to trust from the JWT.

Also reuse the shared getUserRoles helper from stakeholder-access instead
of duplicating the role-parsing, so the two authorization checks can't
drift apart.

Tests: global_admin can still edit another user; a per-tenant admin
(admin/security_admin without global_admin) is now blocked.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: hanapotski

server/__test__/account.test.ts

@@ -712,11 +712,11 @@ describe("Account", () => {
     expect(res.status).toHaveBeenCalledWith(403);
   });
 
-  it("lets an admin update another user's profile", async () => {
+  it("lets a global_admin update another user's profile", async () => {
     const res = mockResponse();
     const req = mockRequest({
       params: { userid: "999" },
-      user: { id: 123, email: "admin@test.com", sub: "admin,global_admin" },
+      user: { id: 123, email: "admin@test.com", sub: "global_admin" },
       body: {
         firstName: "New",
         lastName: "Name",
@@ -739,4 +739,33 @@ describe("Account", () => {
     expect(updateUserProfileMock).toHaveBeenCalledTimes(1);
     expect(res.status).toHaveBeenCalledWith(200);
   });
+
+  it("blocks a per-tenant admin from updating another user's profile (cross-tenant escalation)", async () => {
+    const res = mockResponse();
+    const req = mockRequest({
+      params: { userid: "999" },
+      // admin/security_admin are per-tenant roles; the JWT does not encode which
+      // tenant they apply to, so they must NOT grant a cross-user override.
+      user: {
+        id: 123,
+        email: "tenantadmin@test.com",
+        sub: "admin,security_admin",
+      },
+      body: {
+        firstName: "Attacker",
+        lastName: "Controlled",
+        email: "victim-new@test.com",
+        tenantId: "1",
+      },
+    });
+    const next = mockNext();
+    const updateUserProfileMock =
+      accountService.updateUserProfile as jest.MockedFunction<
+        typeof accountService.updateUserProfile
+      >;
+
+    await accountController.updateUserProfile(req, res, next);
+    expect(updateUserProfileMock).not.toHaveBeenCalled();
+    expect(res.status).toHaveBeenCalledWith(403);
+  });
 });

server/app/controllers/account-controller.ts

@@ -8,6 +8,7 @@ import {
   User,
   Role,
 } from "../../types/account-types";
+import { getUserRoles } from "../helpers/stakeholder-access";
 
 const getAll: RequestHandler<
   never,
@@ -271,18 +272,20 @@ const updateUserProfile: RequestHandler<
 > = async (req, res) => {
   const userid = req.params.userid;
 
-  // Authorization: a user may only update their own profile, unless they hold
-  // an account-management admin role. Prevents the IDOR where any authenticated
-  // (or, previously, unauthenticated) caller could overwrite any user's profile
-  // -- including the email tied to their login (security audit finding #2).
-  const roles = new Set(
-    (req.user?.sub || req.user?.role || "").split(",").filter(Boolean)
-  );
-  const isAccountAdmin =
-    roles.has("admin") ||
-    roles.has("security_admin") ||
-    roles.has("global_admin");
-  if (String(req.user?.id) !== String(userid) && !isAccountAdmin) {
+  // Authorization: a user may only update their own profile, unless they are a
+  // global_admin. Prevents the IDOR where any authenticated (or, previously,
+  // unauthenticated) caller could overwrite any user's profile -- including the
+  // email tied to their login (security audit finding #2).
+  //
+  // Only global_admin is allowed to override, NOT admin/security_admin: those
+  // are per-tenant roles (from the login_tenant join) but the JWT does not
+  // encode which tenant they were granted for, so honoring them here would let
+  // an admin of one tenant edit users in another tenant (cross-tenant privilege
+  // escalation). global_admin is a login-level, non-tenant-scoped flag, so it is
+  // safe to trust from the JWT.
+  const roles = getUserRoles(req.user);
+  const isGlobalAdmin = roles.has("global_admin");
+  if (String(req.user?.id) !== String(userid) && !isGlobalAdmin) {
     return res.status(403).json({
       isSuccess: false,
       code: "FORBIDDEN",

server/app/helpers/stakeholder-access.ts

@@ -18,7 +18,7 @@ type StakeholderAccessRequest = {
   };
 };
 
-const getUserRoles = (user?: StakeholderAccessUser) =>
+export const getUserRoles = (user?: StakeholderAccessUser) =>
   new Set((user?.sub || user?.role || "").split(",").filter(Boolean));
 
 const isRestrictedDataEntryUser = (user?: StakeholderAccessUser) => {