diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command.rb index 2336a9530055..6caf5d2cf20b 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command.rb @@ -33,8 +33,8 @@ module Nextcloud class CopyTemplateFolderCommand using ServiceResultRefinements - def self.call(storage:, source_path:, destination_path:) - new(storage).call(source_path:, destination_path:) + def self.call(auth_strategy:, storage:, source_path:, destination_path:) + new(storage).call(auth_strategy:, source_path:, destination_path:) end def initialize(storage) @@ -42,19 +42,18 @@ def initialize(storage) @data = ResultData::CopyTemplateFolder.new(id: nil, polling_url: nil, requires_polling: false) end - def call(source_path:, destination_path:) + def call(auth_strategy:, source_path:, destination_path:) valid_input_result = validate_inputs(source_path, destination_path).on_failure { |failure| return failure } remote_urls = build_origin_urls(**valid_input_result.result) - ensure_remote_folder_does_not_exist(remote_urls[:destination_url]).on_failure { |failure| return failure } + ensure_remote_folder_does_not_exist(auth_strategy, remote_urls[:destination_url]).on_failure do |failure| + return failure + end - copy_folder(**remote_urls).on_failure { |failure| return failure } + copy_folder(auth_strategy, **remote_urls).on_failure { |failure| return failure } - get_folder_id(valid_input_result.result[:destination_path]).on_success do |command_result| - return ServiceResult - .success(result: @data.with(id: command_result.result[destination_path]["fileid"])) - end + get_folder_id(valid_input_result.result[:destination_path]) end private @@ -87,45 +86,67 @@ def build_origin_urls(source_path:, destination_path:) { source_url:, destination_url: } end - def ensure_remote_folder_does_not_exist(destination_url) - response = OpenProject.httpx.basic_auth(@storage.username, @storage.password).head(destination_url) + def ensure_remote_folder_does_not_exist(auth_strategy, destination_url) + response = Authentication[auth_strategy].call(storage: @storage) { |http| http.head(destination_url) } case response in { status: 200..299 } - Util.error(:conflict, "Destination folder already exists.") + ServiceResult.failure(result: :conflict, + errors: Util.storage_error( + response:, code: :conflict, source: self.class, + log_message: "The copy would overwrite an already existing folder" + )) in { status: 401 } - Util.error(:unauthorized, "unauthorized (validate_destination)") + ServiceResult.failure(result: :unauthorized, + errors: Util.storage_error(response:, code: :unauthorized, source: self.class)) in { status: 404 } ServiceResult.success else - Util.error(:unknown, "Unexpected response (validate_destination): #{response.code}", response) + ServiceResult.failure(result: :error, + errors: Util.storage_error(response:, code: :error, source: self.class)) + end + end + + def copy_folder(auth_strategy, source_url:, destination_url:) + response = Authentication[auth_strategy].call(storage: @storage) do |http| + http.request("COPY", source_url, headers: { "Destination" => destination_url, "Depth" => "infinity" }) end + + handle_response(response) end - def copy_folder(source_url:, destination_url:) - response = OpenProject - .httpx - .basic_auth(@storage.username, @storage.password) - .request("COPY", source_url, headers: { "Destination" => destination_url, "Depth" => "infinity" }) + def handle_response(response) + source = self.class case response in { status: 200..299 } ServiceResult.success(message: "Folder was successfully copied") in { status: 401 } - Util.error(:unauthorized, "Unauthorized (copy_folder)") + ServiceResult.failure(result: :unauthorized, + errors: Util.storage_error(response:, code: :unauthorized, source:)) + in { status: 403 } + ServiceResult.failure(result: :forbidden, + errors: Util.storage_error(response:, code: :forbidden, source:)) in { status: 404 } - Util.error(:not_found, "Project folder not found (copy_folder)") + ServiceResult.failure(result: :not_found, + errors: Util.storage_error(response:, code: :not_found, source:, + log_message: "Template folder not found")) in { status: 409 } - Util.error(:conflict, Util.error_text_from_response(response)) + ServiceResult.failure(result: :conflict, + errors: Util.storage_error( + response:, code: :conflict, source:, + log_message: Util.error_text_from_response(response) + )) else - Util.error(:unknown, "Unexpected response (copy_folder): #{response.status}", response) + ServiceResult.failure(result: :error, + errors: Util.storage_error(response:, code: :error, source:)) end end def get_folder_id(destination_path) Registry .resolve("#{@storage.short_provider_type}.queries.file_ids") - .call(storage: @storage, path: destination_path) + .call(storage: @storage, path: destination_path).map { |result| @data.with(id: result[destination_path]["fileid"]) } end 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 6a1745cb17cc..23ebcad28f5b 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 @@ -54,6 +54,13 @@ def webdav_request_with_depth(number) { headers: { "Depth" => number } } end + def storage_error(response:, code:, source:, log_message: nil) + # Some errors, like timeouts, aren't json responses so we need to adapt + data = StorageErrorData.new(source:, payload: response.to_s) + + StorageError.new(code:, data:, log_message:) + end + def error(code, log_message = nil, data = nil) ServiceResult.failure( result: code, # This is needed to work with the ConnectionManager token refresh mechanism. diff --git a/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command_spec.rb b/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command_spec.rb index ae17be709bcf..4360ddf57915 100644 --- a/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/copy_template_folder_command_spec.rb @@ -43,6 +43,7 @@ let(:destination_path) { "boring-destination" } let(:source_url) { "#{url}/remote.php/dav/files/#{CGI.escape(origin_user_id)}/#{source_path}" } let(:destination_url) { "#{url}/remote.php/dav/files/#{CGI.escape(origin_user_id)}/#{destination_path}" } + let(:auth_strategy) { Storages::Peripherals::StorageInteraction::AuthenticationStrategies::NextcloudStrategies::UserLess.call } subject { described_class.new(storage) } @@ -51,14 +52,14 @@ describe "parameter validation" do it "source_path cannot be blank" do - result = subject.call(source_path: "", destination_path: "destination") + result = subject.call(auth_strategy:, source_path: "", destination_path: "destination") expect(result).to be_failure expect(result.errors.log_message).to eq("Source and destination paths must be present.") end it "destination_path cannot blank" do - result = subject.call(source_path: "source", destination_path: "") + result = subject.call(auth_strategy:, source_path: "source", destination_path: "") expect(result).to be_failure expect(result.errors.log_message).to eq("Source and destination paths must be present.") @@ -68,10 +69,10 @@ describe "remote server overwrite protection" do it "destination_path must not exist on the remote server" do stub_request(:head, destination_url).to_return(status: 200) - result = subject.call(source_path:, destination_path:) + result = subject.call(auth_strategy:, source_path:, destination_path:) expect(result).to be_failure - expect(result.errors.log_message).to eq("Destination folder already exists.") + expect(result.errors.log_message).to eq("The copy would overwrite an already existing folder") end end @@ -112,7 +113,7 @@ end it "must be successful" do - result = subject.call(source_path:, destination_path:) + result = subject.call(auth_strategy:, source_path:, destination_path:) expect(result).to be_success expect(result.result.id).to eq("349") @@ -134,7 +135,7 @@ end it "returns a :conflict failure if the copy fails" do - result = subject.call(source_path:, destination_path:) + result = subject.call(auth_strategy:, source_path:, destination_path:) expect(result).to be_failure expect(result.errors.code).to eq(:conflict)