Merge pull request #79 from zaikio/bugfix/refreshing

Always delete refreshed tokens and add .find_usable_access_token function

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.8.0] - 2021-03-30
+
+* Always destroy old access token after successful Hub API call in `Zaikio::AccessToken#refresh!` and return `nil` if refreshing fails.
+* Add `.find_usable_access_token` helper method to get a token without making a Hub API call to refresh it
+
 ## 0.7.2 - 2021-03-25
 
 * Replace dependency on `rails` with a more specific dependency on `railties` and friends
@@ -47,3 +52,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## 0.4.2 - 2021-01-11
 
 * Fix compatibility issues with Ruby 3.0
+
+[Unreleased]: https://github.com/zaikio/zaikio-oauth_client/compare/v0.8.0..HEAD
+[0.8.0]: https://github.com/zaikio/zaikio-oauth_client/compare/v0.7.2..v0.8.0

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    zaikio-oauth_client (0.7.2)
+    zaikio-oauth_client (0.8.0)
       actionpack (>= 5.0.0)
       activerecord (>= 5.0.0)
       activesupport (>= 5.0.0)

app/models/zaikio/access_token.rb

@@ -78,15 +78,17 @@ def refresh!
           attributes.slice("token", "refresh_token")
         ).refresh!
 
-        access_token = self.class.build_from_access_token(
+        destroy
+
+        self.class.build_from_access_token(
           refreshed_token,
           requested_scopes: requested_scopes
-        )
-
-        transaction { destroy if access_token.save! }
-
-        access_token
+        ).tap(&:save!)
       end
+    rescue OAuth2::Error => e
+      raise unless e.code == "invalid_grant"
+
+      nil
     end
   end
 end

lib/zaikio/oauth_client.rb

@@ -57,34 +57,50 @@ def with_auth(options_or_access_token, &block)
         end
       end
 
-      def get_access_token(client_name: nil, bearer_type: "Person", bearer_id: nil, scopes: nil) # rubocop:disable Metrics/MethodLength
-        client_name ||= self.client_name
-        client_config = client_config_for(client_name)
+      # Finds the best possible access token, using the DB or an API call
+      #   * If the token has expired, it will be refreshed using the refresh_token flow
+      #     (if this fails, we fallback to getting a new token using client_credentials)
+      #   * If the token does not exist, we'll get a new one using the client_credentials flow
+      def get_access_token(bearer_id:, client_name: nil, bearer_type: "Person", scopes: nil)
+        client_config = client_config_for(client_name || self.client_name)
         scopes ||= client_config.default_scopes_for(bearer_type)
 
-        access_token = Zaikio::AccessToken.where(audience: client_config.client_name)
-                                          .usable(
-                                            bearer_type: bearer_type,
-                                            bearer_id: bearer_id,
-                                            requested_scopes: scopes
-                                          )
-                                          .first
-
-        if access_token.blank?
-          access_token = Zaikio::AccessToken.build_from_access_token(
-            client_config.token_by_client_credentials(
-              bearer_type: bearer_type,
-              bearer_id: bearer_id,
-              scopes: scopes
-            ),
-            requested_scopes: scopes
+        token = find_usable_access_token(client_name: client_config.client_name,
+                                         bearer_type: bearer_type,
+                                         bearer_id: bearer_id,
+                                         requested_scopes: scopes)
+
+        token = token.refresh! if token&.expired?
+
+        token ||= fetch_new_token(client_config: client_config,
+                                  bearer_type: bearer_type,
+                                  bearer_id: bearer_id,
+                                  scopes: scopes)
+        token
+      end
+
+      # Finds the best usable access token. Note that this token may have expired and
+      # would require refreshing.
+      def find_usable_access_token(client_name:, bearer_type:, bearer_id:, requested_scopes:)
+        Zaikio::AccessToken
+          .where(audience: client_name)
+          .usable(
+            bearer_type: bearer_type,
+            bearer_id: bearer_id,
+            requested_scopes: requested_scopes
           )
-          access_token.save!
-        elsif access_token&.expired?
-          access_token = access_token.refresh!
-        end
+          .first
+      end
 
-        access_token
+      def fetch_new_token(client_config:, bearer_type:, bearer_id:, scopes:)
+        Zaikio::AccessToken.build_from_access_token(
+          client_config.token_by_client_credentials(
+            bearer_type: bearer_type,
+            bearer_id: bearer_id,
+            scopes: scopes
+          ),
+          requested_scopes: scopes
+        ).tap(&:save!)
       end
 
       def get_plain_scopes(scopes)

lib/zaikio/oauth_client/version.rb

@@ -1,5 +1,5 @@
 module Zaikio
   module OAuthClient
-    VERSION = "0.7.2".freeze
+    VERSION = "0.8.0".freeze
   end
 end

test/zaikio/oauth_client_test.rb

@@ -208,6 +208,73 @@ def setup
     assert_not_equal access_token, refreshed_token
   end
 
+  test "when refreshing fails, fallback to client_credentials flow" do
+    Zaikio::JWTAuth.stubs(:revoked_token_ids).returns([])
+    access_token = Zaikio::AccessToken.create!(
+      bearer_type: "Organization",
+      bearer_id: "123",
+      audience: "warehouse",
+      token: "abc",
+      refresh_token: "def",
+      expires_at: 1.hour.ago,
+      scopes: %w[directory.organization.r directory.something.r],
+      requested_scopes: %w[directory.organization.r directory.something.r]
+    )
+
+    stub_request(:post, "http://hub.zaikio.test/oauth/access_token")
+      .with(
+        basic_auth: %w[abc secret],
+        body: {
+          "grant_type" => "refresh_token",
+          "refresh_token" => access_token.refresh_token
+        },
+        headers: {
+          "Accept" => "application/json"
+        }
+      )
+      .to_return(status: 400, body: {
+        "error" => "invalid_grant",
+        "error_description" => "The application, grant, refresh token or device authorization could not be found."
+      }.to_json, headers: { "Content-Type" => "application/json" })
+
+    stub_request(:post, "http://hub.zaikio.test/oauth/access_token")
+      .with(
+        basic_auth: %w[abc secret],
+        body: {
+          "grant_type" => "client_credentials",
+          "scope" => "Org/123.directory.something.r"
+        },
+        headers: {
+          "Accept" => "application/json"
+        }
+      )
+      .to_return(status: 200, body: {
+        "access_token" => org_token,
+        "refresh_token" => "refresh_token",
+        "token_type" => "bearer",
+        "scope" => "directory.something.r",
+        "audiences" => ["warehouse"],
+        "expires_in" => 600,
+        "bearer" => {
+          "id": "123",
+          "type": "Organization"
+        }
+      }.to_json, headers: { "Content-Type" => "application/json" })
+
+    new_token = Zaikio::OAuthClient.get_access_token(
+      bearer_type: "Organization",
+      bearer_id: "123",
+      scopes: %w[directory.something.r]
+    )
+    assert_not new_token.expired?
+    assert_equal %w[directory.something.r], new_token.scopes
+    assert_equal "123", new_token.bearer_id
+    assert_equal "Organization", new_token.bearer_type
+    assert_equal org_token, new_token.token
+    assert_equal "5df4590e-7382-4a31-a57f-ae0e0ce902f2", new_token.id
+    assert_nil new_token.refresh_token # not set in client credentials
+  end
+
   test "gets token via client credentials if refresh token is not present" do
     Zaikio::JWTAuth.stubs(:revoked_token_ids).returns([])
     Zaikio::AccessToken.create!(