From 7ec1fa212d23911792674e947863f3e71f91834f Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 30 May 2016 16:57:39 +0200 Subject: [PATCH] Make authentication service for Container Registry to be compatible with < Docker 1.11 --- CHANGELOG | 1 + app/controllers/jwt_controller.rb | 2 +- ...ntainer_registry_authentication_service.rb | 4 +- ...er_registry_authentication_service_spec.rb | 46 +++++++------------ 4 files changed, 20 insertions(+), 33 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3e8b0a2af01..a32fa90453e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ v 8.9.0 (unreleased) - 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 gitlab importer failing to import new projects due to missing credentials 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 98ef9d21035..2d114f59ca4 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.not_to include(:token) } - end - shared_examples 'a forbidden' do it { is_expected.to include(http_status: 403) } it { is_expected.not_to 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