Commit b89d3faf authored by Stan Hu's avatar Stan Hu Committed by Yorick Peterse

Merge branch...

Merge branch 'make-container-registry-authentication-service-compatible-with-older-docker' into 'master'

Make authentication service for Container Registry to be compatible with < Docker 1.11

This removes the usage of `offline_token` which is only present when using `Docker 1.11.x` instead we relay on `scope`. This should make it compatible with any client starting from 1.6 (I did test only 1.8 and up).

Right now we return 403 if unauthorized user doesn't have access to anything. In all other cases we return token, but with empty `access`, which simply disallow requested action.

See merge request !4363
parent ad65b561
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.9.0 (unreleased)
- Allow forking projects with restricted visibility level
- Improve note validation to prevent errors when creating invalid note via API
- Redesign navigation for project pages
- Fix groups API to list only user's accessible projects
- Redesign account and email confirmation emails
- Use gitlab-shell v3.0.0
- Add DB index on users.state
- Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database
- Changed the Slack build message to use the singular duration if necessary (Aran Koning)
- Fix issues filter when ordering by milestone
- Todos will display target state if issuable target is 'Closed' or 'Merged'
- Remove 'main language' feature
- Projects pending deletion will render a 404 page
- Measure queue duration between gitlab-workhorse and Rails
- Make authentication service for Container Registry to be compatible with < Docker 1.11
v 8.8.3 v 8.8.3
- Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312 - Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312
- Fixed JS error when trying to remove discussion form. !4303 - Fixed JS error when trying to remove discussion form. !4303
......
...@@ -32,7 +32,7 @@ class JwtController < ApplicationController ...@@ -32,7 +32,7 @@ class JwtController < ApplicationController
end end
def auth_params def auth_params
params.permit(:service, :scope, :offline_token, :account, :client_id) params.permit(:service, :scope, :account, :client_id)
end end
def authenticate_project(login, password) def authenticate_project(login, password)
......
...@@ -5,9 +5,7 @@ module Auth ...@@ -5,9 +5,7 @@ module Auth
def execute def execute
return error('not found', 404) unless registry.enabled return error('not found', 404) unless registry.enabled
if params[:offline_token] unless current_user || project
return error('unauthorized', 401) unless current_user || project
else
return error('forbidden', 403) unless scope return error('forbidden', 403) unless scope
end end
......
...@@ -14,7 +14,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -14,7 +14,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
allow_any_instance_of(JSONWebToken::RSAToken).to receive(:key).and_return(rsa_key) allow_any_instance_of(JSONWebToken::RSAToken).to receive(:key).and_return(rsa_key)
end end
shared_examples 'an authenticated' do shared_examples 'a valid token' do
it { is_expected.to include(:token) } it { is_expected.to include(:token) }
it { expect(payload).to include('access') } it { expect(payload).to include('access') }
end end
...@@ -28,10 +28,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -28,10 +28,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
}] }]
end end
it_behaves_like 'an authenticated' it_behaves_like 'a valid token'
it { expect(payload).to include('access' => access) } it { expect(payload).to include('access' => access) }
end end
shared_examples 'an inaccessible' do
it_behaves_like 'a valid token'
it { expect(payload).to include('access' => []) }
end
shared_examples 'a pullable' do shared_examples 'a pullable' do
it_behaves_like 'a accessible' do it_behaves_like 'a accessible' do
let(:actions) { ['pull'] } let(:actions) { ['pull'] }
...@@ -50,11 +55,6 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -50,11 +55,6 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
end end
end end
shared_examples 'an unauthorized' do
it { is_expected.to include(http_status: 401) }
it { is_expected.to_not include(:token) }
end
shared_examples 'a forbidden' do shared_examples 'a forbidden' do
it { is_expected.to include(http_status: 403) } it { is_expected.to include(http_status: 403) }
it { is_expected.to_not include(:token) } it { is_expected.to_not include(:token) }
...@@ -75,12 +75,8 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -75,12 +75,8 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
context 'allow to use offline_token' do context 'allow to use scope-less authentication' do
let(:current_params) do it_behaves_like 'a valid token'
{ offline_token: true }
end
it_behaves_like 'an authenticated'
end end
context 'allow developer to push images' do context 'allow developer to push images' do
...@@ -120,19 +116,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -120,19 +116,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
{ scope: "repository:#{project.path_with_namespace}:pull,push" } { scope: "repository:#{project.path_with_namespace}:pull,push" }
end end
it_behaves_like 'a forbidden' it_behaves_like 'an inaccessible'
end end
end end
context 'project authorization' do context 'project authorization' do
let(:current_project) { create(:empty_project) } let(:current_project) { create(:empty_project) }
context 'allow to use offline_token' do context 'allow to use scope-less authentication' do
let(:current_params) do it_behaves_like 'a valid token'
{ offline_token: true }
end
it_behaves_like 'an authenticated'
end end
context 'allow to pull and push images' do context 'allow to pull and push images' do
...@@ -158,7 +150,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -158,7 +150,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
context 'disallow for private' do context 'disallow for private' do
let(:project) { create(:empty_project, :private) } let(:project) { create(:empty_project, :private) }
it_behaves_like 'a forbidden' it_behaves_like 'an inaccessible'
end end
end end
...@@ -169,7 +161,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -169,7 +161,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
context 'disallow for all' do context 'disallow for all' do
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
it_behaves_like 'a forbidden' it_behaves_like 'an inaccessible'
end end
end end
end end
...@@ -184,18 +176,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -184,18 +176,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
{ scope: "repository:#{project.path_with_namespace}:pull" } { scope: "repository:#{project.path_with_namespace}:pull" }
end end
it_behaves_like 'a forbidden' it_behaves_like 'an inaccessible'
end end
end end
end end
context 'unauthorized' do context 'unauthorized' do
context 'disallow to use offline_token' do context 'disallow to use scope-less authentication' do
let(:current_params) do it_behaves_like 'a forbidden'
{ offline_token: true }
end
it_behaves_like 'an unauthorized'
end end
context 'for invalid scope' do context 'for invalid scope' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment