Skip to content

[E2E] Harden group access token spec

John McDonnell requested to merge jmd/harden-group-access-tokens-spec into master

What does this MR do and why?

[E2E] Harden group access token spec

This appears to be caused by cases where sidekiq doesn't run jobs in the background fast enough for the newly created group access token to be associated to the projects, leading to a 403 status.

By adding an additional background check here, we can verify that the token has been successfully associated to the project prior to executing the tests in question.

Fixes #433393 (closed)
Fixes #431378 (closed)

How to set up and validate locally

We can replicate this manually by

  1. Create a group, and a project within said group
  2. gdk stop rails-background-jobs
  3. navigate to group > settings > access tokens, and add a new group access token
  4. use this group access token to make a request to the project in the group
curl --location 'http://gdk.test:3000/api/v4/projects/gitlab-qa-sandbox-group-5%2Fqa-test-2023-12-21-16-30-51-bbffce07761805cc%2Fproject-for-group-access-token-24c2fc8457460c8e/repository/files/QA+Test+-+File+name' \
    --header 'PRIVATE-TOKEN: <<ACCESS_TOKEN>>' \
    --header 'Content-Type: application/json' \
    --data '{
        "branch": "main",
        "commit_message": "commit_messages",
        "content": "aGVsbG8gd29ybGQ="
    }'
  1. Viewing http://gdk.test:3000/admin/background_jobs, navigate into Queues > default and we can see an enqueue job Groups::EnterpriseUsers::AssociateWorker which I believe is the item of interest to us
  2. gdk start rails-background-jobs and wait for the enqueued jobs to clear
  3. Retry the request, and you'll note you no long get the 403 error

This can be replicated via the tests by applying some delays to sidekiq before running bundle exec rspec qa/specs/features/api/10_govern/group_access_token_spec.rb which mostly was able to create a consistent reproducer for me.

Sample diff to add delay in sidekiq
diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb
index 8df12671f265..469daa53350f 100644
--- a/config/initializers/sidekiq.rb
+++ b/config/initializers/sidekiq.rb
@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 module SidekiqLogArguments
   def self.enabled?
     Gitlab::Utils.to_boolean(ENV['SIDEKIQ_LOG_ARGUMENTS'], default: true)
@@ -32,7 +33,17 @@ def enable_semi_reliable_fetch_mode?
 
 enable_json_logs = Gitlab.config.sidekiq.log_format != 'text'
 
+class DelayedProcessingMiddleware
+  def call(_worker, _job, _queue)
+    sleep(3)
+    yield
+  end
+end
+
 Sidekiq.configure_server do |config|
+  config.server_middleware do |chain|
+    chain.add DelayedProcessingMiddleware
+  end
   config[:strict] = false
   config[:queues] = Gitlab::SidekiqConfig.expand_queues(config[:queues])

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.

Edited by John McDonnell

Merge request reports