Skip to content

SSO enforcement for web activity should only apply to current_user

Bogdan Denkovych requested to merge bdenkovych-issue-467267 into master

What does this MR do and why?

Gitlab::Auth::GroupSaml::SsoEnforcer class implements SSO enforcement for web activity. This class is being used on policy layer for groups[1] and projects[1] to restrict access to group resources via web requests for users without SAML SSO session when SSO enforcement for web activity is enabled for the group.

To not affect user access during other (Git or API)activities, Gitlab::Auth::GroupSaml::SsoEnforcer checks whether it is called in the context of the web activity[1, 2].

In #215155 (closed) we updated SSO enforcement for web activity to

Project/Group visibility Enforce SSO setting Member with identity Member without identity Non-member or not signed in
Private Off Not enforced Enforced Not enforced Not enforced
Private On Enforced Enforced Enforced
Public Off Not enforced Enforced Not enforced Not enforced
Public On Enforced Enforced Not enforced

to enforce group SAML SSO for web activity to users with the group SAML identity even if Enforce SSO setting is Off. That change was expected and requested by organizations because it allows enforce SAML SSO for their employees and external contributors(that don't have the organization SAML identity) collaborate with the organization. However, this change enabled use cases that have issues because of SSO enforcement, see Non-SAML users cannot tag SAML users for assign... (#467267).

We use declarative policies/users abilities to filter out not allowed for issue assignment users from selected users by the current_user[1, [2, 3, 4]]. As another known example, we use declarative policies/users abilities to show issue participants[1, 2]. There could be other similar use cases of declarative policies/users abilities.

The problem is SSO enforcement for web activity affects abilities for all users in context of web activity of current_user - it should not - SSO enforcement should only apply to the current_user in that case.

After debugging I found that Gitlab::Auth::GroupSaml::SsoEnforcer checks whether it is called in the context of the web activity, but it does not check whether the web activity belongs to user passed to that class. That is the root cause of the reported issue... As per reported issue, Non-SAML user(without SAML SSO session) can access group resources and assign users with the group SAML identity. In context of this assignment request, abilities for users with the SAML identity are evaluated in context of web activity/session of Non-SAML user. Session/Web activity of Non-SAML user does not include SAML SSO session and users with SAML identity are filtered out from assignees and from participants list as per the existing implementation(because it is in a web activity context and SSO enforcement for web activity always applies to users with SAML identity).

SSO enforcement for web activity/Gitlab::Auth::GroupSaml::SsoEnforcer should only apply to the current_user(Non-SAML user in that case).

That issue #467267 is not reproducible for the case when user with SAML identity assigns other users or views participants because they should have SAML session to do so, hence abilities for each users are evaluated in context of session/web activity that has SAML session, that is why other users abilities are not affected in that case.

SSO enforcement for web activity should only apply to current_user. This MR implements that.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

See "Steps to reproduce" in #467267.

Edited by Bogdan Denkovych

Merge request reports