Open timbl opened 1 year ago
Hi @timbl , sorry for the delayed response! We'll look into this soon.
@NSeydoux - any progress? This is biting many many people.
I'm looking into this now, and I started a test to reproduce locally, but since it requires waiting it's going to be a while. I'd be interested in a bit more details form people experiencing this.
First, what Identity Providers is this happening against?
Then, it would be helpful to have a bit more info about the network log leading to this issue. In particular, I'd be interested in the POST request to the /register
endpoint, to have the dynamically registered client id and redirect URL, as well as the redirections to the /authorize
endpoint that causes the issue, to be able to compare the client id and redirect URL.
I am getting it against solidcommunity.net. Today I tried it with both chrome and firefox and simply trying to access my pod gives me the 400 error on both. Clearing cookies and all site data in the console has no effect. This is the url that I get redirected to when I attempt to access https://jeff-zucker.solidcommunity.net/ :
These are the request headers :
GET /authorize?client_id=a25fc2bd57cb9c3e6f668b7cfc071d0d&redirect_uri=https%3A%2F%2Fjeff-zucker.solidcommunity.net%2F%3Fstate%3D77b54cff18f345188f2aa202e06bfe11&response_type=code&scope=openid%20offline_access%20webid&state=039ee31869214bf2be40fa53921c7c09&code_challenge=XpT9tSAtELBamOSXyLmBxTBDFF1wtzDVzocc-c4trlU&code_challenge_method=S256&prompt=none&response_mode=query HTTP/1.1
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9
Cache-Control: no-cache
Connection: keep-alive
DNT: 1
Host: solidcommunity.net
Pragma: no-cache
Referer: https://jeff-zucker.solidcommunity.net/
Sec-Fetch-Dest: document
Sec-Fetch-Mode: navigate
Sec-Fetch-Site: same-site
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/116.0.0.0 Safari/537.36
sec-ch-ua: "Chromium";v="116", "Not)A;Brand";v="24", "Google Chrome";v="116"
sec-ch-ua-arch: "x86"
sec-ch-ua-bitness: "64"
sec-ch-ua-full-version: "116.0.5845.96"
sec-ch-ua-full-version-list: "Chromium";v="116.0.5845.96", "Not)A;Brand";v="24.0.0.0", "Google Chrome";v="116.0.5845.96"
sec-ch-ua-mobile: ?0
sec-ch-ua-model: ""
sec-ch-ua-platform: "Linux"
sec-ch-ua-platform-version: "5.4.0"
sec-ch-ua-wow64: ?0
Thanks for the quick response!
I am getting it against solidcommunity.net. Today I tried it with both chrome and firefox and simply trying to access my pod gives me the 400 error on both.
Am I correct to say this means you:
These are the request headers :
If you clear local storage, and log in, do you see a request to the /register
endpoint prior to the request to /authorize
? If so, the response should include both a client id, client secret and redirect URL (among other things). Do the client ID and redirect URL match those in the request to authorize
?
In particular, I think the issue comes from the state
component in the redirect URL, which should not include dynamic parts like this.
No, I do not get an opportunity to login. I simply type the URL in the browser, hit enter and get the error. As I said, clearing the local cache does nothing. I can not login, logout, or view a page. I'm not sure how I could see the request to /register, if the browser shows it, I don't know where to find it.
If you clear local storage, and log in
I am not able to login, the 400 error appears as soon as I enter a URL on my pod in the browser.
After a screensharing session yesterday (thanks to @jeff-zucker for making himself available), we determined that the issue appears to come from the way SolidOS sometimes registers the redirect URL at some point during dynamic client registration. One way to fix the issue is to:
solidClientAuthenticationUser:
This should get users facing the issue sorted out. In the meantime, I'll implement a fix in the library to prevent the bug from happening in SolidOS, but that may lead to issues popping up in SolidOS, so the team will need to be careful when doing the migration.
Thanks for finding a solution.
but that may lead to issues popping up in SolidOS, so the team will need to be careful when doing the migration.
Le mer. 23 août 2023 à 13:10, Zwifi @.***> a écrit :
After a screensharing session yesterday (thanks to @jeff-zucker https://github.com/jeff-zucker for making himself available), we determined that the issue appears to come from the way SolidOS sometimes registers the redirect URL at some point during dynamic client registration. One way to fix the issue is to:
- Temporarily disable JS on the browser (to prevent from being redirected)
- Visit your Pod with SolidOS (nothing should happen, because JS is disabled)
- Clear the local storage items prefixed with solidClientAuthenticationUser:
- Re-enable JS
This should get users facing the issue sorted out. In the meantime, I'll implement a fix in the library to prevent the bug from happening in SolidOS, but that may lead to issues popping up in SolidOS, so the team will need to be careful when doing the migration.
— Reply to this email directly, view it on GitHub https://github.com/inrupt/solid-client-authn-js/issues/2891#issuecomment-1689768604, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ5TZV32Z6ADRLVKYXWJIDXWXQJFANCNFSM6AAAAAAX3C3FFQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>
I would like to have some more understanding on how SolidOS is concerned when doing the migration.
My current working hypothesis is that in some edge cases, SolidOS sets the redirectURL to a value that includes OpenID query parameters. That does not appear to be the case in the happy path, because I could not reproduce the error on my end, and Jeff didn't appear to have the issue after he cleared his local storage. Therefore, when doing the migration, the solidOS team may have to go poke around and try to trigger the issue, because something that was previously accepted by the library will now be rejected. Note that since accepting this redirectURL resulted in a broken state for the application, forbidding it in the first place is not a breaking change, but rather a bugfix :).
Trying to go to an other URL on the same pod like https://jeff-zucker.solidcommunity.net/public/ could resolve the issue or not ?
I'm not familiar enough with SolidOS to be able to tell for sure, but I don't think so. Local storage is partitioned by origin, so loading a different path under the same origin will result in the same client information to be available to the library in storage, so I suspect that would not solve the issue. It is worth giving it a try though, as my answer is only as good as my understanding of SolidOS, meaning very shallow at best ^^.
Jeff is there a way to reproduce the error after clearing JavaScript. I'm wondering if the issue is related to SolidOS or solidcommunity.net implementation. Do we have the same issue with inrupt.net ?
Le mer. 23 août 2023, 15:20, Zwifi @.***> a écrit :
I would like to have some more understanding on how SolidOS is concerned when doing the migration.
My current working hypothesis is that in some edge cases, SolidOS sets the redirectURL to a value that includes OpenID query parameters. That does not appear to be the case in the happy path, because I could not reproduce the error on my end, and Jeff didn't appear to have the issue after he cleared his local storage. Therefore, when doing the migration, the solidOS team may have to go poke around and try to trigger the issue, because something that was previously accepted by the library will now be rejected. Note that since accepting this redirectURL resulted in a broken state for the application, forbidding it in the first place is not a breaking change, but rather a bugfix :).
Trying to go to an other URL on the same pod like https://jeff-zucker.solidcommunity.net/public/ could resolve the issue or not ?
I'm not familiar enough with SolidOS to be able to tell for sure, but I don't think so. Local storage is partitioned by origin, so loading a different path under the same origin will result in the same client information to be available to the library in storage, so I suspect that would not solve the issue. It is worth giving it a try though, as my answer is only as good as my understanding of SolidOS, meaning very shallow at best ^^.
— Reply to this email directly, view it on GitHub https://github.com/inrupt/solid-client-authn-js/issues/2891#issuecomment-1689954549, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ5TZQLRYQ6I7Y4UCSBRPTXWX7S7ANCNFSM6AAAAAAX3C3FFQ . You are receiving this because you commented.Message ID: @.***>
I have not been able to reproduce the error and have no idea what caused it to begin with. When I get the error there is no way to login on my own pod on any page. If I go to your pod and login I can then navigate to my page but when I logout and try to re-login on one of my own pages, same error as before.
One thing I did notice is that when we finally stopped javascript, cleared the localStorage, restarted javascript, went to pod home, logged in ... it redirected me to https://jeff-zucker.solidcommunity.net/#me even though I had not entered the #me part in the location bar.
This is a very interesting this. There should never be this extension
Le mer. 23 août 2023, 16:52, Jeff Zucker @.***> a écrit :
I have not been able to reproduce the error and have no idea what caused it to begin with. When I get the error there is no way to login on my own pod on any page. If I go to your pod and login I can then navigate to my page but when I logout and try to re-login on one of my own pages, same error as before.
One thing I did notice is that when we finally stopped javascript, cleared the localStorage, restarted javascript, went to pod home, logged in ... it redirected me to https://jeff-zucker.solidcommunity.net/#me even though I had not entered the #me part in the location bar.
— Reply to this email directly, view it on GitHub https://github.com/inrupt/solid-client-authn-js/issues/2891#issuecomment-1690109781, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ5TZWL5ORQ6S4ZVNVHUTDXWYKJ3ANCNFSM6AAAAAAX3C3FFQ . You are receiving this because you commented.Message ID: @.***>
@timbl is facing this problem on timbl.com which is running CSS. (he is logging in with inrupt.net IdP) We saw this problem when IdP is on NSS though. Maybe this helps too.
I can reproduce the issue by polluting local storage with an invalid redirect URL, and the issue is fixed by the changes introduced in #3103 , so when this change is merged and released you'll be able to upgrade SolidOS and that should get users out of trouble.
Sounds good. Will you put the condition into the test suite?
And will you add the UrIs which mismatched to the error message or console log or both?
There is a non-regression test that covers the additional check on the redirect URL, and the offending URL is part of the error message, along with the validation criteria: https://github.com/inrupt/solid-client-authn-js/blob/ac6988c96ebfeaf67a76d59d8c15672c55407cab/packages/browser/src/ClientAuthentication.ts#L68
As a continuation of the discussion in https://forum.solidproject.org/t/podbrowser-deprecation-announcement/6212/8:
I’ll check if this fix has been applied to PodBrowser. In the meantime, a solution that works to clear the local storage is to disable Javascript before going to https://podbrowser.inrupt.com
: it prevents the initial redirect to happen, and allows to remain on the desired origin to clear the storage.
In other words, users of the authentication library shouldn’t hopefully run into this issue if they are on the latest version.
Out of curiosity @NoelDeMartin, could you check the values in storage, and in particular the redirect URL for the registered client? In the past, this issue has been caused by the redirect URL being overwritten with a value including OpenID query parameters, due to a race condition on redirect I haven't been able to reproduce locally because of its random nature.
Ok thanks for the tip @NSeydoux, it is working now :).
I haven't cleared up the storage though, in case it's useful to debug. What value do you want to see exactly?
This is everything I have in local storage:
This is the value for issuerConfig:https://solidcommunity.net
:
And this is the value for the second solidClientAuthenticationUser
(the first one only has a sessionId
):
The redirectUrl
including ?state=...
is the culprit. There's a race condition somewhere that gets this in storage in replacement of the same redirect URL without the state
parameter. The 1.17.2 version of @inrupt/solid-client-authn-*
should prevent this from happening.
@NSeydoux For people logging into timbl.com it certainly wasn't a race condition --- anyone whoo left their browser logged in overnight would get the issue and have to 5restart with an incognito mode window. So very repeatable, you just had to wait. Looking forward to the fix comg through.
Search terms you've used
Found https://github.com/inrupt/solid-client-authn-js/issues/2738 seems similar issue, but was supposed to be fixed
Impacted package
Which packages do you think might be impacted by the bug ?
Bug description
The login process fails with a JSON error message and a 400 error code in the client network log
To Reproduce
Expected result
Login works - see solidos page
Environment
Please run
npx envinfo --system --npmPackages --binaries --npmGlobalPackages --browsers
in your project folder and paste the output here:
Additional information
Running with CSS