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

Potential client issues when cache contains invalid values #2088

Open
1 task done
joachimvh opened this issue Apr 19, 2022 · 7 comments
Open
1 task done

Potential client issues when cache contains invalid values #2088

joachimvh opened this issue Apr 19, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@joachimvh
Copy link

Search terms you've used

session, cache

Impacted package

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

  • solid-client-authn-browser

Bug description

This is not really a bug (I think), but more of a potential situation that can arise. I'm mostly curious about who is responsible here and how this behaviour should be caught. I accidentally discovered this when doing tests with mashlib, but this can be reproduced using packages/browser/examples/demoClientApp.

To Reproduce

  1. Start CSS (or any Solid server I would imagine) and register a user
  2. Change this line to point to your local server (e.g., const defaultIssuer = 'http://localhost:3000/;):
    const defaultIssuer = preconfiguedIdpList[1];
  3. Start the demo client app (at http://localhost:3001) and log in
  4. Remove the registered clients from the Solid server (can be done in CSS by remove the .internal/idp/adapter/ folder).
  5. Refresh http://localhost:3001.
  6. The page will immediately try to redirect to something like http://localhost:3000/.oidc/auth?client_id=4CFIF-J_Ul8JTW2Yvk-Oc&redirect_uri=http%3A%2F%2Flocalhost%3A3001%2F&response_type=code&scope=openid%20offline_access%20webid&state=9e26631265fe40bb859b8760e81c9311&code_challenge=Lz5wSueifjca4C2uxsz8y0H0Myozx-UC72yODlrjjgU&code_challenge_method=S256&prompt=none&response_mode=query
  7. The user will get stuck on a 400 page showing the error that the client_id is unknown by the server

The error is expected as the server no longer knows the client. The problem is that there is now no way to use the client any more: every time the user tries to browse to the client it will immediately get the redirect above. The only way to solve it is to clear the browser cache for localhost.

The thing I'm wondering now is: is this expected/acceptable behaviour?
If not, who should handle this more gracefully?
Should the server still call the callback URI even in case of error?
Should the client somehow check if the browser cache still contains valid values?
Should it not automatically try to redirect if it detects a cache?

Environment

v1.11.7 of the browser authn client

@joachimvh joachimvh added the bug Something isn't working label Apr 19, 2022
@NSeydoux
Copy link
Contributor

At a first glance, I would say that the issue is on the server-side, but it may be a wrong understanding of the prompt=none parameter. Based on the description in https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest, I would expect the user to be redirected back to the client with an error in the redirect IRI query params, which the library would pick up to clear the local storage from the now invalid client identifier. The user would basically be back to the page they just refreshed, unauthenticated, and could successfully go through login from scratch. The fact that the OP shows an error page seems contradictory with prompt=none. Does that make sense?

@joachimvh
Copy link
Author

It makes sense. I'm not sure what the correct way is to interpret this for the prompt parameter, so I'll see how feasible it is to let the OIDC library perform a redirect instead just to improve the potential user experience.

@joachimvh
Copy link
Author

Had a deeper look into the OIDC library which caused me to find some more relevant information. The library does redirect in case of error, but only if it can verify that the provided redirect_uri in the request is valid. Since the client metadata was removed in the example, the redirect_uri can never be valid so the redirect does not occur.

This is also what the OpenID spec says at https://openid.net/specs/openid-connect-core-1_0.html#AuthError

Unless the Redirection URI is invalid, the Authorization Server returns the Client to the Redirection URI specified in the Authorization Request with the appropriate error and state parameters.

Now of course the argument could be made that it's simply the server's fault for deleting the client metadata, but turns out that this is not really something that can be fixed server-side when following that requirement.

@joachimvh joachimvh reopened this Apr 26, 2022
@ludwigschub
Copy link

I'm also experiencing this issue in projektor when trying to re-login to inrupt's ESS after re-opening the page. The user is then stuck in the login flow and i don't know how to catch this error client-side.

Does the solution maybe include a reset of the login flow when this error occurs so that users just start again at the username/password form?

@NSeydoux
Copy link
Contributor

@ludwigschubi could you give a bit more details about your issue so that we could reproduce ? In particular, I have two questions:

  • Is the client using a Solid-OIDC Client Identifier, or is it defaulting to dynamic client registration?
  • How long does the page remain closed before attempting to log in again ?

@ludwigschub
Copy link

ludwigschub commented May 16, 2022

@NSeydoux It's using dynamic client registration (ref)

The exact error is:

{
  "error": "invalid_client",
  "error_description": "Invalid client_id"
}

I didn't use a client identifier resource because I am not sure how it will work when a user tries to use an idp other than ess or how to set it conditionally based on the version of the used idp. Maybe that isn't even needed and done internally by solid-client-authn-js?

How long does the page remain closed before attempting to log in again ?

Not sure actually, I just went back and forth a lot until it started working again, i guess it deleted the cached client information at some point while doing that

Sorry that I can't give more info on how to reproduce but I also don't really know what causes it, I just heard from people using the app that after 1-2 days of consecutive logging in it started raising the error

@ludwigschub
Copy link

ludwigschub commented Jul 28, 2022

It seems to have been resolved itself after adding the client identifier resource

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants