Skip to content

[BE] Add optional SAML auth flow to MergeRequest Approvals

In order to handle SAML Auth for merge request (MR) approvals we need to adapt how the services and controllers currently control flow.

Implementation plan

  1. Change service that approves MR with SAML in mind (EE::MergeRequests::AprovalService)
  2. override eligible_for_approval?(merge_request) 1. Checks for (short–try 5 seconds initially) timeout
  3. Returns false on timeout
  4. override execute 1. check for if saml approval is active 1. if user is not saml_authorized_for_merge_request_approval? return ServiceResponse with message: 'saml_auth_for_approval_timed_out' 1. elseif user is saml_authorized_for_merge_request_approval? approve MR with super
  5. Create dedicated route(&& controller) SamlMergeRequestApprovalController ? or just change existing flow to handl MergeRequestSAMLApprovalAuthError and then
  6. Store current path in session_for(:redirect) and redirect user to omni auth (Add current path as callback to SAML IdP?) 1. Attempt another approval on MR when auth succeeds 1. Redirect user back to MR

In Pseudo code

module EE
  module MergeRequests
    module  ApprovalService
      APPROVE_TIMEOUT = 5.seconds

      override: execute
      def execute
        return ServiceResponse.error(message: 'saml_auth_for_approval_timed_out') unless saml_authorized_for_merge_request_approval?
        return if incorrect_approval_password?(merge_request)

        super
      end
      
      private 

      def needs_saml_auth?
        merge_request.group.merge_request_requires_saml_auth_for_approval?
      end

      def needs_password?
        merge_request.project.require_password_to_approve?
      end

      def eligible_for_approval?(merge_request:)
        super && saml_authorized_for_merge_request_approval?
      end

      def saml_authorized_for_merge_request_approval?()
        return true unless needs_saml_auth?

        group = current_user.group
        saml_provider = group.root_saml_provider
        Gitlab::Auth::GroupSaml::SsoState.new(saml_provider.id).active_since?(APPROVE_TIMEOUT.ago)`
      end
    end
  end
end
Edited by SAM FIGUEROA