iTwin / auth-clients

MIT License
6 stars 3 forks source link

Misleading comment about silent redirect url #179

Open linasburneika opened 1 year ago

linasburneika commented 1 year ago

Comment tells that if silent redirect url is not configured, then the primary redirect url is reused and automatic silent renew will work:

https://github.com/iTwin/auth-clients/blob/252cd1e346c1f67f530a2cd4214cd5e6a92334e4/packages/browser/src/types.ts#L57

In reality however usermanager expects silentRedirectUri to be provided:

silent_redirect_uri: basicSettings.silentRedirectUri

without that access token obtained via interactive sign-in will get you 1 hour of work and then will fail abruptly.

Configuration of userManager allows 2 different redirect urls (interactive vs silent) for a reason. In case of silent signin, handling of response is done in a hidden iframe, where you want to load a minimal amount of code just to do processing. Official oidc-client sample even suggests to use a static html page for that:

https://github.com/authts/oidc-client-ts/blob/main/samples/Parcel/src/user-manager/sample-silent.html

If you reuse primary redirect uri also for silent signin, then it will load your complete app in the iframe. In case of itwin-viewer based app this is megabytes of javascript, which will hurt your RAM usage.

However if you don't configure silentRedirectUri, there is no way to know your configuration is not valid. The underlying oidc-client needs it strictly:

https://github.com/authts/oidc-client-ts/blob/73c4ba86b13fa34d0ac33204a0dad998334e8a20/src/UserManager.ts#LL271C34-L271C34

But auth-client wrapper swallows the error:

https://github.com/iTwin/auth-clients/blob/252cd1e346c1f67f530a2cd4214cd5e6a92334e4/packages/browser/src/Client.ts#L204

ben-polinsky commented 5 months ago

@ben-polinsky reminder to get this done this week. We've had another user confused by this.