Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does not stay logged in after refresh #423

Closed
jaxoncreed opened this issue Oct 14, 2020 · 29 comments
Closed

Does not stay logged in after refresh #423

jaxoncreed opened this issue Oct 14, 2020 · 29 comments

Comments

@jaxoncreed
Copy link
Contributor

Describe the bug
After a successful login, if you refresh the page (or you lose the auth session instance for any other reason), the application will not still be logged in.

To Reproduce
Steps to reproduce the behavior:

  1. Run the example at https://github.com/inrupt/solid-client-authn-js/tree/master/packages/browser/examples/single/bundle
  2. Log into a server.
  3. Refresh the page

Expected behavior
Solid-client-authn should use a refresh token to get a new key so that a user stays logged in when they refresh.

Environment

  System:
    OS: macOS 10.15.6
    CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
    Memory: 220.08 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 14.4.0 - /usr/local/bin/node
    npm: 6.14.4 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Browsers:
    Chrome: 86.0.4240.75
    Firefox: 81.0
    Safari: 14.0
  npmPackages:
    @babel/core: ^7.8.6 => 7.10.5 
    @babel/preset-env: ^7.8.6 => 7.10.4 
    @babel/preset-react: ^7.8.3 => 7.10.4 
    @inrupt/solid-client-authn-browser: file://../../../ => 0.2.1 
    babel-loader: ^8.0.6 => 8.1.0 
    html-loader: ^0.5.5 => 0.5.5 
    html-webpack-plugin: ^3.2.0 => 3.2.0 
    react: ^16.13.0 => 16.13.1 
    react-dom: ^16.13.0 => 16.13.1 
    regenerator-runtime: ^0.13.3 => 0.13.7 
    webpack: ^4.41.6 => 4.44.0 
    webpack-cli: ^3.3.11 => 3.3.12 
    webpack-dev-server: ^3.10.3 => 3.11.0 
  npmGlobalPackages:
    @inrupt/generator-solid-react: 0.7.2
    @inrupt/solid-auth-fetcher: 0.0.6
    expo-cli: 3.27.4
    lerna: 3.22.1
    npm: 6.14.4
    ochat-api: 1.0.0
    redis-commander: 0.7.0
    static-server: 2.2.1
    typescript: 3.9.5
    yo: 3.1.1

Additional context
This probably is just due to the new closure version of storage. To fix this refresh token flow should be triggered upon startup.

@Vinnl
Copy link
Contributor

Vinnl commented Oct 15, 2020

I think the challenge here is that we also can't securely store the refresh token anywhere that would allow us to keep a hold onto it after a page refresh. Or at least, we haven't found a way yet - hopefully we'll be able to :/

@NSeydoux
Copy link
Contributor

Exactly: in the case of a single page app, there is no secure way (as far as we can tell) to keep a token (access or refresh) without exposing it to potentially malicious third-party scripts. Working with a server-side component is required to persist the tokens securely.

I think a possible mitigation could be to use a cookie-based mechanism, as does the data browser against NSS: in this case, as long as the cookie is valid (and it will remain valid even after refresh), the browser can still access resources even if the token expires. Note that this requires to work with a bearer token, and not a dpop one, which means with the current codebase it would not work against NSS, since NSS's bearer token is a legacy ad-hoc token, and not a regular OAuth2 bearer token.

@jaxoncreed
Copy link
Contributor Author

Indeed for a single page app, one way to refresh is using cookies on the IDP's domain and triggering a refresh via an iFrame. This is silent authentication (Here's an article about how silent authentication works on Auth0: https://auth0.com/docs/authorization/configure-silent-authentication)

But, you can also use the refresh token flow in the browser with Refresh Token Rotation: https://auth0.com/blog/securing-single-page-applications-with-refresh-token-rotation/

@timbl
Copy link

timbl commented Oct 19, 2020

This is quite a big usability issue.

For comparison, If I log in to bank (say), it seems that I can go to a protected page and refresh it fine, and also copy the URL into a new tab and that works. (And the bank is only one click to log in compared to 4 with solid). I wonder how they solve this problem.

Not looking forward to debugging with the need to log in every refresh either. Hit error > set breakpoint the line before> refresh ...

Also the normal tricks as user, like dragging any solid thing onto the tab bar to get a new tab about it, or Cmd/clicking something to open it in a new tab, and so on.

@jaxoncreed
Copy link
Contributor Author

jaxoncreed commented Oct 19, 2020

@timbl this problem will only exist for single page applications. Applications with a server-side component can store credentials on the server then use cookie credentials to link it to a client.

It is unfortunate that we can't maintain a login state on single page applications like the databrowser anymore, but I get the concern for security that the Inrupt team is talking about because Pods do potentially have financial and medical data.

I think that maintaining the token on the client side could be returned eventually once we have client constraining access control as long as the client doesn't try to access sensitive data.

@michielbdejong
Copy link

Hm, if it has been decided that this is the functionality ISCAJ will offer (which is what I gather from Solid OS gitter), and we want to use ISCAJ in Solid OS, Darcy, and other Solid apps (which we do!), then we will all need to come up with a way to make our page refreshes work. :)

Considering there are 5 actors: the server hosting the app, the idp, the server hosting the data, the auth server (which hosts the auth dialog with the 'Approve' button), and the browser containing the JS runtime running the SPA.

I don't want to use a cookie on the server hosting the data, that just feels dirty, and it's scary if you combine it with CORS. It would feel like ISCAJ's high security at the core drives others to use terrible security in the "garden" around ISCAJ's "castle".

But what if the browser would only remembers the user's webid? Just remembering the webid is acceptable, right?

Then, with a cookie from the IDP (also acceptable I think), then it can silently get a new DPop token from the IDP using the cookie, use that to silently* get a new access token from the auth server, and be back in business after a page refresh.

@Vinnl
Copy link
Contributor

Vinnl commented Oct 20, 2020

@timbl Agreed that it is a massive usability problem. As for how a bank can solve it: they can store secrets on the server, and then associated those with the user via an HTTP-only cookie. Given that we don't control the server, unfortunately that is not an option.

@michielbdejong It has not been decided that this package will not offer login after refresh - just that we don't know how to do it yet.

I'm not sure I follow your proposed solution. What would the IDP cookie contain? If that would be possible, then the security issue (any script on the page being able to independently perform actions on your behalf against your Pod) would be back, so as far as I know that's exactly what our other mechanisms are trying to prevent.

From my point of view, Refresh Token Rotation is the most viable solution I can see, but we have yet to confer with a security specialist about the risks, and that would also require server-side/spec changes.

@jaxoncreed
Copy link
Contributor Author

Then, with a cookie from the IDP (also acceptable I think), then it can silently get a new DPop token from the IDP using the cookie, use that to silently* get a new access token from the auth server, and be back in business after a page refresh.

If that would be possible, then the security issue (any script on the page being able to independently perform actions on your behalf against your Pod) would be back, so as far as I know that's exactly what our other mechanisms are trying to prevent.

Yep, @michielbdejong, you're talking about silent authentication https://auth0.com/docs/authorization/configure-silent-authentication. I had an offline discussion with @ajacksified. While silent authentication is standard for OIDC, it doesn't hold up to the security standards of banking because an injected script could trigger silent authentication. So, @Vinnl is right.

Really, I only see one solution: encourage Solid app developers to have a server side component instead of building single page applications. I know that's going a bit away from some of the philosophy, but based on the conversations I've had, it seems to be a requirement for security.

@michielbdejong
Copy link

@travis
Copy link
Contributor

travis commented Nov 30, 2020

Is there currently a way to configure the browser auth library to bring this behavior back, even in a less secure way? I'd love to be able to get this behavior (staying logged in after refresh) back as I build demo apps and prototypes and updating to the latest version of the library really makes the user and developer experience miserable in the ways Tim suggested above. I don't need the less secure setup to be the default, but I don't need strong security at this stage of the app lifecycle and am hopeful that this will be solved inside this library by the time I do.

I've tried various combinations of passing along a localStorage based IStorage as the insecureStorage and secureStorage for both Sessions and SessionManagers, but haven't been able to get the original Session back - I may be getting confused by dependency injection so please let me know if there's some magic I'm missing.

Thanks!

@travis
Copy link
Contributor

travis commented Nov 30, 2020

also, apologies if I'm missing something obvious, but curious about this:

While silent authentication is standard for OIDC, it doesn't hold up to the security standards of banking because an injected script could trigger silent authentication.

if I were able to inject a script on a bank's website wouldn't I be able to use their HttpOnly cookie auth to take any actions their website enabled me to perform without the end user's knowledge? What makes OIDC silent authentication based on an HttpOnly cookie on the IDP more risky?

Thanks for all of your work on this - fwiw I've been bumping up against this particular failing of web browser implementations for my whole career so I appreciate that y'all are tackling it in this context!

@NSeydoux
Copy link
Contributor

NSeydoux commented Dec 1, 2020

Unfortunately @travis, there is currently no way to work around this. However, we are working on making it possible to support this use case, with the caveat that it introduces an important security issue (the one that motivated us to move away from solid-auth-client in the first place). It's then a tradeoff between usability and security, so in the case of a demo app such as the ones you are working on, you should be able to get this behaviour without having to add a server-side component. I'll let you know as soon as I make progress on this :)

@michielbdejong
Copy link

@travis FWIW, you can use https://github.com/solid/solid-auth-fetcher instead

@WhyINeedToFillUsername
Copy link

@jaxoncreed thank you for reporting this, I came here with the same issue.

I am building a single page application (SPA) using angular that would monitor user inboxes. I guess I used an older version with Solid as refreshing page was not an issue. Now, when I try using solid-client-authn-js with inrupt, refreshing page loses login.

I've read through this discussion, as well as the linked issue solid/authentication-panel#84 and I have couple of questions. Hope somebody could help me (@michielbdejong ? :) ).

  1. if there is no workaround with the current version of solid-client-authn-js, is there an older version of the library I could temporarily use that would keep login after refresh?
  2. I don't understand what is the difference between the solid-client-authn-js and forked solid-auth-fetcher. Does the solid-auth-fetcher have the capability of keeping the login state?
  3. if no "keeping login without refresh" is currently possible in SPA without backend, what would I have to do with backend to achieve this? Would the backend have to do the authentication, keep the session, make the calls to pod etc.? Or would it be sufficient only to store the session somehow?

@michielbdejong
Copy link

Maybe @ajacksified or @nicolasmondada can answer your questions!

travis added a commit to understory-garden/swrlit that referenced this issue Dec 16, 2020
the inrupt auth libraries no longer maintain sessions across page
reloads (inrupt/solid-client-authn-js#423)
which is better for strict security, but a dealbreaker for user
experience, so switch to solid-auth-fetcher for now.

the right solution here is to make swrlit auth library agnostic, which
will also involve putting together a cleaner typing system - the types
here are copied from solid-auth-fetcher which is super messy.
@Vinnl
Copy link
Contributor

Vinnl commented Dec 17, 2020

@WhyINeedToFillUsername I'll try to answer just what I know:

  1. Not one that I could point out. There used to be an insecure mechanism, but I don't know if that ever actually got released with a proper version number, and if it did, I don't know what number it was. It was certainly not a stable (v1+) release that passed a security audit, at least. (Because the issue was identified in one.)
  2. The fork is by @michielbdejong I believe, so... I suppose Michiel can explain the motivation behind it?
  3. I suppose it'd be something like you'd describe, but I would estimate that implementing that will probably take longer than that a fix for this issue would take.

Unfortunately I don't think I'd recommend anything other than waiting for a fix. On the plus side, everyone I've spoken about it considers this a very high-priority issue.

@WhyINeedToFillUsername
Copy link

@michielbdejong could I ask about the intentions behind creating https://github.com/solid/solid-auth-fetcher fork and its differences with the original https://github.com/inrupt/solid-client-authn-js, please? :)

@michielbdejong
Copy link

@WhyINeedToFillUsername sure, but these changed over time.
When I first created it, this project (https://github.com/inrupt/solid-client-authn-js) was paused. So solid-auth-fetcher was the only client in the world supporting webid-oidc-dpop.

Later, when this library was published, we wanted to deprecate solid-auth-fetcher, but couldn't because of this issue (#423).

Nowadays, rumour has it this issue is about to be resolved, but unfortunately #538 was marked as a wont-fix, meaning this library will never be fully compatible with the open Solid spec and with the way the Solid OS stack uses the pod (especially how chat conversations update in real-time).

That is why the Solid OS stack, as well as the Solid test suite, will continue using its own solid-auth-fetcher. We all hope this situation will somehow change in the future and we can merge the two back together! :)

@WhyINeedToFillUsername
Copy link

@michielbdejong thank you for the answer.

For the record, I tried using the solid-auth-fetcher in my app, but it does not support logging in with multiple accounts (as it claims at https://github.com/solid/solid-auth-fetcher#logging-in-multiple-users) due to this: solid-contrib#19

Also, it does not work with Angular (I believe for the same reason as inrupt/solid-client-js#608 - missing polyfills). So I couldn't use it in my app.

@matthieu-fesselier
Copy link

Is this issue still up to date? There is a documentation page which explains how to restore session after a page refresh here: https://docs.inrupt.com/developer-tools/javascript/client-libraries/tutorial/restore-session-browser-refresh/

Actually, I tried to set up this by adding the restorePreviousSession parameter to handleIncomingRedirect, and I get an error which prevents the user to be logged in:

Field [sessionId] for user [f4436a4865f94bb793670bec5b1a03ae] is not stored

What happens is:

  • After being logged in, I refresh my page http://localhost
  • It redirects me to http://localhost?code=[code]&state=[state]
  • In the lib, in StorageUtility.js, it looks for the sessionId, which looks in the insecureStorage for the key solidClientAuthenticationUser:[state]
  • It does not find anything, throws the error and the user is not logged in

Did I miss something?

@NSeydoux
Copy link
Contributor

Thanks for the reminder, I forgot to keep this up-to-date.

As you mention, we've added some features to solid-client-authn-browser to resolve this issue, but these features are disabled by default, you have to actively opt-in to enable them, because they may be disruptive to your app depending on how you manage state. If you call handleIncomingredirect with {restorePreviousSession: true}, your app should log back in on reload.

For the exact issue you are seeing, I'm not exactly sure where this is coming from. The steps you describe look correct. https://github.com/inrupt/solid-client-authn-js/tree/main/packages/browser/examples/single/bundle provides an example of running app using this. What app are you running exactly ? I've observed similar issues using parcel sometimes I think, but the problem wasn't easy to reproduce, it only happened occasionally. In your case, the bug is manifesting systematically ?

@NoelDeMartin
Copy link

@NSeydoux I tried the latest version, and I noticed that it works by causing a bunch of redirects. I was already doing this in my app, even though I was asking the user for confirmation instead of doing it automatically.

Since you haven't closed the issue, does that mean that this is just a temporal solution, or do you consider the issue resolved?

Personally I think that it's still not great UX, I have used some websites that do similar things in the past and it always bothered me.

@matthieu-fesselier
Copy link

@NSeydoux

If you call handleIncomingredirect with {restorePreviousSession: true}, your app should log back in on reload.

Yes this is what I did

What app are you running exactly ?

I included solid-client-authn-js in sib-auth, which is our authentication component at Startin'blox (code here)

Is the timing important? Can it be executed too early when the page load?

I've observed similar issues using parcel

In this case, I bundled the lib using webpack

In your case, the bug is manifesting systematically ?

Yes in my case, it happens every time:

  • I log in successfully
  • I refresh the page -> error -> not logged in
  • I refresh the page -> authorization screen -> logged in

@NSeydoux
Copy link
Contributor

NSeydoux commented Apr 1, 2021

@NoelDeMartin Unfortunately, there isn't much of a choice but redirects to achieve this, because any other approach (e.g. refresh tokens) would require to store sensitive information in an unsecure place. In that regard, the issue can be considered resolved, since the missing feature is now available securely.

However, I totally agree that the UX isn't satisfactory. There's a way to implement the redirect not in the main window, but in an iframe, which prevents the main window from blinking during the redirection. iframes come with their own restrictions though, which is why we haven't implemented this approach yet, but we'll try to improve the user experience by doing it at some point. In the meantime, I'm afraid there's no more satisfactory solution :/

@NSeydoux
Copy link
Contributor

NSeydoux commented Apr 1, 2021

@matthieu-fesselier

Is the timing important? Can it be executed too early when the page load?

No you should be alright, in React components we've included this in the useEffect hook, so it's running first thing on page load, and in non-React apps this could be the first line of the script it shouldn't change the behaviour.

I'll have a look at the code and see if I can make sense of it. To some extent, it's "good' that it's happening systematically, I've never been able to investigate this deeply because it only occured once in a blue moon ^^.

@matthieu-fesselier
Copy link

I also tried with inrupt provider to see if it could be related to our IDP, but I face the same bug.
Let me know if you want to test the component to reproduce it!

@NoelDeMartin
Copy link

@NSeydoux Thanks for clarifying.

To be honest, I'm not sure how the redirects are any more secure than storing the token. After all, any sensitive information that is necessary for me to log into my POD is already stored in my computer, because it isn't asking me for a password during the redirects, everything happens without user interaction. If the problem has anything to do with domains and sandboxing, wouldn't it be possible to obtain the token doing fetch requests instead?

I don't know the internal details, so I'm just sharing an uninformed opinion. But I find it difficult to believe that it isn't possible to make Solid apps without redirects at every page refresh, specially since I could do it using solid-auth-client (and that's how most websites work).

Hopefully the iframe solution can fix the UX, but I'm surprised that this is such a pain when logging in is an essential part for any app.

@NoelDeMartin
Copy link

We've been following up on my last comment in the community forum, so if anyone wants to learn more check it out: Authenticating Offline-First Solid Apps.

Thanks @NSeydoux for answering my questions there :).

mrkvon added a commit to mrkvon/friend-crawler that referenced this issue Jul 18, 2021
Field [sessionId] for user [...] is not stored
fix: inrupt/solid-client-authn-js#423 (comment)
mrkvon added a commit to livegraph-org/solid-math that referenced this issue Sep 16, 2021
Field [sessionId] for user [...] is not stored
fix: inrupt/solid-client-authn-js#423 (comment)
@nicolasmondada
Copy link
Contributor

This issue has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants