Merge branch 'docker-registry' into docker-registry-view

3 files changed, 59 insertions, 18 deletions

diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 9bfc1085fb9..a0cda4adc56 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -7,10 +7,10 @@ module Auth
 
       if params[:offline_token]
         return error('forbidden', 403) unless current_user
+      else
+        return error('forbidden', 401) unless scope
       end
 
-      return error('forbidden', 401) unless scope
-
       { token: authorized_token(scope).encoded }
     end
 
@@ -32,7 +32,7 @@ module Auth
       token.issuer = registry.issuer
       token.audience = params[:service]
       token.subject = current_user.try(:username)
-      token[:access] = accesses
+      token[:access] = accesses.compact
       token
     end
 
diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb
index 4462cdde9a3..0c3d3ea7019 100644
--- a/spec/lib/json_web_token/rsa_token_spec.rb
+++ b/spec/lib/json_web_token/rsa_token_spec.rb
@@ -1,5 +1,17 @@
 describe JSONWebToken::RSAToken do
-  let(:rsa_key) { generate_key }
+  let(:rsa_key) do
+    OpenSSL::PKey::RSA.new <<-eos.strip_heredoc
+      -----BEGIN RSA PRIVATE KEY-----
+      MIIBOgIBAAJBAMA5sXIBE0HwgIB40iNidN4PGWzOyLQK0bsdOBNgpEXkDlZBvnak
+      OUgAPF+rME4PB0Yl415DabUI40T5UNmlwxcCAwEAAQJAZtY2pSwIFm3JAXIh0cZZ
+      iXcAfiJ+YzuqinUOS+eW2sBCAEzjcARlU/o6sFQgtsOi4FOMczAd1Yx8UDMXMmrw
+      2QIhAPBgVhJiTF09pdmeFWutCvTJDlFFAQNbrbo2X2x/9WF9AiEAzLgqMKeStSRu
+      H9N16TuDrUoO8R+DPqriCwkKrSHaWyMCIFzMhE4inuKcSywBaLmiG4m3GQzs++Al
+      A6PRG/PSTpQtAiBxtBg6zdf+JC3GH3zt/dA0/10tL4OF2wORfYQghRzyYQIhAL2l
+      0ZQW+yLIZAGrdBFWYEAa52GZosncmzBNlsoTgwE4
+      -----END RSA PRIVATE KEY-----
+    eos
+  end
   let(:rsa_token) { described_class.new(nil) }
   let(:rsa_encoded) { rsa_token.encoded }
 
@@ -13,19 +25,19 @@ describe JSONWebToken::RSAToken do
 
       it { expect{subject}.to_not raise_error }
       it { expect(subject.first).to include('key' => 'value') }
+      it do
+        expect(subject.second).to eq(
+          "typ" => "JWT",
+          "alg" => "RS256",
+          "kid" => "OGXY:4TR7:FAVO:WEM2:XXEW:E4FP:TKL7:7ACK:TZAF:D54P:SUIA:P3B2")
+      end
     end
 
     context 'for invalid key to raise an exception' do
-      let(:new_key) { generate_key }
+      let(:new_key) { OpenSSL::PKey::RSA.generate(512) }
       subject { JWT.decode(rsa_encoded, new_key) }
 
       it { expect{subject}.to raise_error(JWT::DecodeError) }
     end
   end
-
-  private
-
-  def generate_key
-    OpenSSL::PKey::RSA.generate(512)
-  end
 end
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index bae576f1670..409d08e49f1 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -57,15 +57,28 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
     end
   end
 
-  shared_examples 'a forbidden' do
+  shared_examples 'a 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) }
+  end
+
   context 'user authorization' 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'
+    end
+
     context 'allow developer to push images' do
       before { project.team << [current_user, :developer] }
 
@@ -103,13 +116,21 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         { scope: "repository:#{project.path_with_namespace}:pull,push" }
       end
 
-      it_behaves_like 'a forbidden'
+      it_behaves_like 'a unauthorized'
     end
   end
 
   context 'project authorization' do
     let(:current_project) { create(:empty_project) }
 
+    context 'disallow to use offline_token' do
+      let(:current_params) do
+        { offline_token: true }
+      end
+
+      it_behaves_like 'a forbidden'
+    end
+
     context 'allow to pull and push images' do
       let(:current_params) do
         { scope: "repository:#{current_project.path_with_namespace}:pull,push" }
@@ -133,7 +154,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 'a unauthorized'
         end
       end
 
@@ -144,7 +165,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 'a unauthorized'
         end
       end
     end
@@ -165,12 +186,20 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
   end
 
   context 'unauthorized' do
+    context 'disallow to use offline_token' do
+      let(:current_params) do
+        { offline_token: true }
+      end
+
+      it_behaves_like 'a forbidden'
+    end
+
     context 'for invalid scope' do
       let(:current_params) do
         { scope: 'invalid:aa:bb' }
       end
 
-      it_behaves_like 'a forbidden'
+      it_behaves_like 'a unauthorized'
     end
 
     context 'for private project' do
@@ -180,7 +209,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         { scope: "repository:#{project.path_with_namespace}:pull" }
       end
 
-      it_behaves_like 'a forbidden'
+      it_behaves_like 'a unauthorized'
     end
 
     context 'for public project' do
@@ -199,7 +228,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
           { scope: "repository:#{project.path_with_namespace}:push" }
         end
 
-        it_behaves_like 'a forbidden'
+        it_behaves_like 'a unauthorized'
       end
     end
   end