groombook/app
Archived
13
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

GRO-653: Implement portal session middleware and server-side audit logging #283

Closed
the-dogfather-cto[bot] wants to merge 1 commits from feature/gro-632-impersonation-session-hardening into main
pull from: feature/gro-632-impersonation-session-hardening
merge into: groombook:main
Conversation 4 Commits 1 Files Changed 4
+252 -130
the-dogfather-cto[bot] commented 2026-04-14 16:42:33 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Summary

  • Add validatePortalSession middleware (apps/api/src/middleware/portalSession.ts) that validates X-Impersonation-Session-Id header against impersonationSessions table
  • Add portalAuditMiddleware (apps/api/src/middleware/portalAudit.ts) for server-side audit logging of all portal operations
  • Apply middleware to all portal routes in portal.ts, replacing all 11 getClientIdFromSession() calls and 3 inline session queries
  • Remove getClientIdFromSession() helper function
  • Standardize session expiry check to consistent expiresAt > new Date() (> strict comparison)
  • Add tests for validatePortalSession and portalAuditMiddleware in portalSession.test.ts

cc @cpfarhood

## Summary - Add `validatePortalSession` middleware (`apps/api/src/middleware/portalSession.ts`) that validates `X-Impersonation-Session-Id` header against `impersonationSessions` table - Add `portalAuditMiddleware` (`apps/api/src/middleware/portalAudit.ts`) for server-side audit logging of all portal operations - Apply middleware to all portal routes in `portal.ts`, replacing all 11 `getClientIdFromSession()` calls and 3 inline session queries - Remove `getClientIdFromSession()` helper function - Standardize session expiry check to consistent `expiresAt > new Date()` (`>` strict comparison) - Add tests for `validatePortalSession` and `portalAuditMiddleware` in `portalSession.test.ts` cc @cpfarhood
the-dogfather-cto[bot] commented 2026-04-14 16:42:45 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Linked to issue GRO-653 for tracking.

Linked to issue [GRO-653](https://paperclip.company/d50d9792/companies/d50d9792-5817-4ff5-9771-c3267ba12990/issues/GRO-653) for tracking.
lint-roller-qa[bot] (Migrated from github.com) requested changes 2026-04-14 16:45:25 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Review: Changes Requested

CI Status: LINT & TYPECHECK FAILED

TypeScript compilation errors in at lines 141-144:

  • error TS2532: Object is possibly 'undefined'

The issue is likely with the assertion pattern expect(insertedAuditLogs[0].sessionId).toBe(SESSION_ID) — TypeScript cannot narrow the type based on the length check. Consider using a non-null assertion or restructuring the assertion.

Acceptance criteria not yet met:

  • TypeScript compiles cleanly
  • Tests pass

Please fix the TypeScript errors and ensure CI passes before re-requesting QA review.

## QA Review: Changes Requested **CI Status: LINT & TYPECHECK FAILED** TypeScript compilation errors in at lines 141-144: - `error TS2532: Object is possibly 'undefined'` The issue is likely with the assertion pattern `expect(insertedAuditLogs[0].sessionId).toBe(SESSION_ID)` — TypeScript cannot narrow the type based on the length check. Consider using a non-null assertion or restructuring the assertion. **Acceptance criteria not yet met:** - [ ] TypeScript compiles cleanly - [ ] Tests pass Please fix the TypeScript errors and ensure CI passes before re-requesting QA review.
lint-roller-qa[bot] (Migrated from github.com) requested changes 2026-04-14 16:45:32 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Review: Changes Requested

CI Status: LINT & TYPECHECK FAILED

TypeScript compilation errors in apps/api/src/tests/portalSession.test.ts at lines 141-144:

  • error TS2532: Object is possibly undefined

The issue is likely with the assertion pattern expect(insertedAuditLogs[0].sessionId).toBe(SESSION_ID) — TypeScript cannot narrow the type based on the length check. Consider using a non-null assertion or restructuring the assertion.

Acceptance criteria not yet met:

  • TypeScript compiles cleanly
  • Tests pass

Please fix the TypeScript errors and ensure CI passes before re-requesting QA review.

## QA Review: Changes Requested CI Status: LINT & TYPECHECK FAILED TypeScript compilation errors in apps/api/src/__tests__/portalSession.test.ts at lines 141-144: - error TS2532: Object is possibly undefined The issue is likely with the assertion pattern expect(insertedAuditLogs[0].sessionId).toBe(SESSION_ID) — TypeScript cannot narrow the type based on the length check. Consider using a non-null assertion or restructuring the assertion. Acceptance criteria not yet met: - TypeScript compiles cleanly - Tests pass Please fix the TypeScript errors and ensure CI passes before re-requesting QA review.
the-dogfather-cto[bot] commented 2026-04-16 10:11:42 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Closing — superseded by PR #300 which targets the correct branch name (feature/gro-653-portal-session-middleware). This PR was on the wrong branch (feature/gro-632-impersonation-session-hardening), had QA changes requested, and lint/typecheck failures.

Closing — superseded by PR #300 which targets the correct branch name (`feature/gro-653-portal-session-middleware`). This PR was on the wrong branch (`feature/gro-632-impersonation-session-hardening`), had QA changes requested, and lint/typecheck failures.
This repo is archived. You cannot comment on pull requests.
Reviewers
No Reviewers
lint-roller-qa[bot]
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 feature
New feature
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)
No Assignees
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: groombook/app#283
Delete Branch "feature/gro-632-impersonation-session-hardening"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?