kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Support multiple scopes when using authing container registry scopes

Browse source
This commit is contained in:
Kamil Trzciński 2018-07-13 19:45:07 +02:00
parent 3d2dad449d
commit 34ec29b956
5 changed files with 155 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
app/controllers/jwt_controller.rb
View file

@ -54,6 +54,22 @@ class JwtController < ApplicationController
end
def auth_params
params.permit(:service, :scope, :account, :client_id)
params.permit(:service, :account, :client_id)
.merge(additional_params)
end
def additional_params
{ scopes: scopes_param }.compact
end
# We have to parse scope here, because Docker Client does not send an array of scopes,
# but rather a flat list and we loose second scope when being processed by Rails:
# scope=scopeA&scope=scopeB
#
# This method makes to always return an array of scopes
def scopes_param
return unless params[:scope].present?
Array(Rack::Utils.parse_query(request.query_string)['scope'])
end
end

12
app/services/auth/container_registry_authentication_service.rb
View file

@ -9,11 +9,11 @@ module Auth
return error('UNAVAILABLE', status: 404, message: 'registry not enabled') unless registry.enabled
unless scope || current_user || project
unless scopes.any? || current_user || project
return error('DENIED', status: 403, message: 'access forbidden')
end
{ token: authorized_token(scope).encoded }
{ token: authorized_token(*scopes).encoded }
end
def self.full_access_token(*names)
@ -47,10 +47,12 @@ module Auth
end
end
def scope
return unless params[:scope]
def scopes
return [] unless params[:scopes]
@scope ||= process_scope(params[:scope])
@scopes ||= params[:scopes].map do |scope|
process_scope(scope)
end.compact
end
def process_scope(scope)

5
changelogs/unreleased/fix-multiple-scopes.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Support multiple scopes when authing container registry scopes
merge_request: 20617
author:
type: fixed

19
spec/requests/jwt_controller_spec.rb
View file

@ -70,6 +70,25 @@ describe JwtController do
it { expect(service_class).to have_received(:new).with(nil, user, parameters) }
context 'when passing a flat array of scopes' do
# We use this trick to make rails to generate a query_string:
# scope=scope1&scope=scope2
# It works because :scope and 'scope' are the same as string, but different objects
let(:parameters) do
{
:service => service_name,
:scope => 'scope1',
'scope' => 'scope2'
}
end
let(:service_parameters) do
{ service: service_name, scopes: %w(scope1 scope2) }
end
it { expect(service_class).to have_received(:new).with(nil, user, service_parameters) }
end
context 'when user has 2FA enabled' do
let(:user) { create(:user, :two_factor) }

140
spec/services/auth/container_registry_authentication_service_spec.rb
View file

@ -142,7 +142,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'for registry catalog' do
let(:current_params) do
{ scope: "registry:catalog:*" }
{ scopes: ["registry:catalog:*"] }
end
context 'disallow browsing for users without Gitlab admin rights' do
@ -164,7 +164,7 @@ describe Auth::ContainerRegistryAuthenticationService do
end
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'a pushable'
@ -177,7 +177,7 @@ describe Auth::ContainerRegistryAuthenticationService do
end
let(:current_params) do
{ scope: "repository:#{project.full_path}:*" }
{ scopes: ["repository:#{project.full_path}:*"] }
end
it_behaves_like 'an inaccessible'
@ -191,7 +191,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when pulling from root level repository' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull" }
{ scopes: ["repository:#{project.full_path}:pull"] }
end
it_behaves_like 'a pullable'
@ -205,7 +205,7 @@ describe Auth::ContainerRegistryAuthenticationService do
end
let(:current_params) do
{ scope: "repository:#{project.full_path}:*" }
{ scopes: ["repository:#{project.full_path}:*"] }
end
it_behaves_like 'an inaccessible'
@ -218,7 +218,7 @@ describe Auth::ContainerRegistryAuthenticationService do
end
let(:current_params) do
{ scope: "repository:#{project.full_path}:push,pull" }
{ scopes: ["repository:#{project.full_path}:push,pull"] }
end
it_behaves_like 'a pullable'
@ -231,7 +231,7 @@ describe Auth::ContainerRegistryAuthenticationService do
end
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull,push" }
{ scopes: ["repository:#{project.full_path}:pull,push"] }
end
it_behaves_like 'an inaccessible'
@ -244,7 +244,7 @@ describe Auth::ContainerRegistryAuthenticationService do
end
let(:current_params) do
{ scope: "repository:#{project.full_path}:*" }
{ scopes: ["repository:#{project.full_path}:*"] }
end
it_behaves_like 'an inaccessible'
@ -257,7 +257,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'allow anyone to pull images' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull" }
{ scopes: ["repository:#{project.full_path}:pull"] }
end
it_behaves_like 'a pullable'
@ -266,7 +266,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow anyone to push images' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'an inaccessible'
@ -275,7 +275,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow anyone to delete images' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:*" }
{ scopes: ["repository:#{project.full_path}:*"] }
end
it_behaves_like 'an inaccessible'
@ -284,7 +284,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when repository name is invalid' do
let(:current_params) do
{ scope: 'repository:invalid:push' }
{ scopes: ['repository:invalid:push'] }
end
it_behaves_like 'an inaccessible'
@ -298,7 +298,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'for internal user' do
context 'allow anyone to pull images' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull" }
{ scopes: ["repository:#{project.full_path}:pull"] }
end
it_behaves_like 'a pullable'
@ -307,7 +307,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow anyone to push images' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'an inaccessible'
@ -316,7 +316,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow anyone to delete images' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:*" }
{ scopes: ["repository:#{project.full_path}:*"] }
end
it_behaves_like 'an inaccessible'
@ -328,7 +328,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow anyone to pull or push images' do
let(:current_user) { create(:user, external: true) }
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull,push" }
{ scopes: ["repository:#{project.full_path}:pull,push"] }
end
it_behaves_like 'an inaccessible'
@ -338,7 +338,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow anyone to delete images' do
let(:current_user) { create(:user, external: true) }
let(:current_params) do
{ scope: "repository:#{project.full_path}:*" }
{ scopes: ["repository:#{project.full_path}:*"] }
end
it_behaves_like 'an inaccessible'
@ -364,7 +364,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'allow to delete images' do
let(:current_params) do
{ scope: "repository:#{current_project.full_path}:*" }
{ scopes: ["repository:#{current_project.full_path}:*"] }
end
it_behaves_like 'a deletable' do
@ -397,7 +397,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'allow to pull and push images' do
let(:current_params) do
{ scope: "repository:#{current_project.full_path}:pull,push" }
{ scopes: ["repository:#{current_project.full_path}:pull,push"] }
end
it_behaves_like 'a pullable and pushable' do
@ -411,7 +411,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow to delete images' do
let(:current_params) do
{ scope: "repository:#{current_project.full_path}:*" }
{ scopes: ["repository:#{current_project.full_path}:*"] }
end
it_behaves_like 'an inaccessible' do
@ -422,7 +422,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'for other projects' do
context 'when pulling' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull" }
{ scopes: ["repository:#{project.full_path}:pull"] }
end
context 'allow for public' do
@ -489,7 +489,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when pushing' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
context 'disallow for all' do
@ -523,7 +523,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'disallow when pulling' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull" }
{ scopes: ["repository:#{project.full_path}:pull"] }
end
it_behaves_like 'an inaccessible'
@ -534,14 +534,66 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'registry catalog browsing authorized as admin' do
let(:current_user) { create(:user, :admin) }
let(:project) { create(:project, :public) }
let(:current_params) do
{ scope: "registry:catalog:*" }
{ scopes: ["registry:catalog:*"] }
end
it_behaves_like 'a browsable'
end
context 'support for multiple scopes' do
let(:internal_project) { create(:project, :internal) }
let(:private_project) { create(:project, :private) }
let(:current_params) do
{
scopes: [
"repository:#{internal_project.full_path}:pull",
"repository:#{private_project.full_path}:pull"
]
}
end
context 'user has access to all projects' do
let(:current_user) { create(:user, :admin) }
it_behaves_like 'a browsable' do
let(:access) do
[
{ 'type' => 'repository',
'name' => internal_project.full_path,
'actions' => ['pull'] },
{ 'type' => 'repository',
'name' => private_project.full_path,
'actions' => ['pull'] }
]
end
end
end
context 'user only has access to internal project' do
let(:current_user) { create(:user) }
it_behaves_like 'a browsable' do
let(:access) do
[
{ 'type' => 'repository',
'name' => internal_project.full_path,
'actions' => ['pull'] }
]
end
end
end
context 'anonymous access is rejected' do
let(:current_user) { nil }
it_behaves_like 'a forbidden'
end
end
context 'unauthorized' do
context 'disallow to use scope-less authentication' do
it_behaves_like 'a forbidden'
@ -550,7 +602,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'for invalid scope' do
let(:current_params) do
{ scope: 'invalid:aa:bb' }
{ scopes: ['invalid:aa:bb'] }
end
it_behaves_like 'a forbidden'
@ -561,7 +613,7 @@ describe Auth::ContainerRegistryAuthenticationService do
let(:project) { create(:project, :private) }
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull" }
{ scopes: ["repository:#{project.full_path}:pull"] }
end
it_behaves_like 'a forbidden'
@ -572,7 +624,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when pulling and pushing' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull,push" }
{ scopes: ["repository:#{project.full_path}:pull,push"] }
end
it_behaves_like 'a pullable'
@ -581,7 +633,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when pushing' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'a forbidden'
@ -591,7 +643,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'for registry catalog' do
let(:current_params) do
{ scope: "registry:catalog:*" }
{ scopes: ["registry:catalog:*"] }
end
it_behaves_like 'a forbidden'
@ -601,7 +653,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'for deploy tokens' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:pull" }
{ scopes: ["repository:#{project.full_path}:pull"] }
end
context 'when deploy token has read_registry as a scope' do
@ -616,7 +668,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when pushing' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'an inaccessible'
@ -632,7 +684,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when pushing' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'an inaccessible'
@ -648,7 +700,7 @@ describe Auth::ContainerRegistryAuthenticationService do
context 'when pushing' do
let(:current_params) do
{ scope: "repository:#{project.full_path}:push" }
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'an inaccessible'
@ -734,4 +786,26 @@ describe Auth::ContainerRegistryAuthenticationService do
end
end
end
context 'user authorization' do
let(:current_user) { create(:user) }
context 'with multiple scopes' do
let(:project) { create(:project) }
let(:project2) { create }
context 'allow developer to push images' do
before do
project.add_developer(current_user)
end
let(:current_params) do
{ scopes: ["repository:#{project.full_path}:push"] }
end
it_behaves_like 'a pushable'
it_behaves_like 'container repository factory'
end
end
end
end