Restrict CompID SAs invites for .com
What does this MR do and why?
Restrict invitations and permissions for group-level, composite-id, service accounts in some cases on gitlab.com.
For requirements, see the https://gitlab.com/gitlab-org/gitlab/-/issues/579877#note_2896940341
All major user stories that we identified so far are in the "How to test" section below.
This is expected to be released only for gitlab.com in this iteration.
The change will be behind the feature flag to allow faster rollback, if needed.
FF issue: #581714
How to set up and validate locally
Run your local instance as gitlab.com as the change only applied there (e.g. export GITLAB_SIMULATE_SAAS=1).
You need to enable ::Feature.enable(:restrict_invites_for_comp_id_service_accounts) feature flag first.
Ideally, you should also re-test all user stories with the FF disabled to ensure it acts similar to what we currently have in production (basically, no restrictions to invite SAs anywhere).
1 - Invite UX restrictions
Create or modify SA so that it has comp id enforced:
[13] pry(main)> u.composite_identity_enforced
=> true
[14] pry(main)> u.user_type
=> "service_account"Ensure that you can invite that SA into the group where it was originally provisioned (you may need to exclude it from there first), but it will not appear in the invitation search dropdown for any other groups: other top-level groups, other subgroups, except subgroups of the origin group.
Screenshots below
| inviting to origin group | inviting to other group |
|---|---|
|
|
2 - Invite BE restriction (abuse protection)
Even though we hide SAs in invitation picker where needed, we still need BE protection against form manipulation or other abuse.
The simplest way to test it is to use EE::Members::InviteUsersFinder from master (or just return super on else in it) so that we will be able to pick SAs in the UX finder for non-origin groups.
With that, try to invite SA to the group where it was not originally created.
You should see an error.
Note that we don't expect to hit that in UX, so I don't think we should customize the error - it's a security concern.
You should still be able to invite SA into subgroups of the group where it was originated (unless it is already present up the chain).
Screenshot below
3 - Invite API restrictions
To test API, I suggest you exclude target SA from everywhere.
Then, try to invite SA to the group where it was originated - you should succeed
(replace token, IDs of group and SA with yours)
curl "http://gdk.test:3443/api/v4/groups/<SA_ORIGIN_GROUP_ID>/members" \
--request "POST" \
--header "PRIVATE-TOKEN: <YOUR_LOCAL_PAT>" \
--data "user_id=<SA_USER_ID>&access_level=30"Then, try to invite SA to another top-level group your user (which PAT you use) owns: (replace token, IDs of group and SA with yours)
curl "http://gdk.test:3443/api/v4/groups/<ID_OF_ANOTHER_GROUP_NOT_SA_ORIGIN>/members" \
--request "POST" \
--header "PRIVATE-TOKEN: <YOUR_LOCAL_PAT>" \
--data "user_id=<SA_USER_ID>&access_level=30"You will get 403 in your response
{
"message": "403 Forbidden"
}4 - Group-to-group invitation | Members UX: hide Comp ID SAs that are "invited"
When we invite group A with CompID SA into group B, we shouldn't see SA in the group B member list.
Screenshots
4.1. Group Duo Duo has composite id SA. We show that SA as a member of its "origin" group:
4.2. We invited Duo Duo into another group:
4.3. We do not show SA in the group where it was invited as part of the group:
5 - Group-to-group invitation | Permission/Policy level: prevent Comp ID SAs that are "invited" from all abilities within their "non-origin" group
Don't forget that you need to run your instance as gitlab.com ("SaaS mode") and enable the FF.
It's the trickiest user story to test and I haven't found non-hacky way. Please let me know if you have any ideas how make it more black-boxy.
Testing through rails c (for dev/debug)
Gitlab::SafeRequestStore.ensure_request_store do while you experimenting in rails c for CompID to be resolved correctly.
To check CompID, you need a human user.
I picked User.find_by(username: 'alipniagov') and invited it to the origin_group (were SA was created) as Owner (you need enough level to check whatever ability you plan to check). You can work with root (User.first).
Then, I ensured that entire origin_group is group-to-group invited into inv_group (via UX).
sa = User.find(90) # Replace with your Service Account ID
scoped_user = User.find_by(username: 'root') # Replace with another user if needed
origin_group = Group.find(108) # Replace with your group ID where SA was originated
inv_group = Group.find(121) # Replace with the group ID to where you invited origin_group
Gitlab::SafeRequestStore.ensure_request_store do
# Manually set up the composite identity state
identity = Gitlab::Auth::Identity.fabricate(sa)
identity.link!(scoped_user)
# Simulate what resolve_composite_identity_user does
sa.composite_identity_enforced!
puts "Testing with service account (#{sa.username}) having a scoped_user (#{scoped_user.username}) that has composite_identity_enforced! set:"
puts ""
puts "Origin Group: #{sa.can?(:read_group, origin_group)}"
puts "Invited Group: #{sa.can?(:read_group, inv_group)}"
puts ""
puts "Expected: Origin=true, Invited=false"
endWe can see that the SA is capable within origin group.
But it can't do anything outside of it.
Black box test
I haven't tried it yet (as I don't have DAP set up locally), but Keeyan (In Slack) suggested:
the best I can think of is creating a flow that is told to get information from another project or post in another projects issues, when it shouldn't be able to. For example this flow called with "Summarize issues for project
OTHER PROJECT ID" might do the trick:
version: "v1"
environment: ambient
components:
- name: "my_agent"
type: AgentComponent
prompt_id: "my_local_prompt"
inputs: ["context:goal"]
toolset: ["read_issue", "create_issue", "create_issue_note", "run_command", "list_issues"]
prompts:
- prompt_id: "my_local_prompt"
name: "Code Analysis Assistant"
model:
params:
model_class_provider: anthropic
model: claude-sonnet-4-20250514
max_tokens: 32_768
unit_primitives: []
prompt_template:
system: "You are a GitLab Issue Retrieval Agent specialized in fetching and presenting issue information from GitLab projects. Your primary task is to retrieve and display the content of issues from GitLab projects when given a project ID. After completing your analysis, post ONE comprehensive comment with all results using the create_issue_note tool. Do not create multiple comments - consolidate all findings into a single well-formatted comment."
user: "{{goal}}"
placeholder: history
params:
timeout: 180
routers:
- from: "my_agent"
to: "end"
flow:
entry_point: "my_agent"Implementation notes
Which change needed for each "user story"
- To achieve 1 (user invite picker), we need to update
ee/app/finders/ee/members/invite_users_finder.rb - To achieve 2 and 3 (user invite protection on BE and in API), we need to update
ee/app/services/ee/members/groups/creator_service.rb - To achieve 4 (do not render comp-id SAs outside of origin group hierarchy), we need to update
ee/app/models/ee/member.rb - To achieve 5 (restrict permissions of comp-id SAs outside of origin group hierarchy), we need to patch
app/models/ability.rb
For DB 🚄 review
There are two DB-related & Performance areas worth paying extra attention:
1. Ability-related changes
First, changes in Ability#allowed? and ee/app/services/ee/members/groups/creator_service.rb.
The implementation sits in ee/app/models/ee/ability_prepend.rb.
While the change in ability sits on a very busy path, the change makes is a series of short-circuit conditional checks: Saas.feature_available?, :Feature.enabled?, user attributes (on a preloaded object), and finally subject.self_and_ancestor_ids.include?(user.provisioned_by_group_id)
Out of them, subject.self_and_ancestor_ids.include? is the heaviest of all of them but it is widely used across the codebase (where reasonable needed) and Traversal has built-in memoization.
That said, we definitely need to acknowledge the additional set of checks executed within ability check, but please note that these checks will set after the
if opts[:composite_identity_check] == false ||
!user.respond_to?(:composite_identity_enforced?) ||
!user&.composite_identity_enforced?
return result
end
... <--- new codeThat means that our new changes sit behind the existing composite identity pre-filtering and therefore be only called for composite identity users. Which means the "regular" flow will remain unaffected.
On a similar note, the additional complexity we added into ee/app/services/ee/members/groups/creator_service.rb sits behind member.user&.service_account? check (already existed), so it affects only these cases.
override :can_create_new_member?
def can_create_new_member?
if member.user&.service_account?
return false if ::Ability.composite_id_service_account_outside_origin_group?(member.user, source) # <-- our changeI think that makes these two changes relatively safe:
- they sit behind composite-id service-accounts cases
- they reuse existing, widely used interface
self_and_ancestor_ids - it is all additionally behind
.comcheck and feature flag
We should monitor any performance concerns after enabling the feature flag.
2. Members::ServiceAccounts::CompositeIdFinder
The second area is an addition of CompositeIdMembersFinder and CompositeIdUsersFinder which help to filter out IDs of composite-id service-accounts users from members/users relations where they should not be shown.
It is called in two places:
ee/app/finders/ee/members/invite_users_finder.rbee/app/models/ee/member.rb
Important note is that currently in production we don't have any group-level, composite-id-enforced service-accounts (ones we should be excluding) which makes the DB query estimation tricky.
Query 1: InviteUsersFinder context
To get a query similar to what is invoked in our app, I did in rails c:
group = Group.find(121)
current_user = User.first
finder = Members::InviteUsersFinder.new(current_user, group, search: nil)
result = finder.execute.page(1).per(20)
puts result.to_sqlI got the query below.
For Database Lab, I replaced my group-id (121) with gitlab-org namespace: (9970):
SELECT "users".*
FROM "users"
INNER JOIN "user_details" ON "user_details"."user_id" = "users"."id"
WHERE ("users"."state" IN ('active'))
AND "users"."user_type" IN (0,
6,
4,
13)
AND "users"."user_type" IN (0,
1,
2,
3,
4,
5,
7,
8,
9,
10,
11,
13,
15,
16,
17)
AND (users.user_type != 13
OR users.composite_identity_enforced = FALSE
OR user_details.provisioned_by_group_id IS NULL
OR user_details.provisioned_by_group_id IN (9970))
ORDER BY "users"."id" DESC
LIMIT 20
OFFSET 0You can see identical query in rails server output when as well.
DatabaseLab results:
Time: 91.122 ms
- planning: 89.915 ms
- execution: 1.207 ms
- I/O read: 0.000 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 338 (~2.60 MiB) from the buffer pool
- reads: 0 from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0 Plan
Limit (cost=0.88..14.85 rows=20 width=1508) (actual time=0.042..1.115 rows=20 loops=1)
Buffers: shared hit=338
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=0.88..12990493.31 rows=18592922 width=1508) (actual time=0.041..1.112 rows=20 loops=1)
Buffers: shared hit=338
I/O Timings: read=0.000 write=0.000
-> Index Scan Backward using users_pkey on public.users (cost=0.44..3501877.12 rows=18437008 width=1508) (actual time=0.019..0.898 rows=20 loops=1)
Filter: (((users.state)::text = 'active'::text) AND (users.user_type = ANY ('{0,6,4,13}'::integer[])) AND (users.user_type = ANY ('{0,1,2,3,4,5,7,8,9,10,11,13,15,16,17}'::integer[])))
Rows Removed by Filter: 172
Buffers: shared hit=255
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_user_details_on_user_id on public.user_details (cost=0.44..0.50 rows=1 width=16) (actual time=0.010..0.010 rows=1 loops=20)
Index Cond: (user_details.user_id = users.id)
Filter: ((users.user_type <> 13) OR (NOT users.composite_identity_enforced) OR (user_details.provisioned_by_group_id IS NULL) OR (user_details.provisioned_by_group_id = 9970))
Rows Removed by Filter: 0
Buffers: shared hit=83
I/O Timings: read=0.000 write=0.000
Settings: work_mem = '100MB', random_page_cost = '1.5', seq_page_cost = '4', effective_cache_size = '472585MB', jit = 'off'Link: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/45961/commands/140629
Note: Rows Removed by Filter: 0 is expected since group-level composite-id service accounts don't yet exist in production. When they do, the impact will be negligible due to the LIMIT clause and the rarity of service accounts among all users.
Query 2: Member relation (shared_members)
I run:
group = Group.find(121)
puts Member.shared_members(group).to_sqlI got the query below.
For Database Lab, I replaced my group-id (121) with gitlab-org namespace: (9970):
explain SELECT "members"."id",
LEAST("group_group_links"."group_access", "members"."access_level") AS access_level,
"members"."source_id", "members"."source_type", "members"."user_id",
"members"."notification_level", "members"."type", "members"."created_at",
"members"."updated_at", "members"."created_by_id", "members"."invite_email",
"members"."invite_token", "members"."invite_accepted_at", "members"."requested_at",
"members"."expires_at", "members"."ldap", "members"."override", "members"."state",
"members"."invite_email_success", "members"."member_namespace_id",
NULL AS member_role_id, "members"."expiry_notified_at", "members"."request_accepted_at"
FROM "members"
JOIN group_group_links ON members.source_id = group_group_links.shared_with_group_id
INNER JOIN "users" ON "users"."id" = "members"."user_id"
INNER JOIN "user_details" ON "user_details"."user_id" = "users"."id"
WHERE "group_group_links"."shared_group_id" IN
(SELECT "namespaces"."id"
FROM "namespaces"
WHERE "namespaces"."type" = 'Group'
AND "namespaces"."id" = 9970)
AND (users.user_type != 13
OR users.composite_identity_enforced = FALSE
OR user_details.provisioned_by_group_id IS NULL
OR user_details.provisioned_by_group_id IN (9970))DatabaseLab summary:
Time: 91.122 ms
- planning: 89.915 ms
- execution: 1.207 ms
- I/O read: 0.000 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 338 (~2.60 MiB) from the buffer pool
- reads: 0 from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0Plan:
Nested Loop (cost=2.43..206.95 rows=118 width=231) (actual time=12.006..28.840 rows=10 loops=1)
Buffers: shared hit=33 read=75
I/O Timings: read=28.102 write=0.000
-> Nested Loop (cost=1.99..147.15 rows=117 width=217) (actual time=11.757..26.869 rows=10 loops=1)
Buffers: shared hit=23 read=45
I/O Timings: read=26.323 write=0.000
-> Nested Loop (cost=1.55..92.27 rows=117 width=201) (actual time=11.601..24.901 rows=10 loops=1)
Buffers: shared hit=8 read=20
I/O Timings: read=24.636 write=0.000
-> Nested Loop Semi Join (cost=0.99..8.17 rows=2 width=10) (actual time=7.212..7.215 rows=1 loops=1)
Buffers: shared hit=3 read=9
I/O Timings: read=7.085 write=0.000
-> Index Scan using index_group_group_links_on_shared_group_and_shared_with_group on public.group_group_links (cost=0.42..4.55 rows=2 width=18) (actual time=3.023..3.025 rows=1 loops=1)
Index Cond: (group_group_links.shared_group_id = 9970)
Buffers: shared hit=3 read=4
I/O Timings: read=2.960 write=0.000
-> Materialize (cost=0.57..3.59 rows=1 width=4) (actual time=4.186..4.186 rows=1 loops=1)
Buffers: shared read=5
I/O Timings: read=4.125 write=0.000
-> Index Only Scan using index_namespaces_on_type_and_id on public.namespaces (cost=0.57..3.59 rows=1 width=4) (actual time=4.180..4.180 rows=1 loops=1)
Index Cond: ((namespaces.type = 'Group'::text) AND (namespaces.id = 9970))
Heap Fetches: 0
Buffers: shared read=5
I/O Timings: read=4.125 write=0.000
-> Index Scan using index_members_on_source_and_type_and_access_level on public.members (cost=0.56..41.78 rows=27 width=199) (actual time=4.385..17.674 rows=10 loops=1)
Index Cond: (members.source_id = group_group_links.shared_with_group_id)
Buffers: shared hit=5 read=11
I/O Timings: read=17.551 write=0.000
-> Index Scan using index_user_details_on_user_id on public.user_details (cost=0.44..0.47 rows=1 width=16) (actual time=0.194..0.194 rows=1 loops=10)
Index Cond: (user_details.user_id = members.user_id)
Buffers: shared hit=15 read=25
I/O Timings: read=1.687 write=0.000
-> Index Scan using users_pkey on public.users (cost=0.44..0.49 rows=1 width=7) (actual time=0.194..0.194 rows=1 loops=10)
Index Cond: (users.id = members.user_id)
Buffers: shared hit=10 read=30
I/O Timings: read=1.779 write=0.000
Settings: random_page_cost = '1.5', seq_page_cost = '4', effective_cache_size = '472585MB', jit = 'off', work_mem = '100MB'Link: https://console.postgres.ai/gitlab/gitlab-production-main/sessions/45961/commands/140630
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.