Merge pull request #712 from phillbaker/bugfix/token-transactions

Wrap access token grant and refresh  in transactions.

NEWS.md

@@ -4,6 +4,8 @@ User-visible changes worth mentioning.
 
 ---
 
+- [#712] Wrap exchange of grant token for access token and access token refresh in transactions.
+
 ## 3.0.0
 
 ### Other changes

lib/doorkeeper/errors.rb

@@ -6,6 +6,12 @@ class DoorkeeperError < StandardError
     class InvalidAuthorizationStrategy < DoorkeeperError
     end
 
+    class InvalidTokenReuse < DoorkeeperError
+    end
+
+    class InvalidGrantReuse < DoorkeeperError
+    end
+
     class InvalidTokenStrategy < DoorkeeperError
     end
 

lib/doorkeeper/helpers/controller.rb

@@ -41,6 +41,10 @@ def get_error_response_from_exception(exception)
                        :unsupported_response_type
                      when Errors::MissingRequestStrategy
                        :invalid_request
+                     when Errors::InvalidTokenReuse
+                       :invalid_request
+                     when Errors::InvalidGrantReuse
+                       :invalid_grant
                      end
 
         OAuth::ErrorResponse.new name: error_name, state: params[:state]

lib/doorkeeper/oauth/authorization_code_request.rb

@@ -21,11 +21,16 @@ def initialize(server, grant, client, parameters = {})
       private
 
       def before_successful_response
-        grant.revoke
-        find_or_create_access_token(grant.application,
-                                    grant.resource_owner_id,
-                                    grant.scopes,
-                                    server)
+        grant.transaction do
+          grant.lock!
+          raise Errors::InvalidGrantReuse if grant.revoked?
+
+          grant.revoke
+          find_or_create_access_token(grant.application,
+                                      grant.resource_owner_id,
+                                      grant.scopes,
+                                      server)
+        end
       end
 
       def validate_attributes

lib/doorkeeper/oauth/refresh_token_request.rb

@@ -32,8 +32,13 @@ def initialize(server, refresh_token, credentials, parameters = {})
       attr_reader :refresh_token_parameter
 
       def before_successful_response
-        refresh_token.revoke
-        create_access_token
+        refresh_token.transaction do
+          refresh_token.lock!
+          raise Errors::InvalidTokenReuse if refresh_token.revoked?
+
+          refresh_token.revoke
+          create_access_token
+        end
       end
 
       def default_scopes

spec/requests/flows/authorization_code_spec.rb

@@ -128,3 +128,29 @@
     end
   end
 end
+
+describe 'Authorization Code Flow' do
+  before do
+    Doorkeeper.configure do
+      orm DOORKEEPER_ORM
+      use_refresh_token
+    end
+    client_exists
+  end
+
+  context 'issuing a refresh token' do
+    before do
+      authorization_code_exists application: @client
+    end
+
+    it 'second of simultaneous client requests get an error for revoked acccess token' do
+      authorization_code = Doorkeeper::AccessGrant.first.token
+      allow_any_instance_of(Doorkeeper::AccessGrant).to receive(:revoked?).and_return(false, true)
+
+      post token_endpoint_url(code: authorization_code, client: @client)
+
+      should_not_have_json 'access_token'
+      should_have_json 'error', 'invalid_grant'
+    end
+  end
+end

spec/requests/flows/refresh_token_spec.rb

@@ -65,6 +65,14 @@
       should_not_have_json 'refresh_token'
       should_have_json 'error', 'invalid_grant'
     end
+
+    it 'second of simultaneous client requests get an error for revoked acccess token' do
+      allow_any_instance_of(Doorkeeper::AccessToken).to receive(:revoked?).and_return(false, true)
+      post refresh_token_endpoint_url(client: @client, refresh_token: @token.refresh_token)
+
+      should_not_have_json 'refresh_token'
+      should_have_json 'error', 'invalid_request'
+    end
   end
 
   context 'refreshing the token with multiple sessions (devices)' do