have redis check and then write nonce

fixes PLAT-1349

test plan:
regression check grade passback
try using the same nonce twice for grade passback
try using an expired oauth signature

Change-Id: I76ca22568451823c0ed9ae03e815873dda843883
Reviewed-on: https://gerrit.instructure.com/90006
Tested-by: Jenkins
Reviewed-by: Andrew Butterfield <abutterfield@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Nathan Mills <nathanm@instructure.com>

app/controllers/lti_api_controller.rb

@@ -164,8 +164,8 @@ class LtiApiController < ApplicationController
       Canvas::Errors::Reporter.raise_canvas_error(BasicLTI::BasicOutcomes::Unauthorized, "Timestamp too old or too far in the future, request has expired", oauth_error_info)
     end
 
-    nonce = @signature.request.nonce
-    unless Canvas::Redis.lock("nonce:#{@tool.asset_string}:#{nonce}", allowed_delta)
+    cache_key = "nonce:#{@tool.asset_string}:#{@signature.request.nonce}"
+    unless Lti::Security::check_and_store_nonce(cache_key, timestamp, allowed_delta.seconds)
       Canvas::Errors::Reporter.raise_canvas_error(BasicLTI::BasicOutcomes::Unauthorized, "Duplicate nonce detected", oauth_error_info)
     end
   end

lib/lti/message_authenticator.rb

@@ -31,9 +31,7 @@ module Lti
     def valid?
       @valid ||= begin
         valid = lti_message_authenticator.valid_signature?
-        valid &&= @params[:oauth_timestamp].to_i > NONCE_EXPIRATION.ago.to_i
-        valid &&= !Rails.cache.exist?(cache_key)
-        Rails.cache.write(cache_key, 'OK', expires_in: NONCE_EXPIRATION) if valid
+        valid &&= Security::check_and_store_nonce(cache_key, @params[:oauth_timestamp], NONCE_EXPIRATION)
         valid
       end
     end

lib/lti/security.rb

@@ -84,6 +84,23 @@ module Lti
     end
     private_class_method :generate_params_deprecated
 
+    ##
+    ## Timeline Examples for valid and invalid timestamps
+    ##
+    ## |---exp---timestamp---now---|  VALID
+    ##
+    ## |---timestamp---exp---now---| INVALID
+    ##
+    ## |---exp---now---timestamp---| INVALID
+    ##
+    def self.check_and_store_nonce(cache_key, timestamp, expiration)
+      valid = timestamp.to_i > expiration.ago.to_i
+      valid &&= timestamp.to_i <= Time.now.to_i
+      valid &&= !Rails.cache.exist?(cache_key)
+      Rails.cache.write(cache_key, 'OK', expires_in: expiration) if valid
+      valid
+    end
+
 
   end
 end

spec/apis/lti/grade_passback_spec.rb

@@ -120,11 +120,13 @@ describe LtiApiController, type: :request do
 
     if Canvas.redis_enabled?
       it "should not allow the same nonce to be used more than once" do
-        make_call('nonce' => 'not_so_random', 'content-type' => 'none')
-        assert_status(415)
-        make_call('nonce' => 'not_so_random', 'content-type' => 'none')
-        assert_status(401)
-        check_error_response("Duplicate nonce detected")
+        enable_cache do
+          make_call('nonce' => 'not_so_random', 'content-type' => 'none')
+          assert_status(415)
+          make_call('nonce' => 'not_so_random', 'content-type' => 'none')
+          assert_status(401)
+          check_error_response("Duplicate nonce detected")
+        end
       end
     end
 

spec/lib/lti/message_authenticator_spec.rb

@@ -121,7 +121,13 @@ module Lti
       end
 
       it 'rejects a message older than the NONCE_EXPIRATION' do
-        message.oauth_timestamp = (described_class::NONCE_EXPIRATION + 1.minute).ago.to_i
+        enable_cache do
+          validator = nil
+          Timecop.freeze((described_class::NONCE_EXPIRATION + 1.minute).ago) do
+            validator = described_class.new(launch_url, message.signed_post_params(tool.shared_secret))
+          end
+          expect(validator.valid?).to be false
+        end
       end
 
       it "doesn't store the nonce if the signature is invalid" do

spec/lib/lti/security_spec.rb

@@ -54,6 +54,34 @@ describe Lti::Security do
 
     end
 
+    context '.check_and_store_nonce' do
+      it 'rejects a used nonce' do
+        enable_cache do
+          cache_key = 'abcdefghijklmnopqrstuvwxyz'
+          timestamp = 1.minute.ago
+          expiration = 5.minutes
+          params = [cache_key, timestamp, expiration]
+          expect(Lti::Security.check_and_store_nonce(*params)).to be true
+          expect(Lti::Security.check_and_store_nonce(*params)).to be false
+        end
+      end
+
+      it 'rejects a nonce if the timestamp exceeds the expiration' do
+        cache_key = 'abcdefghijklmnopqrstuvwxyz'
+        expiration = 5.minutes
+        timestamp = (expiration + 1.minute).ago.to_i
+        expect(Lti::Security.check_and_store_nonce(cache_key, timestamp, expiration)).to be false
+      end
+
+      it 'rejects a nonce in the future' do
+        cache_key = 'abcdefghijklmnopqrstuvwxyz'
+        expiration = 5.minutes
+        timestamp = 5.minutes.from_now
+        expect(Lti::Security.check_and_store_nonce(cache_key, timestamp, expiration)).to be false
+      end
+
+    end
+
     it "generates a correct signature" do
       signed_params = Lti::Security.signed_post_params(params, launch_url, consumer_key, consumer_secret)