Introduce Keyset pagination for `GET /groups` API endpoint for unauthenticated users.
What does this MR do?
Related to https://gitlab.com/gitlab-org/gitlab/-/issues/331493
With this MR, the GET /groups
endpoint will now error out when using offset pagination when all of the following conditions are met:
- The user is anonymous (ie, user accesses this API without logging in. Such users already only get access to data of public groups (visibility_level = 20))
- The user is requesting for records that falls beyond the count of 50,000 (ie,
params[:per_page] * params[:page]
> 50000) - The feature flag
keyset_pagination_for_groups_api
is enabled.
In such a case, the API will error out with the message: Offset pagination has a maximum allowed offset of 50000 for requests that return objects of type Group. Remaining records can be retrieved using keyset pagination.
This anonymous user will now have the option to get the data of public groups using keyset-pagination
using the same endpoint, but with an additional params[:pagination] = keyset
param included while making the request.
However, the only ordering supported via the params (order_by
and sort
) for using this keyset pagination will be name ASC
.
If the anonymous user uses any other order_by
or sort
combination in this endpoint while using keyset pagination, it will error out with Keyset pagination is not yet available for this type of request
The short version is: We do not allow anonymous users to keep going thru /groups
API using offset pagination anymore beyond 50,000 groups. They will now be forced to use keyset-pagination for the same with name ASC
order.
DB Migration
Up Migration
== 20210901065504 AddIndexOnNameAndIdToPublicGroups: migrating ================
-- transaction_open?()
-> 0.0000s
-- index_exists?(:namespaces, [:name, :id], {:name=>"index_namespaces_public_groups_name_id", :where=>"type = 'Group' AND visibility_level = 20", :algorithm=>:concurrently})
-> 0.0136s
-- execute("SET statement_timeout TO 0")
-> 0.0007s
-- add_index(:namespaces, [:name, :id], {:name=>"index_namespaces_public_groups_name_id", :where=>"type = 'Group' AND visibility_level = 20", :algorithm=>:concurrently})
-> 0.0173s
-- execute("RESET statement_timeout")
-> 0.0007s
== 20210901065504 AddIndexOnNameAndIdToPublicGroups: migrated (0.0349s) =======
Down Migration
== 20210901065504 AddIndexOnNameAndIdToPublicGroups: reverting ================
-- transaction_open?()
-> 0.0000s
-- indexes(:namespaces)
-> 0.0113s
-- execute("SET statement_timeout TO 0")
-> 0.0006s
-- remove_index(:namespaces, {:algorithm=>:concurrently, :name=>"index_namespaces_public_groups_name_id"})
-> 0.0047s
-- execute("RESET statement_timeout")
-> 0.0006s
== 20210901065504 AddIndexOnNameAndIdToPublicGroups: reverted (0.0194s) =======
Query plans are added at !68346 (comment 665497635).
Screenshots or Screencasts (strongly suggested)
How to setup and validate locally (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Security
Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.
-
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