Skip to content
This repository has been archived by the owner on Mar 3, 2022. It is now read-only.

Allow supressing id_token_hint in silent renew #343

Closed
crudh opened this issue Jun 7, 2017 · 12 comments
Closed

Allow supressing id_token_hint in silent renew #343

crudh opened this issue Jun 7, 2017 · 12 comments
Milestone

Comments

@crudh
Copy link

crudh commented Jun 7, 2017

After upgrading to 1.2.0 (or 1.3.0) we have a problem in Edge when trying to do a signinSilent. We get a "Frame window timed out". But it works in Chrome/FF/Safari.

This seems to be related to #116, #187 and possibly #311.

After some digging it seems like the problem is the length of the url supplied to the iframe when trying to do signinSilent. When id_token_hint was included by default, in 1.2.0, the length of our url increased to ~4500 characters.

IFrameWindow.navigate does this._frame.src = params.url and apparently Edge ignores all urls longer than 4096 characters when setting the src of an iframe. It doesn't do anything, no error and no request to the server, just a silent ignore. So the timer always timeouts since the frame is never loaded.

If I modify IFrameWindow and remove id_token_hint from the url, since it isn't mandatory, everything works as expected in Edge.

So right now we either have to use a fork of oidc-client-js or trim the token down. Is there any way we could configure or tell oidc-client-js to not send the id_token_hint when doing a signinSilent?

@crudh
Copy link
Author

crudh commented Jun 7, 2017

Update: Seems like we had a lot of redundant data in the id token that we could remove that solved the issue for us.

Should I close the issue @brockallen or should I leave it open? It's not an obvious problem to track down if you have a large id token. If you want I could submit a PR that logs warnings on too long urls in the iframe or something similar?

@brockallen
Copy link
Member

Update: Seems like we had a lot of redundant data in the id token that we could remove that solved the issue for us.

Good to hear. And yes, in general, keeping id_token as small as possible helps many things. Using the user info endpoint should also help.

As for the issue, maybe what's best is to have a flag to toggle if id_token_hint is sent in silent renewal?

@crudh
Copy link
Author

crudh commented Jun 18, 2017

@brockallen yes, we removed things from the id token that was already sent by the user info endpoint.

a toggle flag would work great for us!

@brockallen brockallen added this to the 1.4.0 milestone Jun 18, 2017
@stuartbloom
Copy link

@crudh just wondering, how did you go about reducing the length of your token/token_hint?

Thanks for any assistance :)

@crudh
Copy link
Author

crudh commented Jun 19, 2017

@stuartbloom I don't have any technical info since I'm just on the frontend parts in this current project. But to summarize we were sending out a bunch of user related data, that aren't really claims, both in the token and from the userinfo endpoint. Now we only output that in the userinfo endpoint and only have the required claims in the token.

Does that answer the question or should I ping one of the devs that did the change?

@GDehnke-Turpin
Copy link

This might sound strange, but to avoid the massive URL, I ended out doing a silent in application redirect to the exact same URL such as mybusiness.com/dashboard. The redirect removes the massive token in the URL and you have a clean SEO experience. Could you possibly show a short snippet of code that you are using.

@crudh
Copy link
Author

crudh commented Jun 19, 2017

@GDehnke-Turpin we also update the url (push a new state to the browser history) after we have received the token in the url from the redirect. We have never had any problem with the regular login redirect and url length, it only affected the iframe for the silent renew.

@stuartbloom
Copy link

stuartbloom commented Jun 19, 2017

@GDehnke-Turpin

this is my logout code :s

    private logout() {
        this.appService.userManager.getUser().then((user) => {
            this.appService.userManager.clearStaleState();
            this.appService.userManager.signoutRedirect();
        });
    }

Thanks

Stuart

@stuartbloom
Copy link

Hey @crudh @GDehnke-Turpin @brockallen

Just wondering if there is any movement on this enhancement? or a clearer workaround?

Thanks

@crudh
Copy link
Author

crudh commented Jul 31, 2017

@stuartbloom not that I know of, I don't think I have the time to work on a PR for this now that it works for us (we are in the last phase in a large project). And sorry for the late answer, just got back from vacation.

@brockallen
Copy link
Member

Done. Added a includeIdTokenInSilentRenew flag. For now defaults to true to be backwards compat.

@brockallen brockallen changed the title MS Edge, signinSilent and id_token_hint problem Allow supressing id_token_hint in silent renew Sep 24, 2017
@william-lohan
Copy link

I'm having the same issue for signout redirect.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

5 participants