Timshel / vaultwarden

Fork from dani-garcia/vaultwarden to add OpendID support.
GNU Affero General Public License v3.0
78 stars 12 forks source link

iOS app doesn't work with oidc AzureAD #15

Closed sandervandegeijn closed 7 months ago

sandervandegeijn commented 8 months ago

With the latest build of this evening: signing in with the iOS app fails with the following error:

image image

Tried on both my ipad and iphone. Can somebody confirm this?

sandervandegeijn commented 8 months ago

Tested the Android app as well: that one is working.

Timshel commented 8 months ago

Hey, Sorry I didn't answer before but I'm afraid I can't do much since I have no IOS devices. I had a quick look at the application code but the error is not specific.

Someone else mention it's working on ios https://github.com/Timshel/vaultwarden/issues/12#issuecomment-1883656746. Are you running the latest version of the app ?

Timshel commented 8 months ago

@Halyul Hey could you confirm the IOS version you are running and is working with the oidc server ? :)

sandervandegeijn commented 8 months ago

Is there a way to extend the logging so I can provide you with more details? Using the latest version yeah :)

Timshel commented 8 months ago

Could try LOG_LEVEL=debug on the server.

Halyul commented 8 months ago

@Halyul Hey could you confirm the IOS version you are running and is working with the oidc server ? :)

Tested with the envs mentioned in my submitted issue, I can login without problems on iOS devices with oidcwarden/vaultwarden-oidc:latest-alpine (entries are shown in the iOS app). However, I'm not using AzureAD.

sandervandegeijn commented 8 months ago

I will ask a couple of colleagues tomorrow to check as well to rule out n=1 :)

sandervandegeijn commented 8 months ago

Okay, I've asked a colleague. iOS 17.3 and latest BW app: same problem. Maybe it's the combination of AzureAD and Vaultwarden oidc? Would like to see some more testcases beside ours, but I do think we have a bug here :)

Timshel commented 8 months ago

I expect we do.

Just had a though are you using the offline_access scope to return a refresh token ?

Because if not then server might not return a refresh_token and it already caused issues in the web client cf : https://github.com/Timshel/vaultwarden/issues/16

sandervandegeijn commented 8 months ago

Vaultwarden was complaining about the refresh token until we added that scope. That's active now and the flow is working for everything except iOS.

Timshel commented 8 months ago

Is it working if you run the server with SSO_ONLY=false and log using email + master password ?

Edit: I expect it should due to the error message but just to be sure :).

sandervandegeijn commented 8 months ago

okay, I've made a screengrab of the login process with the error. But it contains infrastructure details I'd rather not share publicly on github. Would it be possible to share this privately under TLP-RED?

Is it working if you run the server with SSO_ONLY=false and log using email + master password ?

Edit: I expect it should due to the error message but just to be sure :).

Will try

edit: that works with the email address previously used for SSO and the master password I set.

Timshel commented 8 months ago

Would it be possible to share this privately under TLP-RED?

How can I contact you ?

sandervandegeijn commented 8 months ago

yes, sander.vandegeijn@wur.nl please :)

sandervandegeijn commented 8 months ago

I haven't received an email yet, is that correct? No rush, just wondering if I missed something!

Timshel commented 8 months ago

Sent one again.

sandervandegeijn commented 8 months ago

Got it thanks!

Timshel commented 8 months ago

Hey, Had a quick look and no inspiration :(. Working on something atm will try to check it again later.

Realized I never suggested to run the server with LOG_LEVEL=debug. Should give more insight on the retrieved tokens.

sandervandegeijn commented 8 months ago

Tried [1.30.1-9] with the debug logging:

image

Thanks for all the help btw!

Halyul commented 8 months ago

Tried [1.30.1-9] with the debug logging:

image

Thanks for all the help btw!

After upgrading to the latest version of oidcwarden/vaultwarden-oidc:latest-alpine, I have encountered the same error on both android and ios devices, which prevents logging in using sso. The same logging entries are observed.

Only logging in with master password is possible.

Timshel commented 8 months ago

After upgrading to the latest version of oidcwarden/vaultwarden-oidc:latest-alpine, I have encountered the same error on both android and ios devices, which prevents logging in using sso. The same logging entries are observed.

I can reproduce on Android with Keycloak, I expect it's a change of url from the Bitwarden app will have a look.

Timshel commented 8 months ago

@Halyul ok so 1.30.1-10 is building (~40min) and should fix the issue.

@sandervandegeijn I don´t expect it to fix your issue but the debug log should change.

Halyul commented 8 months ago

@Halyul ok so 1.30.1-10 is building (~40min) and should fix the issue.

Can confirm it is now back to normal on both ios and android devices.

sandervandegeijn commented 8 months ago

@Timshel I have mailed you a logfile with some relevant errors. I hope this helps :)

Timshel commented 8 months ago

Hey, So the latest lines from the log you sent me:

[22:14:25.254][response][INFO] (authorize) GET /identity/connect/authorize?<data..> => 307 Temporary Redirect
[22:14:25.673][request][INFO] GET /identity/connect/oidc-signin?code=...
[22:14:25.673][response][INFO] (oidcsignin) GET /identity/connect/oidc-signin?<code>&<state> => 307 Temporary Redirect
[22:14:31.987][vaultwarden::api::core::two_factor][DEBUG] Sending notifications for incomplete 2FA logins

So :

What I note is that there is no /identity/connect/token followup (It was present in the first screenshot).

So something happened and the redirection was not successful.

Just pushed 1.30.1-11 (~40m build) which might help in two ways :

sandervandegeijn commented 8 months ago

Unfortunately no luck with the -11 build, mailed the logs. Will look into it further tomorrow.

Timshel commented 8 months ago

Hey The code in the log look strange. It's really big (562 chars ??) while not being a JWT token.

It appears it was even bigger for a time ???

I would suggest :

From what I can find of a quick search length should not be an issue but I'm unsure what else to suggest :(

Timshel commented 8 months ago

Another suggestion you should be able to read the log of an iphone using idevicesyslog from https://docs.libimobiledevice.org/libimobiledevice/latest/.

Hum you might get nothing: https://github.com/libimobiledevice/libimobiledevice/issues/861

sandervandegeijn commented 8 months ago

will check on it Friday probably, we have a production deployment today and tomorrow. :)

sandervandegeijn commented 8 months ago

I have mailed @Timshel - we weren't able to find more details from the logs on the device itself unfortunately. But I did confirm it's still a problem on [1.30.2-3]

derSoerrn95 commented 7 months ago

Hey, I've found the problem. The iOS app uses another path: The iOS app is doing a GET /identity/sso/prevalidate?domainHint=vaultwarden request instead of doing GET /identity/account/prevalidate?domainHint=vaultwarden request.

So a quick fix could be adding the following to the nginx config (when using a nginx reverse proxy).

    location ~ ^/identity/sso/prevalidate(.*)$ {
        rewrite ^/identity/sso/prevalidate(.*)$ /identity/account/prevalidate$1 permanent;
    }

That is going to rewrite the sso part of the iOS app request into account.

A sustainable fix in Vaultwarden should be adding a http handler for the sso path.

sandervandegeijn commented 7 months ago

Fantastic!! Thanks! @Timshel would it be possible to add this route to the application? Will validate when I'm home again on Friday.

Timshel commented 7 months ago

The Android application is doing the same call last I checked so both endpoints should be present :

@derSoerrn95 Do you have an error when calling the endpoint ?

sandervandegeijn commented 7 months ago

Just tested oidcwarden/vaultwarden-oidc:1.30.2-7 still has the issue.

derSoerrn95 commented 7 months ago

@derSoerrn95 Do you have an error when calling the endpoint ?

@Timshel which endpoint do you mean? The sso one? That returns a 404. But yes you’re right. You have to implement both. The website/browser extension are is using the account endpoint.

Timshel commented 7 months ago

@derSoerrn95 can you send some log and check which version of the server you are running ?

Because since the end of January both endpoint are implemented :

derSoerrn95 commented 7 months ago

Oh, I see. It could be that I'm using an older version 😅 But since I'm using my nginx rewrite rule it works fine. I'm using the simonc/vaultwarden:latest docker image.

Timshel commented 7 months ago

@derSoerrn95 the simonc images has not been updated in 2 months. I'm now pushing the images to dockerhub, so I would recommend switching to it but test it first since there has been lots of changes ^^.

derSoerrn95 commented 7 months ago

yeah, I was too lazy to build it myself :D

derSoerrn95 commented 7 months ago

Ok, too bad. When using your latest image I'm getting a session timeout error.

vaultwarden-server-1  | [2024-02-23 19:12:29.306][request][INFO] GET /icons/sso_host/icon.png
vaultwarden-server-1  | [2024-02-23 19:12:29.314][response][INFO] (icon_internal) GET /icons/<domain>/icon.png => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:32.478][request][INFO] GET /notifications/hub?access_token=XXX
vaultwarden-server-1  | [2024-02-23 19:12:32.480][vaultwarden::api::notifications][INFO] Accepting Rocket WS connection from x.x.x.x
vaultwarden-server-1  | [2024-02-23 19:12:32.485][response][INFO] (websockets_hub) GET /notifications/hub?<data..> => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:35.524][request][INFO] GET /identity/connect/oidc-signin?code=xxx
vaultwarden-server-1  | [2024-02-23 19:12:35.528][response][INFO] (oidcsignin) GET /identity/connect/oidc-signin?<code>&<state> => 307 Temporary Redirect
vaultwarden-server-1  | [2024-02-23 19:12:36.269][request][INFO] GET /api/config
vaultwarden-server-1  | [2024-02-23 19:12:36.269][request][INFO] POST /identity/connect/token
vaultwarden-server-1  | [2024-02-23 19:12:36.269][response][INFO] (config) GET /api/config => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:37.197][vaultwarden::api::identity][INFO] User xx@example.com logged in successfully. IP: x.x.x.x
vaultwarden-server-1  | [2024-02-23 19:12:37.197][response][INFO] (login) POST /identity/connect/token => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:37.270][request][INFO] POST /identity/connect/token
vaultwarden-server-1  | [2024-02-23 19:12:38.215][response][INFO] (login) POST /identity/connect/token => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:38.312][request][INFO] GET /api/config
vaultwarden-server-1  | [2024-02-23 19:12:38.312][response][INFO] (config) GET /api/config => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:40.668][request][INFO] POST /identity/connect/token
vaultwarden-server-1  | [2024-02-23 19:12:41.625][response][INFO] (login) POST /identity/connect/token => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:41.687][request][INFO] POST /api/accounts/verify-password
vaultwarden-server-1  | [2024-02-23 19:12:41.797][response][INFO] (verify_password) POST /api/accounts/verify-password => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:41.881][request][INFO] POST /identity/connect/token
vaultwarden-server-1  | [2024-02-23 19:12:41.882][request][INFO] POST /identity/connect/token
vaultwarden-server-1  | [2024-02-23 19:12:41.884][vaultwarden::auth][ERROR] Invalid refresh token
vaultwarden-server-1  | [2024-02-23 19:12:41.884][vaultwarden::api::identity][ERROR] {"ErrorModel":{"Message":"Invalid refresh token","Object":"error"},"ExceptionMessage":null,"ExceptionStackTrace":null,"InnerExceptionMessage":null,"Message":"Invalid refresh token","Object":"error","ValidationErrors":{"":["Invalid refresh token"]},"error":"","error_description":""}
vaultwarden-server-1  | [2024-02-23 19:12:41.884][response][INFO] (login) POST /identity/connect/token => 401 Unauthorized
vaultwarden-server-1  | [2024-02-23 19:12:41.951][request][INFO] GET /api/config
vaultwarden-server-1  | [2024-02-23 19:12:41.951][response][INFO] (config) GET /api/config => 200 OK
vaultwarden-server-1  | [2024-02-23 19:12:42.824][response][INFO] (login) POST /identity/connect/token => 200 OK

As you can see in the log I'm able to authenticate with my sso provider (authentik). Then I have to pass my master password but after that I'm getting a 401 and the error for an invalid refresh token.

Timshel commented 7 months ago

@derSoerrn95 can you open a separate issue :)

sandervandegeijn commented 7 months ago

Switched to testing docker image: no luck :)

Timshel commented 7 months ago

@sandervandegeijn Hey, made no change recently that would fix the IOS issue :(

I'm not even sure which logging I could add to have more insight.

sandervandegeijn commented 7 months ago

Ah I thought you recreated the rewrite, sorry. If the rewrite is the solution it's a bit more difficult to implement for me (need to implement another reverse proxy on k8s.

Can I help with diagnosing this in any way?

Timshel commented 7 months ago

The support for both has existed for some time.

In the log you sent me I believe it was always ok (it's called before redirecting to the SSO), the issue arises later on.

Edit: Had a look again of the IOS log you sent me on Feb 2 :

[2024-02-02 20:21:36.684][response][INFO] (_prevalidate) GET /identity/account/prevalidate => 200 OK
sandervandegeijn commented 7 months ago

Thanks. It would help a lot if we could debug the ios side, but I don't have any experience with that...

https://contributing.bitwarden.com/getting-started/clients/mobile/

hmm that's not very helpful.

Relevant code: https://github.com/bitwarden/mobile/blob/cf8d801c55a4621408da36f385652632e0a60390/src/Core/Pages/Accounts/LoginSsoPageViewModel.cs#L126

Timshel commented 7 months ago

I'm not sure why you are looking at the prevalidate call. From what I remember of your issue you are correctly redirected to the SSO (which happen after the prevalidate); and the failure happen after login on the SSO and during the redirection to the client.

Did this change ?

sandervandegeijn commented 7 months ago

No and I'm not looking at the call specifically, that was @derSoerrn95 . But I have identified which code breaks in the mobile app. I don't understand why because both the iOS and Android use the same code base it seems. Really weird.

spatical commented 7 months ago

Did a bit of testing with trying with my iPhone to log in to my vaultwarden using your latest docker image and SSO turned on and configured with Google. SSO works great on web, but then I get the same error on iOS like is being discussed here.

I used Charles as a proxy from my phone to log all of the URL requests/responses that occur during the SSO process. Then I compared with the same when authenticating directly with an organization set up with SSO directly on bitwarden.com.

The last step that does the redirect to bitwarden://sso-callback is the one step that does seem to be different.

Comparison:

From vaultwarden: bitwarden://sso-callback?code=eyJ0eXAiOiJKV1QiLCJhbGciOiJ___gZHpugK_q7zdjBo_ByimnEE7JMzpK9_IGjVSVUSY4OnYDxmBsV_YYmm_ClD3vRhva0Y0x8MpXmvEloa_z2cfrIkg8JC2b2AMILjZGSibmr22--1L2wUKc3JxZxpYNABRE9__r971axsgKRUjKGC0DMLw&state=Dntu4dyatAQLtoHFMohN7xrElg4EKgffmIvk14aFukbVRcobQ0CifNJ7lC944UPz

From bitwarden: bitwarden://sso-callback/?code=1A5616F52234A656384___157FFA847__BF62988EE24-1&scope=api%20offline_access&state=7WZqgetsR7Kn8liu6UQg3Reid13vREV6LAjuBVuRTtHx1TcOwnGjeedY7Dhku2G8&iss=https%3A%2F%2Fidentity.bitwarden.

I've shortened both of those since I think something in that code is probably sensitive. But the big difference that I can see is that our vaultwarden passes a JWT as the "code" but bitwarden is just passing the code itself. In addition, bitwarden also has additional parameters of "scope" and "iss".

I'll see if I can provide some more details tomorrow, but in case this spurs any ideas perhaps this helps.

Timshel commented 7 months ago

First, thanks for the investigation :)

This issue predates the switch to wrap the code in a JWT token, and I had wondered earlier if when too long it could cause issue but from what I could find limits are way higher. So hopefully nothing there.

Will add scope and an iss parameter with the DOMAIN config. It does not appear to be used by the Bitwarden code, but maybe it's used in the Xamarin code specific to IOS.