Fix response code issue + code clean up in the virtual registries API specs

David Fernandezrequested to merge
10io-cleanup-vreg-specs into master

🧹 Context

While working on the virtual registries APIs, I noticed that a block was duplicated multiple times. I created a shared example with the help of Duo 🤖 .

While doing so, I noticed an inconsistency in returned status codes for private groups. Duo found the explanation:

  • For APIs that are below the group resources /groups/:id, the #find_group helper function is used which has will automatically return 404 Not Found if the user doesn't have the read_group permission on it.
  • For APIs that are below the virtual registry resources, let's say /upstreams/:id, we simply find_by the target resource and then run the permissions check. This will return 403 Forbidden.

We should follow the behavior that we have in #find_group. If private resource + non member user = 404 Not Found.

🤔 What does this MR do and why?

  • Update all registry and upstream APIs to run an additional check when finding the registry or upstream objects: we will check read_group on the target group.
  • Cleanup all the specs by using a shared example.

This is part of the virtual registries work. This MR impacts two formats, here are their statuses:

  • container: we're still implementing the first iteration.
  • maven: we're in beta with a feature flag that is enabled by default.

📖 References

📺 Screenshots or screen recordings

No UI changes.

⚗️ How to set up and validate locally

Run the affected specs:

$ bundle exec rspec ee/spec/requests/api/virtual_registries/container/registries_spec.rb
$ bundle exec rspec ee/spec/requests/api/virtual_registries/container/upstreams_spec.rb
$ bundle exec rspec ee/spec/requests/api/virtual_registries/packages/maven/registries_spec.rb
$ bundle exec rspec ee/spec/requests/api/virtual_registries/packages/maven/upstreams_spec.rb

Let's check the status codes for each format.

  • Have an EE license (virtual registries are a EE only feature)
  • Enable the maven virtual registry with: Feature.enable(:maven_virtual_registry)
  • Enable the container virtual registry with: Feature.enable(:container_virtual_registries)
  • Create a user and a PAT with 
    user = FactoryBot.create(:user, email: 'bananas2@test.net') 
pat = FactoryBot.create(:personal_access_token, user: user, scopes: ['api'])
pat.token # Note this

1️⃣ Maven

In a rails console:

group = Group.all.select(&:root?).sample(1).first # make sure that the group visibility is private or
group.update!(visibility: 'private')

registry = ::VirtualRegistries::Packages::Maven::Registry.create!(name: 'bananas', group: group) # note the id
upstream = ::VirtualRegistries::Packages::Maven::Upstream.create!(name: 'bananas', url: 'https://gitlab.com/maven3', group: group, registries: [registry]) # note the id

# stub file upload
def fixture_file_upload(*args, **kwargs)
  Rack::Test::UploadedFile.new(*args, **kwargs)
end

cache_entry = FactoryBot.create(:virtual_registries_packages_maven_cache_entry, upstream: upstream, group: upstream.group)
Base64.urlsafe_encode64("#{cache_entry.upstream_id} #{cache_entry.relative_path}") # this is the cache entry id

Let's try some curl commands:

curl command With this MR On master
curl -v "http://gdk.test:8000/api/v4/groups/<group id>/-/virtual_registries/packages/maven/registries" (anonymous) 404 Not Found 404 Not Found
curl -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/groups/<group id>/-/virtual_registries/packages/maven/registries" (authenticated but not member) 404 Not Found 404 Not Found
curl -v "http://gdk.test:8000/api/v4/virtual_registries/packages/maven/registries/<registry id>" (anonymous) 401 Unauthorized 401 Unauthorized
curl -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/virtual_registries/packages/maven/registries/<registry id>" (authenticated but not member) 404 Not Found 403 Forbidden
curl -v "http://gdk.test:8000/api/v4/virtual_registries/packages/maven/upstreams/<upstream id>" (anonymous) 401 Unauthorized 401 Unauthorized
curl -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/virtual_registries/packages/maven/registries/<registry id>" (authenticated but not member) 404 Not Found 403 Forbidden
curl -X DELETE -v "http://gdk.test:8000/api/v4/virtual_registries/packages/maven/cache_entries/<cache entry id>" (anonymous) 401 Unauthorized 401 Unauthorized
curl -X DELETE -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/virtual_registries/packages/maven/registries/<registry id>" (authenticated but not member) 404 Not Found 403 Forbidden

2️⃣ Container 

group = Group.all.select(&:root?).sample(1).first # make sure that the group visibility is private or
group.update!(visibility: 'private')

registry = ::VirtualRegistries::Container::Registry.create!(name: 'bananas', group: group) # note the id
upstream = ::VirtualRegistries::Container::Upstream.create!(name: 'bananas', url: 'https://gitlab.com/container', group: group, registries: [registry]) # note the id

# stub file upload
def fixture_file_upload(*args, **kwargs)
  Rack::Test::UploadedFile.new(*args, **kwargs)
end

cache_entry = FactoryBot.create(:virtual_registries_container_cache_entry, upstream: upstream, group: upstream.group)
Base64.urlsafe_encode64("#{cache_entry.upstream_id} #{cache_entry.relative_path}") # this is the cache entry id

Let's try some curl commands:

curl command With this MR On master
curl -v "http://gdk.test:8000/api/v4/groups/<group id>/-/virtual_registries/container/registries" (anonymous) 404 Not Found 404 Not Found
curl -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/groups/<group id>/-/virtual_registries/container/registries" (authenticated but not member) 404 Not Found 404 Not Found
curl -v "http://gdk.test:8000/api/v4/virtual_registries/container/registries/<registry id>" (anonymous) 401 Unauthorized 401 Unauthorized
curl -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/virtual_registries/container/registries/<registry id>" (authenticated but not member) 404 Not Found 403 Forbidden
curl -v "http://gdk.test:8000/api/v4/virtual_registries/container/upstreams/<upstream id>" (anonymous) 401 Unauthorized 401 Unauthorized
curl -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/virtual_registries/container/registries/<registry id>" (authenticated but not member) 404 Not Found 403 Forbidden
curl -X DELETE -v "http://gdk.test:8000/api/v4/virtual_registries/container/cache_entries/<cache entry id>" (anonymous) 401 Unauthorized 401 Unauthorized
curl -X DELETE -v -H "Private-Token: <user PAT>" "http://gdk.test:8000/api/v4/virtual_registries/container/registries/<registry id>" (authenticated but not member) 404 Not Found 403 Forbidden

🔮 Conclusions

As we can see this MR provides more consistency in the status code returned when:

  • user is anonymous.
  • user is authenticated but not member of the group that hosts the virtual registry objects.

🏎️ MR acceptance checklist

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

Merge request reports