Merge branch 'fix_geo_ssh_push_proxy' into 'master'

Geo: Fix SSH push proxy

Closes gitlab-org/quality/nightly#77 and gitlab-org/quality/nightly#78

See merge request gitlab-org/gitlab-ee!9597

doc/administration/geo/replication/security_review.md

@@ -271,6 +271,7 @@ questions from [owasp.org](https://www.owasp.org).
   - LFS and File ID.
   - Upload and File ID.
   - Job Artifact and File ID.
+- Geo JWTs scopes are not enforced for Git Access yet, but will be in a future version (currently scheduled for GitLab 11.10).
 
 ### What access requirements have been defined for URI and Service calls?
 

ee/app/controllers/ee/projects/git_http_controller.rb

@@ -48,7 +48,6 @@ module EE
       def authenticate_user
         return super unless geo_request?
         return render_bad_geo_auth('Bad token') unless decoded_authorization
-        return render_bad_geo_auth('Unauthorized scope') unless jwt_scope_valid?
 
         # grant access
         @authentication_result = ::Gitlab::Auth::Result.new(nil, project, :geo, [:download_code, :push_code]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
@@ -58,14 +57,6 @@ module EE
         render_bad_geo_auth("Invalid signature time ")
       end
 
-      def jwt_scope_valid?
-        decoded_authorization[:scope] == ::Gitlab::Geo::JwtRequestDecoder.build_repository_scope(repository_type, project.id)
-      end
-
-      def repository_type
-        wiki? ? 'wiki' : 'repository'
-      end
-
       def decoded_authorization
         strong_memoize(:decoded_authorization) do
           ::Gitlab::Geo::JwtRequestDecoder.new(request.headers['Authorization']).decode

ee/app/services/geo/base_sync_service.rb

@@ -104,7 +104,7 @@ module Geo
     # Build a JWT header for authentication
     def jwt_authentication_header
       authorization = ::Gitlab::Geo::RepoSyncRequest.new(
-        scope: ::Gitlab::Geo::JwtRequestDecoder.build_repository_scope(type, project.id)
+        scope: project.repository.full_path
       ).authorization
 
       { "http.#{remote_url}.extraHeader" => "Authorization: #{authorization}" }

ee/changelogs/unreleased/9310-reduce-the-scope-of-geo-jwt-step-2.yml

 ---
-title: Enforce Geo JWT tokens scope
+title: Enforce Geo JWT tokens scope for file uploads and Geo API
 merge_request: 9502
 author:
 type: changed

ee/lib/gitlab/geo/git_push_ssh_proxy.rb

@@ -105,10 +105,14 @@ module Gitlab
       def base_headers
         @base_headers ||= {
           'Geo-GL-Id' => gl_id,
-          'Authorization' => Gitlab::Geo::BaseRequest.new.authorization
+          'Authorization' => Gitlab::Geo::BaseRequest.new(scope: auth_scope).authorization
         }
       end
 
+      def auth_scope
+        URI.parse(primary_repo).path.gsub(%r{^\/|\.git$}, '')
+      end
+
       def get(url, headers)
         request(url, Net::HTTP::Get, headers)
       end

ee/lib/gitlab/geo/jwt_request_decoder.rb

@@ -12,10 +12,6 @@ module Gitlab
         token_type == ::Gitlab::Geo::BaseRequest::GITLAB_GEO_AUTH_TOKEN_TYPE
       end
 
-      def self.build_repository_scope(repository_type, project_id)
-        [repository_type, project_id].join('-')
-      end
-
       attr_reader :auth_header
 
       def initialize(auth_header)

ee/spec/lib/gitlab/geo/git_push_ssh_proxy_spec.rb

@@ -69,6 +69,20 @@ describe Gitlab::Geo::GitPushSSHProxy, :geo do
         let(:info_refs_headers) { base_headers.merge('Content-Type' => 'application/x-git-upload-pack-request') }
         let(:info_refs_http_body_full) { "001f# service=git-receive-pack\n0000#{info_refs_body_short}" }
 
+        context 'authorization header is scoped' do
+          it 'passes the scope when .info_refs is called' do
+            expect(Gitlab::Geo::BaseRequest).to receive(:new).with(scope: project.repository.full_path)
+
+            subject.info_refs
+          end
+
+          it 'passes the scope when .push is called' do
+            expect(Gitlab::Geo::BaseRequest).to receive(:new).with(scope: project.repository.full_path)
+
+            subject.push(info_refs_body_short)
+          end
+        end
+
         context 'with a failed response' do
           let(:error_msg) { 'execution expired' }
 

ee/spec/lib/gitlab/geo/jwt_request_decoder_spec.rb

@@ -7,12 +7,6 @@ describe Gitlab::Geo::JwtRequestDecoder do
 
   subject { described_class.new(request.headers['Authorization']) }
 
-  describe ".build_repository_scope" do
-    it 'returns a scope that consolidates repository type and project id' do
-      expect(described_class.build_repository_scope('wiki', 5)).to eq('wiki-5')
-    end
-  end
-
   describe '#decode' do
     it 'decodes correct data' do
       expect(subject.decode).to eq(data)

ee/spec/requests/git_http_geo_spec.rb

@@ -21,7 +21,6 @@ describe "Git HTTP requests (Geo)" do
   let!(:key_for_user_without_push_access) { create(:key, user: user_without_push_access) }
 
   let(:env) { valid_geo_env }
-  let(:auth_token_with_invalid_scope) { Gitlab::Geo::BaseRequest.new(scope: "invalid-#{project.id}").authorization }
 
   before do
     project.add_maintainer(user)
@@ -347,50 +346,6 @@ describe "Git HTTP requests (Geo)" do
         end
       end
     end
-
-    context 'invalid scope' do
-      let(:repository_path) { project.full_path }
-
-      subject do
-        make_request
-        response
-      end
-
-      def make_request
-        get "/#{repository_path}.git/info/refs", params: { service: 'git-upload-pack' }, headers: env
-      end
-
-      shared_examples_for 'unauthorized because of invalid scope' do
-        it { is_expected.to have_gitlab_http_status(:unauthorized) }
-
-        it 'returns correct error' do
-          expect(subject.parsed_body).to eq('Geo JWT authentication failed: Unauthorized scope')
-        end
-      end
-
-      context 'invalid scope of Geo JWT token' do
-        let(:env) { geo_env(auth_token_with_invalid_scope) }
-
-        include_examples 'unauthorized because of invalid scope'
-      end
-
-      context 'Geo JWT token scopes for wiki and repository are not interchangeable' do
-        context 'wiki scope' do
-          let(:auth_token_with_valid_wiki_scope) { Gitlab::Geo::BaseRequest.new(scope: "wiki-#{project.id}").authorization }
-          let(:env) { geo_env(auth_token_with_valid_wiki_scope) }
-
-          include_examples 'unauthorized because of invalid scope'
-        end
-
-        context 'respository scope' do
-          let(:repository_path) { project.wiki.full_path }
-          let(:auth_token_with_valid_repository_scope) { Gitlab::Geo::BaseRequest.new(scope: "repository-#{project.id}").authorization }
-          let(:env) { geo_env(auth_token_with_valid_repository_scope) }
-
-          include_examples 'unauthorized because of invalid scope'
-        end
-      end
-    end
   end
 
   def valid_geo_env