Fix Rack Attack incorrectly rate limiting runner API

What does this MR do and why?

To update a job status, the runner uses the PUT /api/v4/:jobs endpoint with the job token in two places:

  1. The PRIVATE-TOKEN header
  2. The token parameter in the JSON body

Previously AuthFinders looked up the PAT and raised an unauthorized exception because no user was found. Instead, it should continue to see if it can authenticate the job with the token parameter.

This commit makes access_token return blank if it has the CI build token prefix so that the exception is not raised. That way Rack Attack can then ensure the request is authenticated with the job token.

References

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

  1. Enable Rack Attack: in /admin -> Settings -> Network click on all the checkboxes, and click Save.
  2. Check out this branch.
  3. Apply this patch:
diff --git a/lib/gitlab/rack_attack/request.rb b/lib/gitlab/rack_attack/request.rb
index 4a6902594ccc..d0bcb93f8b9e 100644
--- a/lib/gitlab/rack_attack/request.rb
+++ b/lib/gitlab/rack_attack/request.rb
@@ -83,6 +83,7 @@ def throttle?(throttle, authenticated:)
       end
 
       def throttle_unauthenticated_api?
+        Rails.logger.info "=== #{path}: unauthenticated?: #{unauthenticated?}"
         api_request? &&
           !should_be_skipped? &&
           !frontend_request? &&
  1. gdk restart rails-web
  2. Now run a CI job.

tail -f log/development.log | grep "===" and look for the /api/jobs/:id request, and unauthenticated? should all be false.

Edited by Stan Hu

Merge request reports

Loading