Improper rack-attack discriminator for `authenticated_packages_api` with a deploy token

🔥 Problem

We currently have several rack-attack throttles configured. One of them is throttle_authenticated_packages_api. When this throttle is triggered, the discriminator is the user_id.

That user_id comes from https://gitlab.com/gitlab-org/gitlab/-/blob/542ca3dcfd56e9e5581af62ff16b939c895ae097/lib/gitlab/rack_attack/request.rb#L14 which in turn will use https://gitlab.com/gitlab-org/gitlab/-/blob/542ca3dcfd56e9e5581af62ff16b939c895ae097/lib/gitlab/auth/request_authenticator.rb#L32.

The problem is that this function doesn't read deploy token objects.

The discriminator will thus be nil = the throttle evluation will be aborted = all packages api with deploy token will not be throttled.

The same problem was discovered for the dependency proxy routes in !71532 (merged)

🚒 Solution

The straightforward solution would be to update #find_sessionless_user so that it returns deploy tokens. Unfortunately, that function is used in rack-attack throttles and here: https://gitlab.com/gitlab-org/gitlab/-/blob/542ca3dcfd56e9e5581af62ff16b939c895ae097/app/controllers/concerns/sessionless_authentication.rb. That concern is used in several rails controller.

Given that impacting several rails controller at once is not a great idea, we will need an alternative.

Introduce #find_sessionless_user_or_deploy_token which uses #find_sessionless_user and upgrade it with finders for deploy tokens. Once we have that, the discriminator could be the object type + id. For example: user.23 or deploy_token.23. Using only the id will not work as we can have clashes between user ids and deploy token ids.

Edited Oct 07, 2021 by David Fernandez
Assignee Loading
Time tracking Loading