Timshel / vaultwarden

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

SSO_authority and Nextcloud as OIDC provider fails on redirect, not allowing path suffix #58

Open 5-tamra opened 1 month ago

5-tamra commented 1 month ago

Hey thank you for implementing this!

I am trying to use vaultwarden together with nextcloud as OIDC provider.

The Nextcloud OIDC package has its discovery endpoint at "/index.php/.well-known/openid-configuration" which seems not compatible with the current way of configuring SSO for vaultwarden.

When i set SSO_AUTHORITY=https://nextcloud.example.org it will be redirected and the authentication fails with:

"Failed to discover OpenID provider: Server returned invalid response: 
HTTP status code 301 Moved Permanently at 
https://nextcloud.example.org/.well-known/openid-configuration"

and when i set it to SSO_AUTHORITY=https://nextcloud.example.org/index.php it fails with

"Failed to discover OpenID provider: 
Validation error: unexpected issuer URI 'https://nextcloud.example.org/' 
(expected 'https://nextcloud.example.org/index.php')"

This is probably fixable by configuring a forwarding on the nextcloud server, but this is not possible, when the nextcloud is externally hostet.

Is there a possibility to get this working, by vaultwarden alone?

Timshel commented 1 month ago

Hey,

This is mainly handled by the underlying openidconnect lib which append the .well-known/openid-configuration part in accordance with the specification : https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationRequest.

It's possible there is some way in the openidconnect lib to override the issuer validation, but I don't plan to explore/add it.

5-tamra commented 1 month ago

Thanks for your reply. I don't know, if it is even needed to override the issuer validation. It seems, that it is just failing because of the redirect. The 301 HTTP response is handled as an error, while it could be accepted as well. At least the maintainer of the nextcloud-oidc implementation advises so.

But again, this is probably something which belongs to the openidconnect crate itself.

Timshel commented 1 month ago

A yes the issuer part was for the mismatch when settings SSO_AUTHORITY=https://nextcloud.example.org/index.php .

For the 301 redirection it's not in spec either (A successful response MUST use the 200 OK HTTP status code and return a JSON object ...) but it might be easier to still allow the redirection will check it.

Timshel commented 1 month ago

Hey

So tested it rapidly it's possible to inject a reqwest client which allow redirection but due to some conflict in dependencies version it means reimporting once more reqwest and since it's a bit verbose while being out of spec I don't think I will integrate it for now.

The next version of openidconnect-rs should resolve the version conflict so will decide then if I integrate it.

In the meantime if you want I pushed the modification in a separate branch so you can compile it yourself if you want to test it.

Timshel commented 1 day ago

Any feedback ?