Skip to content

"Verify SAML Configuration" lets users see Response XML, NameID and validations

What

Improves "Test SAML SSO" button by actively verifying the configuration and displaying useful information from the SAML Response.

  • Bypasses sign-in and account linking and returns the user direct to a results section of the settings page.
  • Stores the full SAMLResponse in Redis and then shows this in the UI with indentation, highlighting and a "Copy" button for sharing it with support.
  • Highlights important sections of the response via disabled inputs above the XML area. Currently these are
    • Displays NameID Format and shows an inline validation error if it isn't the recommended value.
    • Displays the NameID and includes an error if it is blank or if it does not match the value linked to the current user.
  • Displays all ruby-saml validation errors at the top of the new section. Before only the first error could be captured as this raised an exception.

Closes #33714 (closed)

Why

We could do more to help customers during the initial setup, both to familiarize them with the relevant parts of the SAML response as well as to help them avoid NameID misconfiguration that could cause problems later on.

The main aim is to automatically identify sub-optimal configurations, but this MR also helps with the following:

Screenshots

Valid Invalid
Valid SAML Response Verify SAML Configuration with error messages

Demo

Demo Video

Reviewing

To test this live you'll want to configure the group_saml omniauth provider and set up an identity provider for testing. Instructions for this are at https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/saml.md.

My gitlab-compose-kit setup can be found in gitlab-compose-kit!46 (diffs), which I've been using to experiment with incorrect values for NameID and NameID format.

Risks

  • We'll want to verify that this doesn't have a negative impact on Redis memory usage or bloat the session. The large 10kb SAMLResponse could hold the single threaded redis connection open for too long, so we'll need to ensure we only store and retrieve this when a group admin is testing their configuration. To reduce memory bloat I've included a DEL immediately after retrieving the value, as well as a 5 minute timeout in case the process is interrupted.
  • We'll need to make sure that changes here can't be abused by an attacker. In particular we'll want to review the change of behaviour in the GroupSAML strategy.

Todo

~~Lots, in particular the #TODO: comments scattered through the prototype 😉 ~~

  • Add QA test verifying that SAML response XML is shown
  • Add QA test verifying that fingerprint missmatch results in an error with that description
  • Add QA test for NameID missing/missmatched
  • Move SAMLResponse store call from controller to strategy
  • Strategy bypasses sign-in/link attempts and redirects to RelayState
  • Clicking test loads the page at a sensible anchor to scroll down
  • Validate 'generic' ruby-saml errors and display them
  • Validation tests for NameID blank, not matching
  • Validation tests for NameIdFormat
  • Implement validation of NameID contains @ or low entropy
  • Implement validation allowing emailAddress NameID format if NameID contains an @, conditional on already adding an error to :name_id for that case
  • Optionally re-colour some validation errors to warnings
  • ResponseCheck tests: simple methods, .load method, redis methods.
  • Consider extracting redis actions to a subclass .new(session_id).
  • Consider using feature flag for this.
  • Consider spiting out NameID related validations if they will need significant work

Follow up issues to consider creating after merge

  • Validate assertions for first-name/last-name/email when Group Managed Accounts is enabled
  • Consider using SAML response to generate fingerprint for easier configuration, ensuring this doesn't pose a security risk by making it too easy to accept a wrong certificate
  • Docs: Update troubleshooting docs to mention using this to view XML instead of a browser tool. Update setup docs to recommend using this after saving an initial configuration, followed by enabling and re-saving.
  • Identify common ruby-saml errors like fingerprint mismatch, audience restriction, not-on-or-after and display those sections of XML with instructions on how to fix.

Acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by James Edwards-Jones

Merge request reports

Loading