fix(security): harden user router role guards + drop self-service email change

Three high-severity issues in user router:

1. user.update accepted both `role` and `roles[]` from input but only
   guarded the singular `role`. A PROGRAM_ADMIN could pass `roles:
   ['SUPER_ADMIN']` and self-escalate. Now applies the same guards to the
   array field and uses both fields when checking the target's current
   admin tier.

2. user.updateRoles only blocked SUPER_ADMIN grants; PROGRAM_ADMIN could
   grant PROGRAM_ADMIN laterally and could pass `roles: []` against any
   existing SUPER_ADMIN to silently demote them. Now blocks PROGRAM_ADMIN
   grants and refuses to mutate any target who currently holds SUPER_ADMIN
   or PROGRAM_ADMIN unless the caller is SUPER_ADMIN.

3. user.bulkUpdateRoles had the same omission and additionally let a
   PROGRAM_ADMIN strip SUPER_ADMIN from every peer admin in one call. Now
   requires SUPER_ADMIN for any add/remove of admin-tier roles, blocks
   modifying admin targets entirely from non-super-admins, and adds a
   PROGRAM_ADMIN self-demote guard.

Plus: user.updateProfile previously let any authenticated user silently
overwrite their own email with no verification or notification — turning
any short-lived session compromise into permanent account takeover via
password reset on the new address. Email is removed from the input
schema; the profile page email field is now read-only with a "contact
an administrator" hint.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matt
2026-04-29 03:29:09 +02:00
parent a1c293028a
commit 89e637843a
2 changed files with 109 additions and 37 deletions

View File

@@ -106,7 +106,6 @@ export default function ProfileSettingsPage() {
const handleSaveProfile = async () => {
try {
await updateProfile.mutateAsync({
email: email || undefined,
name: name || undefined,
bio,
phoneNumber: phoneNumber || null,
@@ -229,11 +228,13 @@ export default function ProfileSettingsPage() {
id="email"
type="email"
value={email}
onChange={(e) => setEmail(e.target.value)}
readOnly
disabled
placeholder="you@example.com"
/>
<p className="text-xs text-muted-foreground">
This will be used for login and all notification emails.
Used for login and notifications. Contact an administrator to
change your email address.
</p>
</div>

View File

@@ -96,7 +96,6 @@ export const userRouter = router({
updateProfile: protectedProcedure
.input(
z.object({
email: z.string().email().optional(),
name: z.string().min(1).max(255).optional(),
bio: z.string().max(1000).optional(),
phoneNumber: z.string().max(20).optional().nullable(),
@@ -111,35 +110,19 @@ export const userRouter = router({
})
)
.mutation(async ({ ctx, input }) => {
// Email is intentionally NOT in the input schema. Allowing self-service
// email changes turns any short-lived session compromise into permanent
// account takeover via password reset on the new address. Email changes
// require an admin-driven flow (or a future verified-change procedure).
const {
bio,
expertiseTags,
availabilityJson,
preferredWorkload,
digestFrequency,
email,
...directFields
} = input
const normalizedEmail = email?.toLowerCase().trim()
if (normalizedEmail !== undefined) {
const existing = await ctx.prisma.user.findFirst({
where: {
email: normalizedEmail,
NOT: { id: ctx.user.id },
},
select: { id: true },
})
if (existing) {
throw new TRPCError({
code: 'CONFLICT',
message: 'Another account already uses this email address',
})
}
}
// If bio is provided, merge it into metadataJson
let metadataJson: Prisma.InputJsonValue | undefined
if (bio !== undefined) {
@@ -155,7 +138,6 @@ export const userRouter = router({
where: { id: ctx.user.id },
data: {
...directFields,
...(normalizedEmail !== undefined && { email: normalizedEmail }),
...(metadataJson !== undefined && { metadataJson }),
...(expertiseTags !== undefined && { expertiseTags }),
...(digestFrequency !== undefined && { digestFrequency }),
@@ -564,15 +546,25 @@ export const userRouter = router({
where: { id },
})
if (targetUser.role === 'SUPER_ADMIN' && ctx.user.role !== 'SUPER_ADMIN') {
const callerIsSuperAdmin = ctx.user.role === 'SUPER_ADMIN'
const targetHasSuperAdmin =
targetUser.role === 'SUPER_ADMIN' || targetUser.roles.includes('SUPER_ADMIN')
const targetHasProgramAdmin =
targetUser.role === 'PROGRAM_ADMIN' || targetUser.roles.includes('PROGRAM_ADMIN')
if (targetHasSuperAdmin && !callerIsSuperAdmin) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Cannot modify super admin',
})
}
// Prevent non-super-admins from changing admin roles
if (data.role && targetUser.role === 'PROGRAM_ADMIN' && ctx.user.role !== 'SUPER_ADMIN') {
// Prevent non-super-admins from changing admin roles (singular OR array)
if (
(data.role || data.roles) &&
targetHasProgramAdmin &&
!callerIsSuperAdmin
) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can change admin roles',
@@ -580,13 +572,26 @@ export const userRouter = router({
}
// Prevent non-super-admins from assigning super admin or admin role
if (data.role === 'SUPER_ADMIN' && ctx.user.role !== 'SUPER_ADMIN') {
// — check both the singular `role` field AND the `roles[]` array.
if (data.role === 'SUPER_ADMIN' && !callerIsSuperAdmin) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can assign super admin role',
})
}
if (data.role === 'PROGRAM_ADMIN' && ctx.user.role !== 'SUPER_ADMIN') {
if (data.role === 'PROGRAM_ADMIN' && !callerIsSuperAdmin) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can assign admin role',
})
}
if (data.roles?.includes('SUPER_ADMIN') && !callerIsSuperAdmin) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can assign super admin role',
})
}
if (data.roles?.includes('PROGRAM_ADMIN') && !callerIsSuperAdmin) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can assign admin role',
@@ -1804,10 +1809,30 @@ export const userRouter = router({
roles: z.array(z.nativeEnum(UserRole)).min(1),
}))
.mutation(async ({ ctx, input }) => {
// Guard: only SUPER_ADMIN can grant SUPER_ADMIN
if (input.roles.includes('SUPER_ADMIN') && ctx.user.role !== 'SUPER_ADMIN') {
const callerIsSuperAdmin = ctx.user.role === 'SUPER_ADMIN'
// Guard: only SUPER_ADMIN can grant SUPER_ADMIN or PROGRAM_ADMIN
if (input.roles.includes('SUPER_ADMIN') && !callerIsSuperAdmin) {
throw new TRPCError({ code: 'FORBIDDEN', message: 'Only super admins can grant super admin role' })
}
if (input.roles.includes('PROGRAM_ADMIN') && !callerIsSuperAdmin) {
throw new TRPCError({ code: 'FORBIDDEN', message: 'Only super admins can grant admin role' })
}
// Guard: only SUPER_ADMIN can modify a user who currently holds SUPER_ADMIN
// or PROGRAM_ADMIN — otherwise a PROGRAM_ADMIN could demote peers/super-admins.
const target = await ctx.prisma.user.findUniqueOrThrow({
where: { id: input.userId },
select: { role: true, roles: true },
})
const targetHasAdmin =
target.role === 'SUPER_ADMIN' ||
target.role === 'PROGRAM_ADMIN' ||
target.roles.includes('SUPER_ADMIN') ||
target.roles.includes('PROGRAM_ADMIN')
if (targetHasAdmin && !callerIsSuperAdmin) {
throw new TRPCError({ code: 'FORBIDDEN', message: 'Only super admins can change admin roles' })
}
// Set primary role to highest privilege role
const rolePriority: UserRole[] = ['SUPER_ADMIN', 'PROGRAM_ADMIN', 'JURY_MEMBER', 'MENTOR', 'OBSERVER', 'AWARD_MASTER', 'APPLICANT', 'AUDIENCE']
@@ -1835,26 +1860,72 @@ export const userRouter = router({
}),
)
.mutation(async ({ ctx, input }) => {
// Self-demote guard
const callerIsSuperAdmin = ctx.user.role === 'SUPER_ADMIN'
// Self-demote guards
if (input.removeRole === 'SUPER_ADMIN' && input.userIds.includes(ctx.user.id)) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'You cannot remove SUPER_ADMIN from self',
})
}
// Privilege guard: only SUPER_ADMIN can grant SUPER_ADMIN
if (input.addRole === 'SUPER_ADMIN' && ctx.user.role !== 'SUPER_ADMIN') {
if (input.removeRole === 'PROGRAM_ADMIN' && input.userIds.includes(ctx.user.id)) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can grant super admin role',
message: 'You cannot remove PROGRAM_ADMIN from self',
})
}
// Privilege guards: only SUPER_ADMIN may add/remove SUPER_ADMIN or PROGRAM_ADMIN.
// Without the symmetric remove-side guard a PROGRAM_ADMIN could strip
// SUPER_ADMIN from peers; without the add-side PROGRAM_ADMIN guard a
// PROGRAM_ADMIN could grant peer-admin laterally.
if (
(input.addRole === 'SUPER_ADMIN' || input.removeRole === 'SUPER_ADMIN') &&
!callerIsSuperAdmin
) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can change super admin role',
})
}
if (
(input.addRole === 'PROGRAM_ADMIN' || input.removeRole === 'PROGRAM_ADMIN') &&
!callerIsSuperAdmin
) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can change admin role',
})
}
const targets = await ctx.prisma.user.findMany({
where: { id: { in: input.userIds } },
select: { id: true, name: true, email: true, roles: true, mentorOnboardingSentAt: true },
select: { id: true, name: true, email: true, role: true, roles: true, mentorOnboardingSentAt: true },
})
// Block modifying any target who currently holds an admin tier role
// unless the caller is SUPER_ADMIN. This prevents a PROGRAM_ADMIN from
// using a non-admin add/remove (e.g. addRole: MENTOR) to mutate the
// record of a SUPER_ADMIN target — even though Prisma would only touch
// `roles[]`, the audit trail and downstream logic shouldn't allow
// peer admins to mutate higher-tier accounts at all.
if (!callerIsSuperAdmin) {
const adminTarget = targets.find(
(t) =>
t.role === 'SUPER_ADMIN' ||
t.role === 'PROGRAM_ADMIN' ||
t.roles.includes('SUPER_ADMIN') ||
t.roles.includes('PROGRAM_ADMIN'),
)
if (adminTarget) {
throw new TRPCError({
code: 'FORBIDDEN',
message: 'Only super admins can modify admin accounts',
})
}
}
let updated = 0
let alreadyHadRole = 0
const newlyMentor: typeof targets = []