Skip to content

Fix problem with Groups API search argument

What does this MR do?

Related to #258215 (closed) and #273177 (closed)

This change introduces a fix to Groups API search query parameter that was returning invalid results. The reason for that is: we were not using source_type in the search query. So we were looking for groups with routes that were partially matching route.path or route.name, however we were not checking if these routes were created for Namespace.

Example:

We have Namespace with ID=1 and route.path = great-group We have Namespace with ID=2 and route.path = other-group We have Project with ID=2 and route.path = great-project

We are doing search with query: great

Before this change:

We are getting all Namespace with Source IDs fetched from Routes. First we fetch Source IDs from Routes where path = '%great%', since we are not checking Source Type we are receiving Source ID: 1 and 2. Then we are getting Namespaces with these IDs: we are incorrectly returning Namespaces with path: great-group and other-group.

After this change:

We are getting all Namespace with Source IDs fetched from Routes. First we fetch Source IDs from Routes where path = '%great%' and Source Type = Namespace, we are receiving Source ID: 1. Then we are getting Namespaces with these IDs: we are correctly returning Namespace with path: great-group only.

Queries

Before this change:

SELECT "namespaces".*
FROM "namespaces"
WHERE "namespaces"."type" = 'Group'
  AND (
    "namespaces"."id" IN (
      SELECT "routes"."source_id"
      FROM "routes"
      WHERE ("routes"."path" ILIKE '%threat-insights%' OR "routes"."name" ILIKE '%threat-insights%')
    )
  )
LIMIT 20 OFFSET 0

https://explain.depesz.com/s/o36q6 (~120ms)

After this change:

SELECT "namespaces".*
FROM "namespaces"
WHERE "namespaces"."type" = 'Group'
  AND (
    "namespaces"."id" IN (
      SELECT "routes"."source_id"
      FROM "routes"
      WHERE ("routes"."path" ILIKE '%threat-insights%' OR "routes"."name" ILIKE '%threat-insights%') AND ("routes"."source_type" = 'Namespace')
    )
  )
LIMIT 20 OFFSET 0

https://explain.depesz.com/s/OdBmc (~130ms)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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 Alan (Maciej) Paruszewski

Merge request reports