Automatic merge of gitlab-org/gitlab-ce master

.gitlab/ci/qa.gitlab-ci.yml

@@ -19,6 +19,8 @@ package-and-qa-manual:
   except:
     refs:
       - master
+      - /(^docs[\/-].+|.+-docs$)/
+      - /(^qa[\/-].*|.*-qa$)/
   when: manual
   needs: ["build-qa-image", "gitlab:assets:compile pull-cache"]
 

lib/gitlab/jwt_authenticatable.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module JwtAuthenticatable
+    # Supposedly the effective key size for HMAC-SHA256 is 256 bits, i.e. 32
+    # bytes https://tools.ietf.org/html/rfc4868#section-2.6
+    SECRET_LENGTH = 32
+
+    def self.included(base)
+      base.extend(ClassMethods)
+    end
+
+    module ClassMethods
+      include Gitlab::Utils::StrongMemoize
+
+      def decode_jwt_for_issuer(issuer, encoded_message)
+        JWT.decode(
+          encoded_message,
+          secret,
+          true,
+          { iss: issuer, verify_iss: true, algorithm: 'HS256' }
+        )
+      end
+
+      def secret
+        strong_memoize(:secret) do
+          Base64.strict_decode64(File.read(secret_path).chomp).tap do |bytes|
+            raise "#{secret_path} does not contain #{SECRET_LENGTH} bytes" if bytes.length != SECRET_LENGTH
+          end
+        end
+      end
+
+      def write_secret
+        bytes = SecureRandom.random_bytes(SECRET_LENGTH)
+        File.open(secret_path, 'w:BINARY', 0600) do |f|
+          f.chmod(0600) # If the file already existed, the '0600' passed to 'open' above was a no-op.
+          f.write(Base64.strict_encode64(bytes))
+        end
+      end
+    end
+  end
+end

lib/gitlab/workhorse.rb

@@ -15,9 +15,7 @@ module Gitlab
     ALLOWED_GIT_HTTP_ACTIONS = %w[git_receive_pack git_upload_pack info_refs].freeze
     DETECT_HEADER = 'Gitlab-Workhorse-Detect-Content-Type'.freeze
 
-    # Supposedly the effective key size for HMAC-SHA256 is 256 bits, i.e. 32
-    # bytes https://tools.ietf.org/html/rfc4868#section-2.6
-    SECRET_LENGTH = 32
+    include JwtAuthenticatable
 
     class << self
       def git_http_ok(repository, repo_type, user, action, show_all_refs: false)
@@ -187,34 +185,12 @@ module Gitlab
         path.readable? ? path.read.chomp : 'unknown'
       end
 
-      def secret
-        @secret ||= begin
-          bytes = Base64.strict_decode64(File.read(secret_path).chomp)
-          raise "#{secret_path} does not contain #{SECRET_LENGTH} bytes" if bytes.length != SECRET_LENGTH
-
-          bytes
-        end
-      end
-
-      def write_secret
-        bytes = SecureRandom.random_bytes(SECRET_LENGTH)
-        File.open(secret_path, 'w:BINARY', 0600) do |f|
-          f.chmod(0600) # If the file already existed, the '0600' passed to 'open' above was a no-op.
-          f.write(Base64.strict_encode64(bytes))
-        end
-      end
-
       def verify_api_request!(request_headers)
         decode_jwt(request_headers[INTERNAL_API_REQUEST_HEADER])
       end
 
       def decode_jwt(encoded_message)
-        JWT.decode(
-          encoded_message,
-          secret,
-          true,
-          { iss: 'gitlab-workhorse', verify_iss: true, algorithm: 'HS256' }
-        )
+        decode_jwt_for_issuer('gitlab-workhorse', encoded_message)
       end
 
       def secret_path

spec/lib/gitlab/jwt_authenticatable_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::JwtAuthenticatable do
+  let(:test_class) do
+    Class.new do
+      include Gitlab::JwtAuthenticatable
+
+      def self.secret_path
+        Rails.root.join('tmp', 'tests', '.jwt_shared_secret')
+      end
+    end
+  end
+
+  before do
+    begin
+      File.delete(test_class.secret_path)
+    rescue Errno::ENOENT
+    end
+
+    test_class.write_secret
+  end
+
+  describe '.secret' do
+    subject(:secret) { test_class.secret }
+
+    it 'returns 32 bytes' do
+      expect(secret).to be_a(String)
+      expect(secret.length).to eq(32)
+      expect(secret.encoding).to eq(Encoding::ASCII_8BIT)
+    end
+
+    it 'accepts a trailing newline' do
+      File.open(test_class.secret_path, 'a') { |f| f.write "\n" }
+
+      expect(secret.length).to eq(32)
+    end
+
+    it 'raises an exception if the secret file cannot be read' do
+      File.delete(test_class.secret_path)
+
+      expect { secret }.to raise_exception(Errno::ENOENT)
+    end
+
+    it 'raises an exception if the secret file contains the wrong number of bytes' do
+      File.truncate(test_class.secret_path, 0)
+
+      expect { secret }.to raise_exception(RuntimeError)
+    end
+  end
+
+  describe '.write_secret' do
+    it 'uses mode 0600' do
+      expect(File.stat(test_class.secret_path).mode & 0777).to eq(0600)
+    end
+
+    it 'writes base64 data' do
+      bytes = Base64.strict_decode64(File.read(test_class.secret_path))
+
+      expect(bytes).not_to be_empty
+    end
+  end
+
+  describe '.decode_jwt_for_issuer' do
+    let(:payload) { { 'iss' => 'test_issuer' } }
+
+    it 'accepts a correct header' do
+      encoded_message = JWT.encode(payload, test_class.secret, 'HS256')
+
+      expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.not_to raise_error
+    end
+
+    it 'raises an error when the JWT is not signed' do
+      encoded_message = JWT.encode(payload, nil, 'none')
+
+      expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.to raise_error(JWT::DecodeError)
+    end
+
+    it 'raises an error when the header is signed with the wrong secret' do
+      encoded_message = JWT.encode(payload, 'wrongsecret', 'HS256')
+
+      expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.to raise_error(JWT::DecodeError)
+    end
+
+    it 'raises an error when the issuer is incorrect' do
+      payload['iss'] = 'somebody else'
+      encoded_message = JWT.encode(payload, test_class.secret, 'HS256')
+
+      expect { test_class.decode_jwt_for_issuer('test_issuer', encoded_message) }.to raise_error(JWT::DecodeError)
+    end
+  end
+end

spec/lib/gitlab/workhorse_spec.rb

@@ -200,57 +200,6 @@ describe Gitlab::Workhorse do
     end
   end
 
-  describe ".secret" do
-    subject { described_class.secret }
-
-    before do
-      described_class.instance_variable_set(:@secret, nil)
-      described_class.write_secret
-    end
-
-    it 'returns 32 bytes' do
-      expect(subject).to be_a(String)
-      expect(subject.length).to eq(32)
-      expect(subject.encoding).to eq(Encoding::ASCII_8BIT)
-    end
-
-    it 'accepts a trailing newline' do
-      File.open(described_class.secret_path, 'a') { |f| f.write "\n" }
-      expect(subject.length).to eq(32)
-    end
-
-    it 'raises an exception if the secret file cannot be read' do
-      File.delete(described_class.secret_path)
-      expect { subject }.to raise_exception(Errno::ENOENT)
-    end
-
-    it 'raises an exception if the secret file contains the wrong number of bytes' do
-      File.truncate(described_class.secret_path, 0)
-      expect { subject }.to raise_exception(RuntimeError)
-    end
-  end
-
-  describe ".write_secret" do
-    let(:secret_path) { described_class.secret_path }
-    before do
-      begin
-        File.delete(secret_path)
-      rescue Errno::ENOENT
-      end
-
-      described_class.write_secret
-    end
-
-    it 'uses mode 0600' do
-      expect(File.stat(secret_path).mode & 0777).to eq(0600)
-    end
-
-    it 'writes base64 data' do
-      bytes = Base64.strict_decode64(File.read(secret_path))
-      expect(bytes).not_to be_empty
-    end
-  end
-
   describe '#verify_api_request!' do
     let(:header_key) { described_class::INTERNAL_API_REQUEST_HEADER }
     let(:payload) { { 'iss' => 'gitlab-workhorse' } }