solid-client-authn-js icon indicating copy to clipboard operation
solid-client-authn-js copied to clipboard

solid-client-authn-browser should use storage abstraction for storing a current session or offer control of a namespace in the localstorage

Open Maximvdw opened this issue 3 years ago • 6 comments

Search terms you've used

currentSession, prefix localstorage, localstorage, sub diretory,

Impacted package

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

  • [x] solid-client-authn-browser
  • [ ] solid-client-authn-node
  • [ ] solid-client-authn-core
  • [ ] oidc-client-ext
  • [ ] Other (please specify): ...

Bug description

I have a use case where I have 2 or more solid apps in the same domain under a different path. Each app uses localstorage for session management with a unique prefix (e.g. app1.string.solidClientAuthenticationUser:...). This works fine apart for the currentSession in combination with the restorePreviousSession option. Logging in to another application will cause the restore session to fail for other apps in the domain.

According to the description: https://github.com/inrupt/solid-client-authn-js/blob/3fd7bb44de656bf044e9b09bb3f29fcada5e8de4/packages/browser/src/Session.ts#L227-L233 the reason for not using the storage abstraction layer is because of the browser-only context where this current session is stored. However, as a developer there is no control with this decision. There is also no way to change the namespace of the localstorage to include a prefix.

To Reproduce

  1. Create two applications under the same domain in different sub directories (/app1, /app2 )
  2. Use a custom storage method (localstorage) with a unique prefix for each sub directory
  3. Log in to /app1 with restorePreviousSession enabled
  4. Refresh the page (you will not have to log in)
  5. Visit /app2 with ``restorePreviousSession` enabled and log in
  6. Refresh the page (you will not have to log in)
  7. Visit /app1 again, you will have to log in again

Expected result

I would expect solid-client-authn-browser to use the storage method that is provided, or to have some control in the key that is used (i.e. a prefix). Developers are also not able to handle the storing of a current session themselves due to the unexported silentlyAuthenticate.

Personally I feel that it should use the provided storage method - as this method is used to find the session information https://github.com/inrupt/solid-client-authn-js/blob/3fd7bb44de656bf044e9b09bb3f29fcada5e8de4/packages/browser/src/Session.ts#L350-L368

Actual result

The client library has full control over the key causing unexpected behaviour as mentioned in the bug report.

Environment

    @inrupt/solid-client-authn-browser: ^1.11.7 => 1.11.7

Maximvdw avatar Apr 22 '22 15:04 Maximvdw

and update here?

Maximvdw avatar Dec 08 '23 09:12 Maximvdw

Is there any change required to make this fix happen? This is a huge limitation for same-domain solid apps

Maximvdw avatar Apr 03 '24 10:04 Maximvdw

Hi @Maximvdw , I'm sorry this hasn't been replied to. Currently, there is no support for same-origin apps, as these would end up sharing resources in the browser that are partitioned by origin and we want to keep isolated. Storage is one such resource.

I understand this feature request is a workaround that limitation, but two apps under the same origin would still be able to go into each others local storage, even if we had a namespacing mechanism which would only make this work on the happy path.

Supporting same-origin apps is not on the roadmap, so I don't think this issue will be fixed anytime soon if ever.

NSeydoux avatar Apr 03 '24 15:04 NSeydoux

Apart from same-origin apps, I found this problem also when writing an application that manages two different oidc contexts. Many other libraries do use the oidc. prefix to store session management, and as I've just found out, this causes loops in authentication between the two, because this library clears the same storage. (see e.g. oidc-client-ts).

Maybe a simple change of this prefix with a more specific one for solid-client-authn-js will at least solve this problem, whitout the need to add support for same origin apps or the ability to set a dynamic prefix.

At least documentation should mention this as 'oidc' really is a common prefix to use.

Chiyo-no-sake avatar May 18 '24 11:05 Chiyo-no-sake

@Chiyo-no-sake this is most probably caused by us using a fork of oidc-client-js (before oidc-client-ts was a thing). Since they come from the same codebase, they use the same naming scheme. I'll look if this is configurable.

May I ask why you are using two OpenID client libraries side-by-side?

NSeydoux avatar May 21 '24 07:05 NSeydoux

I need my user to log in both to their SOLID Pod via this library and to the company's IDP via oidc-client-ts, to interact with their APIs. The prefix is configurable for oidc-client-ts indeed, but I think it's easy to encounter this problem if it's not clear to the user which prefixes in the storage are managed via this library, and the user tries to use the same prefix for other logic

Chiyo-no-sake avatar May 23 '24 09:05 Chiyo-no-sake