From 031bae9e21f5264b6a02f8eec839e538760df5ad Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Mon, 12 Aug 2024 16:24:47 +0200 Subject: [PATCH] [#55975] introduced auth strategy for set permissions command - https://community.openproject.org/work_packages/55975 - using auth strategy in nextcloud command set_permissions --- .../nextcloud/set_permissions_command.rb | 59 +++--- .../one_drive/set_permissions_command.rb | 4 +- ...ud_group_folder_properties_sync_service.rb | 6 +- .../one_drive_managed_folder_sync_service.rb | 5 +- .../storages/peripherals/registry_spec.rb | 172 ------------------ 5 files changed, 44 insertions(+), 202 deletions(-) diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/set_permissions_command.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/set_permissions_command.rb index b3d70da6f7c0..ba3ee91542e7 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/set_permissions_command.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/set_permissions_command.rb @@ -37,41 +37,53 @@ class SetPermissionsCommand using ServiceResultRefinements SUCCESS_XPATH = "/d:multistatus/d:response/d:propstat[d:status[text() = 'HTTP/1.1 200 OK']]/d:prop/nc:acl-list" - def self.call(storage:, path:, permissions:) - new(storage).call(path:, permissions:) + + def self.call(storage:, auth_strategy:, path:, permissions:) + new(storage).call(auth_strategy:, path:, permissions:) end def initialize(storage) @storage = storage - @username = storage.username - @password = storage.password end - def call(path:, permissions:) - if path.blank? - return ServiceResult.failure(errors: StorageError.new(code: :invalid_path)) - end + # rubocop:disable Metrics/AbcSize + def call(auth_strategy:, path:, permissions:) + validate_input_data(path).on_failure { return _1 } - with_tagged_logger do - info "Setting permissions #{permissions.inspect} on #{path}" - - body = request_xml_body(permissions[:groups], permissions[:users]) - # This can raise KeyErrors, we probably should just default to enpty Arrays. - response = OpenProject - .httpx - .basic_auth(@username, @password) - .request( - "PROPPATCH", - UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path), - xml: body - ) - - handle_response(response) + username = Util.origin_user_id(caller: self.class, storage: @storage, auth_strategy:) + .on_failure { return _1 } + .result + + Authentication[auth_strategy].call(storage: @storage) do |http| + with_tagged_logger do + info "Setting permissions #{permissions.inspect} on #{path}" + + body = request_xml_body(permissions[:groups], permissions[:users]) + # This can raise KeyErrors, we probably should just default to empty Arrays. + response = http + .request( + "PROPPATCH", + UrlBuilder.url(@storage.uri, "remote.php/dav/files", username, path), + xml: body + ) + + handle_response(response) + end end end + # rubocop:enable Metrics/AbcSize + private + def validate_input_data(path) + if path.blank? + ServiceResult.failure(errors: StorageError.new(code: :invalid_path)) + else + ServiceResult.success + end + end + # rubocop:disable Metrics/AbcSize def handle_response(response) error_data = StorageErrorData.new(source: self.class, payload: response) @@ -125,6 +137,7 @@ def request_xml_body(groups_permissions, users_permissions) end end.to_xml end + # rubocop:enable Metrics/AbcSize end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/set_permissions_command.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/set_permissions_command.rb index 4c71a3f8e25e..0964c821fc51 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/set_permissions_command.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/one_drive/set_permissions_command.rb @@ -45,8 +45,8 @@ def delete? = permission_ids.any? && user_ids.empty? def update? = permission_ids.any? && user_ids.any? end - def self.call(storage:, path:, permissions:, auth_strategy:) - new(storage).call(path:, permissions:, auth_strategy:) + def self.call(storage:, auth_strategy:, path:, permissions:) + new(storage).call(auth_strategy:, path:, permissions:) end def initialize(storage) diff --git a/modules/storages/app/services/storages/nextcloud_group_folder_properties_sync_service.rb b/modules/storages/app/services/storages/nextcloud_group_folder_properties_sync_service.rb index 341283f1853e..36ec9fd5a113 100644 --- a/modules/storages/app/services/storages/nextcloud_group_folder_properties_sync_service.rb +++ b/modules/storages/app/services/storages/nextcloud_group_folder_properties_sync_service.rb @@ -146,7 +146,7 @@ def set_folders_permissions(remote_admins, project_storage) } } - set_permissions.call(storage: @storage, **command_params).on_failure do |service_result| + set_permissions.call(storage: @storage, auth_strategy:, **command_params).on_failure do |service_result| log_storage_error(service_result.errors, folder:) add_error(:set_folder_permission, service_result.errors, options: { folder: }) end @@ -178,7 +178,7 @@ def hide_inactive_folders(remote_folders) groups: { "#{@storage.group}": NO_PERMISSIONS } } } - set_permissions.call(storage: @storage, **command_params).on_failure do |service_result| + set_permissions.call(storage: @storage, auth_strategy:, **command_params).on_failure do |service_result| log_storage_error(service_result.errors, folder: path, context: "hide_folder") add_error(:hide_inactive_folders, service_result.errors, options: { path: }) end @@ -263,7 +263,7 @@ def ensure_root_folder_permissions(group_folder, username, group) } } - set_permissions.call(storage: @storage, **command_params).on_failure do |service_result| + set_permissions.call(storage: @storage, auth_strategy:, **command_params).on_failure do |service_result| log_storage_error(service_result.errors, { folder: group_folder }) add_error(:ensure_root_folder_permissions, service_result.errors, options: { group:, username: }).fail! end diff --git a/modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb b/modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb index 9e53aafd4e94..1389fac5e5c2 100644 --- a/modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb +++ b/modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb @@ -79,7 +79,7 @@ def apply_permission_to_folders end def set_folder_permissions(folder_id, permissions) - set_permissions.call(storage: @storage, path: folder_id, permissions:, auth_strategy:) + set_permissions.call(storage: @storage, auth_strategy:, path: folder_id, permissions:) end def ensure_folders_exist(folder_map) @@ -104,7 +104,7 @@ def hide_inactive_folders(folder_map) info "Hiding folder with ID #{item_id} as it does not belong to any active project" # FIXME: Set permissions wont ever fail. - set_permissions.call(storage: @storage, path: item_id, permissions:, auth_strategy:) + set_permissions.call(storage: @storage, auth_strategy:, path: item_id, permissions:) .on_failure do |service_result| log_storage_error(service_result.errors, item_id:, context: "hide_folder") add_error(:hide_inactive_folders, service_result.errors, options: { path: folder_map[item_id] }) @@ -208,6 +208,7 @@ def client_tokens_scope = OAuthClientToken.where(oauth_client: @storage.oauth_cl def admin_client_tokens_scope = OAuthClientToken.where(oauth_client: @storage.oauth_client, user: User.admin.active) def root_folder = Peripherals::ParentFolder.new("/") + def auth_strategy = userless.call # @param attribute [Symbol] attribute to which the error will be tied to diff --git a/modules/storages/spec/common/storages/peripherals/registry_spec.rb b/modules/storages/spec/common/storages/peripherals/registry_spec.rb index 6b7f409f0169..4657f0375011 100644 --- a/modules/storages/spec/common/storages/peripherals/registry_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/registry_spec.rb @@ -213,178 +213,6 @@ end end - describe "#set_permissions_command" do - let(:path) { "OpenProject/JediProject" } - let(:permissions) do - { - users: { - OpenProject: 31, - "Obi-Wan": 31, - "Qui-Gon": 31 - }, - groups: { - OpenProject: 0 - } - } - end - - let(:expected_request_body) do - <<~XML - - - - - - - group - OpenProject - 31 - 0 - - - user - OpenProject - 31 - 31 - - - user - Obi-Wan - 31 - 31 - - - user - Qui-Gon - 31 - 31 - - - - - - XML - end - - context "with Nextcloud storage type selected" do - context "with outbound request" do - before do - stub_request(:proppatch, "#{url}/remote.php/dav/files/OpenProject/OpenProject/JediProject") - .with( - body: expected_request_body, - headers: { - "Authorization" => "Basic T3BlblByb2plY3Q6T3BlblByb2plY3RTZWN1cmVQYXNzd29yZA==" - } - ) - .to_return(expected_response) - end - - context "when permissions can be set" do - let(:expected_response_body) do - <<~XML - - - - /remote.php/dav/files/OpenProject/OpenProject/Project%231 - - - - - HTTP/1.1 200 OK - - - - XML - end - let(:expected_response) do - { - status: 207, - body: expected_response_body, - headers: {} - } - end - - it "returns success when permissions can be set" do - result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path:, permissions:) - expect(result).to be_success - end - end - - context "when the password is wrong" do - let(:expected_response_body) do - <<~XML - - - Sabre\DAV\Exception\NotAuthenticated - No public access to this resource., No 'Authorization: Basic' header found. Either the client didn't send one, or the server is misconfigured, No 'Authorization: Bearer' header found. Either the client didn't send one, or the server is mis-configured, No 'Authorization: Basic' header found. Either the client didn't send one, or the server is misconfigured - - XML - end - let(:expected_response) do - { - status: 401, - body: expected_response_body, - headers: {} - } - end - - it "returns failure" do - result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path:, permissions:) - expect(result).to be_failure - end - end - - context "when the NC control user cannot read(see) the project folder" do - let(:expected_response_body) do - <<~XML - - - Sabre\DAV\Exception\NotFound - File with name /OpenProject/JediProject could not be located - - XML - end - let(:expected_response) do - { - status: 404, - body: expected_response_body, - headers: {} - } - end - - it "returns failure" do - result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path:, permissions:) - expect(result).to be_failure - end - end - end - - context "when forbidden values are given as folder" do - it "raises an ArgumentError on nil" do - result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: nil, permissions:) - - expect(result).to be_failure - expect(result.errors.code).to eq(:invalid_path) - end - - it "returns a :invalid_path Failure on empty string" do - result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: "", permissions:) - - expect(result).to be_failure - expect(result.errors.code).to eq(:invalid_path) - end - end - end - end - describe "#file_ids_query" do let(:nextcloud_subpath) { "" } let(:url) { "https://example.com#{nextcloud_subpath}" }