[478] Backend: Add immutable ledger of admin impersonation sessions with expiry enforcement#654
Merged
Conversation
|
@yinkscss 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! 🚀 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds time-bounded impersonation sessions for super-admins and enforces session validation before allowing /admin/impersonate/:wallet, with persistence via Prisma (plus memory/hybrid fallback).
Changes:
- Introduces impersonation session + ledger service with session validation, listing, and termination.
- Adds new admin routes to manage sessions and requires
x-impersonation-session-idfor impersonation reads. - Adds Prisma schema/migration and tests for the new impersonation session behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/tracing.ts | Changes OTEL-disabled span handling to use a NOOP span. |
| backend/src/index.ts | Adds impersonation session CRUD endpoints and requires session validation for impersonation. |
| backend/src/impersonationSessionService.ts | Implements session storage (memory/prisma/hybrid), ledger writes, validation, and listing. |
| backend/src/tests/impersonationSessions.test.ts | Adds tests for session creation, validation, expiry, listing, and role enforcement. |
| backend/src/tests/governance.test.ts | Updates existing governance test to include impersonation session header requirement. |
| backend/prisma/schema.prisma | Adds models for impersonation sessions and ledger entries. |
| backend/prisma/migrations/.../migration.sql | Creates DB tables and indexes for sessions/ledger. |
| backend/.env.example | Documents new impersonation session env vars. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1299
to
+1335
| if (!sessionId) { | ||
| req.adminAuditAction = 'admin.impersonate.session.required'; | ||
| res.status(400).json({ | ||
| error: 'Bad Request', | ||
| status: 400, | ||
| message: 'x-impersonation-session-id header is required', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const validation = await validateImpersonationSession(sessionId, wallet, actingAdminAddress); | ||
| if (!validation.ok) { | ||
| const statusCode = validation.reason === 'not_found' ? 404 : 403; | ||
| req.adminAuditAction = | ||
| validation.reason === 'expired' | ||
| ? 'admin.impersonate.expired' | ||
| : 'admin.impersonate.session.invalid'; | ||
| req.adminAuditMetadata = { | ||
| ...req.adminAuditMetadata, | ||
| validationReason: validation.reason, | ||
| }; | ||
| res.status(statusCode).json({ | ||
| error: statusCode === 404 ? 'Not Found' : 'Forbidden', | ||
| status: statusCode, | ||
| message: | ||
| validation.reason === 'expired' | ||
| ? 'Impersonation session has expired; start a new session to continue' | ||
| : validation.reason === 'ended' | ||
| ? 'Impersonation session has ended; start a new session to continue' | ||
| : validation.reason === 'wallet_mismatch' | ||
| ? 'Session target wallet does not match requested wallet' | ||
| : validation.reason === 'actor_mismatch' | ||
| ? 'Session actor does not match requesting admin' | ||
| : 'Impersonation session not found', | ||
| }); | ||
| return; | ||
| } |
Comment on lines
+73
to
+75
| function createId(prefix: string): string { | ||
| return `${prefix}_${Date.now()}_${Math.random().toString(36).slice(2, 10)}`; | ||
| } |
Comment on lines
+1110
to
+1128
| try { | ||
| const session = await startImpersonationSession({ | ||
| actor: actingAdminAddress, | ||
| apiKeyHash, | ||
| targetWallet, | ||
| reason, | ||
| ipAddress, | ||
| userAgent, | ||
| }); | ||
|
|
||
| req.adminAuditAction = 'admin.impersonate.session.started'; | ||
| req.adminAuditMetadata = { | ||
| ...req.adminAuditMetadata, | ||
| sessionId: session.id, | ||
| expiresAt: session.expiresAt, | ||
| }; | ||
|
|
||
| res.status(201).json({ session }); | ||
| } catch (error) { |
Comment on lines
+54
to
+55
| const inMemorySessions = new Map<string, ImpersonationSessionRecord>(); | ||
| const inMemoryLedger: ImpersonationLedgerEntryRecord[] = []; |
Comment on lines
+91
to
+96
| const NOOP_SPAN = { | ||
| setAttributes: () => {}, | ||
| setStatus: () => {}, | ||
| recordException: () => {}, | ||
| end: () => {}, | ||
| } as unknown as Span; |
| attributes?: Record<string, string | number | boolean>, | ||
| ): Promise<T> { | ||
| if (!OTEL_ENABLED) return fn(trace.getTracer(SERVICE_NAME).startSpan(name)); | ||
| if (!OTEL_ENABLED) return fn(NOOP_SPAN); |
Comment on lines
+77
to
+85
| it('rejects expired sessions and requires a new session record', async () => { | ||
| process.env.IMPERSONATION_SESSION_TTL_SECONDS = '1'; | ||
|
|
||
| const sessionResponse = await startSession(); | ||
| const sessionId = sessionResponse.body.session.id; | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 1100)); | ||
|
|
||
| const expiredResponse = await request(app) |
…ent. Sessions require explicit start/stop with reason and actor audit trail, and impersonation reads reject expired sessions unless a new session is created.
…ion. Registers default admin keys in test setup and updates test wallets to valid 56-character base32 addresses so governance checks pass in CI.
Aligns validation, referral, and transaction tests with the 56-character base32 address format enforced by request schemas.
yinkscss
force-pushed
the
fix/478-backend-add-immutable-ledger-of-admin-impersonation-sessions-with-expiry-enforcement
branch
from
29802bf to
471e984
Compare
Prevents CI migrate deploy from failing when a stale dev.db already contains tables pending migration.
…allets. Sets high rate-limit ceilings before app bootstrap and fixes referral integration fixtures to use valid Stellar addresses.
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
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
AdminImpersonationSessionand append-onlyAdminImpersonationLedgerEntryPrisma models for immutable session trackingPOST /admin/impersonate/sessions), list (GET /admin/impersonate/sessions), end (DELETE /admin/impersonate/sessions/:id)x-impersonation-session-idheader for wallet impersonation reads; expired sessions cannot be extended and must be replaced with a new session recordIMPERSONATION_SESSION_TTL_SECONDSCloses #478
Test plan
npm test -- --testPathPattern=impersonationSessions(6 tests passing)