Merge branch '3868-reduce-the-scope-of-geo-jwt-json-web-tokens-step1' into 'master'

Reduce the scope of Geo JWT (JSON Web Tokens). Step 1

Closes #3868

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

doc/administration/geo/replication/security_review.md

@@ -265,6 +265,13 @@ questions from [owasp.org](https://www.owasp.org).
 ### What session management requirements have been defined?
 
 - Geo JWTs are defined to last for only two minutes before needing to be regenerated.
+- Geo JWTs are generated for one of the following specific scopes:
+  - Geo API access.
+  - Git access.
+  - LFS and File ID.
+  - Upload and File ID.
+  - Job Artifact and File ID.
+- Geo JWTs scopes are not enforced, but will be in a future version (currently scheduled for GitLab 11.9).
 
 ### What access requirements have been defined for URI and Service calls?
 

ee/app/services/geo/base_sync_service.rb

@@ -103,10 +103,14 @@ module Geo
 
     # Build a JWT header for authentication
     def jwt_authentication_header
-      authorization = ::Gitlab::Geo::RepoSyncRequest.new.authorization
+      authorization = ::Gitlab::Geo::RepoSyncRequest.new(scope: gl_repository).authorization
       { "http.#{remote_url}.extraHeader" => "Authorization: #{authorization}" }
     end
 
+    def gl_repository
+      "#{type}-#{project.id}"
+    end
+
     def remote_url
       Gitlab::Geo.primary_node.url + repository.full_path + '.git'
     end
@@ -126,7 +130,7 @@ module Geo
 
       temp_repo.create_from_snapshot(
         ::Gitlab::Geo.primary_node.snapshot_url(temp_repo),
-        ::Gitlab::Geo::RepoSyncRequest.new.authorization
+        ::Gitlab::Geo::RepoSyncRequest.new(scope: ::Gitlab::Geo::API_SCOPE).authorization
       )
     rescue => err
       log_error('Snapshot attempt failed', err)

ee/app/services/geo/node_status_post_service.rb

@@ -52,7 +52,7 @@ module Geo
     end
 
     def headers
-      Gitlab::Geo::BaseRequest.new.headers
+      Gitlab::Geo::BaseRequest.new(scope: ::Gitlab::Geo::API_SCOPE).headers
     end
 
     def timeout

ee/lib/gitlab/geo.rb

@@ -14,6 +14,8 @@ module Gitlab
       oauth_application
     ).freeze
 
+    API_SCOPE = 'geo_api'
+
     def self.current_node
       self.cache_value(:current_node, as: GeoNode) { GeoNode.current_node }
     end

ee/lib/gitlab/geo/file_transfer.rb

@@ -22,7 +22,9 @@ module Gitlab
         {
           id: upload.model_id,
           type: upload.model_type,
-          checksum: upload.checksum
+          checksum: upload.checksum,
+          file_type: @file_type,
+          file_id:  @file_id
         }
       end
     end

ee/lib/gitlab/geo/job_artifact_transfer.rb

@@ -17,7 +17,11 @@ module Gitlab
       private
 
       def job_artifact_request_data(job_artifact)
-        { id: @file_id }
+        {
+          id: @file_id,
+          file_type: @file_type,
+          file_id: @file_id
+        }
       end
     end
   end

ee/lib/gitlab/geo/lfs_transfer.rb

@@ -17,7 +17,11 @@ module Gitlab
       private
 
       def lfs_request_data(lfs_object)
-        { checksum: lfs_object.oid }
+        {
+          checksum: lfs_object.oid,
+          file_type: @file_type,
+          file_id:  @file_id
+        }
       end
     end
   end

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

@@ -15,7 +15,9 @@ describe Gitlab::Geo::FileTransfer do
         expect(Pathname.new(subject.filename).absolute?).to be_truthy
         expect(subject.request_data).to eq({ id: upload.model_id,
                                              type: 'User',
-                                             checksum: upload.checksum })
+                                             checksum: upload.checksum,
+                                             file_id: upload.id,
+                                             file_type: :file })
       end
     end
   end

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

@@ -18,7 +18,10 @@ describe Gitlab::Geo::JobArtifactTransfer, :geo do
     end
 
     it 'sets request_data with file_id and file_type' do
-      expect(described_class.new(job_artifact).request_data).to eq(id: job_artifact.id)
+      expect(described_class.new(job_artifact).request_data).to eq(
+        id: job_artifact.id,
+        file_id: job_artifact.id,
+        file_type: :job_artifact)
     end
   end
 end

ee/spec/requests/api/geo_spec.rb

@@ -13,6 +13,10 @@ describe API::Geo do
     { 'X-Gitlab-Token' => secondary_node.system_hook.token }
   end
 
+  let(:not_found_req_header) do
+    Gitlab::Geo::TransferRequest.new(transfer.request_data.merge(file_id: 100000)).headers
+  end
+
   shared_examples 'with terms enforced' do
     before do
       enforce_terms
@@ -62,7 +66,7 @@ describe API::Geo do
 
       context 'attachment does not exist' do
         it 'responds with 404' do
-          get api("/geo/transfers/attachment/100000"), headers: req_header
+          get api("/geo/transfers/attachment/100000"), headers: not_found_req_header
 
           expect(response).to have_gitlab_http_status(404)
         end
@@ -101,7 +105,7 @@ describe API::Geo do
 
       context 'avatar does not exist' do
         it 'responds with 404' do
-          get api("/geo/transfers/avatar/100000"), headers: req_header
+          get api("/geo/transfers/avatar/100000"), headers: not_found_req_header
 
           expect(response).to have_gitlab_http_status(404)
         end
@@ -154,7 +158,7 @@ describe API::Geo do
 
       context 'when the Upload record does not exist' do
         it 'responds with 404' do
-          get api("/geo/transfers/file/100000"), headers: req_header
+          get api("/geo/transfers/file/100000"), headers: not_found_req_header
 
           expect(response).to have_gitlab_http_status(404)
         end
@@ -163,10 +167,8 @@ describe API::Geo do
 
     describe 'GET /geo/transfers/lfs/1' do
       let(:lfs_object) { create(:lfs_object, :with_file) }
-      let(:req_header) do
-        transfer = Gitlab::Geo::LfsTransfer.new(lfs_object)
-        Gitlab::Geo::TransferRequest.new(transfer.request_data).headers
-      end
+      let(:transfer) { Gitlab::Geo::LfsTransfer.new(lfs_object) }
+      let(:req_header) { Gitlab::Geo::TransferRequest.new(transfer.request_data).headers }
 
       before do
         allow_any_instance_of(Gitlab::Geo::TransferRequest).to receive(:requesting_node).and_return(secondary_node)
@@ -207,7 +209,7 @@ describe API::Geo do
 
       context 'LFS object does not exist' do
         it 'responds with 404' do
-          get api("/geo/transfers/lfs/100000"), headers: req_header
+          get api("/geo/transfers/lfs/100000"), headers: not_found_req_header
 
           expect(response).to have_gitlab_http_status(404)
         end
@@ -216,7 +218,7 @@ describe API::Geo do
   end
 
   describe 'POST /geo/status', :postgresql do
-    let(:geo_base_request) { Gitlab::Geo::BaseRequest.new }
+    let(:geo_base_request) { Gitlab::Geo::BaseRequest.new(scope: ::Gitlab::Geo::API_SCOPE) }
 
     let(:data) do
       {

ee/spec/requests/api/project_snapshots_spec.rb

@@ -14,7 +14,7 @@ describe API::ProjectSnapshots do
     end
 
     it 'requests project repository raw archive from Geo primary as Geo secondary' do
-      req = Gitlab::Geo::BaseRequest.new
+      req = Gitlab::Geo::BaseRequest.new(scope: ::Gitlab::Geo::API_SCOPE)
       allow(req).to receive(:requesting_node) { secondary }
 
       get api("/projects/#{project.id}/snapshot", nil), params: {}, headers: req.headers

ee/spec/requests/git_http_geo_spec.rb

@@ -12,7 +12,7 @@ describe "Git HTTP requests (Geo)" do
   set(:secondary) { create(:geo_node) }
 
   # Ensure the token always comes from the real time of the request
-  let!(:auth_token) { Gitlab::Geo::BaseRequest.new.authorization }
+  let!(:auth_token) { Gitlab::Geo::BaseRequest.new(scope: "repository-#{project.id}").authorization }
   let!(:user) { create(:user) }
   let!(:user_without_any_access) { create(:user) }
   let!(:user_without_push_access) { create(:user) }