Merge branch 'tc-geo-ntp-time-error' into 'master'

More descriptive error when clocks between Geo nodes are out of sync

Closes #4276

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

changelogs/unreleased-ee/tc-geo-ntp-time-error.yml

+---
+title: More descriptive error when clocks between Geo nodes are out of sync
+merge_request: 3860
+author:
+type: changed

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

@@ -40,6 +40,8 @@ module EE
         render_bad_geo_auth('Bad token')
       rescue ::Gitlab::Geo::InvalidDecryptionKeyError
         render_bad_geo_auth("Invalid decryption key")
+      rescue ::Gitlab::Geo::InvalidSignatureTimeError
+        render_bad_geo_auth("Invalid signature time ")
       end
 
       def render_bad_geo_auth(message)

ee/lib/gitlab/geo/jwt_request_decoder.rb

 module Gitlab
   module Geo
     InvalidDecryptionKeyError = Class.new(StandardError)
+    InvalidSignatureTimeError = Class.new(StandardError)
 
     class JwtRequestDecoder
       include LogHelpers
@@ -55,6 +56,10 @@ module Gitlab
           data = JSON.parse(message['data']) if message
           data&.deep_symbolize_keys!
           data
+        rescue JWT::ImmatureSignature, JWT::ExpiredSignature
+          message = "Signature not within leeway of #{IAT_LEEWAY} seconds. Check your system clocks!"
+          log_error(message)
+          raise InvalidSignatureTimeError.new(message)
         rescue JWT::DecodeError => e
           log_error("Error decoding Geo request: #{e}")
           return

lib/api/geo.rb

@@ -49,7 +49,7 @@ module API
           unless auth_header && Gitlab::Geo::JwtRequestDecoder.new(auth_header).decode
             unauthorized!
           end
-        rescue Gitlab::Geo::InvalidDecryptionKeyError => e
+        rescue Gitlab::Geo::InvalidDecryptionKeyError, Gitlab::Geo::SignatureTimeInvalidError => e
           render_api_error!(e.to_s, 401)
         end
       end

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

@@ -33,16 +33,16 @@ describe Gitlab::Geo::JwtRequestDecoder do
       Timecop.travel(30.seconds.ago) { expect(subject.decode).to eq(data) }
     end
 
-    it 'fails to decode after expiring' do
+    it 'raises InvalidSignatureTimeError after expiring' do
       subject
 
-      Timecop.travel(2.minutes) { expect(subject.decode).to be_nil }
+      Timecop.travel(2.minutes) { expect { subject.decode }.to raise_error(Gitlab::Geo::InvalidSignatureTimeError) }
     end
 
-    it 'fails to decode when clocks are not in sync' do
+    it 'raises InvalidSignatureTimeError to decode when clocks are not in sync' do
       subject
 
-      Timecop.travel(2.minutes.ago) { expect(subject.decode).to be_nil }
+      Timecop.travel(2.minutes.ago) { expect { subject.decode }.to raise_error(Gitlab::Geo::InvalidSignatureTimeError) }
     end
 
     it 'raises invalid decryption key error' do