[53368] fixing issues from PR

opf · Mar 12, 2024 · 2afdf60 · 2afdf60
1 parent d268b53
commit 2afdf60
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 43 deletions.
diff --git a/.../storages/app/common/storages/peripherals/oauth_configurations/nextcloud_configuration.rb b/.../storages/app/common/storages/peripherals/oauth_configurations/nextcloud_configuration.rb
@@ -32,7 +32,7 @@ module Storages
   module Peripherals
     module OAuthConfigurations
       class NextcloudConfiguration < ConfigurationInterface
-        Util = ::Storages::Peripherals::StorageInteraction::Nextcloud::Util
+        Util = StorageInteraction::Nextcloud::Util
 
         attr_reader :oauth_client
 
@@ -45,11 +45,9 @@ def initialize(storage)
         # rubocop:enable Lint/MissingSuper
 
         def authorization_state_check(token)
-          util = ::Storages::Peripherals::StorageInteraction::Nextcloud::Util
-
           authorization_check_wrapper do
             OpenProject.httpx.get(
-              util.join_uri_path(@uri, '/ocs/v1.php/cloud/user'),
+              Util.join_uri_path(@uri, '/ocs/v1.php/cloud/user'),
               headers: {
                 'Authorization' => "Bearer #{token}",
                 'OCS-APIRequest' => 'true',
@@ -64,7 +62,7 @@ def extract_origin_user_id(rack_access_token)
         end
 
         def to_httpx_oauth_config
-          ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthConfiguration.new(
+          StorageInteraction::AuthenticationStrategies::OAuthConfiguration.new(
             client_id: @oauth_client.client_id,
             client_secret: @oauth_client.client_secret,
             issuer: URI(Util.join_uri_path(@uri, "/index.php/apps/oauth2/api/v1")).normalize,

diff --git a/.../storages/app/common/storages/peripherals/oauth_configurations/one_drive_configuration.rb b/.../storages/app/common/storages/peripherals/oauth_configurations/one_drive_configuration.rb
@@ -32,6 +32,8 @@ module Storages
   module Peripherals
     module OAuthConfigurations
       class OneDriveConfiguration < ConfigurationInterface
+        Util = StorageInteraction::OneDrive::Util
+
         attr_reader :oauth_client
 
         # rubocop:disable Lint/MissingSuper
@@ -45,27 +47,23 @@ def initialize(storage)
         # rubocop:enable Lint/MissingSuper
 
         def authorization_state_check(access_token)
-          util = ::Storages::Peripherals::StorageInteraction::OneDrive::Util
-
           authorization_check_wrapper do
             OpenProject.httpx.get(
-              util.join_uri_path(@uri, '/v1.0/me'),
+              Util.join_uri_path(@uri, '/v1.0/me'),
               headers: { 'Authorization' => "Bearer #{access_token}", 'Accept' => 'application/json' }
             )
           end
         end
 
         def extract_origin_user_id(rack_access_token)
-          util = ::Storages::Peripherals::StorageInteraction::OneDrive::Util
-
           OpenProject.httpx.get(
-            util.join_uri_path(@uri, '/v1.0/me'),
+            Util.join_uri_path(@uri, '/v1.0/me'),
             headers: { 'Authorization' => "Bearer #{rack_access_token.access_token}", 'Accept' => 'application/json' }
           ).raise_for_status.json['id']
         end
 
         def to_httpx_oauth_config
-          ::Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthConfiguration.new(
+          StorageInteraction::AuthenticationStrategies::OAuthConfiguration.new(
             client_id: @oauth_client.client_id,
             client_secret: @oauth_client.client_secret,
             issuer: @oauth_uri,

diff --git a/...p/common/storages/peripherals/storage_interaction/authentication_strategies/basic_auth.rb b/...p/common/storages/peripherals/storage_interaction/authentication_strategies/basic_auth.rb
@@ -41,14 +41,18 @@ def call(storage:, http_options: {})
             username = storage.username
             password = storage.password
 
-            if username.blank? || password.blank?
-              log_message = 'Cannot authenticate storage with basic auth. Password or username not configured.'
-              data = ::Storages::StorageErrorData.new(source: self, payload: storage)
-              return Error.create(code: :error, log_message:, data:)
-            end
+            return build_failure(storage) if username.blank? || password.blank?
 
             yield OpenProject.httpx.basic_auth(username, password).with(http_options)
           end
+
+          private
+
+          def build_failure(storage)
+            log_message = 'Cannot authenticate storage with basic auth. Password or username not configured.'
+            data = ::Storages::StorageErrorData.new(source: self, payload: storage)
+            Failures::Builder.call(code: :error, log_message:, data:)
+          end
         end
       end
     end

diff --git a/...action/authentication_strategies/error.rb → ...ion/authentication_strategies/failures.rb b/...action/authentication_strategies/error.rb → ...ion/authentication_strategies/failures.rb
@@ -32,8 +32,8 @@ module Storages
   module Peripherals
     module StorageInteraction
       module AuthenticationStrategies
-        class Error
-          def self.create(code:, log_message:, data:)
+        module Failures
+          Builder = ->(code:, log_message:, data:) do
             storage_error = ::Storages::StorageError.new(code:, log_message:, data:)
             ServiceResult.failure(result: code, errors: storage_error)
           end

diff --git a/...es/peripherals/storage_interaction/authentication_strategies/o_auth_client_credentials.rb b/...es/peripherals/storage_interaction/authentication_strategies/o_auth_client_credentials.rb
@@ -40,18 +40,14 @@ def self.strategy
           def call(storage:, http_options: {}, &)
             config = storage.oauth_configuration.to_httpx_oauth_config
 
-            if config.complete?
-              create_http_and_yield(issuer: config.issuer,
-                                    client_id: config.client_id,
-                                    client_secret: config.client_secret,
-                                    scope: config.scope,
-                                    http_options:,
-                                    &)
-            else
-              log_message = 'Cannot authenticate storage with client credential oauth flow. Storage not configured.'
-              data = ::Storages::StorageErrorData.new(source: self, payload: storage)
-              Error.create(code: :error, log_message:, data:)
-            end
+            return build_failure(storage) unless config.complete?
+
+            create_http_and_yield(issuer: config.issuer,
+                                  client_id: config.client_id,
+                                  client_secret: config.client_secret,
+                                  scope: config.scope,
+                                  http_options:,
+                                  &)
           end
 
           private
@@ -68,13 +64,19 @@ def create_http_and_yield(issuer:, client_id:, client_secret:, scope:, http_opti
                                 .with(http_options)
             rescue HTTPX::HTTPError => e
               data = ::Storages::StorageErrorData.new(source: self, payload: e.response.json)
-              return Error.create(code: :unauthorized,
-                                  log_message: 'Error while fetching OAuth access token.',
-                                  data:)
+              return Failures::Builder.call(code: :unauthorized,
+                                            log_message: 'Error while fetching OAuth access token.',
+                                            data:)
             end
 
             yield http
           end
+
+          def build_failure(storage)
+            log_message = 'Cannot authenticate storage with client credential oauth flow. Storage not configured.'
+            data = ::Storages::StorageErrorData.new(source: self, payload: storage)
+            Failures::Builder.call(code: :error, log_message:, data:)
+          end
         end
       end
     end

diff --git a/...n/storages/peripherals/storage_interaction/authentication_strategies/o_auth_user_token.rb b/...n/storages/peripherals/storage_interaction/authentication_strategies/o_auth_user_token.rb
@@ -48,9 +48,9 @@ def call(storage:, http_options: {}, &)
             current_token = OAuthClientToken.find_by(user_id: @user, oauth_client_id: config.oauth_client.id)
             if current_token.nil?
               data = ::Storages::StorageErrorData.new(source: self)
-              return Error.create(code: :unauthorized,
-                                  log_message: 'Authorization failed. No user access token found.',
-                                  data:)
+              return Failures::Builder.call(code: :unauthorized,
+                                            log_message: 'Authorization failed. No user access token found.',
+                                            data:)
             end
 
             opts = http_options.merge({ headers: { 'Authorization' => "Bearer #{current_token.access_token}" } })
@@ -81,9 +81,9 @@ def refresh_and_retry(config, http_options, token, &)
                                         .with(http_options)
             rescue HTTPX::HTTPError => e
               data = ::Storages::StorageErrorData.new(source: self, payload: e.response.json)
-              return Error.create(code: :unauthorized,
-                                  log_message: 'Error while refreshing OAuth token.',
-                                  data:)
+              return Failures::Builder.call(code: :unauthorized,
+                                            log_message: 'Error while refreshing OAuth token.',
+                                            data:)
             end
 
             response = yield http_session
@@ -92,9 +92,9 @@ def refresh_and_retry(config, http_options, token, &)
               success = update_refreshed_token(token, http_session)
               unless success
                 data = ::Storages::StorageErrorData.new(source: self)
-                return Error.create(code: :error,
-                                    log_message: 'Error while persisting updated access token.',
-                                    data:)
+                return Failures::Builder.call(code: :error,
+                                              log_message: 'Error while persisting updated access token.',
+                                              data:)
               end
             end