Skip to content

refactor(client): use SDK storage helper in AuthenticationProvider.getLoginToken#40482

Open
ggazzo wants to merge 1 commit into
refactor/drop-unstore-login-token-monkey-patchfrom
refactor/auth-provider-login-token-helper
Open

refactor(client): use SDK storage helper in AuthenticationProvider.getLoginToken#40482
ggazzo wants to merge 1 commit into
refactor/drop-unstore-login-token-monkey-patchfrom
refactor/auth-provider-login-token-helper

Conversation

@ggazzo
Copy link
Copy Markdown
Member

@ggazzo ggazzo commented May 11, 2026

Summary

Drop the last Accounts.storageLocation reference in the client.

#40477 introduced getLoginToken via Accounts.storageLocation.getItem(Accounts.LOGIN_TOKEN_KEY). #40479 then centralized that same access pattern behind getStoredItem(STORAGE_KEYS.LOGIN_TOKEN) but couldn't touch this line because the two PRs were in flight at the same time. This is the catch-up.

-getLoginToken: () => Accounts.storageLocation.getItem(Accounts.LOGIN_TOKEN_KEY) ?? null,
+getLoginToken: () => getStoredItem(STORAGE_KEYS.LOGIN_TOKEN),

No behaviour change: same window.localStorage backend, same 'Meteor.loginToken' key. The Accounts import stays in this file because callLoginMethod / _unstoreLoginToken / loggingIn are still used here.

Test plan

  • yarn eslint on the changed file: 0 errors
  • Manual: open /oauth/authorize?... while logged in; the hidden access_token field is populated from the stored login token and the form submits.
  • Manual: log in, refresh; useLoginToken() continues to return the same token for any consumer that reads it (e.g. AuthorizationFormPage).

Summary by CodeRabbit

  • Refactor
    • Improved internal authentication token storage handling to make login token retrieval more reliable, reducing intermittent sign-in issues and improving session persistence across app restarts.

Review Change Stack

Review Change Stack

Task: ARCH-2137

@ggazzo ggazzo requested a review from a team as a code owner May 11, 2026 21:03
@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot Bot commented May 11, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 11, 2026

⚠️ No Changeset found

Latest commit: eda14b6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4f99ac03-6b35-4bba-96b6-b13f64f7d4fd

📥 Commits

Reviewing files that changed from the base of the PR and between e4cbd03 and eda14b6.

📒 Files selected for processing (1)
  • apps/meteor/client/providers/AuthenticationProvider/AuthenticationProvider.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/meteor/client/providers/AuthenticationProvider/AuthenticationProvider.tsx
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build

Walkthrough

AuthenticationProvider switches login-token retrieval to the SDK storage helper: import adds getStoredItem and getLoginToken now returns getStoredItem(STORAGE_KEYS.LOGIN_TOKEN).

Changes

Login Token Storage Migration

Layer / File(s) Summary
Login Token Storage Retrieval
apps/meteor/client/providers/AuthenticationProvider/AuthenticationProvider.tsx
Imports getStoredItem with STORAGE_KEYS; updates getLoginToken to return getStoredItem(STORAGE_KEYS.LOGIN_TOKEN) instead of Accounts.storageLocation.getItem(Accounts.LOGIN_TOKEN_KEY).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • RocketChat/Rocket.Chat#40479: Both PRs replace direct Accounts.storageLocation usage with the shared SDK storage helper for the login token.

Suggested labels

area: authentication

Suggested reviewers

  • tassoevan
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring AuthenticationProvider.getLoginToken to use an SDK storage helper instead of direct Accounts API calls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • ARCH-2137: Request failed with status code 401

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.66%. Comparing base (47df1e1) to head (eda14b6).

Additional details and impacted files

Impacted file tree graph

@@                                Coverage Diff                                 @@
##           refactor/drop-unstore-login-token-monkey-patch   #40482      +/-   ##
==================================================================================
+ Coverage                                           69.64%   69.66%   +0.01%     
==================================================================================
  Files                                                3318     3318              
  Lines                                              122002   122002              
  Branches                                            21783    21810      +27     
==================================================================================
+ Hits                                                84973    84994      +21     
+ Misses                                              33701    33677      -24     
- Partials                                             3328     3331       +3     
Flag Coverage Δ
e2e 59.13% <100.00%> (+<0.01%) ⬆️
e2e-api 47.12% <ø> (+0.86%) ⬆️
unit 70.41% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ggazzo ggazzo added this to the 8.5.0 milestone May 11, 2026
@ggazzo
Copy link
Copy Markdown
Member Author

ggazzo commented May 11, 2026

/jira ARCH-2116

…tLoginToken

up the AuthenticationProvider so the last Accounts.storageLocation /
Accounts.LOGIN_TOKEN_KEY reference in the client is gone.

No behaviour change: same window.localStorage backend, same 'Meteor.loginToken'
key. The Accounts import stays because callLoginMethod / _unstoreLoginToken /
loggingIn are still used here.
@ggazzo ggazzo force-pushed the refactor/auth-provider-login-token-helper branch from e4cbd03 to eda14b6 Compare May 11, 2026 22:54
@ggazzo ggazzo changed the base branch from develop to refactor/drop-unstore-login-token-monkey-patch May 11, 2026 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant