Skip to content

SCIX-752: Fix verify middleware crash and implement cross-domain session synchronization#736

Merged
thostetler merged 2 commits intoadsabs:masterfrom
thostetler:SCIX-752/Verify-middleware-needs-to-bootstrap-if-incoming-session-has-no-token
Dec 17, 2025
Merged

SCIX-752: Fix verify middleware crash and implement cross-domain session synchronization#736
thostetler merged 2 commits intoadsabs:masterfrom
thostetler:SCIX-752/Verify-middleware-needs-to-bootstrap-if-incoming-session-has-no-token

Conversation

@thostetler
Copy link
Member

@thostetler thostetler commented Dec 16, 2025

Users clicking email verification links crashed with "Cannot read property 'access_token' of undefined" because the verify middleware tried to access session.token.access_token without checking if the token existed.

  • Added defensive null check in verifyMiddleware before accessing session token
  • Fixed hash encoding to use hex format (prevents potential collisions)
  • Fixed apiCookieHash null check to match actual return value

Cookie synchronization now properly handles cross-domain session sharing between BBB and Scix:

  • Domain attribute stripped (host-only, more secure)
  • SameSite forced to lax (allows cross-site navigation, prevents CSRF)
  • Only syncs when cookie value actually changes (prevents race conditions)

Deployment Note

⚠️ Hash encoding fix will invalidate existing sessions - users must re-authenticate after deployment.

…tion

Fixed critical bug where users clicking email verification links crashed due to missing session token checks. Implemented proper Sidecar Session pattern for seamless cross-domain session sharing between legacy BBB and Next.js apps.

- Added defensive null checks in verifyMiddleware to handle missing session.token
- Implemented cookie synchronization with domain stripping and SameSite=Lax per spec
- Fixed hash encoding to use proper hex format instead of comma-separated decimals
- Fixed apiCookieHash presence check to match empty string return value
- Migrated middleware to use edge-compatible error handler
- Added comprehensive documentation for Sidecar Session architecture

Note: Hash encoding fix will invalidate existing sessions requiring users to re-authenticate.
@thostetler thostetler marked this pull request as ready for review December 16, 2025 20:55
@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

❌ Patch coverage is 27.65957% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.7%. Comparing base (6fcc443) to head (fd6cc7f).

Files with missing lines Patch % Lines
src/middlewares/initSession.ts 29.0% 59 Missing ⚠️
src/middlewares/verifyMiddleware.ts 10.0% 9 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #736     +/-   ##
========================================
- Coverage    69.9%   69.7%   -0.1%     
========================================
  Files         220     219      -1     
  Lines       24478   24509     +31     
  Branches     1304    1303      -1     
========================================
- Hits        17086   17066     -20     
- Misses       7357    7408     +51     
  Partials       35      35             
Files with missing lines Coverage Δ
src/middleware.ts 37.9% <100.0%> (ø)
src/middlewares/verifyMiddleware.ts 15.5% <10.0%> (-1.2%) ⬇️
src/middlewares/initSession.ts 30.9% <29.0%> (-2.1%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thostetler thostetler merged commit 2a31cbf into adsabs:master Dec 17, 2025
4 checks passed
@thostetler thostetler deleted the SCIX-752/Verify-middleware-needs-to-bootstrap-if-incoming-session-has-no-token branch December 17, 2025 19:38
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.

2 participants