Fix ExternalUsernameSanitizer matching nested project paths

What does this MR do and why?

ExternalUsernameSanitizer#unique_by_namespace queried Namespace.all.find_by_path_or_name(s) to check whether a slugified username was already taken. That query also matches ProjectNamespace records, whose path column stores only the last URL segment (for example root/quokka has path: "quokka"). As a result, a first-time LDAP/OAuth login for a user actually named quokka was treated as colliding with an unrelated nested project, and got suffixed to quokka1.

Namespace.username_reserved? already answers this exact question correctly — it excludes project namespaces and scopes to top-level namespaces — and is already used by UsersController#exists for the same purpose. This MR just reuses it instead of the duplicated, buggy lookup.

References

#602502 (closed)

Screenshots or screen recordings

N/A — backend-only bug fix, no UI change.

How to set up and validate locally

  1. bundle exec rspec spec/lib/gitlab/auth/external_username_sanitizer_spec.rb
  2. See the new context with a nested project sharing the same path segment for the exact repro from the issue.

Database Review Information

Local GDK results (not production-representative)

Local testing against the 13-row namespaces table shows Seq Scan plans for both queries, which does not reflect production behavior:

OLD query (Namespace.all.find_by_path_or_name):

Seq Scan on namespaces  (cost=0.00..1.22 rows=2 width=367) (actual time=0.031..0.032 rows=0 loops=1)
Filter: ((lower((path)::text) = 'quokka_query_plan_test'::text) OR (lower((name)::text) = 'quokka_query_plan_test'::text))
Rows Removed by Filter: 13
Planning Time: 8.111 ms
Execution Time: 0.059 ms

NEW query (Namespace.without_project_namespaces.top_level.find_by_path_or_name, via username_reserved?):

Seq Scan on namespaces  (cost=0.00..1.25 rows=1 width=367) (actual time=0.019..0.019 rows=0 loops=1)
Filter: ((parent_id IS NULL) AND ((type)::text <> 'Project'::text) AND ((lower((path)::text) = 'quokka_query_plan_test'::text) OR (lower((name)::text) = 'quokka_query_plan_test'::text)))
Rows Removed by Filter: 13
Planning Time: 0.312 ms
Execution Time: 0.040 ms

Production-scale index eligibility

A partial index index_namespaces_on_path_for_top_level_non_projects exists on lower(path), filtered to WHERE (parent_id IS NULL AND type <> 'Project') — an exact match for the new query's scoping. It was added in migration 20221122155149_add_index_for_paths_on_non_projects.rb (commit 7c651a11ea14, Nov 2022) specifically to fix timeouts on path-uniqueness checks during group creation for popular path names. A separate full-table index, index_on_namespaces_lower_name, covers the name branch of the OR.

Conclusion

The OLD query has no parent_id/type predicates, so it can't use the scoped partial index at all — it falls back to the broader, unscoped index_on_namespaces_lower_path / index_on_namespaces_lower_name indexes, scanning across every namespace type including all project namespaces. The NEW query's scope matches the partial index's filter exactly, so at production scale it's index-eligible in a way the old query wasn't — a strict improvement, not just "no worse."

Edited by Sergey Pechenko

Merge request reports

Loading
Loading