From fd1438975474cb4950a8b444e4d6c7ce04bdca10 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Mon, 30 Sep 2024 16:38:37 +0200 Subject: [PATCH] [#57709] rework add user command - https://community.openproject.org/work_packages/57709 - use authentication strategy pattern --- .../nextcloud/add_user_to_group_command.rb | 103 ++++++++++-------- .../nextcloud_managed_folder_sync_service.rb | 8 +- .../storages/peripherals/registry_spec.rb | 41 ------- ...tcloud_managed_folder_sync_service_spec.rb | 6 +- 4 files changed, 65 insertions(+), 93 deletions(-) diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command.rb index 04ab1c6171b1..0205180b3e3d 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command.rb @@ -35,66 +35,73 @@ module Nextcloud class AddUserToGroupCommand include TaggedLogging + def self.call(storage:, auth_strategy:, user:, group:) + new(storage).call(auth_strategy:, user:, group:) + end + def initialize(storage) @storage = storage - @username = storage.username - @password = storage.password - @group = storage.group end - # rubocop:disable Metrics/AbcSize - def self.call(storage:, user:, group: storage.group) - new(storage).call(user:, group:) + def call(auth_strategy:, user:, group:) + with_tagged_logger do + Authentication[auth_strategy].call(storage: @storage, http_options:) do |http| + url = UrlBuilder.url(@storage.uri, "ocs/v1.php/cloud/users", user, "groups") + info "Adding #{user} to #{group} through #{url}" + + response = http.post(UrlBuilder.url(@storage.uri, "ocs/v1.php/cloud/users", user, "groups"), + form: { "groupid" => CGI.escapeURIComponent(group) }) + + handle_response(response) + end + end end - def call(user:, group: @group) - with_tagged_logger do - url = UrlBuilder.url(@storage.uri, "ocs/v1.php/cloud/users", user, "groups") - info "Adding #{user} to #{group} through #{url}" + private + + def http_options + Util.ocs_api_request + end - response = OpenProject.httpx - .basic_auth(@username, @password) - .with(headers: { "OCS-APIRequest" => "true" }) - .post( - UrlBuilder.url(@storage.uri, "ocs/v1.php/cloud/users", user, "groups"), - form: { "groupid" => CGI.escapeURIComponent(group) } - ) + def handle_response(response) + error_data = StorageErrorData.new(source: self.class, payload: response) - error_data = StorageErrorData.new(source: self.class, payload: response) + case response + in { status: 200..299 } + handle_success_response(response) + in { status: 405 } + Util.error(:not_allowed, "Outbound request method not allowed", error_data) + in { status: 401 } + Util.error(:not_found, "Outbound request destination not found", error_data) + in { status: 404 } + Util.error(:unauthorized, "Outbound request not authorized", error_data) + in { status: 409 } + Util.error(:conflict, Util.error_text_from_response(response), error_data) + else + Util.error(:error, "Outbound request failed", error_data) + end + end - case response - in { status: 200..299 } - statuscode = Nokogiri::XML(response.body.to_s).xpath("/ocs/meta/statuscode").text + def handle_success_response(response) + error_data = StorageErrorData.new(source: self.class, payload: response) + statuscode = Nokogiri::XML(response.body.to_s).xpath("/ocs/meta/statuscode").text - case statuscode - when "100" - info "User has been added to the group" - ServiceResult.success - when "101" - Util.error(:error, "No group specified", error_data) - when "102" - Util.error(:group_does_not_exist, "Group does not exist", error_data) - when "103" - Util.error(:user_does_not_exist, "User does not exist", error_data) - when "104" - Util.error(:insufficient_privileges, "Insufficient privileges", error_data) - when "105" - Util.error(:failed_to_add, "Failed to add user to group", error_data) - end - in { status: 405 } - Util.error(:not_allowed, "Outbound request method not allowed", error_data) - in { status: 401 } - Util.error(:not_found, "Outbound request destination not found", error_data) - in { status: 404 } - Util.error(:unauthorized, "Outbound request not authorized", error_data) - in { status: 409 } - Util.error(:conflict, Util.error_text_from_response(response), error_data) - else - Util.error(:error, "Outbound request failed", error_data) - end + case statuscode + when "100" + info "User has been added to the group" + ServiceResult.success + when "101" + Util.error(:error, "No group specified", error_data) + when "102" + Util.error(:group_does_not_exist, "Group does not exist", error_data) + when "103" + Util.error(:user_does_not_exist, "User does not exist", error_data) + when "104" + Util.error(:insufficient_privileges, "Insufficient privileges", error_data) + when "105" + Util.error(:failed_to_add, "Failed to add user to group", error_data) end end - # rubocop:enable Metrics/AbcSize end end end diff --git a/modules/storages/app/services/storages/nextcloud_managed_folder_sync_service.rb b/modules/storages/app/services/storages/nextcloud_managed_folder_sync_service.rb index de53a1eaba0d..056230716708 100644 --- a/modules/storages/app/services/storages/nextcloud_managed_folder_sync_service.rb +++ b/modules/storages/app/services/storages/nextcloud_managed_folder_sync_service.rb @@ -116,10 +116,12 @@ def add_remove_users_to_group(group, username) end def add_users_to_remote_group(users_to_add) + group = @storage.group + users_to_add.each do |user| - add_user_to_group.call(storage: @storage, user:).error_and do |error| - add_error(:add_user_to_group, error, options: { user:, group: @storage.group, reason: error.log_message }) - log_storage_error(error, group: @storage.group, user:, reason: error.log_message) + add_user_to_group.call(storage: @storage, auth_strategy:, user:, group:).error_and do |error| + add_error(:add_user_to_group, error, options: { user:, group:, reason: error.log_message }) + log_storage_error(error, group:, user:, reason: error.log_message) end end end diff --git a/modules/storages/spec/common/storages/peripherals/registry_spec.rb b/modules/storages/spec/common/storages/peripherals/registry_spec.rb index bcf7ee97c0f5..4130e22f4db7 100644 --- a/modules/storages/spec/common/storages/peripherals/registry_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/registry_spec.rb @@ -106,47 +106,6 @@ end end - describe "#add_user_to_group_command" do - let(:expected_response) do - { - status: 200, - body: expected_response_body, - headers: {} - } - end - let(:expected_response_body) do - <<~XML - - - - ok - 100 - OK - - - - - - XML - end - - before do - stub_request(:post, "https://example.com/ocs/v1.php/cloud/users/#{origin_user_id}/groups") - .with( - headers: { - "Authorization" => "Basic T3BlblByb2plY3Q6T3BlblByb2plY3RTZWN1cmVQYXNzd29yZA==", - "OCS-APIRequest" => "true" - } - ) - .to_return(expected_response) - end - - it "adds user to the group" do - result = registry.resolve("nextcloud.commands.add_user_to_group").call(storage:, user: origin_user_id) - expect(result).to be_success - end - end - describe "#delete_folder_command" do let(:auth_strategy) { Storages::Peripherals::StorageInteraction::AuthenticationStrategies::BasicAuth.strategy } diff --git a/modules/storages/spec/services/storages/nextcloud_managed_folder_sync_service_spec.rb b/modules/storages/spec/services/storages/nextcloud_managed_folder_sync_service_spec.rb index 0e9c207829af..cce5b7cb51a7 100644 --- a/modules/storages/spec/services/storages/nextcloud_managed_folder_sync_service_spec.rb +++ b/modules/storages/spec/services/storages/nextcloud_managed_folder_sync_service_spec.rb @@ -187,7 +187,11 @@ module Storages # No AuthStrategy on GroupUsers allow(group_users).to receive(:call).with(storage:, group: storage.group).and_return(group_users_result) # Updating the group users - allow(add_user).to receive(:call).with(storage:, user: "single_project_user").and_return(add_user_result) + allow(add_user).to receive(:call).with(storage:, + auth_strategy:, + user: "single_project_user", + group: storage.group) + .and_return(add_user_result) allow(remove_user).to receive(:call).with(storage:, auth_strategy:, user: "cookiemonster",