search

openid / AppAuth-JS

JavaScript client SDK for communicating with OAuth 2.0 and OpenID Connect providers.
Apache License 2.0
975 stars 162 forks source link

RedirectRequestHandler leaks storage entries #208

Open zachmullen opened 2 years ago

zachmullen commented 2 years ago

Expected Behavior

[REQUIRED] Describe expected behavior

When using RedirectRequestHandler, at the conclusion of an OAuth flow, all storage entries should be cleaned up, even if an error occurred.

Describe the problem

[REQUIRED] Actual Behavior

If there was an error during the OAuth flow, storage entries are left behind. This is problematic because a new random value is used as part of the localStorage keys for each new OAuth flow, so not cleaning them up causes localStorage to grow monotonically. This issue was discovered when a user hit the 5MB limit of their localStorage and was no longer able to log into the app, and we found the localStorage filled with many many appauth entries from failed login attempts that happened while the client setup was misconfigured.

This TODO indicates the missing logic.

[REQUIRED] Steps to reproduce the behavior

  1. Use LocalStorageBackend with a RedirectRequestHandler.
  2. Introduce a misconfiguration into your app that causes the OAuth flow to fail.
  3. Perform repeated login attempts.
  4. Inspect localStorage in the browser and notice many different appauth keys.

[REQUIRED] Environment