Restrict user autocompletes to SAML enforced members
What does this MR do?
Solves #13699 (closed)
To satisfy the acceptance criteria, the frontend was changed to pass a new param saml_provider_id
when SAML/SSO is enforced by the group. When that parameter is passed the Autocomplete::UsersFinder
will modify the query to only include users that have an identity for the given SAML provider.
Local Testing
- Install an EE license: https://about.gitlab.com/handbook/developer-onboarding/#working-on-gitlab-ee.
- Enable the following feature flags:
group_saml
,enforced_sso
, andenforced_sso_requires_session
- Add
group_saml
toconfig/gitlab.yml
. See https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/doc/howto/saml.md#gitlab-configuration. Note: Ensure this is added to thedevelopment:
section of your config - Follow the instructions in https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/doc/howto/saml.md for setting up a local SAML provider using docker. You will need to enable HTTPS
- Create a group
- Navigate to "Settings -> SAML SSO"
- Toggle on "Enable SAML authentication for this group." and add the "Identity provider single sign on URL" and "Certificate fingerprint" from https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/doc/howto/saml.md#configuring-the-group
- Navigate to "GitLab single sign on URL" found in "Settings -> SAML SSO" and authorize your account.
- In "Settings -> SAML SSO" enable "Enforce SSO-only authentication for this group."
- In a private/incognito window navigate to the "GitLab single sign on URL" found in "Settings -> SAML SSO" and create a user with the
user2
credentials (https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/doc/howto/saml.md#credentials) - With your group owner account navigate to the "Members" section of the group. The dropdown should now only show user's with SAML enabled.
For database review
Before modification
query:
SELECT
"users".*
FROM
"users"
WHERE ("users"."state" IN ('active'))
AND (ghost IS NOT TRUE)
AND ("users"."user_type" IS NULL
OR "users"."user_type" NOT IN (2, 1, 3))
ORDER BY
"users"."name" ASC
LIMIT 20;
plan:
Limit (cost=0.56..3.61 rows=20 width=1205) (actual time=26.154..70.718 rows=20 loops=1)
Buffers: shared read=25 dirtied=1
I/O Timings: read=68.955
-> Index Scan using index_users_on_name on public.users (cost=0.56..818240.12 rows=5359135 width=1205) (actual time=26.152..70.702 rows=20 loops=1)
Filter: ((users.ghost IS NOT TRUE) AND ((users.state)::text = 'active'::text) AND ((users.user_type IS NULL) OR (users.user_type <> ALL ('{2,1,3}'::integer[]))))
Rows Removed by Filter: 1
Buffers: shared read=25 dirtied=1
I/O Timings: read=68.955
summary:
Time: 71.296 ms
- planning: 0.482 ms
- execution: 70.814 ms
- I/O read: 68.955 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 0 from the buffer pool
- reads: 25 (~200.00 KiB) from the OS file cache, including disk I/O
- dirtied: 1 (~8.00 KiB)
- writes: 0
After modification
query:
SELECT DISTINCT
"users".*
FROM
"users"
INNER JOIN "identities" ON "identities"."user_id" = "users"."id"
WHERE ("users"."state" IN ('active'))
AND (ghost IS NOT TRUE)
AND ("users"."user_type" IS NULL
OR "users"."user_type" NOT IN (2, 1, 3))
AND "identities"."saml_provider_id" = 3
ORDER BY
"users"."name" ASC
LIMIT 20
plan:
Limit (cost=58.99..60.90 rows=9 width=1205) (actual time=11.321..11.322 rows=1 loops=1)
Buffers: shared hit=24 read=6
I/O Timings: read=10.920
-> Unique (cost=58.99..60.90 rows=9 width=1205) (actual time=11.319..11.320 rows=1 loops=1)
Buffers: shared hit=24 read=6
I/O Timings: read=10.920
-> Sort (cost=58.99..59.01 rows=9 width=1205) (actual time=11.317..11.317 rows=1 loops=1)
Sort Key: users.name, users.id, users.email, users.encrypted_password, users.reset_password_token, users.reset_password_sent_at, users.remember_created_at, users.sign_in_count, users.current_sign_in_at, users.last_sign_in_at, users.current_sign_in_ip, users.last_sign_in_ip, users.created_at, users.updated_at, users.admin, users.projects_limit, users.skype, users.linkedin, users.twitter, users.bio, users.failed_attempts, users.locked_at, users.username, users.can_create_group, users.can_create_team, users.color_scheme_id, users.password_expires_at, users.created_by_id, users.avatar, users.confirmation_token, users.confirmed_at, users.confirmation_sent_at, users.unconfirmed_email, users.hide_no_ssh_key, users.website_url, users.last_credential_check_at, users.admin_email_unsubscribed_at, users.notification_email, users.hide_no_password, users.password_automatically_set, users.location, users.public_email, users.encrypted_otp_secret, users.encrypted_otp_secret_iv, users.encrypted_otp_secret_salt, users.otp_required_for_login, users.otp_backup_codes, users.dashboard, users.project_view, users.consumed_timestep, users.layout, users.hide_project_limit, users.unlock_token, users.note, users.otp_grace_period_started_at, users.external, users.organization, users.incoming_email_token, users.auditor, users.ghost, users.require_two_factor_authentication_from_group, users.two_factor_grace_period, users.notified_of_own_activity, users.last_activity_on, users.preferred_language, users.email_opted_in, users.email_opted_in_ip, users.email_opted_in_source_id, users.email_opted_in_at, users.theme_id, users.accepted_term_id, users.feed_token, users.private_profile, users.roadmap_layout, users.include_private_contributions, users.commit_email, users.group_view, users.managing_group_id, users.bot_type, users.first_name, users.last_name, users.static_object_token, users.role, users.user_type
Sort Method: quicksort Memory: 27kB
Buffers: shared hit=24 read=6
I/O Timings: read=10.920
-> Nested Loop (cost=0.72..58.85 rows=9 width=1205) (actual time=11.004..11.007 rows=1 loops=1)
Buffers: shared hit=1 read=6
I/O Timings: read=10.920
-> Index Scan using index_identities_on_saml_provider_id on public.identities (cost=0.29..18.65 rows=9 width=4) (actual time=4.453..4.454 rows=1 loops=1)
Index Cond: (identities.saml_provider_id = 3)
Buffers: shared read=3
I/O Timings: read=4.424
-> Index Scan using users_pkey on public.users (cost=0.43..4.46 rows=1 width=1205) (actual time=6.540..6.542 rows=1 loops=1)
Index Cond: (users.id = identities.user_id)
Filter: ((users.ghost IS NOT TRUE) AND ((users.state)::text = 'active'::text) AND ((users.user_type IS NULL) OR (users.user_type <> ALL ('{2,1,3}'::integer[]))))
Rows Removed by Filter: 0
Buffers: shared hit=1 read=3
I/O Timings: read=6.496
summary:
Time: 13.167 ms
- planning: 1.620 ms
- execution: 11.547 ms
- I/O read: 10.920 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 24 (~192.00 KiB) from the buffer pool
- reads: 6 (~48.00 KiB) from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
Screenshots
Page | Before | After |
---|---|---|
Root Group | ||
Subgroup |
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Edited by Heinrich Lee Yu