hard: role-gated endpoints trust stale JWT role claims

Author: Godfrey-Delight

context/progress-tracker.md

@@ -26,6 +26,21 @@ pure chore/docs commits). Direct pushes to main must also be logged here.
 - Tests: refresh-family rotation, replay → family-wide revocation + audit
   event, blocked-user denial within TTL bound, cache expiry re-query,
   cleanup job deletes-only-expired.
+<<<<<<< Updated upstream
+=======
+=======
+## 2026-08-26
+
+- Centralized role authorization on server truth: updated `UserStatusService` to cache user status and role with a 30s staleness bound (`USER_STATUS_CACHE_TTL_MS = 30_000`), and updated `RolesGuard` to enforce datastore roles instead of relying on un-enforced JWT role claims. Stale-token attacks are now rejected with 403 `AUTH_ROLE_FORBIDDEN`.
+- Added admin-only role-management endpoint `POST /admin/users/:wallet/role/reset` in `AdminRolesController`, guarded by `JwtAuthGuard` and `AdminGuard`, and audited via `@AuditAction('admin_users', 'RESET_USER_ROLE')` and `AuditInterceptor`.
+- Wired cache invalidation (`userStatusService.invalidate(wallet)`) into `setRole` and admin role reset, ensuring role changes take effect immediately on local server instance and within 30s across instances.
+- Added unit tests for `RolesGuard`, `AdminRolesController`, `UserStatusService`, and `UsersService.setRole`.
+- Fixed registration race conditions in `AuthService.register()` by eliminating application-side pre-checks (`findByWallet`, `checkUsernameExists`) and relying directly on DB-level UNIQUE constraints (`users.wallet_address`, `users.username`).
+- Added idempotent migration `20260826130000_ensure_users_unique_constraints.sql` to ensure unique indexes exist on `users.wallet_address` and `users.username`.
+- Updated `UsersRepository.createProfile()` to catch PostgreSQL unique constraint violation error `23505` and map to structured 409 `ConflictException` (`AUTH_WALLET_EXISTS`, `AUTH_USERNAME_TAKEN`).
+- Added cleanup handlers (`deleteAvatar`, `deleteUserById`) in `AuthService.register()` and `UsersRepository` to ensure failed registrations do not leave orphaned avatar files or partial user records.
+>>>>>>> Stashed changes
+>>>>>>> Stashed changes
 
 ## 2026-07-23
 

src/auth/guards/roles.guard.ts

@@ -4,8 +4,10 @@ import {
   ExecutionContext,
   ForbiddenException,
   SetMetadata,
+  Optional,
 } from '@nestjs/common';
 import { Reflector } from '@nestjs/core';
+import { UserStatusService } from '../../modules/auth/user-status.service';
 
 export const ROLES_KEY = 'roles';
 
@@ -19,18 +21,22 @@ export const ROLES_KEY = 'roles';
 export const Roles = (...roles: string[]) => SetMetadata(ROLES_KEY, roles);
 
 /**
- * Enforces the role claim carried in the JWT (set by JwtStrategy.validate).
+ * Enforces user role authorization based on server truth in the datastore
+ * (resolved via short-TTL cached UserStatusService).
  *
  * - Routes without @Roles metadata are unaffected.
- * - Tokens without a role claim (role not chosen yet, or token issued
- *   before the role was set) are rejected with 403; the client must call
- *   POST /auth/refresh after setting a role to obtain the claim.
+ * - The JWT role claim is treated as a hint only; live datastore role is enforced.
+ * - Roles revoked or changed server-side take effect within USER_STATUS_CACHE_TTL_MS (30s)
+ *   or immediately upon cache invalidation.
  */
 @Injectable()
 export class RolesGuard implements CanActivate {
-  constructor(private readonly reflector: Reflector) {}
+  constructor(
+    private readonly reflector: Reflector,
+    @Optional() private readonly userStatusService?: UserStatusService,
+  ) {}
 
-  canActivate(context: ExecutionContext): boolean {
+  async canActivate(context: ExecutionContext): Promise<boolean> {
     const requiredRoles = this.reflector.getAllAndOverride<string[]>(ROLES_KEY, [
       context.getHandler(),
       context.getClass(),
@@ -41,7 +47,21 @@ export class RolesGuard implements CanActivate {
       .switchToHttp()
       .getRequest<{ user?: { wallet: string; role?: string | null } }>();
 
-    if (!user?.role || !requiredRoles.includes(user.role)) {
+    if (!user) {
+      throw new ForbiddenException({
+        code: 'AUTH_ROLE_FORBIDDEN',
+        message: `This action requires one of the following roles: ${requiredRoles.join(', ')}.`,
+      });
+    }
+
+    let currentRole: string | null = null;
+    if (this.userStatusService && user.wallet) {
+      currentRole = await this.userStatusService.getRole(user.wallet);
+    } else {
+      currentRole = user.role ?? null;
+    }
+
+    if (!currentRole || !requiredRoles.includes(currentRole)) {
       throw new ForbiddenException({
         code: 'AUTH_ROLE_FORBIDDEN',
         message: `This action requires one of the following roles: ${requiredRoles.join(', ')}. If you just selected your role, refresh your access token.`,
@@ -50,3 +70,5 @@ export class RolesGuard implements CanActivate {
     return true;
   }
 }
+
+

src/database/repositories/users.repository.ts

@@ -226,6 +226,31 @@ export class UsersRepository {
         return (data as { wallet_address: string; role: UserRole } | null) ?? null;
     }
 
+    /**
+     * Admin override: sets or resets the user's role regardless of whether a role was set previously.
+     * Returns the updated user row, or null if the user does not exist.
+     */
+    async forceSetRole(
+        wallet: string,
+        role: UserRole | null,
+    ): Promise<{ wallet_address: string; role: UserRole | null } | null> {
+        const { data, error } = await this.supabaseService
+            .getServiceRoleClient()
+            .from('users')
+            .update({ role })
+            .eq('wallet_address', wallet)
+            .select('wallet_address, role')
+            .maybeSingle();
+
+        if (error) {
+            throw new InternalServerErrorException({
+                code: 'DATABASE_ROLE_UPDATE_FAILED',
+                message: 'Failed to update user role.',
+            });
+        }
+        return (data as { wallet_address: string; role: UserRole | null } | null) ?? null;
+    }
+
     // --- REGISTRATION METHODS ---
 
     async checkUsernameExists(username: string): Promise<boolean> {

src/modules/admin/admin-roles.controller.ts

@@ -0,0 +1,71 @@
+import {
+  Controller,
+  Post,
+  Param,
+  Body,
+  NotFoundException,
+  UseGuards,
+  UseInterceptors,
+  HttpCode,
+  HttpStatus,
+} from '@nestjs/common';
+import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth, ApiParam } from '@nestjs/swagger';
+import { JwtAuthGuard } from '../../common/guards/jwt-auth.guard';
+import { AdminGuard } from '../../auth/guards/admin.guard';
+import { AuditInterceptor } from '../../common/interceptors/audit.interceptor';
+import { AuditAction } from '../../common/decorators/audit-action.decorator';
+import { UsersRepository } from '../../database/repositories/users.repository';
+import { UserStatusService } from '../auth/user-status.service';
+import { AdminResetRoleDto } from './dto/admin-reset-role.dto';
+
+@ApiTags('admin')
+@Controller('admin')
+@UseGuards(JwtAuthGuard, AdminGuard)
+@ApiBearerAuth()
+@UseInterceptors(AuditInterceptor)
+export class AdminRolesController {
+  constructor(
+    private readonly usersRepository: UsersRepository,
+    private readonly userStatusService: UserStatusService,
+  ) {}
+
+  @Post('users/:wallet/role/reset')
+  @HttpCode(HttpStatus.OK)
+  @AuditAction('admin_users', 'RESET_USER_ROLE')
+  @ApiOperation({
+    summary: 'Reset or override user role (Admin only)',
+    description:
+      'Allows admins to reset a user\'s permanent role back to null or override it with a specific role. ' +
+      'Immediately invalidates the user\'s server-side status cache and logs an audit event.',
+  })
+  @ApiParam({ name: 'wallet', description: 'Target user wallet address' })
+  @ApiResponse({ status: 200, description: 'Role reset/updated successfully' })
+  @ApiResponse({ status: 401, description: 'Unauthorized — missing or invalid JWT' })
+  @ApiResponse({ status: 403, description: 'Forbidden — wallet is not in ADMIN_WALLETS' })
+  @ApiResponse({ status: 404, description: 'User not found' })
+  async resetUserRole(
+    @Param('wallet') wallet: string,
+    @Body() dto?: AdminResetRoleDto,
+  ) {
+    const targetRole = dto?.role ?? null;
+    const updated = await this.usersRepository.forceSetRole(wallet, targetRole);
+
+    if (!updated) {
+      throw new NotFoundException({
+        code: 'USERS_NOT_FOUND',
+        message: 'User not found.',
+      });
+    }
+
+    this.userStatusService.invalidate(wallet);
+
+    return {
+      success: true,
+      data: {
+        wallet: updated.wallet_address,
+        role: updated.role,
+      },
+      message: 'User role updated successfully.',
+    };
+  }
+}

src/modules/admin/admin.module.ts

@@ -1,11 +1,15 @@
 import { Module } from '@nestjs/common';
 import { AuditController } from './audit.controller';
 import { AuditService } from './audit.service';
+import { AdminRolesController } from './admin-roles.controller';
 import { SupabaseService } from '../../database/supabase.client';
+import { UsersRepository } from '../../database/repositories/users.repository';
+import { UserStatusService } from '../auth/user-status.service';
+import { AdminGuard } from '../../auth/guards/admin.guard';
 
 @Module({
-  controllers: [AuditController],
-  providers: [AuditService, SupabaseService],
-  exports: [AuditService],
+  controllers: [AuditController, AdminRolesController],
+  providers: [AuditService, SupabaseService, UsersRepository, UserStatusService, AdminGuard],
+  exports: [AuditService, UserStatusService],
 })
 export class AdminModule {}

src/modules/admin/dto/admin-reset-role.dto.ts

@@ -0,0 +1,14 @@
+import { ApiPropertyOptional } from '@nestjs/swagger';
+import { IsIn, IsOptional } from 'class-validator';
+import { UserRole } from '../../../database/repositories/users.repository';
+
+export class AdminResetRoleDto {
+  @ApiPropertyOptional({
+    description: 'New role to assign (sponsor | vendor | mentor), or null to reset/remove the role',
+    enum: ['sponsor', 'vendor', 'mentor'],
+    nullable: true,
+  })
+  @IsOptional()
+  @IsIn(['sponsor', 'vendor', 'mentor', null])
+  role?: UserRole | null;
+}

src/modules/auth/auth.module.ts

@@ -12,6 +12,8 @@ import { UsersRepository } from '../../database/repositories/users.repository';
 import { getJwtConfig } from '../../config/jwt.config';
 import { AdminModule } from '../admin/admin.module';
 
+import { RolesGuard } from '../../auth/guards/roles.guard';
+
 @Module({
   imports: [
     PassportModule.register({ defaultStrategy: 'jwt' }),
@@ -23,7 +25,7 @@ import { AdminModule } from '../admin/admin.module';
     AdminModule,
   ],
   controllers: [AuthController],
-  providers: [AuthService, JwtStrategy, UserStatusService, ApiKeyGuard, SupabaseService, ConfigService, UsersRepository],
-  exports: [AuthService, JwtStrategy, UserStatusService, ApiKeyGuard, PassportModule],
+  providers: [AuthService, JwtStrategy, UserStatusService, ApiKeyGuard, RolesGuard, SupabaseService, ConfigService, UsersRepository],
+  exports: [AuthService, JwtStrategy, UserStatusService, ApiKeyGuard, RolesGuard, PassportModule],
 })
 export class AuthModule {}

src/modules/auth/user-status.service.ts

@@ -2,67 +2,79 @@ import { Injectable, Logger, UnauthorizedException } from '@nestjs/common';
 import { SupabaseService } from '../../database/supabase.client';
 
 /**
- * How long a user's status may be served from cache before re-checking the
- * database. This is the documented staleness bound for blocking enforcement:
- * a blocked wallet can keep using valid access tokens for AT MOST this many
- * seconds (plus the remaining lifetime of its current access token is NOT
- * granted — requests within this window are the only grace period).
+ * How long a user's status and role may be served from cache before re-checking
+ * the database. This is the documented staleness bound for server-truth enforcement:
+ * role changes or blocked wallets take effect within AT MOST this many seconds
+ * (or immediately when invalidate() is called after role/status changes).
  */
 export const USER_STATUS_CACHE_TTL_MS = 30_000;
 
-interface CachedStatus {
+interface CachedUserState {
   status: string;
+  role: string | null;
   expiresAt: number;
 }
 
 /**
- * Short-TTL in-memory cache of user account status, consulted on every
- * authenticated request by JwtStrategy so that blocked wallets lose API
- * access within USER_STATUS_CACHE_TTL_MS instead of waiting for their
- * access token to expire naturally.
+ * Short-TTL in-memory cache of user account status and role, consulted on every
+ * authenticated request by JwtStrategy and RolesGuard so that authorization decisions
+ * rely on server truth rather than un-enforced JWT claims.
  *
- * A local in-memory Map is used deliberately instead of Redis: the check
- * runs on every request, one Redis round trip per request would double
- * auth latency, and a 30s staleness bound does not justify shared state.
- * On multi-instance deployments each instance maintains its own cache with
- * the same bound.
+ * A local in-memory Map is used deliberately instead of Redis: checks run on every
+ * request, one Redis round trip per request would double auth latency, and a 30s
+ * staleness bound does not justify shared state. On multi-instance deployments each
+ * instance maintains its own cache with the same bound.
  */
 @Injectable()
 export class UserStatusService {
   private readonly logger = new Logger(UserStatusService.name);
-  private readonly cache = new Map<string, CachedStatus>();
+  private readonly cache = new Map<string, CachedUserState>();
 
   constructor(private readonly supabaseService: SupabaseService) {}
 
   /**
-   * Returns the user's status ('active', 'blocked', ...), serving from the
-   * cache when fresh. Never throws for DB errors — fails open so a database
-   * blip cannot lock out every authenticated user; the failure is logged.
+   * Returns the user's current status and role, serving from cache when fresh.
+   * Never throws for DB errors — fails open so a database blip cannot lock out
+   * every authenticated user; the failure is logged.
    */
-  async getStatus(wallet: string): Promise<string> {
+  async getUserState(wallet: string): Promise<{ status: string; role: string | null }> {
     const cached = this.cache.get(wallet);
     if (cached && cached.expiresAt > Date.now()) {
-      return cached.status;
+      return { status: cached.status, role: cached.role };
     }
     let status = 'active';
+    let role: string | null = null;
     try {
       const client = this.supabaseService.getServiceRoleClient();
       const { data, error } = await client
         .from('users')
-        .select('status')
+        .select('status, role')
         .eq('wallet_address', wallet)
         .maybeSingle();
-      if (!error && data?.status) {
-        status = data.status;
+      if (!error && data) {
+        if (data.status) status = data.status;
+        if (data.role !== undefined) role = data.role ?? null;
       }
       if (error) {
-        this.logger.error(`Failed to read status for ${wallet}: ${error.message}`);
+        this.logger.error(`Failed to read user state for ${wallet}: ${error.message}`);
       }
     } catch (err) {
-      this.logger.error(`User status lookup failed for ${wallet}`, err);
+      this.logger.error(`User state lookup failed for ${wallet}`, err);
     }
-    this.cache.set(wallet, { status, expiresAt: Date.now() + USER_STATUS_CACHE_TTL_MS });
-    return status;
+    this.cache.set(wallet, { status, role, expiresAt: Date.now() + USER_STATUS_CACHE_TTL_MS });
+    return { status, role };
+  }
+
+  /** Returns the user's status ('active', 'blocked', ...), serving from cache when fresh. */
+  async getStatus(wallet: string): Promise<string> {
+    const state = await this.getUserState(wallet);
+    return state.status;
+  }
+
+  /** Returns the user's current role ('sponsor', 'vendor', 'mentor', null), serving from cache when fresh. */
+  async getRole(wallet: string): Promise<string | null> {
+    const state = await this.getUserState(wallet);
+    return state.role;
   }
 
   /** Throws AUTH_USER_BLOCKED when the wallet's account is suspended. */
@@ -73,8 +85,9 @@ export class UserStatusService {
     }
   }
 
-  /** Test/admin helper: drops cached status so the next check hits the DB. */
+  /** Test/admin helper: drops cached state so the next check hits the DB. */
   invalidate(wallet: string): void {
     this.cache.delete(wallet);
   }
 }
+

src/modules/users/users.module.ts

@@ -3,16 +3,15 @@ import { UsersController } from './users.controller';
 import { UsersService } from './users.service';
 import { UsersRepository } from '../../database/repositories/users.repository';
 import { SupabaseService } from '../../database/supabase.client';
+import { AuthModule } from '../auth/auth.module';
 
 /**
  * Users feature module.
- *
- * Note: JwtAuthGuard is NOT listed as a provider here — it lives in AuthModule
- * (created in API-03) and is resolved from there by NestJS's DI container.
  */
 @Module({
+    imports: [AuthModule],
     controllers: [UsersController],
     providers: [UsersService, UsersRepository, SupabaseService],
-    exports: [UsersService],
+    exports: [UsersService, UsersRepository],
 })
 export class UsersModule { }