handleIncomingRedirect restore all query parameters but shouldn't

[scenaristeur]

Bug description

handleIncomingRedirect with restorePreviousSession restore all query parameters but i don't want

I want the user is logged back when he visits the app and has already logged but not to restore all the query parameters that were in the url when he loggin.
so, to restore his session, i use
import * as sc from '@inrupt/solid-client-authn-browser' ... sc.handleIncomingRedirect({restorePreviousSession : true})

code here https://github.com/scenaristeur/booklice/blob/9f15b5c81878fc1551b4513518b64b107e4f6276/src/plugins/vue-solid.js#L63

But the query parameters are lost when using "restorePreviousSession"

if the url is http://127.0.0.1:8080/?url=https://the.url.com&title=bookmark&text=blabla when the user login,
the next time he visits the app with some other query parameters, like http://127.0.0.1:8080/?url=https://another_url.com&title=something_else&text=tiptop but the new query parameters are replaced with the one that were registered when he logged in.

To Reproduce

1. open https://scenaristeur.github.io/booklice/?url=the.url.com&title=bookmark&text=blabla
2. Login
3. open https://scenaristeur.github.io/booklice/?url=https://another_url.com&title=something_else&text=tiptop
4. the query parameters in the url are replaced with the one that was registered when login
5. if the user log out he can now access https://scenaristeur.github.io/booklice/?url=https://another_url.com&title=something_else&text=tiptop

Expected result

the redirect should be with ?url=https://another_url.com&title=something_else&text=tiptop

Actual result

the redirect is always with ?url=the.url.com&title=bookmark&text=blabla

Environment

npx : 1 installé(s) en 1.295s

  System:
    OS: Linux 5.4 Linux Mint 20.1 (Ulyssa)
    CPU: (4) x64 Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz
    Memory: 221.99 MB / 7.60 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 14.17.1 - /usr/bin/node
    npm: 6.14.13 - /usr/bin/npm
  Browsers:
    Chromium: 91.0.4472.114
    Firefox: 89.0.2
  npmPackages:
    @inrupt/solid-client: ^1.9.0 => 1.9.0 
    @inrupt/solid-client-authn-browser: ^1.9.1 => 1.9.1 
    @inrupt/vocab-common-rdf: ^0.7.4 => 0.7.4 
    @inrupt/vocab-inrupt-common: ^0.7.4 => 0.7.4 
    @inrupt/vocab-solid-common: ^0.7.4 => 0.7.4 
    @vue/cli-plugin-babel: ~4.5.0 => 4.5.13 
    @vue/cli-plugin-eslint: ~4.5.0 => 4.5.13 
    @vue/cli-plugin-pwa: ~4.5.0 => 4.5.13 
    @vue/cli-plugin-router: ~4.5.0 => 4.5.13 
    @vue/cli-plugin-vuex: ~4.5.0 => 4.5.13 
    @vue/cli-service: ~4.5.0 => 4.5.13 
    babel-eslint: ^10.1.0 => 10.1.0 
    bootstrap: ^5.0.2 => 5.0.2 
    bootstrap-vue: ^2.21.2 => 2.21.2 
    core-js: ^3.6.5 => 3.15.1 
    eslint: ^6.7.2 => 6.8.0 
    eslint-plugin-vue: ^6.2.2 => 6.2.2 
    register-service-worker: ^1.7.1 => 1.7.2 
    vue: ^2.6.11 => 2.6.14 
    vue-router: ^3.2.0 => 3.5.2 
    vue-template-compiler: ^2.6.11 => 2.6.14 
    vuex: ^3.4.0 => 3.6.2 
  npmGlobalPackages:
    @vue/cli: 4.5.13
    ember-cli: 3.26.1
    ember-rdfa-semapps: 0.0.0
    npm-check-updates: 11.6.0
    npm: 6.14.13

Additional information

Another redirection is used to allow hosting on github pages

• https://github.com/scenaristeur/booklice/blob/9f15b5c81878fc1551b4513518b64b107e4f6276/public/404.html#L9
• https://github.com/scenaristeur/booklice/blob/9f15b5c81878fc1551b4513518b64b107e4f6276/public/index.html#L15

but it has never been a problem, it was already used on poPock with solid-auth-client https://scenaristeur.github.io/solid-vue-panes/bookmarks?title=Popock&url=https%3A%2F%2Fscenaristeur.github.io%2Fsolid-vue-panes%2F&text=My%20favorite%20Pod%20Manager

[Vinnl]

Hi @scenaristeur, the query parameters are not lost, but you'll have to restore them yourself, since routing is app-specific — solid-client-authn-browser cannot know how to do so for you.

How to do so is described in the Use Session Restore Event Handler docs, but basically, you can pass a callback to onSessionRestore, which will be called with the URL with query parameters.

Please let us know whether that resolves your issue.

[scenaristeur]

thxs @Vinnl I got it working

[scenaristeur]

I got it working, but their is something that i don't understand :
The redirection always redirect with the url paramaters that i had when i logged in :

see it here https://scenaristeur.github.io/booklice/
example :

• my app has two pages :

/ for home
/about for "A propos"

if i run to /about then login all params url are send to authentication.
then i go to / for home.
Then when I come back tomorrow or when i refresh the page, the handleredirection process should redirect me to / home but it always redirect me to the /about page, the one i used when I logged in .

2021-07-03.14-41-37.mp4

[Vinnl]

Hi @scenaristeur, I reproduced the scenario you described above, and I indeed get sent back to /about as well. However, looking at your code, you log the URL passed to you in onSessionRestore, and url is the URL of the home page, rather than of the "about" page:

So the URL given to you by solid-client-authn-browser is correct.

I am not too familiar with Vue, but it appears that the way you restore that URL isn't quite correct. Given that query is undefined, it appears that your calling

        router.push({path: '?'+query})

results in ?undefined being appended to the current path (resulting in /about?undefined), rather than it being replaced by /.

[scenaristeur]

I need to keep the ?query parameter as the app is a PWA, progressive web app that you can install on your mobile phone and use to share your from other app https://web.dev/web-share-target/, sending some parameters like

"share_target": {
  "action": "/share-target/",
  "method": "GET",
  "params": {
    "title": "title",
    "text": "text",
    "url": "url"
  }
}

nevermind, for what is my problem here, and the next demo, i've removed that query parameter in the redirection
removed the vuejs redirect
and removed the url param in await sc.handleIncomingRedirect({restorePreviousSession : params.restore, url: window.location.href})

but it strange that the /about always come back
let's say the pwa open my app at http://127.0.0.1:8080/about?one=two&three=four or conforming with the share api at
http://127.0.0.1:8080/about?title=Solid&test=CoolProject&url=https://forum.solidproject.org/

if i login on this page, then close the app and reopen on the / rootbase path, i always be redirected to the url + params with the one i've been logged in. even if i have another route, or other query parameters, ALWAYS the ones when i've logged in come back, so i don't think this is a vuejs-side issue. Where are those parameters stored ? on the server or on the browser ?
see :

2021-07-07.11-09-29.mp4

[Vinnl]

If you open your app at http://127.0.0.1:8080/about?title=Solid&test=CoolProject&url=https://forum.solidproject.org/, then after you get redirected to your identity provider, the url passed to your handler in onSessionRestore should have a value of http://127.0.0.1:8080/about?title=Solid&test=CoolProject&url=https://forum.solidproject.org/, so you can use that to send the user back to that page. However, instead of sending the user to that page, you just append ?title=Solid&test=CoolProject&url=https://forum.solidproject.org/ to the current page, so the user might end up at http://127.0.0.1:8080/other-path?title=Solid&test=CoolProject&url=https://forum.solidproject.org/.

The value of url (i.e. http://127.0.0.1:8080/about?title=Solid&test=CoolProject&url=https://forum.solidproject.org/) gets stored in the browser. The value of the redirect URL (e.g. http://127.0.0.1:8080/other-path) is remembered by the Pod server.

[scenaristeur]

Hi @Vinnl , really sorry, but there is always something going wrong,
See I open one app stored on https://scenaristeur.github.io/booklice and connect on this app.
Then I open another app in another tab and I'm always been redirected to the i've logged. The two apps are two separate github repos with two different gh-pages :

I absolutely don't understand why

2021-07-15.00-44-35.mp4

same thing but other way open game-sync, then connect, then open booklice and redirected to game-sync (the page where i've logged in)

2021-07-15.00-51-28.mp4

[Vinnl]

Unfortunately cookies are always shared across an origin (scenaristeur.github.io in this case), otherwise e.g. you wouldn't be able to navigate from github.com/inrupt/solid-client-authn-js to github.com/scenaristeur/booklice while staying logged in to your GitHub account. Thus, different apps all running on the same origin (scenaristeur.github.io) will share the same sessions and hence remember the same redirectUrl.

Since you're using GitHub Pages, you could look at creating different GitHub organizations for your different projects if you don't want that.

[AJamesPhillips]

Thanks for this @Vinnl . onSessionRestore is what I needed. In my case I'm using a hash to route the single page application and the hash was disappearing completely.

I had read this tutorial page which introduced handleIncomingRedirect, but I did not see the more detailed page you have linked to.

I think we should either add onSessionRestore to the example on the first page, or link explicitly to the second page from the first page.

Also, I'm not sure, but I think the behaviour seems unnecessary? It would appear to me we could set a flag on the first session restore so the subsequent ones do not use this initial redirectURL, and or the handleIncomingRedirect should correctly take the current page url and set that as the redirect location before it reloads the page so that on it's second, third etc invocations it has the correct redirect url? Then we could get rid of onSessionRestore entirely? I'm sure this has already been thought of so it would be interesting to know if there's an outstanding issue to implement this or if it can't be done for some reason.

[AJamesPhillips]

Specifically I used this code:

onSessionRestore((url) => {
    if (document.location.toString() !== url) history.replaceState(null, "", url)
})

[Vinnl]

Pinging @NSeydoux to respond to that, since I no longer work at Inrupt and don't really have time to look into this :)

[AJamesPhillips]

So I had to use the following code as I have two separate apps running on the same domain. One at /app and a second at /sandbox which meant that logging whilst at /app then trying to access /sandbox resulted in onSessionRestore getting the /app url to redirect to which was wrong.

Instead I had to add the following code:

handle_login.ts

// The process offered by Solid does not seem to work
const LOCAL_STORAGE_KEY__SOLID_SESSION_RESTORE_URL = "solid_session_restore_url"


export async function correct_path ()
{
    const url = localStorage.getItem(LOCAL_STORAGE_KEY__SOLID_SESSION_RESTORE_URL)
    if (!url) return

    const url_object = new URL(url)

    // Paths are the same so nothing to do here.
    // For the hash component it seems fine and I think that's because in my application the path determines
    // which application code is loaded (/app vs /sandbox) but I could see how two SPAs on the
    // same domain but which the query parameter or hash determined which would load would
    // need to do a check here.
    if (document.location.pathname === url_object.pathname) return

    const new_url = new URL(document.location.href)
    new_url.pathname = url_object.pathname
    // Force the page to change to the new URL, e.g. correct from attempting to:
    // 1.  sign in to `/sandbox`
    // 2.  having solid reuse your old path at `/app` and incorrectly redirect you there
    // When the page reloads it will hit the path equality conditional above and correctly return early.
    document.location.href = new_url.toString()

    return Promise.reject()
}


export async function finish_login ()
{
    localStorage.setItem(LOCAL_STORAGE_KEY__SOLID_SESSION_RESTORE_URL, window.location.toString())

    await handleIncomingRedirect({ restorePreviousSession: true })

    // This line is not reached until you are successfully logged in
    localStorage.setItem(LOCAL_STORAGE_KEY__SOLID_SESSION_RESTORE_URL, "")
}

index.tsx

function restore_session (): Promise<void>
{
    
    root_el.innerHTML = "Requesting Solid login session..."

    // See https://github.com/inrupt/solid-client-authn-js/issues/1473#issuecomment-902808449
    onSessionRestore(url => {
        if (document.location.toString() !== url)
        {
            // I do not think this is needed anymore but I'm not 100% sure now.
            // I think the application only now needs:
            //            history.replaceState(null, "", url)
            //
            if (document.location.pathname !== new URL(url).pathname) document.location.href = url
            else history.replaceState(null, "", url)
        }
    })

    return correct_path().then(finish_login)
}


restore_session().then(() => render(<App />, root_el))

[scenaristeur]

@AJamesPhillips happy to see some other with the same problem and that you found a potential workaround, but i basically think that the redirect target should by default be the page where the user were before the session check. Not the one where he was when he logged in for the first time (can be two hours ago !). And we should be able to host many apps on the same domain in subfolder or in subdomains

[scenaristeur]

@Vinnl ,
You say :
'''
Hi @scenaristeur, the query parameters are not lost, but you'll have to restore them yourself, since routing is app-specific — solid-client-authn-browser cannot know how to do so for you.
'''
But I think solid-client-auth should not restore the old parameters !

[Vinnl]

@scenaristeur I don't understand - that already is the case? SCA doesn't restore the old parameters; it merely remembers them for you so you can restore them yourself if you wish. (And it by definition can't redirect the user to the page they were at, because it's not SCA that does the redirection - it's the Solid identity provider, and that only knows the page the user was on when they first logged in. So all SCA can do for you is remember the page the user was coming from, and inform your app when the Solid identity provider redirects the user back. But only the app is then in a position to actually restore the parameters if needed (or not, if not).)

[AJamesPhillips]

I thought more about this, specifically the desire to have multiple applications on a domain either under different subdomains, paths, or perhaps even determined by query parameters or the hash.

From a security point of view if you signed in at: good_app.github.io and latter got sent a link to evil_app.github.io then you obviously do not want the second to have the first's access rights. And I think this is achieved by the call to handleIncomingRedirect({ restorePreviousSession: true }), via the Solid identity provider, returning you to the page you first signed in at? It would be good to see this motivation confirmed in a specification doc.

Using this redirect to the first signin url as a default behaviour I think makes sense and has to stay (from a security) point of view? But also, and I'm not certain, but when you initially authorise an application, if the application could ask for a scope for redirects then that could allow the Solid identity provider to "safely" redirect you to a "session restoring" URL that's different from your initial sign in URL? i.e.:

1. user goes to good_app.github.io/path_one or example.com/good_app/?query=one and clicks sign in (for the first time).
2. good_app calls SCA (Solid Client Auth) code which <inserts steps I'm not 100% clear on> eventually after the user accepts, redirects them back to example.com/good_app/?query=one
3. When a few seconds, or 3 hours later, the user is now on a different page, e.g. good_app.github.io/path_TWO or example.com/good_app/?query=TWO or clicks a link to go to open a new tab on a different page of the application.
4. The SCA code runs but the Solid identity provider returns the user to the initial URL at signin.

• At the moment we have to do various url restoring hacks which will not work in all situations as it requires coordination between different applications and from a user perspective is quite confusing to go to app1, click sign in, get send back to app2 and have app2 loads then send you back to app1...
• Instead perhaps if in step2 the SCA code also allowed, and the identity provider also supported a scope, then:

2. good_app calls SCA (Solid Client Auth) code and also provides a scope of good_app.github.io/* or example.com/good_app/*
3. good_app sends a redirect url that is different from the initial sign in URL but that is within the scope of URL redirects, i.e. example.com/good_app/?query=two is allowed by example.com/good_app/*
4. in step 4: the Solid identity provider checks the new requested URL, finds it's valid, and redirects the user back there instead.

Would that work or does it have some problems / other disadvantages in it?

[NSeydoux]

Sorry I took so much time to reply this issue.

it would be interesting to know if there's an outstanding issue to implement this or if it can't be done for some reason.

I think that's an excellent starting point: why is the library behaving the way it is currently ? The problem we want to solve is the following: a user is logged in a SPA, which means routing happens client-side. The user logs is on some page, https://example.org/some-page/?some-query=some-value. This IRI is used as a redirect IRI in the call to login, and handleIncomingRedirect is then called from this page too. The user is logged in, all is well.

Note that when calling login, if the client is neither statically registered nor using a Client Identifier, which is currently the case for the majority of clients, the client goes through a process called Dynamic Client Registration, during which the client gives some information about itself to the OIDC provider, including but not limited to its valid redirect IRIs. This is important for later.

Then, the user navigates client-side to a different page, https://example.org/another-page/. Client-side routing means that there has been no full reload of the browser page, which preserves the context, and means the same session is still logged in and is able to make authenticated requests. All is still well. Let's now pretend that the user does a full refresh of the page: the context is lost, and the access token, which for security reasons is only stored in-memory, is no longer available. The app can no longer do authenticated request, and the user should log in again, which is annoying. However, we can assume that there is still a valid cookie for the browser on the OIDC provider, and that the user won't be prompted for their login/password if we redirected them there, and would directly be redirected to the client-controlled redirect IRI.

The problem we want to solve happens at this point: for the authentication to be completely silent (transparent to the user), the client must be known to the OIDC provider, so we have to reuse the same client ID/secret as previously used, even if they were dynamically registered. Which means we also MUST reuse the original redirect IRI (in our case https://example.org/some-page/?some-query=some-value), asking the OIDC provider to redirect the user to https://example.org/another-page/ with the same client credentials would be refused and the user would not be logged in. That is why the onSessionRestore callback receives the IRI where the redirection takes place as a parameter: in order to silently log the user in, we must redirect them from the new IRI to the OIDC provider, which redirects them to the original refirect IRI, where the user is logged back in, and where the onSessionRestore callback is called with the new IRI as a parameter to enable client-side navigation back where the user refreshed the page. Otherwise, the use experience is awful, because they refresh a page and end up on a different page, even if the intermediary redirects already make for a somewhat degraded user experience.

So @scenaristeur, part of the issue is that the redirect IRI must exactly match, query parameters included. Unfortunately, I don't see another approach besides overriding the query parameters in the callback passed to onSessionRestore. You could also consider storing their value in local storage before calling login, and provide a query-less IRI as a redirect IRI to the login function. However, you'd have to use the onLogin callback to restore the query parameters so that you app can actually use them, so I don't know to what extent it's a win for you.

[NSeydoux]

@AJamesPhillips what you propose here is a very good point:

if the application could ask for a scope for redirects then that could allow the Solid identity provider to "safely" redirect you to a "session restoring" URL that's different from your initial sign in URL?

Asking for such a scope is done via using a proper Client Identifier, and not going through Dynamic Client Registration, as explained on this page. In your client profile document, you may specify as many redirect IRIs as you wish, in which case we could consider using the refreshed IRI as a redirect IRI too, which already carries an assumption that any page in your application the user may refresh is a valid redirect IRI. This for instance does not work for IRIs with dynamic parameters, such as the one @scenaristeur described.

One possible workaround would be to provide a transformation callback that gets applied to the IRI in order to compute the associated redirect IRI. This way, one could:

• store dynamic parameters in a redirect-persistant place, e.g. the local storage
• remove dynamic parameters from the IRI so that it matches a valid registered redirect IRI
• restore the dynamic parameters on the onSessionRestore callback to implement client-side navigation using the parameters

in this case, we could use the current IRI as a redirect IRI if the user either provided an IRI transformation callback and/or is using a Client Identifier. I'm a bit concerned this would get rather involved and bug-prine though, so I'm interested in your feedbacks as you're hitting such issues yourselves.

[AJamesPhillips]

@NSeydoux thank you for the response. There's obviously a lot of complexity going on here but I think there's some low hanging fruit. Firstly it would be good to clarify the role of redirectUrl in the documentation by saying:

When a refresh token is available, i.e. on subsequent calls to login, the OIDC provider does NOT redirect to redirectUrl, instead it redirects to the original redirectUrl value. This is for security to ensure a malicious application hosted on the same domain can not hijack the session via the refresh token. Use onSessionRestore to access the newer redirectUrl value"

(I would submit a tentative PR but I can't find where that documentation is on Github)

the client gives some information about itself to the OIDC provider, including but not limited to its valid redirect IRIs

I think I've misunderstood you as looking at the code I can't see a list of redirectIRIs supported? If possible it would be great to extend the tutorial / docs with: "providing a list of redirectIRIs can enable the same, or different applications on the same domain to get the Solid OIDC provider to redirect them to the newer (correct) URL instead of back to the URL the login was first started with, for example... ".

[AJamesPhillips]

And thank you for your comments here @NSeydoux . If I correctly understand what we're all saying: One of the most extreme cases of where this seems to currently fail is where we have different applications on the same top level domain. Currently I do not think the use of a proper Client Identifier and redirect_uris would solve this as it is a fundamental limitation of browsers? i.e. I think browsers would need to support cookies limited to specific paths. Summary: sorry @scenaristeur but we can't host two different applications on the same TLD or subdomain and expect them to not share Solid auth details. Is that correct @NSeydoux ?

Browser's do however support limitation of cookie scope to subdomains right?... I wanted to check that this would enable @scenaristeur to host two different applications on scenaristeur.github.io and not have them know anything about each other? This would still not help @scenaristeur and my second example because the two apps are on different paths on the same subdomain (or top level domain with no subdomain). But limiting the scope of cookies to the different subdomains would allow a user to authorise an application on scenaristeur.github.io to access their Solid pod and not risk getting it highjacked by evil_app.github.io ?

[NSeydoux]

I think I've misunderstood you as looking at the code I can't see a list of redirectIRIs supported?

indeed, when logging in, only one redirect IRI may be specified, because this gives the OIDC provider an unambiguous information on where the user should be redirected after they log in. If the client is dynamically registered (the default behaviour), then only this redirect IRI will eve be authorized for the given client instance. This constraint stems from the OIDC specification. However, the Solid-OIDC specification defines a new, Solid-specific way of approaching this by allowing the client to host its own profile document, where it can define a list of valid redirect IRIs instead of one. Note that the redirect IRI used at login time still needs to exactly match one of these pre-registered IRIs, IRI patterns are not authorized.

Also, to clarify, any call to login uses the provided redirectUrl. The issue we want to solve only manifests itself after the user has logged in once, when they refresh the page essentially. Then, handleIncomingRedirect is called, and it's at this point that we 'implicitly' call login if restorePreviousSession is set to true. Because of this implicitness, we have to figure out the values for the parameters, hence the absence of control of the user on the redirect IRI. The exact page relevant to this use case is this one https://docs.inrupt.com/developer-tools/javascript/client-libraries/tutorial/restore-session-browser-refresh/.

The problem with having multiple applications on the same origin is that they would share the same local storage (rather than cookies being shared, since cookies in this case only get involved at the OIDC provider level), which is used to detect whether a session is currently active and to kickstart the silent authentication process if applicable. This means only one redirect IRI is expected by origin, which is the problematic assumption we made. However, if we remove this assumption, the problem becomes: how can solid-client-authn-browser tell which app is currently active for a given IRI, since multiple apps would share the same local storage ? The problem being that with client-side navidation being an option, there is no way to know where is the root of any app in the current path... So yes @AJamesPhillips, two applications under the same origin would share the same storage, which contains authentication details, and I can only think of brittle heuristics to discriminate them.

Overall, there seems to have two discussions going on in this issue:

• one regarding the query params being restored, which I think has been responded to by @Vinnl and myself, and for which a workaround exists. @scenaristeur , you're the original reporter of this, do you agree that even though it's not the behaviour you'd prefer, there are workarounds for this issue, either not including the query parameters in the original redirect IRI (and managing them locally), or using the onSessionRestore callback ? If not, I'm happy to continue the conversation, and otherwise this issue can be closed.
• one regarding hosting several apps under the same origin, which is currently not possible for solid-client-authn-browser. I can create a dedicated issue for this to host the dedicated conversation in a different issue so that other developers with similar questions can find it more easily.

[scenaristeur]

Overall, there seems to have two discussions going on in this issue:

yes there are two issues. thxs all, i'll try what was said with query parameters.

and follow #1647 for the second