Merge pull request #1744 from ransombriggs/allow-refresh-tokens-to-be…

…-revoked-after-expire

Allow for expired refresh tokens to be revoked

CHANGELOG.md

@@ -11,6 +11,7 @@ Add your entry here.
 
 - [#1752] Bump the range of supported Ruby and Rails versions
 - [#1747] Fix unknown pkce method error when configured
+- [#1744] Allow for expired refresh tokens to be revoked
 
 ## 5.8.0
 

app/controllers/doorkeeper/tokens_controller.rb

@@ -113,19 +113,38 @@ def revoke_token
       # The authorization server responds with HTTP status code 200 if the token
       # has been revoked successfully or if the client submitted an invalid
       # token
-      token.revoke if token&.accessible?
+      revocable_token.revoke if revocable_token.revocable?
     end
 
     def token
-      @token ||=
+      revocable_token&.token
+    end
+
+    def revocable_token
+      return @revocable_token if defined? @revocable_token
+
+      @revocable_token =
         if params[:token_type_hint] == "refresh_token"
-          Doorkeeper.config.access_token_model.by_refresh_token(params["token"])
+          refresh_token
         else
-          Doorkeeper.config.access_token_model.by_token(params["token"]) ||
-            Doorkeeper.config.access_token_model.by_refresh_token(params["token"])
+          access_token || refresh_token
         end
     end
 
+    def refresh_token
+      token = Doorkeeper.config.access_token_model.by_refresh_token(params["token"])
+      return unless token
+
+      RevocableTokens::RevocableRefreshToken.new(token)
+    end
+
+    def access_token
+      token = Doorkeeper.config.access_token_model.by_token(params["token"])
+      return unless token
+
+      RevocableTokens::RevocableAccessToken.new(token)
+    end
+
     def strategy
       @strategy ||= server.token_request(params[:grant_type])
     end

lib/doorkeeper.rb

@@ -34,6 +34,11 @@ module Request
     autoload :Token, "doorkeeper/request/token"
   end
 
+  module RevocableTokens
+    autoload :RevocableAccessToken, "doorkeeper/revocable_tokens/revocable_access_token"
+    autoload :RevocableRefreshToken, "doorkeeper/revocable_tokens/revocable_refresh_token"
+  end
+
   module OAuth
     autoload :BaseRequest, "doorkeeper/oauth/base_request"
     autoload :AuthorizationCodeRequest, "doorkeeper/oauth/authorization_code_request"

lib/doorkeeper/revocable_tokens/revocable_access_token.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module RevocableTokens
+    class RevocableAccessToken
+      attr_reader :token
+
+      def initialize(token)
+        @token = token
+      end
+
+      def revocable?
+        token.accessible?
+      end
+
+      def revoke
+        token.revoke
+      end
+    end
+  end
+end

lib/doorkeeper/revocable_tokens/revocable_refresh_token.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module RevocableTokens
+    class RevocableRefreshToken
+      attr_reader :token
+
+      def initialize(token)
+        @token = token
+      end
+
+      def revocable?
+        !token.revoked?
+      end
+
+      def revoke
+        token.revoke
+      end
+    end
+  end
+end

spec/controllers/tokens_controller_spec.rb

@@ -172,7 +172,15 @@
   # https://datatracker.ietf.org/doc/html/rfc7009#section-2.2
   describe "POST #revoke" do
     let(:client) { FactoryBot.create(:application) }
-    let(:access_token) { FactoryBot.create(:access_token, application: client) }
+    let(:revoked_at) { nil }
+    let(:access_token) do
+      FactoryBot.create(
+        :access_token,
+        application: client,
+        revoked_at: revoked_at,
+        use_refresh_token: true
+      )
+    end
 
     context "when associated app is public" do
       let(:client) { FactoryBot.create(:application, confidential: false) }
@@ -183,8 +191,89 @@
         expect(response.status).to eq 200
       end
 
-      it "revokes the access token" do
-        post :revoke, params: { client_id: client.uid, token: access_token.token }
+      it "does not revoke the access token when token_type_hint == refresh_token" do
+        post :revoke, params: {
+          client_id: client.uid,
+          token: access_token.token,
+          token_type_hint: "refresh_token",
+        }
+
+        expect(response.status).to eq 200
+
+        expect(access_token.reload).to have_attributes(revoked?: false)
+      end
+
+      it "revokes the refresh token when token_type_hint == refresh_token" do
+        post :revoke, params: {
+          client_id: client.uid,
+          token: access_token.refresh_token,
+          token_type_hint: "refresh_token",
+        }
+
+        expect(response.status).to eq 200
+
+        expect(access_token.reload).to have_attributes(revoked?: true)
+      end
+
+      it "revokes the refresh token when token_type_hint not passed" do
+        post :revoke, params: {
+          client_id: client.uid,
+          token: access_token.refresh_token,
+        }
+
+        expect(response.status).to eq 200
+
+        expect(access_token.reload).to have_attributes(revoked?: true)
+      end
+
+      context "when access_token has already been revoked" do
+        let(:revoked_at) { 1.day.ago.floor }
+
+        it "does not update the revoked_at when the access token has already been revoked" do
+          post :revoke, params: {
+            client_id: client.uid,
+            token: access_token.token,
+          }
+
+          expect(response.status).to eq 200
+
+          expect(access_token.reload).to have_attributes(revoked_at: revoked_at)
+        end
+
+        it "does not update the revoked_at when the refresh token has already been revoked" do
+          post :revoke, params: {
+            client_id: client.uid,
+            token: access_token.refresh_token,
+          }
+
+          expect(response.status).to eq 200
+
+          expect(access_token.reload).to have_attributes(revoked_at: revoked_at)
+        end
+      end
+
+      it "does not revoke when the access token has expired" do
+        access_token.update!(created_at: access_token.created_at - access_token.expires_in - 1)
+
+        post :revoke, params: {
+          client_id: client.uid,
+          token: access_token.token,
+        }
+
+        expect(response.status).to eq 200
+
+        expect(access_token.reload).to have_attributes(revoked?: false)
+      end
+
+      it "revokes the refresh token after the access token has expired" do
+        access_token.update!(created_at: access_token.created_at - access_token.expires_in - 1)
+
+        post :revoke, params: {
+          client_id: client.uid,
+          token: access_token.refresh_token,
+        }
+
+        expect(response.status).to eq 200
 
         expect(access_token.reload).to have_attributes(revoked?: true)
       end