search

kinde-oss / kinde-auth-pkce-js

Kinde vanilla JavaScript authentication for SPAs using PKCE flows. Can be used with Vue / Angular or any JS framework
https://docs.kinde.com/developer-tools/sdks/frontend/javascript-sdk/
MIT License
17 stars 12 forks source link

Bug: createKindeClient.ts creates an extra history entry #56

Open mchr3k opened 1 year ago

mchr3k commented 1 year ago

Prerequisites

Describe the issue

https://github.dev/kinde-oss/kinde-auth-pkce-js/blob/main/src/createKindeClient.ts

handleRedirectToApp(...) calls window.history.pushState({}, '', url); on line 258. Is there any chance that this could be changed to https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState The use of pushState(...) means that a history entry gets generated which means that if a user click the Back button in their browser after login, they end up on back on the URL which has the ?code=... query args, which get stripped back out by Kinde and redirects them forward again to the page they just tried to press Back on.

Library URL

https://github.dev/kinde-oss/kinde-auth-pkce-js

Library version

3.0.27

Operating system(s)

macOS

Operating system version(s)

14.0

Further environment details

No response

Reproducible test case URL

No response

Additional information

No response

DaveOrDead commented 1 year ago

@mchr3k thanks for raising this one. We can definitely update it to replaceState but it would come with it's own caveats.

If we make it replaceState and the user clicks the back button they would return to the Kinde auth flow - however each login flow can only be run once for security, so they would end up seeing an error screen as that instance of the auth flow has already completed.

Just trying to understand the use case where do you anticipate the user is trying to access when they click the Back button?

mchr3k commented 1 year ago

Perhaps my use case is a little bit unusual. I have been doing testing locally without using isDangerouslyUseLocalStorage. This means that every time I click a link in my app, the browser treats this as a page navigation, which unloads the login state, triggering a new login.

So from a browser history point of view, what I end up with is:

  1. Homepage (I click on a link to a Detail page)
  2. Detail page (initiates Kinde login, which redirects back to redirectUri)
  3. redirectUri?code=... (uses pushState to change the URL to redirectUri)
  4. redirectUri (my own code uses replaceState to change the URL to the Detail page)

This results in a history stack:

  1. Homepage
  2. Detail page
  3. redirectUri?code=...
  4. Detail page

If pushState changed to replaceState then I think I would get a history stack like this:

  1. Homepage
  2. Detail page
  3. Detail page

Which isn't perfect, but is better since it wouldn't trigger the current behavior where navigating back to redirectUri?code=... results in an automatic forward navigation.

However, if I try this out after an interactive sign in flow, I can navigate back to the Google signin screen, but when I pick an account, this is then rejected by Kinde:

Screenshot 2023-11-24 at 10 05 57

Presumably this is related to what you said:

however each login flow can only be run once for security, so they would end up seeing an error screen as that instance of the auth flow has already completed.

Could this error case be handled in a more friendly way? It would be nice if the user got a button to try logging in again.