Skip to content

fix: [PDI-3249] - Use sessionStorage for codeVerifier and nonce: update OAuth and storage#13570

Merged
pmakode-akamai merged 10 commits intolinode:developfrom
mkaminsk-akamai:sessionStorage_for_codeVerifier
Apr 15, 2026
Merged

fix: [PDI-3249] - Use sessionStorage for codeVerifier and nonce: update OAuth and storage#13570
pmakode-akamai merged 10 commits intolinode:developfrom
mkaminsk-akamai:sessionStorage_for_codeVerifier

Conversation

@mkaminsk-akamai
Copy link
Copy Markdown
Contributor

@mkaminsk-akamai mkaminsk-akamai commented Apr 9, 2026

Description 📝

@pmakode-akamai This is a code snipplet which can help to reproduce the case when user who has multiple CM tabs open receives message in console that codeVerifier not being found. You can reproduce it locally when you point local APIV4 environment to production APIV4 so CM will get 401s each time.
Without changes to storage.ts you should be able to reproduce the message with codeVerifier not being found.
If you apply changes from storage.ts the issue seems to be gone, since when there are multiple tabs open the codeVerifier may overwritten in local storage, thus I switching in this code snipplet to use sessionStorage seems to resolve it.

Scope 🚢

Upon production release, changes in this PR will be visible to:

  • All customers
  • Some customers (e.g. in Beta or Limited Availability)
  • No customers / Not applicable

Target release date 🗓️

April End

How to test 🧪

Reproduction steps

You can use this config for reproducing it: Login in DevCloud will work fine, APIv4 will give 401s. You can use this client id from DevCloud 'c19a605edc88b07242fa':

REACT_APP_APP_ROOT='http://localhost:3000'
REACT_APP_API_ROOT='https://api.linode.com/v4'
REACT_APP_LOGIN_ROOT='https://login.devcloud.linode.com'
REACT_APP_CLIENT_ID='c19a605edc88b07242fa'

Verification steps

  • Add alert('No code codeVerifier found in local storage when running OAuth callback.'); after L208 here
  • No code verifier found in local storage. Please try logging in again. alert should not appear while testing in multiple tabs. (Note: this alert message appears intermittently without these changes, and these PR changes are supposed to fix this problem)

Note

alert('No code codeVerifier found in local storage when running OAuth callback.'); should be removed before merging this PR

Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support

  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All tests and CI checks are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@mkaminsk-akamai mkaminsk-akamai requested a review from a team as a code owner April 9, 2026 12:02
@mkaminsk-akamai mkaminsk-akamai marked this pull request as draft April 9, 2026 12:03
@mkaminsk-akamai mkaminsk-akamai changed the title Use sessionStorage for codeVerifier: update OAuth and storage Use sessionStorage for codeVerifier and nonce: update OAuth and storage Apr 9, 2026
@pmakode-akamai pmakode-akamai changed the title Use sessionStorage for codeVerifier and nonce: update OAuth and storage fix: [PDI-3249] - Use sessionStorage for codeVerifier and nonce: update OAuth and storage Apr 9, 2026
@pmakode-akamai pmakode-akamai self-requested a review April 9, 2026 12:15
Copy link
Copy Markdown
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

I think this is a good approach - am not seeing any regression from my initial testing and this def addresses the multiple tab issue.

This fixes an edge case with multiple tabs session expiration, however we will still have to decide it that's a fix worth introducing. It appears safe but it will have to be tested well. thanks @mkaminsk-akamai

Comment thread packages/manager/src/OAuth/oauth.ts Outdated
@pmakode-akamai pmakode-akamai added the Bug Fixes for regressions or bugs label Apr 10, 2026
@pmakode-akamai pmakode-akamai marked this pull request as ready for review April 10, 2026 12:14
@pmakode-akamai pmakode-akamai force-pushed the sessionStorage_for_codeVerifier branch from 29629f9 to 4beb82e Compare April 14, 2026 09:43
Copy link
Copy Markdown
Contributor

@pmakode-akamai pmakode-akamai left a comment

Choose a reason for hiding this comment

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

Validated the latest changes on multiple tabs and didn't observe any regressions ✅

@pmakode-akamai pmakode-akamai added the Add'tl Approval Needed Waiting on another approval! label Apr 14, 2026
@pmakode-akamai pmakode-akamai force-pushed the sessionStorage_for_codeVerifier branch from 40db52f to e788dec Compare April 14, 2026 11:10
@linode-gh-bot
Copy link
Copy Markdown

Cloud Manager UI test results

🔺 1 failing test on test run #9 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing901 Passing11 Skipped40m 45s

Details

Failing Tests
SpecTest
object-storage.e2e.spec.tsCloud Manager Cypress Tests→object storage end-to-end tests » can update bucket access

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/objectStorage/object-storage.e2e.spec.ts"

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Apr 14, 2026
Copy link
Copy Markdown
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

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

Thanks @pmakode-akamai and @mkaminsk-akamai! I wasn't able to observe any regressions. These are some of the flows I tested:

  • ✅ Signing in against develop, then switching to this branch and remaining signed in as expected on this branch (i.e. to simulate what'll happen to users who are already signed in after we deploy)
  • ✅ Signing in fresh against this branch
  • ✅ Logging out in one tab, confirmed other tabs get booted to login

I tested all this a few times, and I'm happy to take a second look if you make any other changes but otherwise lgtm

(cc @abailly-akamai)

@pmakode-akamai pmakode-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Apr 15, 2026
@pmakode-akamai pmakode-akamai merged commit 9fc994d into linode:develop Apr 15, 2026
34 of 35 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

5 participants