Skip to content

Conversation

enrico-kaack-comp
Copy link
Contributor

What this PR does / why we need it:
We had a race condition when the session onSend hook was called before the encryptedSession onSend hook, therefore the session data was old and invalid.
This changes the encryptedSession implementation to be called on every save and therefore we dont have this order issue anymore

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

@n3rdc4ptn n3rdc4ptn requested a review from Copilot July 10, 2025 13:40
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures encrypted session operations are fully awaited to eliminate the race condition between session and encryptedSession hooks, and refactors the encrypted-session plugin to save on every store operation.

  • Await prepareOidcLoginRedirect and all encryptedSession set/delete/clear calls in auth routes and proxy plugin.
  • Convert prepareOidcLoginRedirect to async and add awaits in auth-utils.
  • Refactor encrypted-session plugin to use decorateRequest with a per-request store and immediate saves on set/delete/clear.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
server/routes/auth-onboarding.js Added await on OIDC redirect prep and session operations, return replies
server/routes/auth-mcp.js Mirror awaits for OIDC redirect and session operations, return replies
server/plugins/http-proxy.js Added await to encryptedSession.clear, set, and delete calls
server/plugins/auth-utils.js Made prepareOidcLoginRedirect async and awaited session writes
server/encrypted-session.js Replaced hooks with decorateRequest, created per-request store, immediate saves
Comments suppressed due to low confidence (1)

server/plugins/http-proxy.js:46

  • Ensure the onRequest hook callback is declared as async; using await inside a non-async function will cause a runtime error.
        await request.encryptedSession.clear(); //TODO: also clear user encrpytion key?

@n3rdc4ptn n3rdc4ptn merged commit 4f775ba into main Jul 10, 2025
5 checks passed
@n3rdc4ptn n3rdc4ptn deleted the fix/race-condition branch July 10, 2025 13:54
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