Skip to content

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 by David Fernandez