diff options
author | Stan Hu <stanhu@gmail.com> | 2016-05-31 03:18:33 +0000 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-06-02 13:33:56 +0200 |
commit | b89d3faf55e605a8ffa46fbff6a12cb333dc1e11 (patch) | |
tree | df08ef6e28460785a33c97c1fa64e196ea401588 | |
parent | ad65b561931137b5ad8a55a2bfca6d24384b0572 (diff) | |
download | gitlab-ce-b89d3faf55e605a8ffa46fbff6a12cb333dc1e11.tar.gz |
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
-rw-r--r-- | CHANGELOG | 17 | ||||
-rw-r--r-- | app/controllers/jwt_controller.rb | 2 | ||||
-rw-r--r-- | app/services/auth/container_registry_authentication_service.rb | 4 | ||||
-rw-r--r-- | spec/services/auth/container_registry_authentication_service_spec.rb | 46 |
4 files changed, 36 insertions, 33 deletions
diff --git a/CHANGELOG b/CHANGELOG index 60e012917da..be9ac842e0e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,22 @@ 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 - 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 diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 156ab2811d6..cee3b6c43e7 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -32,7 +32,7 @@ class JwtController < ApplicationController end def auth_params - params.permit(:service, :scope, :offline_token, :account, :client_id) + params.permit(:service, :scope, :account, :client_id) end def authenticate_project(login, password) diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 2bbab643e69..5090bd8f6e6 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -5,9 +5,7 @@ module Auth def execute return error('not found', 404) unless registry.enabled - if params[:offline_token] - return error('unauthorized', 401) unless current_user || project - else + unless current_user || project return error('forbidden', 403) unless scope end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 3f4a1ced2b6..11858301844 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -14,7 +14,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do allow_any_instance_of(JSONWebToken::RSAToken).to receive(:key).and_return(rsa_key) end - shared_examples 'an authenticated' do + shared_examples 'a valid token' do it { is_expected.to include(:token) } it { expect(payload).to include('access') } end @@ -28,10 +28,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do }] end - it_behaves_like 'an authenticated' + it_behaves_like 'a valid token' it { expect(payload).to include('access' => access) } end + shared_examples 'an inaccessible' do + it_behaves_like 'a valid token' + it { expect(payload).to include('access' => []) } + end + shared_examples 'a pullable' do it_behaves_like 'a accessible' do let(:actions) { ['pull'] } @@ -50,11 +55,6 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do 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 it { is_expected.to include(http_status: 403) } it { is_expected.to_not include(:token) } @@ -75,12 +75,8 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do let(:project) { create(:project) } let(:current_user) { create(:user) } - context 'allow to use offline_token' do - let(:current_params) do - { offline_token: true } - end - - it_behaves_like 'an authenticated' + context 'allow to use scope-less authentication' do + it_behaves_like 'a valid token' end context 'allow developer to push images' do @@ -120,19 +116,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull,push" } end - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end context 'project authorization' do let(:current_project) { create(:empty_project) } - context 'allow to use offline_token' do - let(:current_params) do - { offline_token: true } - end - - it_behaves_like 'an authenticated' + context 'allow to use scope-less authentication' do + it_behaves_like 'a valid token' end context 'allow to pull and push images' do @@ -158,7 +150,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for private' do let(:project) { create(:empty_project, :private) } - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end @@ -169,7 +161,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for all' do let(:project) { create(:empty_project, :public) } - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end end @@ -184,18 +176,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull" } end - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end end context 'unauthorized' do - context 'disallow to use offline_token' do - let(:current_params) do - { offline_token: true } - end - - it_behaves_like 'an unauthorized' + context 'disallow to use scope-less authentication' do + it_behaves_like 'a forbidden' end context 'for invalid scope' do |