[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
- Change service that approves MR with SAML in mind (
EE::MergeRequests::AprovalService
) - override
eligible_for_approval?(merge_request)
1. Checks for (short–try5
seconds initially) timeout - Returns false on timeout
- override
execute
1. check for if saml approval is active 1. if user is notsaml_authorized_for_merge_request_approval?
returnServiceResponse
withmessage: 'saml_auth_for_approval_timed_out'
1. elseif user issaml_authorized_for_merge_request_approval?
approve MR withsuper
- Create dedicated route(&& controller) SamlMergeRequestApprovalController ? or just change existing flow to handl
MergeRequestSAMLApprovalAuthError
and then - 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