Filter out SAML starred projects when SAML session has expired

Peter Hegmanrequested to merge
514406-projects-graphql-query-not-respecting-first-when-not-signed-in-with-saml-4 into master

What does this MR do and why?

Related to #514406 (closed)

In &13066 (closed) we are working on migrating Your work -> Projects from HAML/vanilla JS to Vue with a GraphQL data source. This is currently behind the your_work_projects_vue feature flag.

We noticed an issue with the GraphQL API when group SAML is configured but you are not signed in with SAML. The problem is that the SAML projects are not returned but they still are included in the count which messes up the pagination.

In !179111 (merged) we fixed the issue in app/graphql/resolvers/projects_resolver.rb by adding a concern and some shared RSpec examples. app/graphql/resolvers/user_starred_projects_resolver.rb does not extend app/graphql/resolvers/projects_resolver.rb so we need to include the concern for it to get the same treatment.

References

Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

Database

All SQL and query plans use my user ID (5413811)

Before

Query plan

https://console.postgres.ai/gitlab/gitlab-production-main/sessions/35685/commands/110419

Click to expand 
Limit  (cost=30.44..30.44 rows=2 width=860) (actual time=98.259..98.265 rows=5 loops=1)
   Buffers: shared hit=49 read=49 dirtied=5
   WAL: records=5 fpi=5 bytes=35033
   I/O Timings: read=65.298 write=0.000
   ->  Sort  (cost=30.44..30.44 rows=2 width=860) (actual time=98.257..98.261 rows=5 loops=1)
         Sort Key: (lower((projects.name)::text)), projects.id DESC
         Sort Method: quicksort  Memory: 29kB
         Buffers: shared hit=49 read=49 dirtied=5
         WAL: records=5 fpi=5 bytes=35033
         I/O Timings: read=65.298 write=0.000
         ->  Nested Loop  (cost=13.90..30.43 rows=2 width=860) (actual time=86.746..98.197 rows=5 loops=1)
               Buffers: shared hit=43 read=49 dirtied=5
               WAL: records=5 fpi=5 bytes=35033
               I/O Timings: read=65.298 write=0.000
               ->  HashAggregate  (cost=13.34..13.38 rows=4 width=8) (actual time=30.819..30.832 rows=5 loops=1)
                     Group Key: projects_1.id
                     Buffers: shared hit=13 read=26 dirtied=4
                     WAL: records=4 fpi=4 bytes=27504
                     I/O Timings: read=30.461 write=0.000
                     ->  Nested Loop  (cost=0.99..13.33 rows=4 width=8) (actual time=5.550..30.794 rows=5 loops=1)
                           Buffers: shared hit=13 read=26 dirtied=4
                           WAL: records=4 fpi=4 bytes=27504
                           I/O Timings: read=30.461 write=0.000
                           ->  Index Only Scan Backward using index_users_star_projects_on_user_id_and_project_id on public.users_star_projects  (cost=0.43..3.50 rows=4 width=4) (actual time=2.800..2.811 rows=5 loops=1)
                                 Index Cond: (users_star_projects.user_id = 5413811)
                                 Heap Fetches: 0
                                 Buffers: shared hit=1 read=3
                                 I/O Timings: read=2.780 write=0.000
                           ->  Index Only Scan using idx_projects_on_repository_storage_last_repository_updated_at on public.projects projects_1  (cost=0.56..2.46 rows=1 width=4) (actual time=5.590..5.590 rows=1 loops=5)
                                 Index Cond: (projects_1.id = users_star_projects.project_id)
                                 Heap Fetches: 7
                                 Buffers: shared hit=12 read=23 dirtied=3
                                 WAL: records=3 fpi=3 bytes=23147
                                 I/O Timings: read=27.682 write=0.000
               ->  Index Scan using projects_pkey on public.projects  (cost=0.56..4.26 rows=1 width=828) (actual time=7.026..7.027 rows=1 loops=5)
                     Index Cond: (projects.id = projects_1.id)
                     Filter: ((NOT projects.pending_delete) AND (NOT projects.hidden) AND ((SubPlan 1) OR (projects.visibility_level = ANY ('{10,20}'::integer[]))))
                     Rows Removed by Filter: 0
                     Buffers: shared hit=29 read=23
                     I/O Timings: read=34.837 write=0.000
                     SubPlan 1
                       ->  Index Only Scan using project_authorizations_pkey on public.project_authorizations  (cost=0.58..3.60 rows=1 width=0) (actual time=1.649..1.649 rows=1 loops=5)
                             Index Cond: ((project_authorizations.user_id = 5413811) AND (project_authorizations.project_id = projects.id))
                             Heap Fetches: 0
                             Buffers: shared hit=14 read=9
                             I/O Timings: read=8.130 write=0.000
Settings: seq_page_cost = '4', work_mem = '100MB', effective_cache_size = '472585MB', jit = 'off', random_page_cost = '1.5'

Raw SQL

Click to expand 
SELECT
    "projects"."id",
    "projects"."name",
    "projects"."path",
    "projects"."description",
    "projects"."created_at",
    "projects"."updated_at",
    "projects"."creator_id",
    "projects"."namespace_id",
    "projects"."last_activity_at",
    "projects"."import_url",
    "projects"."visibility_level",
    "projects"."archived",
    "projects"."avatar",
    "projects"."merge_requests_template",
    "projects"."star_count",
    "projects"."merge_requests_rebase_enabled",
    "projects"."import_type",
    "projects"."import_source",
    "projects"."approvals_before_merge",
    "projects"."reset_approvals_on_push",
    "projects"."merge_requests_ff_only_enabled",
    "projects"."issues_template",
    "projects"."mirror",
    "projects"."mirror_last_update_at",
    "projects"."mirror_last_successful_update_at",
    "projects"."mirror_user_id",
    "projects"."shared_runners_enabled",
    "projects"."runners_token",
    "projects"."build_allow_git_fetch",
    "projects"."build_timeout",
    "projects"."mirror_trigger_builds",
    "projects"."pending_delete",
    "projects"."public_builds",
    "projects"."last_repository_check_failed",
    "projects"."last_repository_check_at",
    "projects"."only_allow_merge_if_pipeline_succeeds",
    "projects"."has_external_issue_tracker",
    "projects"."repository_storage",
    "projects"."repository_read_only",
    "projects"."request_access_enabled",
    "projects"."has_external_wiki",
    "projects"."ci_config_path",
    "projects"."lfs_enabled",
    "projects"."description_html",
    "projects"."only_allow_merge_if_all_discussions_are_resolved",
    "projects"."repository_size_limit",
    "projects"."printing_merge_request_link_enabled",
    "projects"."auto_cancel_pending_pipelines",
    "projects"."service_desk_enabled",
    "projects"."cached_markdown_version",
    "projects"."delete_error",
    "projects"."last_repository_updated_at",
    "projects"."disable_overriding_approvers_per_merge_request",
    "projects"."storage_version",
    "projects"."resolve_outdated_diff_discussions",
    "projects"."remote_mirror_available_overridden",
    "projects"."only_mirror_protected_branches",
    "projects"."pull_mirror_available_overridden",
    "projects"."jobs_cache_index",
    "projects"."external_authorization_classification_label",
    "projects"."mirror_overwrites_diverged_branches",
    "projects"."pages_https_only",
    "projects"."external_webhook_token",
    "projects"."packages_enabled",
    "projects"."merge_requests_author_approval",
    "projects"."pool_repository_id",
    "projects"."runners_token_encrypted",
    "projects"."bfg_object_map",
    "projects"."detected_repository_languages",
    "projects"."merge_requests_disable_committers_approval",
    "projects"."require_password_to_approve",
    "projects"."max_pages_size",
    "projects"."max_artifacts_size",
    "projects"."pull_mirror_branch_prefix",
    "projects"."remove_source_branch_after_merge",
    "projects"."marked_for_deletion_at",
    "projects"."marked_for_deletion_by_user_id",
    "projects"."autoclose_referenced_issues",
    "projects"."suggestion_commit_message",
    "projects"."project_namespace_id",
    "projects"."hidden",
    "projects"."organization_id"
FROM
    "projects"
WHERE (EXISTS (
        SELECT
            1
        FROM
            "project_authorizations"
        WHERE
            "project_authorizations"."user_id" = 5413811
            AND (project_authorizations.project_id = projects.id))
        OR projects.visibility_level IN (10, 20))
AND "projects"."pending_delete" = FALSE
AND "projects"."id" IN (
    SELECT
        "projects"."id"
    FROM
        "projects"
        INNER JOIN "users_star_projects" ON "projects"."id" = "users_star_projects"."project_id"
    WHERE
        "users_star_projects"."user_id" = 5413811)
AND "projects"."hidden" = FALSE
ORDER BY
    LOWER("projects"."name") ASC,
    "projects"."id" DESC
LIMIT 21

After

Query plan

https://console.postgres.ai/gitlab/gitlab-production-main/sessions/35685/commands/110420

Click to expand 
 Limit  (cost=45.34..45.34 rows=1 width=860) (actual time=21.161..21.164 rows=5 loops=1)
   Buffers: shared hit=104 read=16
   I/O Timings: read=20.717 write=0.000
   ->  Sort  (cost=45.34..45.34 rows=1 width=860) (actual time=21.160..21.163 rows=5 loops=1)
         Sort Key: (lower((projects.name)::text)), projects.id DESC
         Sort Method: quicksort  Memory: 29kB
         Buffers: shared hit=104 read=16
         I/O Timings: read=20.717 write=0.000
         ->  Nested Loop  (cost=21.22..45.33 rows=1 width=860) (actual time=11.511..21.100 rows=5 loops=1)
               Buffers: shared hit=98 read=16
               I/O Timings: read=20.717 write=0.000
               ->  Nested Loop  (cost=13.90..30.42 rows=2 width=828) (actual time=0.101..0.206 rows=5 loops=1)
                     Buffers: shared hit=79
                     I/O Timings: read=0.000 write=0.000
                     ->  HashAggregate  (cost=13.34..13.38 rows=4 width=8) (actual time=0.066..0.073 rows=5 loops=1)
                           Group Key: projects_1.id
                           Buffers: shared hit=31
                           I/O Timings: read=0.000 write=0.000
                           ->  Nested Loop  (cost=0.99..13.33 rows=4 width=8) (actual time=0.026..0.063 rows=5 loops=1)
                                 Buffers: shared hit=31
                                 I/O Timings: read=0.000 write=0.000
                                 ->  Index Only Scan Backward using index_users_star_projects_on_user_id_and_project_id on public.users_star_projects  (cost=0.43..3.50 rows=4 width=4) (actual time=0.013..0.014 rows=5 loops=1)
                                       Index Cond: (users_star_projects.user_id = 5413811)
                                       Heap Fetches: 0
                                       Buffers: shared hit=4
                                       I/O Timings: read=0.000 write=0.000
                                 ->  Index Only Scan using idx_projects_on_repository_storage_last_repository_updated_at on public.projects projects_1  (cost=0.56..2.46 rows=1 width=4) (actual time=0.009..0.009 rows=1 loops=5)
                                       Index Cond: (projects_1.id = users_star_projects.project_id)
                                       Heap Fetches: 3
                                       Buffers: shared hit=27
                                       I/O Timings: read=0.000 write=0.000
                     ->  Index Scan using projects_pkey on public.projects  (cost=0.56..4.26 rows=1 width=828) (actual time=0.024..0.025 rows=1 loops=5)
                           Index Cond: (projects.id = projects_1.id)
                           Filter: ((NOT projects.pending_delete) AND (NOT projects.hidden) AND ((SubPlan 1) OR (projects.visibility_level = ANY ('{10,20}'::integer[]))))
                           Rows Removed by Filter: 0
                           Buffers: shared hit=48
                           I/O Timings: read=0.000 write=0.000
                           SubPlan 1
                             ->  Index Only Scan using project_authorizations_pkey on public.project_authorizations  (cost=0.58..3.60 rows=1 width=0) (actual time=0.010..0.010 rows=1 loops=5)
                                   Index Cond: ((project_authorizations.user_id = 5413811) AND (project_authorizations.project_id = projects.id))
                                   Heap Fetches: 0
                                   Buffers: shared hit=23
                                   I/O Timings: read=0.000 write=0.000
               ->  Index Scan using namespaces_pkey on public.namespaces  (cost=7.32..7.45 rows=1 width=4) (actual time=4.169..4.169 rows=1 loops=5)
                     Index Cond: (namespaces.id = projects.project_namespace_id)
                     Filter: ((NOT (hashed SubPlan 3)) AND ((namespaces.type)::text = 'Project'::text))
                     Rows Removed by Filter: 0
                     Buffers: shared hit=19 read=16
                     I/O Timings: read=20.717 write=0.000
                     SubPlan 3
                       ->  Nested Loop  (cost=0.72..6.75 rows=1 width=4) (actual time=6.801..6.803 rows=1 loops=1)
                             Buffers: shared hit=2 read=5
                             I/O Timings: read=6.764 write=0.000
                             ->  Index Scan using index_identities_on_user_id on public.identities  (cost=0.43..3.45 rows=1 width=4) (actual time=5.744..5.745 rows=1 loops=1)
                                   Index Cond: (identities.user_id = 5413811)
                                   Filter: (identities.saml_provider_id IS NOT NULL)
                                   Rows Removed by Filter: 0
                                   Buffers: shared read=4
                                   I/O Timings: read=5.721 write=0.000
                             ->  Index Scan using saml_providers_pkey on public.saml_providers  (cost=0.28..3.30 rows=1 width=8) (actual time=1.055..1.055 rows=1 loops=1)
                                   Index Cond: (saml_providers.id = identities.saml_provider_id)
                                   Buffers: shared hit=2 read=1
                                   I/O Timings: read=1.042 write=0.000
Settings: seq_page_cost = '4', work_mem = '100MB', effective_cache_size = '472585MB', jit = 'off', random_page_cost = '1.5'

Raw SQL

Click to expand 
SELECT
    "projects"."id",
    "projects"."name",
    "projects"."path",
    "projects"."description",
    "projects"."created_at",
    "projects"."updated_at",
    "projects"."creator_id",
    "projects"."namespace_id",
    "projects"."last_activity_at",
    "projects"."import_url",
    "projects"."visibility_level",
    "projects"."archived",
    "projects"."avatar",
    "projects"."merge_requests_template",
    "projects"."star_count",
    "projects"."merge_requests_rebase_enabled",
    "projects"."import_type",
    "projects"."import_source",
    "projects"."approvals_before_merge",
    "projects"."reset_approvals_on_push",
    "projects"."merge_requests_ff_only_enabled",
    "projects"."issues_template",
    "projects"."mirror",
    "projects"."mirror_last_update_at",
    "projects"."mirror_last_successful_update_at",
    "projects"."mirror_user_id",
    "projects"."shared_runners_enabled",
    "projects"."runners_token",
    "projects"."build_allow_git_fetch",
    "projects"."build_timeout",
    "projects"."mirror_trigger_builds",
    "projects"."pending_delete",
    "projects"."public_builds",
    "projects"."last_repository_check_failed",
    "projects"."last_repository_check_at",
    "projects"."only_allow_merge_if_pipeline_succeeds",
    "projects"."has_external_issue_tracker",
    "projects"."repository_storage",
    "projects"."repository_read_only",
    "projects"."request_access_enabled",
    "projects"."has_external_wiki",
    "projects"."ci_config_path",
    "projects"."lfs_enabled",
    "projects"."description_html",
    "projects"."only_allow_merge_if_all_discussions_are_resolved",
    "projects"."repository_size_limit",
    "projects"."printing_merge_request_link_enabled",
    "projects"."auto_cancel_pending_pipelines",
    "projects"."service_desk_enabled",
    "projects"."cached_markdown_version",
    "projects"."delete_error",
    "projects"."last_repository_updated_at",
    "projects"."disable_overriding_approvers_per_merge_request",
    "projects"."storage_version",
    "projects"."resolve_outdated_diff_discussions",
    "projects"."remote_mirror_available_overridden",
    "projects"."only_mirror_protected_branches",
    "projects"."pull_mirror_available_overridden",
    "projects"."jobs_cache_index",
    "projects"."external_authorization_classification_label",
    "projects"."mirror_overwrites_diverged_branches",
    "projects"."pages_https_only",
    "projects"."external_webhook_token",
    "projects"."packages_enabled",
    "projects"."merge_requests_author_approval",
    "projects"."pool_repository_id",
    "projects"."runners_token_encrypted",
    "projects"."bfg_object_map",
    "projects"."detected_repository_languages",
    "projects"."merge_requests_disable_committers_approval",
    "projects"."require_password_to_approve",
    "projects"."max_pages_size",
    "projects"."max_artifacts_size",
    "projects"."pull_mirror_branch_prefix",
    "projects"."remove_source_branch_after_merge",
    "projects"."marked_for_deletion_at",
    "projects"."marked_for_deletion_by_user_id",
    "projects"."autoclose_referenced_issues",
    "projects"."suggestion_commit_message",
    "projects"."project_namespace_id",
    "projects"."hidden",
    "projects"."organization_id"
FROM
    "projects"
    INNER JOIN "namespaces" ON "namespaces"."id" = "projects"."project_namespace_id"
        AND "namespaces"."type" = 'Project'
WHERE (EXISTS (
        SELECT
            1
        FROM
            "project_authorizations"
        WHERE
            "project_authorizations"."user_id" = 5413811
            AND (project_authorizations.project_id = projects.id))
        OR projects.visibility_level IN (10, 20))
AND "projects"."pending_delete" = FALSE
AND "projects"."id" IN (
    SELECT
        "projects"."id"
    FROM
        "projects"
        INNER JOIN "users_star_projects" ON "projects"."id" = "users_star_projects"."project_id"
    WHERE
        "users_star_projects"."user_id" = 5413811)
AND "projects"."hidden" = FALSE
AND (namespaces.traversal_ids[1] NOT IN (
        SELECT
            "saml_providers"."group_id"
        FROM
            "saml_providers"
            INNER JOIN "identities" ON "saml_providers"."id" = "identities"."saml_provider_id"
        WHERE
            "identities"."user_id" = 5413811
            AND "identities"."saml_provider_id" IS NOT NULL
            AND 1 = 1))
ORDER BY
    LOWER("projects"."name") ASC,
    "projects"."id" DESC
LIMIT 21

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

Before After
Screenshot_2025-01-29_at_8.29.43_PM Screenshot_2025-01-29_at_8.26.09_PM

How to set up and validate locally

  1. Setup SAML in GDK - https://gitlab.com/gitlab-org/gitlab-development-kit/blob/main/doc/howto/saml.md
  2. Start GDK in SaaS mode - https://docs.gitlab.com/ee/development/ee_features.html#simulate-a-saas-instance
  3. Create a group
  4. In the admin area give that group an Ultimate license
  5. Configure SAML for that group - https://gitlab.com/gitlab-org/gitlab-development-kit/blob/main/doc/howto/saml.md
  6. Create a few private projects in that group and star all of them
  7. Sign in with SAML
  8. Create a few personal projects and star them
  9. Go to /-/user_settings/password/edit and set a password
  10. Sign out, then sign back in with your password
  11. Change app/assets/javascripts/api.js#L8 to 5
  12. Go to /rails/features and enable the your_work_projects_vue feature flag
  13. Go to /dashboard/projects/starred
  14. The count should be correct
Edited by Peter Hegman

Merge request reports