"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
- 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:
- Makes it easier to diagnose problems with the SAML response. Previously a browser plugin was recommended: https://docs.gitlab.com/ee/user/group/saml_sso/#saml-debugging-tools
- Avoids us prematurely linking the group admin's account to a mis-configured NameID during setup by bypassing the identity-linking flow.
Screenshots
Valid | Invalid |
---|---|
Demo
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 aDEL
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
-
Changelog entry -
Documentation created/updated or follow-up review issue created -
Merge request performance guidelines -
Database guides
Performance and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
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