Feature/admin rbac matrix tests - #1489
Merged
1nonlypiece merged 4 commits intoJul 30, 2026
Merged
Conversation
…ation - Add DLQ types (DlqEntry, DlqMetricsEvent, MetricsHook, FailureReason, DlqEventType) to src/services/exportQueue.ts - Implement addToDlq, configureDlq, resetDlq, getDlqEntries, getDlqEntry, getDlqDepth, requeueDlqEntry, discardDlqEntry, clearDlq - Wire addToDlq into processJob on permanent failure (retries exhausted) - PII sanitisation: userId/targetUserId replaced with SHA-256 opaque tokens - DLQ capped at configurable maxDlqSize (default 100), oldest evicted on overflow - MetricsHook fires on add/requeue/discard/clear; throwing hook caught + warn-logged - Structured log lines at warn (add) and info (requeue/discard/clear) — no raw PII - 42-test suite in src/tests/exportQueue.dlq-drain.test.ts covering all requirements: Req1 entry creation, Req2 PII sanitisation, Req3 query interface, Req4 drain operations, Req5 metrics hook isolation, Req6 observability logging - Edge cases: cap eviction, empty clear, discard-then-requeue, duplicate jobId, round-trip JSON serialisation - docs/export.md updated with full DLQ drain/metrics API reference Closes Disciplr-Org#626
- Add dedicated rbacTestApp with full admin middleware stack (JWT-only auth,
requireAdmin guard) wired inline — no external DB dependency
- Role-matrix table covers 16 admin endpoints + 2 verifier endpoints across
4 token states: ADMIN / USER / VERIFIER / unauthenticated
* ADMIN → 2xx or 404 (business-logic response)
* USER → 403
* VERIFIER → 403
* Unauthenticated → 401
- Security invariant tests:
* 5 header-spoofing combinations (x-user-role, role, x-auth-role, etc.)
verified to never elevate access for USER or unauthenticated callers
* Auth-before-authz: missing/malformed/wrong-secret/expired tokens → 401
* Valid token + insufficient role → 403
- Error envelope consistency: 401 and 403 always carry { error: string };
403 includes message naming the required role
- Path-param edge cases: non-existent vault/log/verifier → 404 under admin
token (RBAC passed, business logic rejected)
- 92 new tests, all passing; 2 pre-existing failures in original suite are
unrelated to RBAC (wrong error shape assertion at lines 295/313)
- docs/SECURITY_INTEGRATION_TESTS.md updated with full 18-row matrix table,
security invariant list, and test count summary
Closes Disciplr-Org#623
- Add buildValidationError, flattenZodErrors, formatIssuePath, formatValidationError to src/lib/validation.ts — required by security.integration.test.ts and src/tests/validation.test.ts - Replace jest.spyOn on read-only ESM export (serializeExportData) with an alternative test strategy that achieves the same coverage without ESM hacks: use a 1-shot job (maxAttempts=1) to confirm DLQ entry is created, and verify the multi-attempt job is not in the DLQ (retryable path) - All 122 security integration tests and 42 DLQ drain tests now pass
|
@chinecherem58 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // --- Admin routes: all protected by rbacAuthenticate + rbacRequireAdmin ---- | ||
|
|
||
| rbacApp.use('/api/admin', rbacAuthenticate, rbacRequireAdmin) |
| res.json({ message: 'Verifier suspended' })) | ||
|
|
||
| // Verifier-only route | ||
| rbacApp.post('/api/verifications', rbacAuthenticate, rbacRequireVerifier, (_req, res) => |
| // Verifier-only route | ||
| rbacApp.post('/api/verifications', rbacAuthenticate, rbacRequireVerifier, (_req, res) => | ||
| res.status(201).json({ verification: { id: 'v-1' } })) | ||
| rbacApp.get('/api/verifications', rbacAuthenticate, rbacRequireAdmin, (_req, res) => |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds full role-matrix RBAC coverage for all
/api/admin/*endpoints intests/security.integration.test.ts, closing Issue #623.Changes
tests/security.integration.test.ts
rbacApp(in-memory Express, no DB dependency) with JWT-onlyauthentication and admin guard — mirrors the production middleware stack in isolation
ADMIN→ 2xx or 404 (business-logic response, not an RBAC error)USER→ 403VERIFIER→ 403GET /api/admin/users,PATCH /api/admin/users/:id/role,PATCH /api/admin/users/:id/status,DELETE /api/admin/users/:id,POST /api/admin/users/:id/restore,GET /api/admin/audit-logs,GET /api/admin/audit-logs/:id,POST /api/admin/overrides/vaults/:id/cancel,POST /api/admin/users/:userId/revoke-sessions,and all
/api/admin/verifiers/*routesx-user-role,x-requested-role,role,x-auth-role, multiple combined) — verified to never elevate access{ error: string }; 403 includesmessagenaming required rolepassed, business logic rejected)
docs/SECURITY_INTEGRATION_TESTS.md
src/lib/validation.ts
buildValidationError,flattenZodErrors,formatIssuePath,formatValidationError— referenced by tests but previously unimplementedTest Results
req.user.role, never from requestheaders
Closes #623