Merge pull request #1778 from hannahyeates/fix-make-token-revocation-idempotent

fix: ensure that token revocation is idempotent

Author: nbulaj

CHANGELOG.md

@@ -9,6 +9,7 @@ User-visible changes worth mentioning.
 
 Add your entry here.
 - [#1775] Fix Applications Secret Not Null Constraint generator
+- [#1778] Ensure that token revocation is idempotent by checking that that token has not already been revoked before revoking.
 
 ## 5.8.2
 

lib/doorkeeper/models/concerns/revocable.rb

@@ -9,6 +9,7 @@ module Revocable
       # @param clock [Time] time object
       #
       def revoke(clock = Time)
+        return if revoked?
         update_attribute(:revoked_at, clock.now.utc)
       end
 

spec/lib/models/revocable_spec.rb

@@ -10,12 +10,26 @@
   end
 
   describe "#revoke" do
+    let(:revoked_at) { nil }
+
+    before do
+      allow(fake_object).to receive(:revoked_at).and_return(revoked_at)
+    end
+
     it "updates :revoked_at attribute with current time" do
       utc = double utc: double
       clock = double now: utc
       expect(fake_object).to receive(:update_attribute).with(:revoked_at, clock.now.utc)
       fake_object.revoke(clock)
     end
+
+    context "when the object is already revoked" do
+      let(:revoked_at) { Time.now.utc - 1000 }
+
+      it "does not update :revoked_at attribute" do
+        expect(fake_object).not_to receive(:update_attribute)
+      end
+    end
   end
 
   describe "#revoked?" do