Skip to content

hard: role-gated endpoints trust stale JWT role claims - #128

Merged
EmeditWeb merged 3 commits into
StepFi-app:mainfrom
Godfrey-Delight:feat/server-truth-role-authorization-admin-reset
Aug 26, 2026
Merged

hard: role-gated endpoints trust stale JWT role claims#128
EmeditWeb merged 3 commits into
StepFi-app:mainfrom
Godfrey-Delight:feat/server-truth-role-authorization-admin-reset

Conversation

@Godfrey-Delight

Copy link
Copy Markdown
Contributor

🔗 Related Issue

Closes #122


🔖 Title

feat(auth): enforce server-truth role authorization and add audited admin role reset


📝 Description

Centralized role-based authorization on server truth rather than un-enforced JWT role claims. RolesGuard now resolves live user roles from the datastore via UserStatusService (cached in-memory with a documented 30-second staleness bound), preventing stale-token attacks where a downgraded user retains role access until token expiration. Added an admin-only role-management endpoint (POST /admin/users/:wallet/role/reset) allowing role resets and overrides with full audit logging via AuditInterceptor. Role changes automatically invalidate the server-side status cache for immediate effect.


🔄 Changes Made

  • Server-Truth Authorization: Extended UserStatusService to cache status and role (getUserState, getRole). Updated RolesGuard to enforce datastore roles rather than trusting JWT claims.
  • Cache Invalidation: Injected UserStatusService into UsersService and wired userStatusService.invalidate(wallet) into setRole() so role selection updates immediately.
  • Admin Role Reset Endpoint: Created AdminRolesController exposing POST /admin/users/:wallet/role/reset, guarded by JwtAuthGuard & AdminGuard, and audited via @AuditAction('admin_users', 'RESET_USER_ROLE') and AuditInterceptor.
  • Repository Layer: Added UsersRepository.forceSetRole(wallet, role) to allow admin role overrides and resets.
  • Module Wiring: Exported RolesGuard and UserStatusService from AuthModule and imported AuthModule in UsersModule, VouchingModule, and AdminModule.
  • Unit Tests: Added test suites for RolesGuard (roles.guard.spec.ts), AdminRolesController (admin-roles.controller.spec.ts), and updated users.service.spec.ts & vendors.service.spec.ts.
  • Documentation: Updated context/progress-tracker.md.

📸 Screenshots (if applicable)

N/A (Backend API logic change)


🗒️ Additional Notes

  • npm run build passed cleanly with 0 TypeScript compilation errors.
  • npm test passed with 34/34 test suites green (388/388 total tests passing).
  • Staleness bound is set to 30 seconds (USER_STATUS_CACHE_TTL_MS = 30_000), matching account blocking enforcement.

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Automated Audit: partial

@Godfrey-Delight Good start — please look into the gaps identified below.

The PR genuinely addresses the root cause of Issue #122: RolesGuard now resolves live user roles from the datastore via UserStatusService (30s cached) instead of trusting JWT claims, admin role reset endpoint exists with audit logging, cache invalidation is wired into role changes, and regression tests cover the stale-token attack path. However, context/progress-tracker.md contains unresolved merge conflict markers (<<<<<<< Updated upstream / ======= / >>>>>>> Stashed changes) which is a blockable defect and indicates the change was not properly landed.

Gaps identified:

  • Unresolved git merge conflict markers in context/progress-tracker.md (<<<<<<< Updated upstream / ======= / >>>>>>> Stashed changes) — must be resolved before merge

CI checks: ✅ PASSED: build-test

Audited by stepfi-audit-bot 🤖

EmeditWeb
EmeditWeb previously approved these changes Aug 26, 2026

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Automated Audit: solves

@Godfrey-Delight Excellent work, thank you! 🎉

The PR directly addresses the root cause of Issue #122: RolesGuard now resolves the user's current role from the datastore via UserStatusService instead of trusting the JWT role claim, which is the core security fix. The admin role reset endpoint (POST /admin/users/:wallet/role/reset) with audit logging, cache invalidation on role changes, and the repository-level forceSetRole method collectively close all four gaps described in the issue. The test suite explicitly covers the stale-token attack path (roles.guard.spec.ts verifying AUTH_ROLE_FORBIDDEN when datastore role is null but JWT carries a role), admin reset with cache invalidation, and setRole invalidation — matching the issue's acceptance criteria.


CI checks: ✅ PASSED: build-test
Merge conflicts: ✅ none — but the PR is blocked (failing/missing required checks or reviews).

Audited by stepfi-audit-bot 🤖

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Automated Audit: partial

@Godfrey-Delight Good start — please look into the gaps identified below.

The PR directly addresses the root cause of issue #122: RolesGuard now resolves the user's current role from the datastore via UserStatusService (30s cache TTL) instead of trusting the stale JWT role claim, with immediate cache invalidation on role changes. An admin role-reset endpoint with audit logging fulfills the recovery-path requirement, and setRole transition rules are enforced (setRoleIfUnset + admin override). Tests cover the critical security scenario — stale-token attack (JWT says vendor, datastore says null → 403) — plus role downgrade, admin reset, and cache invalidation. The @optional() fallback on UserStatusService in RolesGuard is a standard NestJS defensive pattern; in practice the service is always injected via module wiring (AuthModule exports both RolesGuard and UserStatusService, and importing modules are wired).

⚖️ Adjusted by bot policy: confidence 88% is below the 90% threshold for a full approval.


CI checks: ⏳ PENDING: build-test
Merge conflicts: ✅ none — but the PR is blocked (failing/missing required checks or reviews).

Audited by stepfi-audit-bot 🤖

@EmeditWeb
EmeditWeb self-requested a review August 26, 2026 20:39
@EmeditWeb
EmeditWeb merged commit ed6eacc into StepFi-app:main Aug 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hard: role-gated endpoints trust stale JWT role claims — blocked/downgraded users keep permissions until token expiry

2 participants