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

CSA-6 Fix/remove artifact binding #1885

Merged
merged 2 commits into from
Feb 28, 2022
Merged

Conversation

cscharf
Copy link
Contributor

@cscharf cscharf commented Feb 25, 2022

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Remove Artifact binding capabilities from Bitwarden's SSO service for now; they may be added back in the future.
see: https://app.asana.com/0/1169444489336079/1201340159635915/f
see: CSA-6

Code changes

  • Saml2BindingType.cs: Remove the Artifact binding type from the enum
  • DynamicAuthenticationSchemeProvider.cs: Force clear artifact resolution URLs and remove validation/creation of Artifact bindings
  • SsoAuthenticationMiddleware.cs: Add validation to fail on request when it includes the SAMLart parameter. Dont' let the name fool you, this is not to stifle login creativity or self-expression through art, SAMLart is the artifact binding parameter itself and should not be allowed
  • OrganizationSsoRequestModel.cs: Remove validation around artifact binding URL and ensure they're always null, ignore set operations against the model. This is to provide backwards compatibility against JSON requests and stored JSON to prevent serialization/deserialization errors and keep compatibility with older clients.

Testing requirements

  • Ensure SAML SSO (POST and Redirect bindings) still works. A simple login test with both should suffice.
  • Ensure you can still configure SAML SSO via the web vault and the configuration saves and loads as expected
  • Ensure you can still configure OpenID Connect SSO via the web vault and the configuration saves and loads as expected

Documentation updates

cc: @fschillingeriv
Because this removes the underlying functionality for the Artifact binding and associated Artifact Endpoint URL configuration in the UI when configuring SAML, this update will need to be made to the documentation when this goes live (likely in March release). A similar PR will remove this from the UI itself.

Before you submit

  • I have checked for formatting errors (dotnet tool run dotnet-format --check) (required)
  • If making database changes - I have also updated Entity Framework queries and/or migrations
  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

@cscharf cscharf merged commit a7a39fb into master Feb 28, 2022
@cscharf cscharf deleted the fix/remove-artifact-binding branch February 28, 2022 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants