Use Application#confidential? to determine revocation auth eligibility

OAuth applications that obtain an access token using the "implicit" grant flow will have their ID set on the token record. Unfortunately this causes the revocation controller code to think it's as confidential application. Because of this, Doorkeeper enforces oauth client authentication and the revocation call fails.

Fixes #891

Add specs for both public and confidential apps in revocation

app/controllers/doorkeeper/tokens_controller.rb

@@ -58,16 +58,15 @@ def introspect
     # https://tools.ietf.org/html/rfc6749#section-2.1
     # https://tools.ietf.org/html/rfc7009
     def authorized?
-      if token.present?
-        # Client is confidential, therefore client authentication & authorization
-        # is required
-        if token.application_id?
-          # We authorize client by checking token's application
-          server.client && server.client.application == token.application
-        else
-          # Client is public, authentication unnecessary
-          true
-        end
+      return unless token.present?
+      # Client is confidential, therefore client authentication & authorization
+      # is required
+      if token.application_id? && token.application.confidential?
+        # We authorize client by checking token's application
+        server.client && server.client.application == token.application
+      else
+        # Client is public, authentication unnecessary
+        true
       end
     end
 

spec/controllers/tokens_controller_spec.rb

@@ -59,15 +59,67 @@
     end
   end
 
-  describe 'when revoke authorization has failed' do
-    # http://tools.ietf.org/html/rfc7009#section-2.2
-    it 'returns no error response' do
-      token = double(:token, authorize: false, application_id?: true)
-      allow(controller).to receive(:token) { token }
+  # http://tools.ietf.org/html/rfc7009#section-2.2
+  describe 'revoking tokens' do
+    let(:client) { FactoryBot.create(:application) }
+    let(:access_token) { FactoryBot.create(:access_token, application: client) }
+
+    before(:each) do
+      allow(controller).to receive(:token) { access_token }
+    end
+
+    context 'when associated app is public' do
+      let(:client) { FactoryBot.create(:application, confidential: false) }
+
+      it 'returns 200' do
+        post :revoke
+
+        expect(response.status).to eq 200
+      end
+
+      it 'revokes the access token' do
+        post :revoke
+
+        expect(access_token.reload).to have_attributes(revoked?: true)
+      end
+    end
+
+    context 'when associated app is confidential' do
+      let(:client) { FactoryBot.create(:application, confidential: true) }
+      let(:oauth_client) { Doorkeeper::OAuth::Client.new(client) }
 
-      post :revoke
+      before(:each) do
+        allow_any_instance_of(Doorkeeper::Server).to receive(:client) { oauth_client }
+      end
+
+      it 'returns 200' do
+        post :revoke
+
+        expect(response.status).to eq 200
+      end
+
+      it 'revokes the access token' do
+        post :revoke
+
+        expect(access_token.reload).to have_attributes(revoked?: true)
+      end
+
+      context 'when authorization fails' do
+        let(:some_other_client) { FactoryBot.create(:application, confidential: true) }
+        let(:oauth_client) { Doorkeeper::OAuth::Client.new(some_other_client) }
+
+        it 'returns 200' do
+          post :revoke
 
-      expect(response.status).to eq 200
+          expect(response.status).to eq 200
+        end
+
+        it 'does not revoke the access token' do
+          post :revoke
+
+          expect(access_token.reload).to have_attributes(revoked?: false)
+        end
+      end
     end
   end