Skip to content

feat: session manager local storage#454

Draft
tuna1207 wants to merge 1 commit intomasterfrom
fix/session-manager-local-storage
Draft

feat: session manager local storage#454
tuna1207 wants to merge 1 commit intomasterfrom
fix/session-manager-local-storage

Conversation

@tuna1207
Copy link
Member

@tuna1207 tuna1207 commented Feb 16, 2026

Motivation and Context

Jira Link: https://consensyssoftware.atlassian.net/browse/W3APD-5312?atlOrigin=eyJpIjoiNDU0MDYyNWQ2MWExNGQxNjhkMzFiYTQxYzI5N2I2ZDUiLCJwIjoiaiJ9

Description

This PR migrates redirect-session persistence in CustomAuth from StorageHelper to SessionManager, while preserving compatibility with existing stored session keys.

  • Replace StorageHelper usage in src/login.ts with a constructor-initialized SessionManager instance.
  • Enable local cache + server fallback behavior by using useLocalStorage: true.
  • Preserve backward compatibility for redirect session lookup by deriving sessionId from legacy string keys via keccak256(...).slice(2) before calling createSession / authorizeSession.
  • Update storage-method reporting in logs/error paths to use useLocalStorage ? "localStorage" : "server".
    Align src/utils/sessionHelper.ts with the same behavior by adding useLocalStorage: true.

How has this been tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project. (run lint)
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My code requires a db migration.

Note

Medium Risk
Changes the redirect login flow’s state persistence and retrieval path; any mismatch in session ID hashing, storage availability, or session cleanup could break redirect logins across browsers/environments.

Overview
Redirect-mode login state persistence is migrated from the internal StorageHelper to @toruslabs/session-manager in CustomAuth, including orphan cleanup, session creation before redirect, and session authorization/cleanup when handling the redirect result.

To preserve legacy keying, a new getSessionId() hashes the previous string storage key into the hex sessionId format expected by SessionManager, and logging/error messaging is updated to report whether localStorage or server storage was used. fetchDataFromBroadcastServer is also updated to explicitly set useLocalStorage: true.

Written by Cursor Bugbot for commit 940ebb5. This will update automatically on new commits. Configure here.

@tuna1207 tuna1207 marked this pull request as draft February 16, 2026 04:10
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.


if (clearLoginDetails) {
this.storageHelper.clearStorage(`torus_login_${instanceId}`);
this.sessionManager.clearStorage();
Copy link

Choose a reason for hiding this comment

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

Missing sessionId set before clearStorage in success path

High Severity

When storageData is provided to getRedirectResult, the if (!loginDetails) block is skipped, so this.sessionManager.sessionId is never set for the current instanceId. The success-path clearStorage() call then operates on a stale or default sessionId, failing to clear the correct session data. The error path at lines 316–317 correctly sets sessionId before calling clearStorage(), but the success path omits this. The old code always passed the key explicitly to clearStorage, so both paths worked.

Additional Locations (1)

Fix in Cursor Fix in Web

nodeDetailManager: NodeDetailManager;

storageHelper: StorageHelper<LoginDetails>;
useLocalStorage: boolean = true;
Copy link
Member

Choose a reason for hiding this comment

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

no need for flag. always true

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

Comments