Fix "Mismatching redirect URL" #3103
Conversation
The client could register redirect URLs including OpenID Connect query parameters.
If the stored redirect URL is invalid, the silent login will end up in an error. To prevent this crash from happening, when an invalid redirect URL, the storage is cleared, so that the user has to properly log back in.
NSeydoux
force-pushed
the
fix/SDK-3140_prevent-invalid-redirect-url
branch
from
34dc69f
to
08a90df
Compare
| }), | ||
| ]); | ||
|
|
||
| if (typeof redirectUrl === "string" && !isValidRedirectUrl(redirectUrl)) { |
There was a problem hiding this comment.
Are there cases where this could be valid but still not the correct redirect url?
There was a problem hiding this comment.
Potentially, but it would become harder for the library to pick that up. One issue is that the failure happens out of the domain of the client, so the client is not informed that the OpenID Provider errors on their redirect_url, and there is no way for the client to look up what redirect URL has been registered by the client at dynamic registration.
I'm not even sure how the bad redirect URL gets in storage in the first place, but I suspect that using window.location.href plays a role. In that case, a likely cause of failure is picking up the OpenID query params (which is the observed cause of the bug in the reproduced issues).
If what you describe is indeed an issue, we'll need to figure out how to prevent it from happening, but for the time being I think it is low probabllity enough that we don't work on a fix for the time being.
by upgrading @inrupt/solid-client-authn-browser to latest version nodeSolidServer/node-solid-server#1729 inrupt/solid-client-authn-js#2891 inrupt/solid-client-authn-js#3103
by upgrading @inrupt/solid-client-authn-browser to latest version nodeSolidServer/node-solid-server#1729 inrupt/solid-client-authn-js#2891 inrupt/solid-client-authn-js#3103
This PR fixes bug #2891.
This bug was caused by an invalid redirect URL stored with session data. Saving an invalid redirect URL is now prohibited, and in addition the storage of users impacted by this bug will be cleared so that they don't have to do anything manually to clear their local storage. Users affected by this bug will be asked to log back in, as if they logged out.