Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loadAdapter location.assign() vs location.replace() in login #32

Open
madziszyn opened this issue Feb 5, 2025 · 1 comment
Open

loadAdapter location.assign() vs location.replace() in login #32

madziszyn opened this issue Feb 5, 2025 · 1 comment

Comments

@madziszyn
Copy link

Describe the bug

Hi,

Reporting issue we've managed to resolve by patching keycloak-js internally, but posting here so you can check if our solution is valid for more use cases (or secure).
We have SPA (vue.js) that inits keycloak on start than rest of app afterwards:

initKeycloak().finally(() => {
//vue mount here. omitted for clarity
});

We are using standard code flow (below init options):

return keycloakClient.init({
    onLoad: 'login-required',
    checkLoginIframe: false,
  });

We were facing weird behavior after browser page refresh (F5) - sometimes redirect url (with state fragment) was pushed to browser history but most of the time redirect url was replaced in history. In cases when history entry was added user was stuck in loop (browser back button lead to original URL without state fragemtn which causes keycloak init with another redirect).

Code causing this behavior (in keycloak.js loadAdapter func):

login: async function(options) {
    window.location.assign(await kc.createLoginUrl(options));
    return createPromise().promise;
}

location.assign will use by default auto history behavior which will resolve most of the time to push but in some cases to replace - when document is not fully loaded on navingation. More info: https://html.spec.whatwg.org/multipage/nav-history-apis.html#location-object-navigate (point 3).

Our solution was simply changing location.assign with location.replace

location.assign is also used in register - we also changed that but not sure if relevant to this issue.

Version

26.1.0

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Anything else?

No response

@jonkoops
Copy link
Contributor

jonkoops commented Feb 5, 2025

I'd be amicable to replacing location.assign() with location.replace() in Keycloak JS, it seems the more logical behavior. Are there any ill effects you could think of that would possibly create issues for other users. Additionally, perhaps we should consider the new Navigation API as well.

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

No branches or pull requests

2 participants