From 905a5dd7f679af62468702bd95684c35be898ccc Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Fri, 22 Mar 2024 14:15:57 +0100 Subject: [PATCH 1/4] [#53622] use authentication in files info queries - https://community.openproject.org/wp/53622 --- .../nextcloud/file_info_query.rb | 40 +++-- .../nextcloud/files_info_query.rb | 39 ++--- .../one_drive/file_info_query.rb | 15 +- .../one_drive/files_info_query.rb | 11 +- .../one_drive/internal/children_query.rb | 16 +- .../one_drive/internal/drive_item_query.rb | 32 ++-- .../storages/project_storages_controller.rb | 13 +- .../copy/file_links_dependent_service.rb | 26 ++-- .../storages/file_link_sync_service.rb | 8 +- .../api/v3/storage_files/storage_files_api.rb | 8 +- .../nextcloud/files_info_query_spec.rb | 33 +--- .../one_drive/file_info_query_spec.rb | 145 ++++++------------ .../one_drive/files_info_query_spec.rb | 85 +--------- .../file_info_query_invalid_token.yml | 51 ------ .../files_info_query_invalid_token.yml | 51 ------ 15 files changed, 169 insertions(+), 404 deletions(-) delete mode 100644 modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/file_info_query_invalid_token.yml delete mode 100644 modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/files_info_query_invalid_token.yml diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb index f01c030fbf79..2c65be434ee2 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb @@ -33,41 +33,37 @@ class FileInfoQuery using Storages::Peripherals::ServiceResultRefinements FILE_INFO_PATH = "ocs/v1.php/apps/integration_openproject/fileinfo" + Auth = ::Storages::Peripherals::StorageInteraction::Authentication - def initialize(storage) - @uri = storage.uri - @configuration = storage.oauth_configuration + def self.call(storage:, auth_strategy:, file_id:) + new(storage).call(auth_strategy:, file_id:) end - def self.call(storage:, user:, file_id:) - new(storage).call(user:, file_id:) + def initialize(storage) + @storage = storage end - def call(user:, file_id:) - Util.token(user:, configuration: @configuration) do |token| - file_info(file_id, token).map(&parse_json) >> handle_failure >> create_storage_file_info + def call(auth_strategy:, file_id:) + Auth[auth_strategy].call(storage: @storage, http_options: Util.ocs_api_request) do |http| + file_info(http, file_id).map(&parse_json) >> handle_failure >> create_storage_file_info end end private - def file_info(file_id, token) - response = OpenProject - .httpx - .with(headers: { "Authorization" => "Bearer #{token.access_token}", - "Accept" => "application/json", - "OCS-APIRequest" => "true" }) - .get(Util.join_uri_path(@uri, FILE_INFO_PATH, file_id)) + def file_info(http, file_id) + response = http.get(Util.join_uri_path(@storage.uri, FILE_INFO_PATH, file_id)) + error_data = Storages::StorageErrorData.new(source: self.class, payload: response) case response in { status: 200..299 } ServiceResult.success(result: response.body) in { status: 404 } - Util.error(:not_found, "Outbound request destination not found!", response) + Util.error(:not_found, "Outbound request destination not found!", error_data) in { status: 401 } - Util.error(:unauthorized, "Outbound request not authorized!", response) + Util.error(:unauthorized, "Outbound request not authorized!", error_data) else - Util.error(:error, "Outbound request failed!") + Util.error(:error, "Outbound request failed!", error_data) end end @@ -81,15 +77,17 @@ def parse_json def handle_failure ->(response_object) do + error_data = Storages::StorageErrorData.new(source: self.class, payload: response_object) + case response_object.ocs.data.statuscode when 200..299 ServiceResult.success(result: response_object) when 403 - Util.error(:forbidden, "Access to storage file forbidden!", response_object) + Util.error(:forbidden, "Access to storage file forbidden!", error_data) when 404 - Util.error(:not_found, "Storage file not found!", response_object) + Util.error(:not_found, "Storage file not found!", error_data) else - Util.error(:error, "Outbound request failed!", response_object) + Util.error(:error, "Outbound request failed!", error_data) end end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb index 929e27e06aa8..b5bba8ab6f4a 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb @@ -33,17 +33,17 @@ class FilesInfoQuery using Storages::Peripherals::ServiceResultRefinements FILES_INFO_PATH = "ocs/v1.php/apps/integration_openproject/filesinfo" + Auth = ::Storages::Peripherals::StorageInteraction::Authentication - def initialize(storage) - @uri = storage.uri - @configuration = storage.oauth_configuration + def self.call(storage:, auth_strategy:, file_ids:) + new(storage).call(auth_strategy:, file_ids:) end - def self.call(storage:, user:, file_ids: []) - new(storage).call(user:, file_ids:) + def initialize(storage) + @storage = storage end - def call(user:, file_ids: []) + def call(auth_strategy:, file_ids:) if file_ids.nil? return Util.error(:error, "File IDs can not be nil", file_ids) end @@ -52,32 +52,26 @@ def call(user:, file_ids: []) return ServiceResult.success(result: []) end - Util.token(user:, configuration: @configuration) do |token| - files_info(file_ids, token).map(&parse_json) >> handle_failure >> create_storage_file_infos + Auth[auth_strategy].call(storage: @storage, http_options: Util.ocs_api_request) do |http| + files_info(http, file_ids).map(&parse_json) >> handle_failure >> create_storage_file_infos end end private - def files_info(file_ids, token) - response = OpenProject - .httpx - .with(headers: { "Authorization" => "Bearer #{token.access_token}", - "Accept" => "application/json", - "Content-Type" => "application/json", - "OCS-APIRequest" => "true" }) - .post(Util.join_uri_path(@uri.to_s, FILES_INFO_PATH), - json: { fileIds: file_ids }) + def files_info(http, file_ids) + response = http.post(Util.join_uri_path(@storage.uri, FILES_INFO_PATH), json: { fileIds: file_ids }) + error_data = Storages::StorageErrorData.new(source: self.class, payload: response) case response in { status: 200..299 } - ServiceResult.success(result: response.body.to_s) + ServiceResult.success(result: response.body) in { status: 404 } - Util.error(:not_found, "Outbound request destination not found!", response) + Util.error(:not_found, "Outbound request destination not found!", error_data) in { status: 401 } - Util.error(:unauthorized, "Outbound request not authorized!", response) + Util.error(:unauthorized, "Outbound request not authorized!", error_data) else - Util.error(:error, "Outbound request failed!", response) + Util.error(:error, "Outbound request failed!", error_data) end end @@ -94,7 +88,8 @@ def handle_failure if response_object.ocs.meta.status == "ok" ServiceResult.success(result: response_object) else - Util.error(:error, "Outbound request failed!", response_object) + error_data = Storages::StorageErrorData.new(source: self.class, payload: response_object) + Util.error(:error, "Outbound request failed!", error_data) end end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/file_info_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/file_info_query.rb index 379bed8f693d..4d110509cc65 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/file_info_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/file_info_query.rb @@ -34,9 +34,10 @@ module StorageInteraction module OneDrive class FileInfoQuery FIELDS = %w[id name fileSystemInfo file folder size createdBy lastModifiedBy parentReference].freeze + Auth = ::Storages::Peripherals::StorageInteraction::Authentication - def self.call(storage:, user:, file_id:) - new(storage).call(user:, file_id:) + def self.call(storage:, auth_strategy:, file_id:) + new(storage).call(auth_strategy:, file_id:) end def initialize(storage) @@ -44,18 +45,18 @@ def initialize(storage) @delegate = Internal::DriveItemQuery.new(storage) end - def call(user:, file_id:) + def call(auth_strategy:, file_id:) if file_id.nil? return ServiceResult.failure( result: :error, errors: ::Storages::StorageError.new(code: :error, data: StorageErrorData.new(source: self.class), - log_message: 'File ID can not be nil') + log_message: "File ID can not be nil") ) end - Util.using_user_token(@storage, user) do |token| - @delegate.call(token:, drive_item_id: file_id, fields: FIELDS).map(&storage_file_infos) + Auth[auth_strategy].call(storage: @storage) do |http| + @delegate.call(http:, drive_item_id: file_id, fields: FIELDS).map(&storage_file_infos) end end @@ -65,7 +66,7 @@ def call(user:, file_id:) def storage_file_infos ->(json) do StorageFileInfo.new( - status: 'ok', + status: "ok", status_code: 200, id: json[:id], name: json[:name], diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb index 59f2dccebddc..f32ddc461876 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb @@ -35,25 +35,24 @@ module OneDrive class FilesInfoQuery using ServiceResultRefinements - def self.call(storage:, user:, file_ids: []) - new(storage).call(user:, file_ids:) + def self.call(storage:, auth_strategy:, file_ids: []) + new(storage).call(auth_strategy:, file_ids:) end def initialize(storage) @storage = storage - @uri = storage.uri end - def call(user:, file_ids:) + def call(auth_strategy:, file_ids:) if file_ids.nil? return ServiceResult.failure( result: :error, - errors: ::Storages::StorageError.new(code: :error, log_message: 'File IDs can not be nil') + errors: ::Storages::StorageError.new(code: :error, log_message: "File IDs can not be nil") ) end result = file_ids.map do |file_id| - file_info_result = FileInfoQuery.call(storage: @storage, user:, file_id:) + file_info_result = FileInfoQuery.call(storage: @storage, auth_strategy:, file_id:) if file_info_result.failure? && file_info_result.error_source.is_a?(::OAuthClients::ConnectionManager) # errors in the connection manager must short circuit the query and return the error diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/children_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/children_query.rb index 2acd8a2e3704..d1bf993579ad 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/children_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/children_query.rb @@ -34,11 +34,7 @@ module StorageInteraction module OneDrive module Internal class ChildrenQuery - UTIL = ::Storages::Peripherals::StorageInteraction::OneDrive::Util - - def self.call(storage:, http:, folder:, fields: []) - new(storage).call(http:, folder:, fields:) - end + Util = ::Storages::Peripherals::StorageInteraction::OneDrive::Util def initialize(storage) @storage = storage @@ -47,7 +43,7 @@ def initialize(storage) def call(http:, folder:, fields: []) select_url_query = if fields.empty? - '' + "" else "?$select=#{fields.join(',')}" end @@ -58,7 +54,7 @@ def call(http:, folder:, fields: []) private def make_children_request(folder, http, select_url_query) - response = http.get(UTIL.join_uri_path(@uri, uri_path_for(folder) + select_url_query)) + response = http.get(Util.join_uri_path(@uri, uri_path_for(folder) + select_url_query)) handle_responses(response) end @@ -68,13 +64,13 @@ def handle_responses(response) ServiceResult.success(result: response.json(symbolize_keys: true)) in { status: 404 } ServiceResult.failure(result: :not_found, - errors: UTIL.storage_error(response:, code: :not_found, source: self)) + errors: Util.storage_error(response:, code: :not_found, source: self)) in { status: 403 } ServiceResult.failure(result: :forbidden, - errors: UTIL.storage_error(response:, code: :forbidden, source: self)) + errors: Util.storage_error(response:, code: :forbidden, source: self)) in { status: 401 } ServiceResult.failure(result: :unauthorized, - errors: UTIL.storage_error(response:, code: :unauthorized, source: self)) + errors: Util.storage_error(response:, code: :unauthorized, source: self)) else data = ::Storages::StorageErrorData.new(source: self.class, payload: response) ServiceResult.failure(result: :error, errors: ::Storages::StorageError.new(code: :error, data:)) diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb index cb52988648f6..9ddf3b51624b 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb @@ -34,34 +34,26 @@ module StorageInteraction module OneDrive module Internal class DriveItemQuery - UTIL = ::Storages::Peripherals::StorageInteraction::OneDrive::Util - - def self.call(storage:, token:, drive_item_id:, fields: []) - new(storage).call(token:, drive_item_id:, fields:) - end + Util = ::Storages::Peripherals::StorageInteraction::OneDrive::Util def initialize(storage) @storage = storage - @uri = storage.uri end - def call(token:, drive_item_id:, fields: []) + def call(http:, drive_item_id:, fields: []) select_url_query = if fields.empty? - '' + "" else "?$select=#{fields.join(',')}" end - make_file_request(drive_item_id, token, select_url_query) + make_file_request(drive_item_id, http, select_url_query) end private - def make_file_request(drive_item_id, token, select_url_query) - response = OpenProject.httpx.get( - UTIL.join_uri_path(@uri, uri_path_for(drive_item_id) + select_url_query), - headers: { 'Authorization' => "Bearer #{token.access_token}" } - ) + def make_file_request(drive_item_id, http, select_url_query) + response = http.get(Util.join_uri_path(@storage.uri, uri_path_for(drive_item_id) + select_url_query)) handle_responses(response) end @@ -71,13 +63,13 @@ def handle_responses(response) ServiceResult.success(result: response.json(symbolize_keys: true)) in { status: 404 } ServiceResult.failure(result: :not_found, - errors: UTIL.storage_error(response:, code: :not_found, source: self)) + errors: Util.storage_error(response:, code: :not_found, source: self)) in { status: 403 } ServiceResult.failure(result: :forbidden, - errors: UTIL.storage_error(response:, code: :forbidden, source: self)) + errors: Util.storage_error(response:, code: :forbidden, source: self)) in { status: 401 } ServiceResult.failure(result: :unauthorized, - errors: UTIL.storage_error(response:, code: :unauthorized, source: self)) + errors: Util.storage_error(response:, code: :unauthorized, source: self)) else data = ::Storages::StorageErrorData.new(source: self.class, payload: response) ServiceResult.failure(result: :error, errors: ::Storages::StorageError.new(code: :error, data:)) @@ -85,7 +77,11 @@ def handle_responses(response) end def uri_path_for(file_id) - "/v1.0/drives/#{@storage.drive_id}/items/#{file_id}" + if file_id == "/" + "/v1.0/drives/#{@storage.drive_id}/root" + else + "/v1.0/drives/#{@storage.drive_id}/items/#{file_id}" + end end end end diff --git a/modules/storages/app/controllers/storages/project_storages_controller.rb b/modules/storages/app/controllers/storages/project_storages_controller.rb index f210a5c28549..8a63558450d8 100644 --- a/modules/storages/app/controllers/storages/project_storages_controller.rb +++ b/modules/storages/app/controllers/storages/project_storages_controller.rb @@ -37,6 +37,7 @@ class Storages::ProjectStoragesController < ApplicationController before_action :find_project_by_project_id before_action :render_403, unless: -> { User.current.allowed_in_project?(:view_file_links, @project) } + # rubocop:disable Metrics/AbcSize def open if @object.project_folder_automatic? @storage = @object.storage @@ -44,9 +45,7 @@ def open if @object.project_folder_id.present? ::Storages::Peripherals::Registry .resolve("#{@storage.short_provider_type}.queries.file_info") - .call(storage: @storage, - user: current_user, - file_id: @object.project_folder_id) + .call(storage: @storage, auth_strategy:, file_id: @object.project_folder_id) .match( on_success: user_can_read_project_folder, on_failure: user_can_not_read_project_folder @@ -64,8 +63,16 @@ def open end end + # rubocop:enable Metrics/AbcSize + private + def auth_strategy + Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken + .strategy + .with_user(current_user) + end + def user_can_read_project_folder ->(_) do respond_to do |format| diff --git a/modules/storages/app/services/projects/copy/file_links_dependent_service.rb b/modules/storages/app/services/projects/copy/file_links_dependent_service.rb index dd8ce290da7c..27b28a51eaa2 100644 --- a/modules/storages/app/services/projects/copy/file_links_dependent_service.rb +++ b/modules/storages/app/services/projects/copy/file_links_dependent_service.rb @@ -31,11 +31,11 @@ module Projects::Copy class FileLinksDependentService < Dependency def self.human_name - I18n.t(:'projects.copy.work_package_file_links') + I18n.t(:"projects.copy.work_package_file_links") end def source_count - source.work_packages.joins(:file_links).count('file_links.id') + source.work_packages.joins(:file_links).count("file_links.id") end protected @@ -55,11 +55,11 @@ def copy_dependency(*) tmp = state .copied_project_storages .find { |item| item["source"].storage_id == storage_id } - source_project_storage = tmp['source'] - target_project_storage = tmp['target'] + source_project_storage = tmp["source"] + target_project_storage = tmp["target"] storage = source_project_storage.storage - if source_project_storage.project_folder_mode == 'automatic' + if source_project_storage.project_folder_mode == "automatic" files_info_query_result = files_info_query(storage:, file_ids: source_file_links.map(&:origin_id)) folder_files_file_ids_deep_query_result = folder_files_file_ids_deep_query( @@ -71,7 +71,7 @@ def copy_dependency(*) storage_id: old_file_link.storage_id, creator_id: User.current.id, container_id: state.work_package_id_lookup[old_file_link.container_id.to_s], - container_type: 'WorkPackage', + container_type: "WorkPackage", origin_name: old_file_link.origin_name, origin_mime_type: old_file_link.origin_mime_type } @@ -80,7 +80,7 @@ def copy_dependency(*) .find { |i| i.id.to_i == old_file_link.origin_id.to_i } .location - attributes['origin_id'] = + attributes["origin_id"] = if source_project_storage.file_inside_project_folder?(original_file_location) new_file_location = original_file_location.gsub( source_project_storage.project_folder_path_escaped, @@ -99,7 +99,7 @@ def copy_dependency(*) storage_id: old_file_link.storage_id, creator_id: User.current.id, container_id: state.work_package_id_lookup[old_file_link.container_id.to_s], - container_type: 'WorkPackage', + container_type: "WorkPackage", origin_name: old_file_link.origin_name, origin_mime_type: old_file_link.origin_mime_type, origin_id: old_file_link.origin_id @@ -116,7 +116,7 @@ def copy_dependency(*) def files_info_query(storage:, file_ids:) Storages::Peripherals::Registry .resolve("#{storage.short_provider_type}.queries.files_info") - .call(storage:, user: User.current, file_ids:) + .call(storage:, auth_strategy:, file_ids:) .on_failure { |r| add_error!("files_info_query", r.to_active_model_errors) } .result end @@ -128,5 +128,13 @@ def folder_files_file_ids_deep_query(storage:, location:) .on_failure { |r| add_error!("folder_files_file_ids_deep_query", r.to_active_model_errors) } .result end + + private + + def auth_strategy + Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken + .strategy + .with_user(User.current) + end end end diff --git a/modules/storages/app/services/storages/file_link_sync_service.rb b/modules/storages/app/services/storages/file_link_sync_service.rb index c82fa26adc65..b8e5a5ff72b2 100644 --- a/modules/storages/app/services/storages/file_link_sync_service.rb +++ b/modules/storages/app/services/storages/file_link_sync_service.rb @@ -55,7 +55,7 @@ def sync_storage_data(storage_id, file_links) storage = ::Storages::Storage.find(storage_id) ::Storages::Peripherals::Registry .resolve("#{storage.short_provider_type}.queries.files_info") - .call(storage:, user: @user, file_ids: file_links.map(&:origin_id)) + .call(storage:, auth_strategy:, file_ids: file_links.map(&:origin_id)) .map { |file_infos| to_hash(file_infos) } .match( on_success: set_file_link_status(file_links), @@ -68,6 +68,12 @@ def sync_storage_data(storage_id, file_links) ) end + def auth_strategy + Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken + .strategy + .with_user(@user) + end + def to_hash(file_infos) file_infos.index_by { |file_info| file_info.id.to_s }.to_h end diff --git a/modules/storages/lib/api/v3/storage_files/storage_files_api.rb b/modules/storages/lib/api/v3/storage_files/storage_files_api.rb index 96b2250a2c42..3378e5e9f08e 100644 --- a/modules/storages/lib/api/v3/storage_files/storage_files_api.rb +++ b/modules/storages/lib/api/v3/storage_files/storage_files_api.rb @@ -38,7 +38,7 @@ class StorageFilesAPI < ::API::OpenProjectAPI helpers do def validate_upload_request(body) if Storages::Storage::one_drive_without_ee_token?(@storage.provider_type) - log_message = 'The request can not be handled due to invalid or missing Enterprise token.' + log_message = "The request can not be handled due to invalid or missing Enterprise token." return ServiceResult.failure(errors: Storages::StorageError.new(code: :missing_ee_token_for_one_drive, log_message:)) end @@ -48,7 +48,7 @@ def validate_upload_request(body) ServiceResult.success(result: { file_name:, parent: }.transform_keys(&:to_s)) else ServiceResult.failure(errors: Storages::StorageError.new(code: :bad_request, - log_message: 'Request body malformed!')) + log_message: "Request body malformed!")) end end @@ -78,11 +78,11 @@ def auth_strategy ) end - route_param :file_id, type: String, desc: 'Storage file id' do + route_param :file_id, type: String, desc: "Storage file id" do get do Storages::Peripherals::Registry .resolve("#{@storage.short_provider_type}.queries.file_info") - .call(storage: @storage, user: current_user, file_id: params[:file_id]) + .call(storage: @storage, auth_strategy:, file_id: params[:file_id]) .map { |file_info| to_storage_file(file_info) } .match( on_success: ->(storage_file) { diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/files_info_query_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/files_info_query_spec.rb index a9e61226e7bd..ddee8abd7a08 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/files_info_query_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/files_info_query_spec.rb @@ -35,10 +35,12 @@ using Storages::Peripherals::ServiceResultRefinements let(:user) { create(:user) } - let(:storage) do create(:nextcloud_storage_with_local_connection, :as_not_automatically_managed, oauth_client_token_user: user) end + let(:auth_strategy) do + Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken.strategy.with_user(user) + end subject { described_class.new(storage) } @@ -48,7 +50,7 @@ context "without outbound request involved" do context "with an empty array of file ids" do it "returns an empty array" do - result = subject.call(user:, file_ids: []) + result = subject.call(auth_strategy:, file_ids: []) expect(result).to be_success expect(result.result).to eq([]) @@ -57,7 +59,7 @@ context "with nil" do it "returns an error" do - result = subject.call(user:, file_ids: nil) + result = subject.call(auth_strategy:, file_ids: nil) expect(result).to be_failure expect(result.result).to eq(:error) @@ -69,7 +71,7 @@ vcr: "nextcloud/files_info_query_success" do context "with an array of file ids" do it "must return an array of file information when called" do - result = subject.call(user:, file_ids:) + result = subject.call(auth_strategy:, file_ids:) expect(result).to be_success result.match( @@ -83,32 +85,13 @@ end end - context "with outbound request not authorized", - vcr: "nextcloud/files_info_query_unauthorized" do - context "with an array of file ids" do - before do - token = build_stubbed(:oauth_client_token, oauth_client: storage.oauth_client) - allow(Storages::Peripherals::StorageInteraction::Nextcloud::Util) - .to receive(:token) - .and_yield(token) - end - - it "must return an error when called" do - subject.call(user:, file_ids:).match( - on_success: ->(file_infos) { fail "Expected failure, got #{file_infos}" }, - on_failure: ->(error) { expect(error.code).to eq(:unauthorized) } - ) - end - end - end - context "with outbound request not found" do context "with a single file id", vcr: "nextcloud/files_info_query_not_found" do let(:file_ids) { %w[1234] } it "returns an HTTP 200 with individual status code per file ID" do - subject.call(user:, file_ids:).match( + subject.call(auth_strategy:, file_ids:).match( on_success: ->(file_infos) do expect(file_infos.size).to eq(1) expect(file_infos.first.to_h).to include(status: "Not Found", status_code: 404) @@ -125,7 +108,7 @@ let(:file_ids) { %w[182 1234] } it "returns an HTTP 200 with individual status code per file ID" do - subject.call(user:, file_ids:).match( + subject.call(auth_strategy:, file_ids:).match( on_success: ->(file_infos) do expect(file_infos.size).to eq(2) expect(file_infos.map(&:status_code)).to contain_exactly(403, 404) diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb index 50c756d7582f..defacc3139ad 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -require 'spec_helper' +require "spec_helper" require_module_spec_helper RSpec.describe Storages::Peripherals::StorageInteraction::OneDrive::FileInfoQuery, :vcr, :webmock do @@ -36,21 +36,24 @@ let(:user) { create(:user) } let(:storage) { create(:sharepoint_dev_drive_storage, oauth_client_token_user: user) } + let(:auth_strategy) do + Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken.strategy.with_user(user) + end subject { described_class.new(storage) } - describe '#call' do - it 'responds with correct parameters' do + describe "#call" do + it "responds with correct parameters" do expect(described_class).to respond_to(:call) method = described_class.method(:call) - expect(method.parameters).to contain_exactly(%i[keyreq storage], %i[keyreq user], %i[keyreq file_id]) + expect(method.parameters).to contain_exactly(%i[keyreq storage], %i[keyreq auth_strategy], %i[keyreq file_id]) end - context 'without outbound request involved' do - context 'with nil' do - it 'returns an error' do - result = subject.call(user:, file_id: nil) + context "without outbound request involved" do + context "with nil" do + it "returns an error" do + result = subject.call(auth_strategy:, file_id: nil) expect(result).to be_failure expect(result.error_source).to eq(described_class) @@ -60,13 +63,13 @@ end end - context 'with outbound requests successful' do - context 'with a file id requested', vcr: 'one_drive/file_info_query_success_file' do - let(:file_id) { '01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA' } + context "with outbound requests successful" do + context "with a file id requested", vcr: "one_drive/file_info_query_success_file" do + let(:file_id) { "01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA" } # rubocop:disable RSpec/ExampleLength - it 'must return the file information when called' do - result = subject.call(user:, file_id:) + it "must return the file information when called" do + result = subject.call(auth_strategy:, file_id:) expect(result).to be_success result.match( @@ -74,21 +77,21 @@ expect(file_info).to be_a(Storages::StorageFileInfo) expect(file_info.to_h) .to eq({ - status: 'ok', + status: "ok", status_code: 200, - id: '01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA', - name: 'NextcloudHub.md', + id: "01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA", + name: "NextcloudHub.md", size: 1095, - mime_type: 'application/octet-stream', - created_at: Time.parse('2023-09-26T14:45:25Z'), - last_modified_at: Time.parse('2023-09-26T14:46:13Z'), - owner_name: 'Eric Schubert', - owner_id: '0a0d38a9-a59b-4245-93fa-0d2cf727f17a', - last_modified_by_name: 'Eric Schubert', - last_modified_by_id: '0a0d38a9-a59b-4245-93fa-0d2cf727f17a', + mime_type: "application/octet-stream", + created_at: Time.parse("2023-09-26T14:45:25Z"), + last_modified_at: Time.parse("2023-09-26T14:46:13Z"), + owner_name: "Eric Schubert", + owner_id: "0a0d38a9-a59b-4245-93fa-0d2cf727f17a", + last_modified_by_name: "Eric Schubert", + last_modified_by_id: "0a0d38a9-a59b-4245-93fa-0d2cf727f17a", permissions: nil, trashed: false, - location: '/Folder/Subfolder/NextcloudHub.md' + location: "/Folder/Subfolder/NextcloudHub.md" }) end, on_failure: ->(error) { fail "Expected success, got #{error}" } @@ -97,12 +100,12 @@ # rubocop:enable RSpec/ExampleLength end - context 'with a folder id requested', vcr: 'one_drive/file_info_query_success_folder' do - let(:file_id) { '01AZJL5PNQYF5NM3KWYNA3RJHJIB2XMMMB' } + context "with a folder id requested", vcr: "one_drive/file_info_query_success_folder" do + let(:file_id) { "01AZJL5PNQYF5NM3KWYNA3RJHJIB2XMMMB" } # rubocop:disable RSpec/ExampleLength - it 'must return the file information when called' do - result = subject.call(user:, file_id:) + it "must return the file information when called" do + result = subject.call(auth_strategy:, file_id:) expect(result).to be_success result.match( @@ -110,21 +113,21 @@ expect(file_info).to be_a(Storages::StorageFileInfo) expect(file_info.to_h) .to eq({ - status: 'ok', + status: "ok", status_code: 200, - id: '01AZJL5PNQYF5NM3KWYNA3RJHJIB2XMMMB', - name: 'Ümlæûts', + id: "01AZJL5PNQYF5NM3KWYNA3RJHJIB2XMMMB", + name: "Ümlæûts", size: 18007, - mime_type: 'application/x-op-directory', - created_at: Time.parse('2023-10-09T15:26:32Z'), - last_modified_at: Time.parse('2023-10-09T15:26:32Z'), - owner_name: 'Eric Schubert', - owner_id: '0a0d38a9-a59b-4245-93fa-0d2cf727f17a', - last_modified_by_name: 'Eric Schubert', - last_modified_by_id: '0a0d38a9-a59b-4245-93fa-0d2cf727f17a', + mime_type: "application/x-op-directory", + created_at: Time.parse("2023-10-09T15:26:32Z"), + last_modified_at: Time.parse("2023-10-09T15:26:32Z"), + owner_name: "Eric Schubert", + owner_id: "0a0d38a9-a59b-4245-93fa-0d2cf727f17a", + last_modified_by_name: "Eric Schubert", + last_modified_by_id: "0a0d38a9-a59b-4245-93fa-0d2cf727f17a", permissions: nil, trashed: false, - location: '/Folder/Ümlæûts' + location: "/Folder/Ümlæûts" }) end, on_failure: ->(error) { fail "Expected success, got #{error}" } @@ -134,11 +137,11 @@ end end - context 'with outbound request returning not found', vcr: 'one_drive/file_info_query_one_not_found' do - let(:file_id) { 'not_existent' } + context "with outbound request returning not found", vcr: "one_drive/file_info_query_one_not_found" do + let(:file_id) { "not_existent" } - it 'must return not found' do - result = subject.call(user:, file_id:) + it "must return not found" do + result = subject.call(auth_strategy:, file_id:) expect(result).to be_failure expect(result.error_source).to be_a(Storages::Peripherals::StorageInteraction::OneDrive::Internal::DriveItemQuery) @@ -148,60 +151,4 @@ ) end end - - context 'with invalid oauth token', vcr: 'one_drive/file_info_query_invalid_token' do - let(:file_id) { '01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA' } - - before do - token = build_stubbed(:oauth_client_token, oauth_client: storage.oauth_client) - allow(Storages::Peripherals::StorageInteraction::OneDrive::Util) - .to receive(:using_user_token) - .and_yield(token) - end - - it 'must return unauthorized' do - result = subject.call(user:, file_id:) - expect(result).to be_failure - expect(result.error_source).to be_a(Storages::Peripherals::StorageInteraction::OneDrive::Internal::DriveItemQuery) - - result.match( - on_failure: ->(error) { expect(error.code).to eq(:unauthorized) }, - on_success: ->(file_info) { fail "Expected failure, got #{file_info}" } - ) - end - end - - context 'with not existent oauth token' do - let(:file_id) { '01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA' } - let(:user_without_token) { create(:user) } - - it 'must return unauthorized' do - result = subject.call(user: user_without_token, file_id:) - expect(result).to be_failure - expect(result.error_source).to be_a(OAuthClients::ConnectionManager) - - result.match( - on_failure: ->(error) { expect(error.code).to eq(:unauthorized) }, - on_success: ->(file_infos) { fail "Expected failure, got #{file_infos}" } - ) - end - end - - context 'with network errors' do - let(:file_id) { '01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA' } - - before do - request = HTTPX::Request.new(:get, 'https://my.timeout.org/') - httpx_double = class_double(HTTPX, get: HTTPX::ErrorResponse.new(request, 'Timeout happens', {})) - - allow(OpenProject).to receive(:httpx).and_return(httpx_double) - end - - it 'must return an error with wrapped network error response' do - error = subject.call(user:, file_id:) - expect(error).to be_failure - expect(error.result).to eq(:error) - expect(error.error_payload).to be_a(HTTPX::ErrorResponse) - end - end end diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/files_info_query_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/files_info_query_spec.rb index ddb07c5c36d6..2ec58c727af0 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/files_info_query_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/files_info_query_spec.rb @@ -36,6 +36,9 @@ let(:user) { create(:user) } let(:storage) { create(:sharepoint_dev_drive_storage, oauth_client_token_user: user) } + let(:auth_strategy) do + Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken.strategy.with_user(user) + end subject { described_class.new(storage) } @@ -44,13 +47,13 @@ expect(described_class).to respond_to(:call) method = described_class.method(:call) - expect(method.parameters).to contain_exactly(%i[keyreq storage], %i[keyreq user], %i[key file_ids]) + expect(method.parameters).to contain_exactly(%i[keyreq storage], %i[keyreq auth_strategy], %i[key file_ids]) end context "without outbound request involved" do context "with an empty array of file ids" do it "returns an empty array" do - result = subject.call(user:, file_ids: []) + result = subject.call(auth_strategy:, file_ids: []) expect(result).to be_success expect(result.result).to eq([]) @@ -59,7 +62,7 @@ context "with nil" do it "returns an error" do - result = subject.call(user:, file_ids: nil) + result = subject.call(auth_strategy:, file_ids: nil) expect(result).to be_failure expect(result.result).to eq(:error) @@ -79,7 +82,7 @@ # rubocop:disable RSpec/ExampleLength it "must return an array of file information when called" do - result = subject.call(user:, file_ids:) + result = subject.call(auth_strategy:, file_ids:) expect(result).to be_success result.match( @@ -153,7 +156,7 @@ let(:file_ids) { %w[01AZJL5PJTICED3C5YSVAY6NWTBNA2XERU not_existent] } it "must return an array of file information when called" do - result = subject.call(user:, file_ids:) + result = subject.call(auth_strategy:, file_ids:) expect(result).to be_success result.match( @@ -169,77 +172,5 @@ end end end - - context "with invalid oauth token", vcr: "one_drive/files_info_query_invalid_token" do - before do - token = build_stubbed(:oauth_client_token, oauth_client: storage.oauth_client) - allow(Storages::Peripherals::StorageInteraction::OneDrive::Util) - .to receive(:using_user_token) - .and_yield(token) - end - - context "with an array of file ids" do - let(:file_ids) { %w[01AZJL5PKU2WV3U3RKKFF2A7ZCWVBXRTEU] } - - it "must return an array of file information when called" do - result = subject.call(user:, file_ids:) - expect(result).to be_success - - result.match( - on_success: ->(file_infos) do - expect(file_infos.size).to eq(1) - expect(file_infos).to all(be_a(Storages::StorageFileInfo)) - expect(file_infos[0].id).to eq("01AZJL5PKU2WV3U3RKKFF2A7ZCWVBXRTEU") - expect(file_infos[0].status).to eq("InvalidAuthenticationToken") - expect(file_infos[0].status_code).to eq(401) - end, - on_failure: ->(error) { fail "Expected success, got #{error}" } - ) - end - end - end - - context "with not existent oauth token" do - let(:file_ids) { %w[01AZJL5PKU2WV3U3RKKFF2A7ZCWVBXRTEU] } - let(:user_without_token) { create(:user) } - - it "must return unauthorized when called" do - result = subject.call(user: user_without_token, file_ids:) - expect(result).to be_failure - expect(result.error_source).to be_a(OAuthClients::ConnectionManager) - - result.match( - on_failure: ->(error) { expect(error.code).to eq(:unauthorized) }, - on_success: ->(file_infos) { fail "Expected failure, got #{file_infos}" } - ) - end - end - - context "with network errors" do - let(:file_ids) { %w[01AZJL5PKU2WV3U3RKKFF2A7ZCWVBXRTEU] } - - before do - request = HTTPX::Request.new(:get, "https://my.timeout.org/") - httpx_double = class_double(HTTPX, get: HTTPX::ErrorResponse.new(request, "Timeout happens", {})) - - allow(OpenProject).to receive(:httpx).and_return(httpx_double) - end - - it "must return an array of file information when called" do - result = subject.call(user:, file_ids:) - expect(result).to be_success - - result.match( - on_success: ->(file_infos) do - expect(file_infos.size).to eq(1) - expect(file_infos).to all(be_a(Storages::StorageFileInfo)) - expect(file_infos[0].id).to eq("01AZJL5PKU2WV3U3RKKFF2A7ZCWVBXRTEU") - expect(file_infos[0].status).to eq("Timeout happens") - expect(file_infos[0].status_code).to eq(500) - end, - on_failure: ->(error) { fail "Expected success, got #{error}" } - ) - end - end end end diff --git a/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/file_info_query_invalid_token.yml b/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/file_info_query_invalid_token.yml deleted file mode 100644 index d005a0fb4f27..000000000000 --- a/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/file_info_query_invalid_token.yml +++ /dev/null @@ -1,51 +0,0 @@ ---- -http_interactions: -- request: - method: get - uri: https://graph.microsoft.com/v1.0/drives/b!dmVLG22QlE2PSW0AqVB7UOhZ8n7tjkVGkgqLNnuw2OBb-brzKzZAR4DYT1k9KPXs/items/01AZJL5PNCQCEBFI3N7JGZSX5AOX32Z3LA?$select=id,name,fileSystemInfo,file,folder,size,createdBy,lastModifiedBy,parentReference - body: - encoding: US-ASCII - string: '' - headers: - Authorization: - - Bearer - User-Agent: - - httpx.rb/1.2.1 - Accept: - - "*/*" - Accept-Encoding: - - gzip, deflate - response: - status: - code: 401 - message: Unauthorized - headers: - Transfer-Encoding: - - chunked - Content-Type: - - application/json - Content-Encoding: - - gzip - Vary: - - Accept-Encoding - Strict-Transport-Security: - - max-age=31536000 - Request-Id: - - bb2f6d5c-3c01-41a8-be44-3d2bb300998e - Client-Request-Id: - - bb2f6d5c-3c01-41a8-be44-3d2bb300998e - X-Ms-Ags-Diagnostic: - - '{"ServerInfo":{"DataCenter":"Germany West Central","Slice":"E","Ring":"5","ScaleUnit":"005","RoleInstance":"FR3PEPF000002D9"}}' - Www-Authenticate: - - Bearer realm="", authorization_uri="https://login.microsoftonline.com/common/oauth2/authorize", - client_id="00000003-0000-0000-c000-000000000000" - Date: - - Tue, 30 Jan 2024 11:56:00 GMT - body: - encoding: UTF-8 - string: '{"error":{"code":"InvalidAuthenticationToken","message":"IDX14100: - JWT is not well formed, there are no dots (.).\nThe token needs to be in JWS - or JWE Compact Serialization Format. (JWS): ''EncodedHeader.EndcodedPayload.EncodedSignature''. - (JWE): ''EncodedProtectedHeader.EncodedEncryptedKey.EncodedInitializationVector.EncodedCiphertext.EncodedAuthenticationTag''.","innerError":{"date":"2024-01-30T11:56:01","request-id":"bb2f6d5c-3c01-41a8-be44-3d2bb300998e","client-request-id":"bb2f6d5c-3c01-41a8-be44-3d2bb300998e"}}}' - recorded_at: Tue, 30 Jan 2024 11:56:01 GMT -recorded_with: VCR 6.2.0 diff --git a/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/files_info_query_invalid_token.yml b/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/files_info_query_invalid_token.yml deleted file mode 100644 index 6bea5b664d29..000000000000 --- a/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/files_info_query_invalid_token.yml +++ /dev/null @@ -1,51 +0,0 @@ ---- -http_interactions: -- request: - method: get - uri: https://graph.microsoft.com/v1.0/drives/b!dmVLG22QlE2PSW0AqVB7UOhZ8n7tjkVGkgqLNnuw2OBb-brzKzZAR4DYT1k9KPXs/items/01AZJL5PKU2WV3U3RKKFF2A7ZCWVBXRTEU?$select=id,name,fileSystemInfo,file,folder,size,createdBy,lastModifiedBy,parentReference - body: - encoding: US-ASCII - string: '' - headers: - Authorization: - - Bearer - User-Agent: - - httpx.rb/1.2.1 - Accept: - - "*/*" - Accept-Encoding: - - gzip, deflate - response: - status: - code: 401 - message: Unauthorized - headers: - Transfer-Encoding: - - chunked - Content-Type: - - application/json - Content-Encoding: - - gzip - Vary: - - Accept-Encoding - Strict-Transport-Security: - - max-age=31536000 - Request-Id: - - 6bcd1876-dd4b-4a6d-a28c-3ff3130e654d - Client-Request-Id: - - 6bcd1876-dd4b-4a6d-a28c-3ff3130e654d - X-Ms-Ags-Diagnostic: - - '{"ServerInfo":{"DataCenter":"Germany West Central","Slice":"E","Ring":"5","ScaleUnit":"005","RoleInstance":"FR3PEPF000002C3"}}' - Www-Authenticate: - - Bearer realm="", authorization_uri="https://login.microsoftonline.com/common/oauth2/authorize", - client_id="00000003-0000-0000-c000-000000000000" - Date: - - Tue, 30 Jan 2024 11:55:53 GMT - body: - encoding: UTF-8 - string: '{"error":{"code":"InvalidAuthenticationToken","message":"IDX14100: - JWT is not well formed, there are no dots (.).\nThe token needs to be in JWS - or JWE Compact Serialization Format. (JWS): ''EncodedHeader.EndcodedPayload.EncodedSignature''. - (JWE): ''EncodedProtectedHeader.EncodedEncryptedKey.EncodedInitializationVector.EncodedCiphertext.EncodedAuthenticationTag''.","innerError":{"date":"2024-01-30T11:55:53","request-id":"6bcd1876-dd4b-4a6d-a28c-3ff3130e654d","client-request-id":"6bcd1876-dd4b-4a6d-a28c-3ff3130e654d"}}}' - recorded_at: Tue, 30 Jan 2024 11:55:53 GMT -recorded_with: VCR 6.2.0 From 7a128a2cf44d5763c89f2766ec6f33a7deb22cbd Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Fri, 22 Mar 2024 15:32:27 +0100 Subject: [PATCH 2/4] [#53622] fixed error handling for user token authentication --- .../o_auth_user_token.rb | 19 ++++++++++++++++--- .../nextcloud/file_info_query.rb | 3 ++- .../nextcloud/files_info_query.rb | 3 ++- .../storage_interaction/nextcloud/util.rb | 14 +++++++++----- .../one_drive/files_info_query.rb | 4 ++-- .../one_drive/internal/drive_item_query.rb | 6 +++--- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/o_auth_user_token.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/o_auth_user_token.rb index 22bdc4994b75..5f5e2943c5e9 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/o_auth_user_token.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/o_auth_user_token.rb @@ -52,7 +52,7 @@ def call(storage:, http_options: {}, &) data:) end - opts = http_options.merge({ headers: { "Authorization" => "Bearer #{current_token.access_token}" } }) + opts = http_options.deep_merge({ headers: { "Authorization" => "Bearer #{current_token.access_token}" } }) response_with_current_token = yield OpenProject.httpx.with(opts) if response_with_current_token.success? || response_with_current_token.result != :unauthorized @@ -82,10 +82,9 @@ def refresh_and_retry(config, http_options, token, &) .with_access_token .with(http_options) rescue HTTPX::HTTPError => e - data = ::Storages::StorageErrorData.new(source: self.class, payload: e.response.json) return Failures::Builder.call(code: :unauthorized, log_message: "Error while refreshing OAuth token.", - data:) + data: error_data_from_response(e.response)) end response = yield http_session @@ -105,6 +104,20 @@ def refresh_and_retry(config, http_options, token, &) # rubocop:enable Metrics/AbcSize + def error_data_from_response(response) + payload = + case response + in { content_type: { mime_type: "application/json" } } + response.json + in { content_type: { mime_type: "text/xml" } } + response.xml + else + response.body.to_s + end + + ::Storages::StorageErrorData.new(source: self.class, payload:) + end + def update_refreshed_token(token, http_session) oauth = http_session.instance_variable_get(:@options).oauth_session access_token = oauth.access_token diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb index 2c65be434ee2..5a0d3c3cc3f0 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_info_query.rb @@ -44,7 +44,8 @@ def initialize(storage) end def call(auth_strategy:, file_id:) - Auth[auth_strategy].call(storage: @storage, http_options: Util.ocs_api_request) do |http| + http_options = Util.ocs_api_request.deep_merge(Util.accept_json) + Auth[auth_strategy].call(storage: @storage, http_options:) do |http| file_info(http, file_id).map(&parse_json) >> handle_failure >> create_storage_file_info end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb index b5bba8ab6f4a..51866801a84e 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_info_query.rb @@ -52,7 +52,8 @@ def call(auth_strategy:, file_ids:) return ServiceResult.success(result: []) end - Auth[auth_strategy].call(storage: @storage, http_options: Util.ocs_api_request) do |http| + http_options = Util.ocs_api_request.deep_merge(Util.accept_json) + Auth[auth_strategy].call(storage: @storage, http_options:) do |http| files_info(http, file_ids).map(&parse_json) >> handle_failure >> create_storage_file_infos end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/util.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/util.rb index e707eee5c6bf..f8e091d393c4 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/util.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/util.rb @@ -33,17 +33,21 @@ module Storages::Peripherals::StorageInteraction::Nextcloud::Util class << self def escape_path(path) - escaped_path = path.split('/').map { |i| CGI.escapeURIComponent(i) }.join('/') - escaped_path << '/' if path[-1] == '/' + escaped_path = path.split("/").map { |i| CGI.escapeURIComponent(i) }.join("/") + escaped_path << "/" if path[-1] == "/" escaped_path end def ocs_api_request - { headers: { 'OCS-APIRequest' => 'true' } } + { headers: { "OCS-APIRequest" => "true" } } + end + + def accept_json + { headers: { "Accept" => "application/json" } } end def webdav_request_with_depth(number) - { headers: { 'Depth' => number } } + { headers: { "Depth" => number } } end def error(code, log_message = nil, data = nil) @@ -69,7 +73,7 @@ def token(user:, configuration:, &) end, on_failure: ->(_) do error(:unauthorized, - 'Query could not be created! No access token found!', + "Query could not be created! No access token found!", Storages::StorageErrorData.new(source: connection_manager)) end ) diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb index f32ddc461876..59e95c45720d 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/files_info_query.rb @@ -54,8 +54,8 @@ def call(auth_strategy:, file_ids:) result = file_ids.map do |file_id| file_info_result = FileInfoQuery.call(storage: @storage, auth_strategy:, file_id:) if file_info_result.failure? && - file_info_result.error_source.is_a?(::OAuthClients::ConnectionManager) - # errors in the connection manager must short circuit the query and return the error + file_info_result.error_source.module_parent == AuthenticationStrategies + # errors in the authentication strategies must short circuit the query and return the error return file_info_result end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb index 9ddf3b51624b..7179d09597bc 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/internal/drive_item_query.rb @@ -63,13 +63,13 @@ def handle_responses(response) ServiceResult.success(result: response.json(symbolize_keys: true)) in { status: 404 } ServiceResult.failure(result: :not_found, - errors: Util.storage_error(response:, code: :not_found, source: self)) + errors: Util.storage_error(response:, code: :not_found, source: self.class)) in { status: 403 } ServiceResult.failure(result: :forbidden, - errors: Util.storage_error(response:, code: :forbidden, source: self)) + errors: Util.storage_error(response:, code: :forbidden, source: self.class)) in { status: 401 } ServiceResult.failure(result: :unauthorized, - errors: Util.storage_error(response:, code: :unauthorized, source: self)) + errors: Util.storage_error(response:, code: :unauthorized, source: self.class)) else data = ::Storages::StorageErrorData.new(source: self.class, payload: response) ServiceResult.failure(result: :error, errors: ::Storages::StorageError.new(code: :error, data:)) From 00b3dc041e1459eb066ead3c209df51fae0eb2f7 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Tue, 26 Mar 2024 11:00:29 +0100 Subject: [PATCH 3/4] [#53622] updated open file link query --- .../nextcloud/open_file_link_query.rb | 10 +-- .../one_drive/open_file_link_query.rb | 23 +++--- .../app/models/storages/project_storage.rb | 2 +- .../api/v3/file_links/file_links_open_api.rb | 6 +- .../one_drive/file_info_query_spec.rb | 2 +- .../one_drive/open_file_link_query_spec.rb | 79 ++++++------------- .../open_file_link_query_invalid_token.yml | 51 ------------ 7 files changed, 48 insertions(+), 125 deletions(-) delete mode 100644 modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/open_file_link_query_invalid_token.yml diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb index b7e95f71d67a..5a74dcdc87dd 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb @@ -34,17 +34,17 @@ module StorageInteraction module Nextcloud class OpenFileLinkQuery def initialize(storage) - @uri = storage.uri + @storage = storage.uri end - def self.call(storage:, user:, file_id:, open_location: false) - new(storage).call(user:, file_id:, open_location:) + def self.call(storage:, auth_strategy:, file_id:, open_location: false) + new(storage).call(auth_strategy:, file_id:, open_location:) end # rubocop:disable Lint/UnusedMethodArgument - def call(user:, file_id:, open_location: false) + def call(auth_strategy:, file_id:, open_location: false) location_flag = open_location ? 0 : 1 - ServiceResult.success(result: Util.join_uri_path(@uri, "index.php/f/#{file_id}?openfile=#{location_flag}")) + ServiceResult.success(result: Util.join_uri_path(@storage.uri, "index.php/f/#{file_id}?openfile=#{location_flag}")) end # rubocop:enable Lint/UnusedMethodArgument diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query.rb index 1c5f523f3b39..f49282ae8606 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query.rb @@ -34,9 +34,10 @@ module StorageInteraction module OneDrive class OpenFileLinkQuery using ::Storages::Peripherals::ServiceResultRefinements + Auth = ::Storages::Peripherals::StorageInteraction::Authentication - def self.call(storage:, user:, file_id:, open_location: false) - new(storage).call(user:, file_id:, open_location:) + def self.call(storage:, auth_strategy:, file_id:, open_location: false) + new(storage).call(auth_strategy:, file_id:, open_location:) end def initialize(storage) @@ -44,29 +45,27 @@ def initialize(storage) @delegate = Internal::DriveItemQuery.new(storage) end - def call(user:, file_id:, open_location: false) - @user = user - - Util.using_user_token(@storage, user) do |token| + def call(auth_strategy:, file_id:, open_location: false) + Auth[auth_strategy].call(storage: @storage) do |http| if open_location - request_parent_id(token).call(file_id) >> request_web_url(token) + request_parent_id(http).call(file_id) >> request_web_url(http) else - request_web_url(token).call(file_id) + request_web_url(http).call(file_id) end end end private - def request_web_url(token) + def request_web_url(http) ->(file_id) do - @delegate.call(token:, drive_item_id: file_id, fields: %w[webUrl]).map(&web_url) + @delegate.call(http:, drive_item_id: file_id, fields: %w[webUrl]).map(&web_url) end end - def request_parent_id(token) + def request_parent_id(http) ->(file_id) do - @delegate.call(token:, drive_item_id: file_id, fields: %w[parentReference]).map(&parent_id) + @delegate.call(http:, drive_item_id: file_id, fields: %w[parentReference]).map(&parent_id) end end diff --git a/modules/storages/app/models/storages/project_storage.rb b/modules/storages/app/models/storages/project_storage.rb index 51663bc4ecc0..a1fe8fa686bf 100644 --- a/modules/storages/app/models/storages/project_storage.rb +++ b/modules/storages/app/models/storages/project_storage.rb @@ -89,7 +89,7 @@ def open(user) else Peripherals::Registry .resolve("#{storage.short_provider_type}.queries.open_file_link") - .call(storage:, user:, file_id: project_folder_id) + .call(storage:, auth_strategy:, file_id: project_folder_id) end end diff --git a/modules/storages/lib/api/v3/file_links/file_links_open_api.rb b/modules/storages/lib/api/v3/file_links/file_links_open_api.rb index 5989101b21b7..2368aaf8e0a6 100644 --- a/modules/storages/lib/api/v3/file_links/file_links_open_api.rb +++ b/modules/storages/lib/api/v3/file_links/file_links_open_api.rb @@ -33,11 +33,15 @@ class API::V3::FileLinks::FileLinksOpenAPI < API::OpenProjectAPI resources :open do get do + auth_strategy = Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken + .strategy + .with_user(current_user) + Storages::Peripherals::Registry .resolve("#{@file_link.storage.short_provider_type}.queries.open_file_link") .call( storage: @file_link.storage, - user: current_user, + auth_strategy:, file_id: @file_link.origin_id, open_location: ActiveModel::Type::Boolean.new.cast(params[:location]) ) diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb index defacc3139ad..14085dea0def 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/file_info_query_spec.rb @@ -143,7 +143,7 @@ it "must return not found" do result = subject.call(auth_strategy:, file_id:) expect(result).to be_failure - expect(result.error_source).to be_a(Storages::Peripherals::StorageInteraction::OneDrive::Internal::DriveItemQuery) + expect(result.error_source).to be(Storages::Peripherals::StorageInteraction::OneDrive::Internal::DriveItemQuery) result.match( on_failure: ->(error) { expect(error.code).to eq(:not_found) }, diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query_spec.rb index 78ca03cc9b8d..9324d99f2264 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_file_link_query_spec.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -require 'spec_helper' +require "spec_helper" require_module_spec_helper RSpec.describe Storages::Peripherals::StorageInteraction::OneDrive::OpenFileLinkQuery, :vcr, :webmock do @@ -36,58 +36,49 @@ let(:user) { create(:user) } let(:storage) { create(:sharepoint_dev_drive_storage, oauth_client_token_user: user) } - let(:file_id) { '01AZJL5PJTICED3C5YSVAY6NWTBNA2XERU' } + let(:file_id) { "01AZJL5PJTICED3C5YSVAY6NWTBNA2XERU" } + let(:auth_strategy) do + Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken.strategy.with_user(user) + end subject { described_class.new(storage) } - describe '#call' do - it 'responds with correct parameters' do + describe "#call" do + it "responds with correct parameters" do expect(described_class).to respond_to(:call) method = described_class.method(:call) - expect(method.parameters).to contain_exactly(%i[keyreq storage], %i[keyreq user], %i[keyreq file_id], %i[key open_location]) + expect(method.parameters).to contain_exactly(%i[keyreq storage], + %i[keyreq auth_strategy], + %i[keyreq file_id], + %i[key open_location]) end - context 'with outbound requests successful' do - context 'with open location flag not set', vcr: 'one_drive/open_file_link_query_success' do - it 'returns the url for opening the file on storage' do - call = subject.call(user:, file_id:) + context "with outbound requests successful" do + context "with open location flag not set", vcr: "one_drive/open_file_link_query_success" do + it "returns the url for opening the file on storage" do + call = subject.call(auth_strategy:, file_id:) expect(call).to be_success - expect(call.result).to eq('https://finn.sharepoint.com/sites/openprojectfilestoragetests/_layouts/15/Doc.aspx?sourcedoc=%7B3D884033-B88B-4195-8F36-D30B41AB9234%7D&file=Document.docx&action=default&mobileredirect=true') + expect(call.result).to eq("https://finn.sharepoint.com/sites/openprojectfilestoragetests/_layouts/15/Doc.aspx?sourcedoc=%7B3D884033-B88B-4195-8F36-D30B41AB9234%7D&file=Document.docx&action=default&mobileredirect=true") end end - context 'with open location flag set', vcr: 'one_drive/open_file_link_location_query_success' do - it 'returns the url for opening the file on storage' do - call = subject.call(user:, file_id:, open_location: true) + context "with open location flag set", vcr: "one_drive/open_file_link_location_query_success" do + it "returns the url for opening the file on storage" do + call = subject.call(auth_strategy:, file_id:, open_location: true) expect(call).to be_success - expect(call.result).to eq('https://finn.sharepoint.com/sites/openprojectfilestoragetests/VCR/Folder') + expect(call.result).to eq("https://finn.sharepoint.com/sites/openprojectfilestoragetests/VCR/Folder") end end end - context 'with not existent oauth token' do - let(:user_without_token) { create(:user) } - - it 'must return unauthorized when called' do - result = subject.call(user: user_without_token, file_id:) - expect(result).to be_failure - expect(result.error_source).to be_a(OAuthClients::ConnectionManager) - - result.match( - on_failure: ->(error) { expect(error.code).to eq(:unauthorized) }, - on_success: ->(file_infos) { fail "Expected failure, got #{file_infos}" } - ) - end - end - - context 'with not existent file id', vcr: 'one_drive/open_file_link_query_missing_file_id' do - let(:file_id) { 'iamnotexistent' } + context "with not existent file id", vcr: "one_drive/open_file_link_query_missing_file_id" do + let(:file_id) { "iamnotexistent" } - it 'must return not found' do - result = subject.call(user:, file_id:) + it "must return not found" do + result = subject.call(auth_strategy:, file_id:) expect(result).to be_failure - expect(result.error_source).to be_a(Storages::Peripherals::StorageInteraction::OneDrive::Internal::DriveItemQuery) + expect(result.error_source).to be(Storages::Peripherals::StorageInteraction::OneDrive::Internal::DriveItemQuery) result.match( on_failure: ->(error) { expect(error.code).to eq(:not_found) }, @@ -95,25 +86,5 @@ ) end end - - context 'with invalid oauth token', vcr: 'one_drive/open_file_link_query_invalid_token' do - before do - token = build_stubbed(:oauth_client_token, oauth_client: storage.oauth_client) - allow(Storages::Peripherals::StorageInteraction::OneDrive::Util) - .to receive(:using_user_token) - .and_yield(token) - end - - it 'must return unauthorized' do - result = subject.call(user:, file_id:) - expect(result).to be_failure - expect(result.error_source).to be_a(Storages::Peripherals::StorageInteraction::OneDrive::Internal::DriveItemQuery) - - result.match( - on_failure: ->(error) { expect(error.code).to eq(:unauthorized) }, - on_success: ->(file_infos) { fail "Expected failure, got #{file_infos}" } - ) - end - end end end diff --git a/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/open_file_link_query_invalid_token.yml b/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/open_file_link_query_invalid_token.yml deleted file mode 100644 index 2502b6bf767b..000000000000 --- a/modules/storages/spec/support/fixtures/vcr_cassettes/one_drive/open_file_link_query_invalid_token.yml +++ /dev/null @@ -1,51 +0,0 @@ ---- -http_interactions: -- request: - method: get - uri: https://graph.microsoft.com/v1.0/drives/b!dmVLG22QlE2PSW0AqVB7UOhZ8n7tjkVGkgqLNnuw2OBb-brzKzZAR4DYT1k9KPXs/items/01AZJL5PJTICED3C5YSVAY6NWTBNA2XERU?$select=webUrl - body: - encoding: US-ASCII - string: '' - headers: - Authorization: - - Bearer - User-Agent: - - httpx.rb/1.2.1 - Accept: - - "*/*" - Accept-Encoding: - - gzip, deflate - response: - status: - code: 401 - message: Unauthorized - headers: - Transfer-Encoding: - - chunked - Content-Type: - - application/json - Content-Encoding: - - gzip - Vary: - - Accept-Encoding - Strict-Transport-Security: - - max-age=31536000 - Request-Id: - - 79a87165-2edf-413e-bb42-7387830b7e4f - Client-Request-Id: - - 79a87165-2edf-413e-bb42-7387830b7e4f - X-Ms-Ags-Diagnostic: - - '{"ServerInfo":{"DataCenter":"Germany West Central","Slice":"E","Ring":"5","ScaleUnit":"005","RoleInstance":"FR3PEPF00000366"}}' - Www-Authenticate: - - Bearer realm="", authorization_uri="https://login.microsoftonline.com/common/oauth2/authorize", - client_id="00000003-0000-0000-c000-000000000000" - Date: - - Tue, 30 Jan 2024 11:55:25 GMT - body: - encoding: UTF-8 - string: '{"error":{"code":"InvalidAuthenticationToken","message":"IDX14100: - JWT is not well formed, there are no dots (.).\nThe token needs to be in JWS - or JWE Compact Serialization Format. (JWS): ''EncodedHeader.EndcodedPayload.EncodedSignature''. - (JWE): ''EncodedProtectedHeader.EncodedEncryptedKey.EncodedInitializationVector.EncodedCiphertext.EncodedAuthenticationTag''.","innerError":{"date":"2024-01-30T11:55:26","request-id":"79a87165-2edf-413e-bb42-7387830b7e4f","client-request-id":"79a87165-2edf-413e-bb42-7387830b7e4f"}}}' - recorded_at: Tue, 30 Jan 2024 11:55:26 GMT -recorded_with: VCR 6.2.0 From a2e052421e33599604a65d9fc56e505389571372 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Tue, 26 Mar 2024 11:50:52 +0100 Subject: [PATCH 4/4] [#53622] fixed unit tests - fixed some leftovers in open storage query --- .../nextcloud/open_file_link_query.rb | 8 +++---- .../one_drive/open_storage_query.rb | 6 +++--- .../api/v3/file_links/file_links_open_api.rb | 2 +- .../nextcloud/open_file_link_query_spec.rb | 13 +++++++----- .../one_drive/open_storage_query_spec.rb | 21 ------------------- 5 files changed, 16 insertions(+), 34 deletions(-) diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb index 5a74dcdc87dd..fa40a136097f 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query.rb @@ -33,14 +33,14 @@ module Peripherals module StorageInteraction module Nextcloud class OpenFileLinkQuery - def initialize(storage) - @storage = storage.uri - end - def self.call(storage:, auth_strategy:, file_id:, open_location: false) new(storage).call(auth_strategy:, file_id:, open_location:) end + def initialize(storage) + @storage = storage + end + # rubocop:disable Lint/UnusedMethodArgument def call(auth_strategy:, file_id:, open_location: false) location_flag = open_location ? 0 : 1 diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_storage_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_storage_query.rb index 6eaa679f052c..498d2a8a8fca 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_storage_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/open_storage_query.rb @@ -61,13 +61,13 @@ def handle_responses(response) ServiceResult.success(result: response.json(symbolize_keys: true)) in { status: 404 } ServiceResult.failure(result: :not_found, - errors: Util.storage_error(response:, code: :not_found, source: self)) + errors: Util.storage_error(response:, code: :not_found, source: self.class)) in { status: 403 } ServiceResult.failure(result: :forbidden, - errors: Util.storage_error(response:, code: :forbidden, source: self)) + errors: Util.storage_error(response:, code: :forbidden, source: self.class)) in { status: 401 } ServiceResult.failure(result: :unauthorized, - errors: Util.storage_error(response:, code: :unauthorized, source: self)) + errors: Util.storage_error(response:, code: :unauthorized, source: self.class)) else data = ::Storages::StorageErrorData.new(source: self.class, payload: response) ServiceResult.failure(result: :error, errors: ::Storages::StorageError.new(code: :error, data:)) diff --git a/modules/storages/lib/api/v3/file_links/file_links_open_api.rb b/modules/storages/lib/api/v3/file_links/file_links_open_api.rb index 2368aaf8e0a6..a5370269f03a 100644 --- a/modules/storages/lib/api/v3/file_links/file_links_open_api.rb +++ b/modules/storages/lib/api/v3/file_links/file_links_open_api.rb @@ -33,7 +33,7 @@ class API::V3::FileLinks::FileLinksOpenAPI < API::OpenProjectAPI resources :open do get do - auth_strategy = Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken + auth_strategy = Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken .strategy .with_user(current_user) diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query_spec.rb index 584f7ce1ee71..757647cc4e4a 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/open_file_link_query_spec.rb @@ -33,23 +33,26 @@ RSpec.describe Storages::Peripherals::StorageInteraction::Nextcloud::OpenFileLinkQuery do let(:storage) { create(:nextcloud_storage, host: "https://example.com") } - let(:user) { create(:user) } let(:file_id) { "1337" } + let(:auth_strategy) { Storages::Peripherals::StorageInteraction::AuthenticationStrategies::BasicAuth.strategy } it "responds to .call" do expect(described_class).to respond_to(:call) method = described_class.method(:call) - expect(method.parameters).to contain_exactly(%i[keyreq storage], %i[keyreq user], %i[keyreq file_id], %i[key open_location]) + expect(method.parameters).to contain_exactly(%i[keyreq storage], + %i[keyreq auth_strategy], + %i[keyreq file_id], + %i[key open_location]) end it "returns the url for opening the file on storage" do - url = described_class.call(storage:, user:, file_id:).result + url = described_class.call(storage:, auth_strategy:, file_id:).result expect(url).to eq("#{storage.host}/index.php/f/#{file_id}?openfile=1") end it "returns the url for opening the file's location on storage" do - url = described_class.call(storage:, user:, file_id:, open_location: true).result + url = described_class.call(storage:, auth_strategy:, file_id:, open_location: true).result expect(url).to eq("#{storage.host}/index.php/f/#{file_id}?openfile=0") end @@ -57,7 +60,7 @@ let(:storage) { create(:nextcloud_storage, host: "https://example.com/html") } it "returns the url for opening the file on storage" do - url = described_class.call(storage:, user:, file_id:).result + url = described_class.call(storage:, auth_strategy:, file_id:).result expect(url).to eq("#{storage.host}/index.php/f/#{file_id}?openfile=1") end end diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_storage_query_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_storage_query_spec.rb index b8fec04296cb..296eb94e5b3f 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_storage_query_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/one_drive/open_storage_query_spec.rb @@ -57,26 +57,5 @@ expect(call.result).to eq("https://finn.sharepoint.com/sites/openprojectfilestoragetests/VCR") end end - - context "with not existent oauth token" do - let(:user_without_token) { create(:user) } - let(:auth_strategy) do - Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken - .strategy - .with_user(user_without_token) - end - - it "must return unauthorized when called" do - result = subject.call(auth_strategy:) - expect(result).to be_failure - expect(result.error_source) - .to be(Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken) - - result.match( - on_failure: ->(error) { expect(error.code).to eq(:unauthorized) }, - on_success: ->(file_infos) { fail "Expected failure, got #{file_infos}" } - ) - end - end end end