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

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

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)

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
 

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