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

OpenID Connect response_mode handling - flawed discovery, missing validation #5461

Open
1 task done
tumbl3w33d opened this issue Mar 4, 2025 · 4 comments
Open
1 task done
Labels

Comments

@tumbl3w33d
Copy link

Steps To Reproduce

  • You use an IdP that does not support (and actively rejects) response_mode=form_post
  • You use bitwarden server's SSO functionality (OIDC) to login

Expected Result

Bitwarden should have noticed from the discovery information that the IdP does not support form_post and consequently have set the respective query parameter value to query. Thus, the login would work.

Actual Result

My strict IdP rejects users being redirected to it from bitwarden due to the unsupported parameter value.

Screenshots or Videos

No response

Additional Context

I would like to report two issues with the Bitwarden SSO implementation that recently surfaced on our end when we upgraded our IdP.

Flawed discovery of response_modes_supported

  • bitwarden does not consider the list of supported response modes advertised by the IdP
  • the field is optional, however, the choice bitwarden makes (form_post) does not match the specified default if absent (query and fragment)
  • it is also not possible to override this in the bitwarden configuration

Background:

response_modes_supported
OPTIONAL. JSON array containing a list of the OAuth 2.0 response_mode values that this OP supports, as specified in OAuth 2.0 Multiple Response Type Encoding Practices [OAuth.Responses]. If omitted, the default for Dynamic OpenID Providers is ["query", "fragment"].

Source: https://openid.net/specs/openid-connect-discovery-1_0.html

Missing validation of form_post

  • if you choose to go down that form_post road, you should be validating how the response comes in
  • currently you can swap the response_mode query parameter value to query (either manually as user in the browser or on-the-fly in a proxy) and bitwarden will continue processing although it requested a form_post

Context

The first issue currently keeps us from upgrading our IdP because we either need to wait for them to relax validation on their end or we put a proxy-based hack in place.

In my view, you could just switch to query as a quick solution as this is what you have been accepting anyway so the situation does not get worse and it seems to be considered secure enough for the standard to choose it as default. You could allow administrators to change that to form_post in case their IdPs support that or you react to the discovery values.

I could provide more information, but I tried to reduce it to what someone implementing the OIDC authentication on your end would need to know to fix it. If you have OIDC-related questions I can recommend asking in the IdP-issue I linked as there is quite some domain expertise available over there.

Build Version

bitwarden/nginx:2025.2.0, bitwarden/admin:2025.2.0, bitwarden/api:2025.2.0, bitwarden/icons:2025.2.0, bitwarden/web:2025.2.1, bitwarden/mssql:2025.2.0, bitwarden/attachments:2025.2.0, bitwarden/sso:2025.2.0, bitwarden/notifications:2025.2.0, bitwarden/events:2025.2.0, bitwarden/identity:2025.2.0

Environment

Self-Hosted

Environment Details

  • OS: Arch Linux
  • Environment: Docker

Issue Tracking Info

  • I understand that work is tracked outside of Github. A PR will be linked to this issue should one be opened to address it, but Bitwarden doesn't use fields like "assigned", "milestone", or "project" to track progress.
@tumbl3w33d tumbl3w33d added the bug label Mar 4, 2025
@NovaSilentium
Copy link

Hi there,

Your issue appears to be describing the intended behavior of the software as per #954

If you want this to be changed, it would be a feature request.

We use GitHub issues as a place to track bugs and other development related issues. The Bitwarden Community Forums has a Feature Requests section for submitting, voting for, and discussing requests like this one: https://community.bitwarden.com/c/feature-requests/

Please sign up on our forums (https://community.bitwarden.com/signup) and search to see if this request already exists. If so, you can vote for it and contribute to any discussions about it. If not, you can re-create the request there so that it can be properly tracked.

This issue will now be closed.

Thanks!

@tumbl3w33d
Copy link
Author

tumbl3w33d commented Mar 5, 2025

No, the configuration you are referring to is unrelated. This is a bug. Maybe @cscharf could confirm and reopen, please?

@trmartin4
Copy link
Member

Hi @tumbl3w33d. Thank you for reaching out on this ticket. After reviewing internally, we do see that we could make some additional improvements in correctly obeying the standard here. We will need to consider potential compatibility with other customers who may have been leveraging this non-standard behavior for their implementation. I am re-opening this, and we will continue that investigation internally while evaluating remediation options.

@trmartin4 trmartin4 reopened this Mar 5, 2025
@tumbl3w33d
Copy link
Author

Hi Todd, thank you so much for reopening and letting me know you are thinking about a solution.

I guess you can always show warnings about such wrong behavior during a transition phase and inform the operator that this is gonna stop working in the future or at least will disappear behind some kind of "lax validation" config switch for backwards compatibility.

Looking forward to whatever you will come up with!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants