-
Notifications
You must be signed in to change notification settings - Fork 24
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
ldap user reports 'code verifier invalid' in desktop client (sometimes) #309
Comments
That's fishy, maybe also a problem in the client ... cc @TheOneRing |
The error happens only when actually switching user duing the oauth flow. All fresh logins or authorizations of already logged in users work fine. |
Not a 100% regression in oauth2-0.5.1-rc1:
But not reproducable with 0.4.4 -- the client always authenticates correctly. Probably unrelated to ldap. |
But no ldap on demo.owncloud.com |
I'd like to have a look at: 1) Desktop pre-auth2) Browser request & response when clicking the "Authorise" button
(difference with/without user change?) 3) Then the first POST to the token endpoint from the desktop log… |
Code seems to fail in oauth2/lib/Controller/OAuthApiController.php Lines 168 to 171 in 4ab10b0
It seems you're trying to use the S256 challenge method without a code verifier. That's why the method fails. ( oauth2/lib/Db/AuthorizationCode.php Line 77 in 4ab10b0
It's likely a bug with the user switching. If the rest of the flow is working fine, my guess is that we're either checking the code of a different user or we're switching the code challenge method somewhere when we switch the user. I haven't checked the code in depth, so I might be wrong. |
Without user change1)2)
Response headers:
3)20211014_1644_owncloud.log.0.zip With user change1)2)
Response Headers
3) |
Connecting to the same server using android app 2.18.2Same steps as above, first connect with the phone's browser as admin, then When the authorize button is pressed, An error appears onscreen "Legitimierung nicht erfolgreich" The server log has owncloud-server-android-oauth51rc1.log.zip Android logfile android.log.zip
Without switching user before authorization, the login succeeds. |
The autorization code is tied to the user. Changing the user will result in a different code and the verifier no longer matches. Smalls like works as designed ..... |
Then remove the "Switch users to continue" feature from the OAuth 2.0 app? |
sounds reasonable ..... will think about this ... |
Scenario: Client was connected with user1, -> I vote against dropping the switch user feature. |
Reproduced today again with oauth2 0.5.2 on 10.9.0-beta1 and user-ldap 0.16.0 rc2 @jvillafanez @DeepDiver1975 do you see a way to fix this? |
After playing around with this scenario, the question I have is, what should happen with the browser afterwards? I've hacked things a bit with mitmproxy in order to forward the code_challenge and the code_challenge_method to the POST request that happens when you click the "authorize" button (last step, after switching the user and just before the "code verifier invalid" error happens). It seems that the desktop client successfully login, but the browser ends up in the login page "https://test.server/login?redirect_url=%252Fapps%252Foauth2%252Fauthorization-successful" Basically, you had a valid session with a user and that session is lost, so you're forced to log in again. Things might be worse if you end up using the "user2" account (the one you're logging in with the desktop client) instead of the account you had before initiating the desktop login process. Maybe it's better to show a message to the user to force him to logout before initiating the desktop login. The flow would be something like:
This way is more clear to the user that he'll be logged out from ownCloud and that he'll eventually need to login again. After the user has logged out, the user will need to restart the flow from the desktop client (either login again or reopen the browser buttons should work) The expected changes for this solution should be just change the "switch users" button for a "logout" button and some additional message to warn the user about the logout. |
While debugging we would consistently see the That did solve the issue when clicking 'switch user' and then using the same user to log in. It seems to me that change might affect this issue as well. |
Seen with server 10.8.0 with oauth2-0.5.1-rc1 and user_ldap-0.15.4
and testpilotcloud-client 2.9.0-rc1
with "Authorize" and "Switch Users to continue" buttons. Click switch user.
The server database has
Client logfile:
20211012_1038_owncloud.log.1.zip
On a second attempt, the client was logged in, although the server logs an error:
The text was updated successfully, but these errors were encountered: