Re-trust group SAML identities marked untrusted

Smriti Gargrequested to merge
608236-re-trust-group-saml-identities into master

What does this MR do and why?

Background

GitLab 18.10 shipped a security change that stayed in place through 19.3: the API endpoint PATCH /groups/:id/saml/:uid set trusted_extern_uid to false on a group SAML identity whenever a group Owner changed that identity's extern_uid. Once an identity was marked untrusted this way, it was blocked from SAML single sign-on until the user re-linked their identity.

The decision to revert this behavior was made on work item https://gitlab.com/gitlab-org/gitlab/-/work_items/605704. In https://gitlab.com/gitlab-org/gitlab/-/work_items/605704#note_3646406187, Bogdan Denkovych concluded that reverting the endpoint behavior is not enough on its own: a database migration is also needed to set trusted_extern_uid back to true for the group SAML identities that the endpoint had already marked untrusted. His reasons were data integrity, and preventing SSO sign-in issues now and in the future for the affected identities. He also noted that this endpoint was the only application flow that ever changed trusted_extern_uid for group SAML identities. That is why it is safe for this migration to re-trust every identities row with provider = 'group_saml' that currently has trusted_extern_uid set to false, without needing to track which specific rows the endpoint had touched.

The code revert merged in !248753 (merged), which stops the endpoint from distrusting identities going forward.

This MR

This MR is the database follow-up to that decision: it fixes the rows that were already marked untrusted before the revert shipped. It contains only migrations, no application code changes.

Following a suggestion from Bogdan Denkovych in review on the revert MR (!248753 (comment 3783379592)), this is done with two post-deployment migrations:

  1. 20260904050000_add_index_identities_on_provider_and_id.rb adds the index index_identities_on_provider_and_id on identities (provider, id), using add_concurrent_index. This index lets the second migration batch efficiently, following the development guide on efficient use of each_batch. The index shape was discussed with the database maintainer (!253640 (comment 3814439613)), who preferred the full composite over a temporary partial index because it is expected to be needed by upcoming work, so it is intentionally permanent.
  2. 20260904050001_re_trust_group_saml_identities.rb iterates the identities table where provider = 'group_saml' using each_batch with a batch size of 1000, and sets trusted_extern_uid back to true for any row where it is currently false.

Rows where trusted_extern_uid is NULL are intentionally left untouched. The reverted API endpoint only ever set the flag to false, so NULL rows were never affected by that behavior and do not need correcting.

The down method is a deliberate no-op. Once trust is restored, there is no remaining record of which rows were previously untrusted, so there is nothing to reverse.

Related to #608236 (closed), which is closed by the code revert MR rather than by this MR.

References

Database

identities is table_size: small and is not a high-traffic table.

On a Database Lab clone taken for an earlier version of this migration, there were roughly 681,000 group SAML identities, of which only about 62 had trusted_extern_uid set to false. All 62 were last modified after 2026-06-08, which is when the reverted behavior shipped on GitLab.com. The fresh database testing pipeline run described below found 161 affected rows instead, because the endpoint kept marking identities untrusted until the code revert merged on 2026-09-09 (!248753 (merged)).

The batching strategy changed from the earlier pre-split version of this migration: that version selected rows by saml_provider_id IS NOT NULL and also re-trusted rows where trusted_extern_uid was NULL. This version scopes by provider = 'group_saml' and leaves NULL rows untouched, as described above.

The database testing pipeline results for this version (!253640 (comment 3813877081)) measured the index build at 39.2 s, adding +435.21 MiB to the database, and the data migration at 190.5 s, with two marginal timing warnings: worst cases of 276.8 ms for the batch boundary SELECT and 148.4 ms for the batched UPDATE, against averages of 2.2 ms and 5.5 ms, both far below the 100 ms guideline.

A migration spec covers:

  • An untrusted group SAML identity is re-trusted.
  • An already-trusted identity is left unchanged.
  • An identity with trusted_extern_uid: NULL is left unchanged.
  • An untrusted identity on a different provider is left unchanged.

An upgrade note has been added to doc/update/versions/gitlab_19_changes.md, telling administrators that GitLab 18.10 through 19.3 marked identities untrusted and blocked SAML sign-in on extern_uid changes, and that GitLab 19.4 restores trust to the affected identities through this post-deployment migration.

How to set up and validate locally

Run the migration spec:

bundle exec rspec spec/migrations/20260904050001_re_trust_group_saml_identities_spec.rb

Or validate manually:

  1. In the rails console, create an identity with provider: 'group_saml' and trusted_extern_uid: false.
  2. Run bundle exec rails db:migrate.
  3. Confirm the identity's trusted_extern_uid is now true.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist.

Edited by Smriti Garg

Merge request reports