don't refresh oauth token if recently changed

closes #CORE-1113

Change-Id: Id9ba35e2f0f4b4fc710a84d8c94d20b315bee1ab
Reviewed-on: https://gerrit.instructure.com/175522
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>

lib/canvas/oauth/grant_types/refresh_token.rb

@@ -15,9 +15,20 @@ module Canvas::Oauth
       end
 
       def generate_token
-        @_token.access_token.regenerate_access_token
+        # don't regenerate if recently changed
+        if update_recently_refreshed_tokens? || @_token.access_token.updated_at < recent_refresh_threshold
+          @_token.access_token.regenerate_access_token
+        end
         @_token
       end
+
+      def update_recently_refreshed_tokens?
+        Rails.env.test? # keep old behavior only for specs
+      end
+
+      def recent_refresh_threshold
+        Setting.get("oauth_refresh_token_spam_interval", "5").to_i.seconds.ago
+      end
     end
   end
 end

spec/controllers/oauth2_provider_controller_spec.rb

@@ -480,17 +480,41 @@ describe Oauth2ProviderController do
         expect(response.body).to match(/incorrect client/)
       end
 
-      it 'should be able to regenerate access_token multiple times' do
-        post :token, params: base_params.merge(refresh_token: refresh_token)
-        expect(response).to be_successful
-        json = JSON.parse(response.body)
-        expect(json['access_token']).to_not eq old_token.full_token
+      context "multiple refreshes" do
+        before :each do
+          allow_any_instance_of(Canvas::Oauth::GrantTypes::RefreshToken).
+            to receive(:update_recently_refreshed_tokens?).and_return(false)
+        end
 
-        access_token = json['access_token']
-        post :token, params: base_params.merge(refresh_token: refresh_token)
-        expect(response).to be_successful
-        json = JSON.parse(response.body)
-        expect(json['access_token']).to_not eq access_token
+        it 'should be able to regenerate access_token multiple times (with enough time in between)' do
+          post :token, params: base_params.merge(refresh_token: refresh_token)
+          expect(response).to be_successful
+          json = JSON.parse(response.body)
+          expect(json['access_token']).to_not eq old_token.full_token
+
+          Timecop.freeze(1.minute.from_now) do
+            access_token = json['access_token']
+            post :token, params: base_params.merge(refresh_token: refresh_token)
+            expect(response).to be_successful
+            json = JSON.parse(response.body)
+            expect(json['access_token']).to_not eq access_token
+          end
+        end
+
+        it 'should not be able to regenerate access_token multiple times in short succession' do
+          post :token, params: base_params.merge(refresh_token: refresh_token)
+          expect(response).to be_successful
+          json = JSON.parse(response.body)
+          expect(json['access_token']).to_not eq old_token.full_token
+
+          Timecop.freeze(old_token.reload.updated_at + 1.second) do
+            access_token = json['access_token']
+            post :token, params: base_params.merge(refresh_token: refresh_token)
+            expect(response).to be_successful
+            json = JSON.parse(response.body)
+            expect(json['access_token']).to eq access_token # didn't change
+          end
+        end
       end
     end