RestorePreviousSession redirects to the wrong page

[fuubi]

Search terms you've used

• redirect

Impacted package

Which packages do you think might be impacted by the bug ?

• solid-client-authn-browser

Bug description

It is not possible to navigate to the page where the restorePreviousSession prop has been set to true. But for all the other pages the session is restored automatically without setting the prop to true.
I expected that setting the flag would lead to an automatic redirect when navigating to a new page which partially happens. But, in one corner case, it not only restores the session but also redirects to the previous (wrong) page.

To Reproduce

1. Checkout the restorePreviousSession branch: https://github.com/FUUbi/solid-client-authn-js/tree/f5cea590f610508347d968e29c49003bca7ed25f/packages/browser/examples/single/script (let me know if I should send a PR)

I extended the single browser example to have three different HTML-Pages.

On Page-B, I set the redirect prop to true:

 session.handleIncomingRedirect({
   url:window.location.href,
   restorePreviousSession: true                
})

But for A and B, the default value (false) is used.

session .handleIncomingRedirect(window.location.href)

So now what happens is that if we visit A first, it is impossible to reach B.

from	to	result
A	B	A
A	C	C
C	B	C
C	A	A

If we clear the state and visit B first. We can reach it, but then after seeing A or C
we always get redirected to the previous state.

from	to	result
B	A	A
A	B	A

Expected result

I would expect that setting the restore flag is necessary on every page I want the session to be restored automatically. But not go back to the previous (one step back in the page history) page.

Additional Information

This bug hunt was quite an adventure as I experienced bizarre redirect behaviors.
I quickly went through the source code and tried to set the redirect URL by the window reference. But that leads to a Uri mismatch.

{"error":"invalid_request","error_description":"Mismatching redirect uri"}

Unfortunately, I don't have the resources currently to look into it in greater detail, but I hope this gets fixed soon.

[NSeydoux]

Hi @fuubi , thanks for reporting this ! Redirections are indeed a tad tricky with OpenID. The issue we are facing in your case is the following: on one hand, loading a page loses authentication, because for security reasons we only keep access information in memory, and not in storage. On the other hand, when logging in to an OpenID provider, the client specifies a redirect URL, and it is only authorized to get access information at this given URL for the duration of the session.

This means that the behaviour you observe is caused by the following course of action:

• User logs in from page A. The client tells the OpenID issuer to redirect the user back to page A.
• User goes to page B, where restorePreviousSession is true. Loading the page loses access information, so the client will log the user back in. This means the client has to re-specify a redirect IRI to the OpenID provider, and as far as the library can tell, the only client IRI which is valid for sure is page A.
• The OpenID provider returns the user to page A.

A workaround to this is available in client-side frameworks which handle client-side navigation, as described in our docs. However, if you prefer avoiding using such frameworks, I'm afraid the only mitigations are either to have a backend component to your app which serves pages A, B and C, and handles authentication, or to stick to a single-page app.

Can you describe the use case which lead to run into this behaviour, and do you think the previous paragraph offers a possible solution to your issue ?

[fuubi]

Hi @NSeydoux , thanks for your reply. Indeed, I am more or less familiar with the complexity you describe, and I also looked into the implementation a little bit. My use case would be a multi-page app/website with native browser navigation.

The weird thing is that the expected behavior is partially implemented. Although maybe not on purpose, which might cause this unintuitive behavior. Or what is the reason the session is restored if we navigate to B and C in my example setup? Although, we only visited A where the restorePreviousSession is set to true. But for B and C, it is not.

As you suggested, I might just use a client-side navigation framework for now.

[fuubi]

While setting everything up with a client-side navigation framework and next.js I had to take special care of presenting the corresponding state to the user. Users can either be logged out, which means that they are presented with the login page. Next, once they logged in, the app should only show a loading screen before getting the token and while coming back from the redirect. This is the best user experience I can currently realize.

Although the loading times are not too terrible, the user experience suffers when a loading icon appears every time while navigating to a different page. Moreover, in between the loading screen, there is also the OpenID provider's page.

So, now to my idea. What if the auth client keeps a valid code in memory that is URL-Encoded on user navigation? This would drastically reduce the loading time because it is there instantly and can directly request the token. But I am not sure if this flow is acceptable.

Anyway, was just a thought. But I will create a little starter project with what I have currently implemented and share it in the forum.

[smessie]

Is there any update on this? I am experiencing the same issue and it is blocking me in the following way:

I have two applications, both hosted on GitHub Pages under the same organisation.
However, when I first log into org.github.io/A, then I get correctly redirected to org.github.io/A after the login call.
When I then navigate to app B at org.github.io/B, it has the handleIncomingRedirect configured with restorePreviousSession: true so that I do not have to log in every time I visit the app. However, when browsing app B, it does a silentlyAuthenticate login call with as redirect URL org.github.io/A because it finds the StoredSession of app A as they reside at the same domain, seeing app A and B incorrectly as one single multipage application.
This results in a redirect to org.github.io/A, making org.github.io/B inaccessible if I'm logged in into app A.

Instead I would expect that before it tries to restore the session, it first checks if the redirect URLs match, and if not, it does not try to restore the session that actually belongs to a different app.
I already had a look and implemented a small fix for this here, so let me know if you would be open to accept this as a PR.
Although I'm not sure if there might be a better way to solve this or if my fix would have any unwanted side effects but I could not directly see any.

[NSeydoux]

Hi @smessie , thanks for reaching out, and thanks for looking for existing issues matching your problem!

I'm afraid there is a fundamental issue with hosting two conceptually separate apps under two paths of the same domain. Since a lot of browser resources are shared per origin (local storage for instance) , assumptions are made about two pages under the same origin and I don't think you'll get proper isolation between the two apps.

I had a look at your proposed fix, and I think I'm missing something, so let me walk through what this feature is designed for, and you'll confirm or infirm my understanding. The idea is that when you're using a client navigation framework, you can go through the following sequence:

1. Go to https://my-app.example.org/
2. Log in (i.e. get redirected to your OpenID provider, and then back to the app, let's say to https://my-app.example.org/callback
3. Once logged in, navigate through multiple pages actually all served from the client, let's say https://my-app.example.org/foo, https://my-app.example.org/foo/bar, https://my-app.example.org/baz.
4. On that last page, do a full refresh
5. Still be able to navigate authenticated from https://my-app.example.org/baz

Silent authentication happens between steps 4 and 5, where you do a full refresh, and then on reload the library will find the trace of the previous session in local storage, redirect you to the same OpenID provider using the same callback URL https://my-app.example.org/callback, and since there should still be an active cookie on the OpenID provider you are authenticated there, so you're redirect back straight away without having to actively log in with a password or such. You end up on https://my-app.example.org/callback, and then the library emits an event so that you can send the user back to https://my-app.example.org/baz. All this should happen mostly instantly, although since there is a bit of bouncing around you'll typically see your app flickering through the screens.

With your proposed change, since the redirect URL doesn't match the current URL when doing the refresh, the user would end up unauthenticated if I'm understanding correctly, which isn't what is intended originally. Am I reading your code correctly?

I'm afraid the issue you are experiencing is caused by the deeply rooted assumption that two separate apps will be served on two different origins, resulting in isolation you're not getting by serving the two apps under the same origin. That breaks assumptions the library is making, so I don't think this is a use case we will support on the short term.

[smessie]

Thanks for your reply @NSeydoux, I certainly understand that having separate apps under the same origin is not the best choice. However, with technologies like GitHub pages more often being used nowadays to host applications like these, I feel like better support should be added, because now the result is that apps in the same organisation are just inaccessible.

I am not sure if the scenario you point out would give any problems and this is why.
It is only using the current URL like https://my-app.example.org/foo, https://my-app.example.org/foo/bar, https://my-app.example.org/baz if the url parameter was not provided to the silentlyAuthenticate function, but the idea is that you would pass along the redirect url using that parameter.
So every page should do a handleIncomingRedirect to restore the session with the url option set as the redirect URL https://my-app.example.org/callback. (the url option from handleIncomingRedirect is being passed along as parameter for the silentlyAuthenticate function.)
In that way, a different app is recognised by the redirect url and all URLs from above will result in the session being restored.

I can imagine that you do not consider this as the best implementation possible, but as the most important here is that we should add some basic support for technologies like GH pages, I am open for any other implementation.

[NSeydoux]

So every page should do a handleIncomingRedirect to restore the session

Client-side navigation aims at not requiring that, otherwise the navigation experience becomes quite painful because of the latency introduced by the redirects. There is no full refresh on client-side navigation, things are handled by the client-side framework (e.g. NextJS, Vite, React-Router...), so the session doesn't need to be restored because the JS context isn't flushed. I suspect the fix you proposed would break this because the current URL and the redirect URL can end up being different, in which case we still want the silent authentication to go through, which your change would disable.

the most important here is that we should add some basic support for technologies like GH pages

I don't fully agree with that: the most important thing is to provide a good developer experience while keeping user data secure. I'm not sure Github Pages is a good fit for the use case you're describing, so it doesn't ought to be supported. Hosting a single app under GH pages will work fine, and other providers, such as Vercel, allow deployments of apps under a single origin with a nice Github integration.

[smessie]

Client-side navigation aims at not requiring that, otherwise the navigation experience becomes quite painful because of the latency introduced by the redirects.

Well, let's rephrase my sentence. There does not have to be an extra handleIncomingRedirect call if it was not needed there before. The point is that where it occurs in the same app, it should have set the same redirect url, which is what we would expect in the first place anyways.

I suspect the fix you proposed would break this because the current URL and the redirect URL can end up being different, in which case we still want the silent authentication to go through, which your change would disable.

My change is using the configured redirect url as well (only, just to not introduce breaking changes, when the url parameter is not passed along, it would take the current URL. My change uses the configured redirect URL to compare it with the redirect URL configured in the StoredSession to see if the latter belongs to the app trying to restore it, without just assuming it does.

I don't fully agree with that: the most important thing is to provide a good developer experience while keeping user data secure.

Agreed. With "most important here" I was referring to this issue, and that I don't expect that my implementation should be the solution, I care more about a solution then the fact that it should be solved that way, especially if there is one with better developer experience and data security.