Add guardrails for new subroup- and project-level SAs

Aleksei Lipniagovrequested to merge
540774-guardrails-for-new-subgroup-project-sas into master

Summary

Restricts service accounts provisioned by subgroups or projects from creating top-level groups and creating other service accounts, as a policy-level defense-in-depth safeguard.

This replaces the free-tier-based approach from !224247 (closed) and !224691 (closed) with a structurally simpler check: instead of inspecting subscription plans, we look at where the SA was provisioned — subgroup/project-provisioned SAs are restricted, while root-group and instance-level SAs are not.

What does this MR do?

Introduces three model-level methods on User and a shared policy concern SubgroupProvisionedServiceAccountRestriction that prevents subgroup/project-provisioned service accounts from :create_group and :create_service_account.

Definition

A service account is considered "provisioned by subgroup or project" if:

  • provisioned_by_project_id IS NOT NULL, OR
  • provisioned_by_group_id IS NOT NULL and the referenced group has a parent (i.e., is not a root namespace)

Behavior

Provisioning source Can create groups Can create SAs? Behaviour decision
No provisioning source (instance-level SA) Yes — on .com, these SAs are controlled by GitLab admins Yes Keep as-is for Free and Paid, because user and group creation is allowed on SM due to existing APIs but not on .com. We don't have instance level SA on .com
Root (top-level) group Yes Yes Keep as-is for Free and Paid, because user and group creation is allowed on SM due to existing APIs but not on .com
Subgroup No No Keep this at No for Free and Paid
Deeply nested subgroup No No Keep this at No for Free and Paid
Project No No Keep this at No for Free and Paid

This logic is identical on SaaS and self-managed — no subscription/license checks are involved.

Regular (human) users are not affected by this restriction.

Why GlobalPolicy, OrganizationPolicy, GroupPolicy, and ProjectPolicy?

Group creation can happen through two code paths. GlobalPolicy governs the standard create_group ability, while OrganizationPolicy governs group creation scoped to an organization. Both must enforce the restriction for :create_group.

Service account creation happens via GroupPolicy (for group-level SAs) and ProjectPolicy (for project-level SAs). Both must enforce the restriction for :create_service_account.

The SubgroupProvisionedServiceAccountRestriction concern prevents both abilities, but no single policy enables both. Each test asserts only the ability that's actually enabled in that policy:

Policy :create_group enabled? :create_service_account enabled? Tested ability
GlobalPolicy ✅ ❌ :create_group
OrganizationPolicy ✅ ❌ :create_group
GroupPolicy ❌ ✅ :create_service_account
ProjectPolicy ❌ ✅ :create_service_account

Why update_column(:can_create_group, true) in tests?

Service accounts are created with external: true, which forces can_create_group = false via a before_save callback. The tests use update_column to bypass this and set can_create_group = true at the database level, so that the base policy condition passes and we can verify that the new policy logic correctly prevents group creation independently.

How to test manually

Subgroup-provisioned SA

Note: ideally, we do that both in Free (and in particular in Free) group/project and in Paid.
The reason Free is preferred because that's the actual product context for which we model this restrictions.

In Free, we can't use UX to create SAs within Free groups/projects trees, so we may need rails c.

  • Create a top-level group and a subgroup within it
  • Create a SA provisioned by the subgroup:
group = Group.find(<subgroup_id>)
sa = ::Users::AuthorizedCreateService.new(
  User.find_by(admin: true),
  {
    organization_id: Organizations::Organization.default_organization.id,
    name: "SA for #{group.name}",
    username: "service_account_#{group.id}_#{SecureRandom.hex(4)}",
    email: "sa_#{SecureRandom.hex(4)}@service.account.gitlab.com",
    user_type: :service_account,
    provisioned_by_group_id: group.id,
    skip_confirmation: true,
    confirmed_at: Time.current,
    external: true
  }
).execute
sa = sa.payload[:user]
  • Run sa.update_column(:can_create_group, true) so we test the new mechanism
  • Check Ability.allowed?(sa, :create_group) → should be false on this branch, true on master

Root-group-provisioned SA

Same as above, but use the root group's ID for provisioned_by_group_id. Ability.allowed?(sa, :create_group) → should be true (both branches).

Project-provisioned SA

Same approach, but use provisioned_by_project_id: project.id instead of provisioned_by_group_id. Ability.allowed?(sa, :create_group) → should be false on this branch.

MR acceptance checklist

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

Related issues

Related to #540774 Part of &20439 (closed)

Edited by Aleksei Lipniagov

Merge request reports