Skip to content

Fix group SAML error when FF update_oauth_registration_flow is enabled

Hinam Mehra requested to merge 374579-fix-saml-oauth-error into master

What does this MR do and why?

  1. The error was being caused because the new_user created by build_new_user wasn't being persisted at that stage, so auto-accepting terms for the user at that point in the code was resulting in a 500.
  2. So, I have reverted build_new_user method to its original state and I have moved the persist_accepted_terms_if_required to the OmniAuthCallbacksController. It'll also check if the user is persisted before trying to auto-accept the terms.

Screenshots or screen recordings

SAML sign-up with SCIM enabled

SAML sign-up without SCIM

How to set up and validate locally

  1. Enable feature flag update_oauth_registration_flow
  2. Enforce terms
$ bundle exec rails c

> settings = Gitlab::CurrentSettings.current_application_settings
> ApplicationSettings::UpdateService.new(
  settings, nil, terms: 'These are the terms', enforce_terms: true
).execute
  1. Configure a SAML provider such as Okta. Explainer Youtube Video
  2. Sign-in/sign-up from Okta into GitLab

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #374579 (closed)

Edited by Hinam Mehra

Merge request reports