diff --git a/Gemfile b/Gemfile index 2b632665a43..7dfa5bd99d3 100644 --- a/Gemfile +++ b/Gemfile @@ -209,9 +209,9 @@ gem "mini_magick", "~> 4.13.0", require: false gem "validate_url" # Storages support code +gem "dry-auto_inject" gem "dry-container" gem "dry-monads" -gem "dry-auto_inject" # ActiveRecord extension which adds typecasting to store accessors gem "store_attribute", "~> 1.0" diff --git a/modules/storages/app/common/storages/peripherals/managed_folder_identifier/nextcloud.rb b/modules/storages/app/common/storages/peripherals/managed_folder_identifier/nextcloud.rb index 7e6be978d68..5ca9c05ecb4 100644 --- a/modules/storages/app/common/storages/peripherals/managed_folder_identifier/nextcloud.rb +++ b/modules/storages/app/common/storages/peripherals/managed_folder_identifier/nextcloud.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright #++ @@ -10,8 +12,12 @@ module Storages @project = project_storage.project end + def name + "#{@project.name.tr('/', '|')} (#{@project.id})" + end + def path - "/#{@storage.group_folder}/#{@project.name.tr('/', '|')} (#{@project.id})/" + "/#{@storage.group_folder}/#{name}/" end def location diff --git a/modules/storages/app/common/storages/peripherals/managed_folder_identifier/one_drive.rb b/modules/storages/app/common/storages/peripherals/managed_folder_identifier/one_drive.rb index 24c55ab7697..e81c3ac99e1 100644 --- a/modules/storages/app/common/storages/peripherals/managed_folder_identifier/one_drive.rb +++ b/modules/storages/app/common/storages/peripherals/managed_folder_identifier/one_drive.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright #++ @@ -12,6 +14,10 @@ module Storages @project = project_storage.project end + def name + path + end + def path "#{@project.name.gsub(CHARACTER_BLOCKLIST, '_')} (#{@project.id})" end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_ids_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_ids_query.rb index 87851fc3f5c..9c785590b37 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_ids_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/file_ids_query.rb @@ -43,7 +43,7 @@ module Storages def call(path:) query_params = { depth: "1", path:, props: %w[oc:fileid] } - Rails.logger.tagged(self.class).info "Requesting File Ids with the following args: #{query_params.inspect}" + Rails.logger.tagged(self.class).info "Requesting File Ids on path: #{path} and args: #{query_params.inspect}" @query.call(**query_params) end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_query.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_query.rb index 1b9ba3e9f2f..407b382721b 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_query.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_query.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2024 the OpenProject GmbH @@ -222,7 +224,7 @@ module Storages # https://github.com/nextcloud/server/blob/66648011c6bc278ace57230db44fd6d63d67b864/lib/public/Files/DavUtil.php result = [] result << :readable if permissions_string.include?("G") - result << :writeable if %w[CK W].reduce(false) { |s, v| s || permissions_string.include?(v) } + result << :writeable if permissions_string.match?(/W|CK/) result end end diff --git a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/internal/propfind_query_legacy.rb b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/internal/propfind_query_legacy.rb index 32cce7b819b..207b2ccf16b 100644 --- a/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/internal/propfind_query_legacy.rb +++ b/modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/internal/propfind_query_legacy.rb @@ -34,35 +34,9 @@ module Storages module Nextcloud module Internal class PropfindQueryLegacy - # Only for information purposes currently. - # Probably a bit later we could validate `#call` parameters. - # - # DEPTH = %w[0 1 infinity].freeze - # POSSIBLE_PROPS = %w[ - # d:getlastmodified - # d:getetag - # d:getcontenttype - # d:resourcetype - # d:getcontentlength - # d:permissions - # d:size - # oc:id - # oc:fileid - # oc:favorite - # oc:comments-href - # oc:comments-count - # oc:comments-unread - # oc:owner-id - # oc:owner-display-name - # oc:share-types - # oc:checksums - # oc:size - # nc:has-preview - # nc:rich-workspace - # nc:contained-folder-count - # nc:contained-file-count - # nc:acl-list - # ].freeze + def self.call(storage:, depth:, path:, props:) + new(storage).call(depth:, path:, props:) + end def initialize(storage) @storage = storage @@ -71,10 +45,6 @@ module Storages @group = storage.group end - def self.call(storage:, depth:, path:, props:) - new(storage).call(depth:, path:, props:) - end - # rubocop:disable Metrics/AbcSize def call(depth:, path:, props:) Rails.logger.tagged(self.class) do @@ -87,32 +57,33 @@ module Storages xml["d"].prop do props.each do |prop| namespace, property = prop.split(":") - xml[namespace].send(property) + xml[namespace].public_send(property) end end end end.to_xml - response = OpenProject - .httpx - .basic_auth(@username, @password) - .with(headers: { "Depth" => depth }) - .request( - "PROPFIND", - UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path), - xml: body - ) + response = OpenProject + .httpx + .basic_auth(@username, @password) + .with(headers: { "Depth" => depth }) + .request( + "PROPFIND", + UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path), + xml: body + ) error_data = StorageErrorData.new(source: self.class, payload: response) - case response - in { status: 200..299 } - log_response(response) - doc = Nokogiri::XML(response.body.to_s) - Rails.logger.info "Parsing response body" - result = doc.xpath("/d:multistatus/d:response").each_with_object({}) do |resource_section, hash| - source_path = UrlBuilder.path(@storage.uri.path, "/remote.php/dav/files", @username) - resource = CGI.unescape(resource_section.xpath("d:href").text.strip).gsub!(source_path, "") + case response + in { status: 200..299 } + log_response(response) + log_message "Parsing XML response body" + doc = Nokogiri::XML(response.body.to_s) + Rails.logger.info "Parsing response body" + result = doc.xpath("/d:multistatus/d:response").each_with_object({}) do |resource_section, hash| + source_path = UrlBuilder.path(@storage.uri.path, "/remote.php/dav/files", @username) + resource = CGI.unescape(resource_section.xpath("d:href").text.strip).gsub!(source_path, "") hash[resource] = {} @@ -123,7 +94,7 @@ module Storages end end - Rails.logger.info "Response parsed found: #{result.inspect}" + log_message "Response parsed found: #{result.inspect}" ServiceResult.success(result:) in { status: 405 } log_response(response) @@ -142,7 +113,11 @@ module Storages # rubocop:enable Metrics/AbcSize def log_response(response) - Rails.logger.info "Storage responded with a #{response.status} code." + log_message "Storage responded with a #{response.status} code." + end + + def log_message(message) + Rails.logger.info message end end end 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 eed0d4b2855..a10484ef367 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 @@ -28,87 +28,113 @@ # See COPYRIGHT and LICENSE files for more details. #++ -module Storages::Peripherals::StorageInteraction::Nextcloud - class SetPermissionsCommand - using Storages::Peripherals::ServiceResultRefinements +module Storages + module Peripherals + module StorageInteraction + module Nextcloud + class SetPermissionsCommand + using ServiceResultRefinements - def initialize(storage) - @storage = storage - @username = storage.username - @password = storage.password - end + 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:) + end - def self.call(storage:, path:, permissions:) - new(storage).call(path:, permissions:) - end + def initialize(storage) + @storage = storage + @username = storage.username + @password = storage.password + end - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/PerceivedComplexity - def call(path:, permissions:) - raise ArgumentError if path.blank? + def call(path:, permissions:) + if path.blank? + return ServiceResult.failure(errors: StorageError.new(code: :invalid_path)) + end - users_permissions = permissions.fetch(:users) - groups_permissions = permissions.fetch(:groups) + with_tagged_logger do + info "Setting permissions #{permissions.inspect} on #{path}" - body = Nokogiri::XML::Builder.new do |xml| - xml["d"].propertyupdate( - "xmlns:d" => "DAV:", - "xmlns:nc" => "http://nextcloud.org/ns" - ) do - xml["d"].set do - xml["d"].prop do - xml["nc"].send(:"acl-list") do - groups_permissions.each do |group, group_permissions| - xml["nc"].acl do - xml["nc"].send(:"acl-mapping-type", "group") - xml["nc"].send(:"acl-mapping-id", group) - xml["nc"].send(:"acl-mask", "31") - xml["nc"].send(:"acl-permissions", group_permissions.to_s) - end - end - users_permissions.each do |user, user_permissions| - xml["nc"].acl do - xml["nc"].send(:"acl-mapping-type", "user") - xml["nc"].send(:"acl-mapping-id", user) - xml["nc"].send(:"acl-mask", "31") - xml["nc"].send(:"acl-permissions", user_permissions.to_s) + 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) + end + end + + private + + # rubocop:disable Metrics/AbcSize + def handle_response(response) + error_data = StorageErrorData.new(source: self.class, payload: response) + + case response + in { status: 200..299 } + doc = Nokogiri::XML(response.body.to_s) + if doc.xpath(SUCCESS_XPATH).present? + info "Permissions set" + ServiceResult.success(result: :success) + else + Util.error(:permission_not_set, "nc:acl properly has not been set for #{path}", error_data) + end + in { status: 404 } + Util.error(:not_found, "Outbound request destination not found", error_data) + in { status: 401 } + Util.error(:unauthorized, "Outbound request not authorized", error_data) + else + Util.error(:error, "Outbound request failed", error_data) + end + end + + def request_xml_body(groups_permissions, users_permissions) + Nokogiri::XML::Builder.new do |xml| + xml["d"].propertyupdate( + "xmlns:d" => "DAV:", + "xmlns:nc" => "http://nextcloud.org/ns" + ) do + xml["d"].set do + xml["d"].prop do + xml["nc"].send(:"acl-list") do + groups_permissions.each do |group, group_permissions| + xml["nc"].acl do + xml["nc"].send(:"acl-mapping-type", "group") + xml["nc"].send(:"acl-mapping-id", group) + xml["nc"].send(:"acl-mask", "31") + xml["nc"].send(:"acl-permissions", group_permissions.to_s) + end + end + users_permissions.each do |user, user_permissions| + xml["nc"].acl do + xml["nc"].send(:"acl-mapping-type", "user") + xml["nc"].send(:"acl-mapping-id", user) + xml["nc"].send(:"acl-mask", "31") + xml["nc"].send(:"acl-permissions", user_permissions.to_s) + end + end + end end end end - end + end.to_xml + end + # rubocop:enable Metrics/AbcSize + + def with_tagged_logger(&) + Rails.logger.tagged(self.class, &) + end + + def info(message) + Rails.logger.info message end end - end.to_xml - - response = OpenProject - .httpx - .basic_auth(@username, @password) - .request( - "PROPPATCH", - Storages::UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path), - xml: body - ) - - error_data = Storages::StorageErrorData.new(source: self.class, payload: response) - - case response - in { status: 200..299 } - doc = Nokogiri::XML(response.body.to_s) - if doc.xpath("/d:multistatus/d:response/d:propstat[d:status[text() = 'HTTP/1.1 200 OK']]/d:prop/nc:acl-list").present? - ServiceResult.success(result: :success) - else - Util.error(:error, "nc:acl properly has not been set for #{path}", error_data) - end - in { status: 404 } - Util.error(:not_found, "Outbound request destination not found", error_data) - in { status: 401 } - Util.error(:unauthorized, "Outbound request not authorized", error_data) - else - Util.error(:error, "Outbound request failed", error_data) end end - - # rubocop:enable Metrics/AbcSize - # rubocop:enable Metrics/PerceivedComplexity end end diff --git a/modules/storages/app/models/storages/project_storage.rb b/modules/storages/app/models/storages/project_storage.rb index 3575337a0d3..20204e47aa9 100644 --- a/modules/storages/app/models/storages/project_storage.rb +++ b/modules/storages/app/models/storages/project_storage.rb @@ -65,6 +65,10 @@ module Storages managed_folder_identifier.path end + def managed_project_folder_name + managed_folder_identifier.name + end + def project_folder_location managed_folder_identifier.location end diff --git a/modules/storages/app/models/storages/storage.rb b/modules/storages/app/models/storages/storage.rb index 4dfa1910af7..22dddd75ec0 100644 --- a/modules/storages/app/models/storages/storage.rb +++ b/modules/storages/app/models/storages/storage.rb @@ -54,7 +54,6 @@ module Storages store_attribute :provider_fields, :automatically_managed, :boolean store_attribute :provider_fields, :health_notifications_enabled, :boolean, default: true - store_attribute :provider_fields, :detailed_logging_enabled, :boolean, default: false has_many :file_links, class_name: "Storages::FileLink" belongs_to :creator, class_name: "User" diff --git a/modules/storages/app/models/storages/storage_error.rb b/modules/storages/app/models/storages/storage_error.rb index 4ea1bf57c9e..68af9fa27c8 100644 --- a/modules/storages/app/models/storages/storage_error.rb +++ b/modules/storages/app/models/storages/storage_error.rb @@ -40,10 +40,9 @@ module Storages @data = data end - def to_active_model_errors(klass = self) - errors = ActiveModel::Errors.new(klass) - attr = data.source.to_s.demodulize.underscore.to_sym - errors.add(attr, code) + def to_active_model_errors + errors = ActiveModel::Errors.new(self) + errors.add(:storage_error, code, message: log_message) errors end 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 f8654d38c9b..e43f8a1ca58 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 @@ -30,13 +30,12 @@ module Storages class NextcloudGroupFolderPropertiesSyncService - using Peripherals::ServiceResultRefinements - extend ActiveModel::Naming extend ActiveModel::Translation - PERMISSIONS_MAP = { read_files: 1, write_files: 2, create_files: 4, delete_files: 8, share_files: 16 }.freeze + using Peripherals::ServiceResultRefinements + PERMISSIONS_MAP = { read_files: 1, write_files: 2, create_files: 4, delete_files: 8, share_files: 16 }.freeze PERMISSIONS_KEYS = PERMISSIONS_MAP.keys.freeze ALL_PERMISSIONS = PERMISSIONS_MAP.values.sum NO_PERMISSIONS = 0 @@ -61,8 +60,8 @@ module Storages def call with_logging do - log_message "Starting AMPF Sync for Nextcloud Storage #{@storage.id}" - prepare_remote_folders.on_failure { return _1 } + info "Starting AMPF Sync for Nextcloud Storage #{@storage.id}" + prepare_remote_folders.on_failure { return @result } apply_permissions_to_folders end end @@ -74,26 +73,24 @@ module Storages # @param options [Hash] optional extra parameters for the message generation # @return [ServiceResult] def add_error(attribute, storage_error, options: {}) - if storage_error == :error - @result.errors.add(:base, storage_error, **options) + case storage_error + when :error, :unauthorized + @result.errors.add(:base, storage_error.code, **options) else - @result.errors.add(attribute, storage_error, **options) + @result.errors.add(attribute, storage_error.code, **options) end + @result end # @return [ServiceResult] def prepare_remote_folders - remote_folders = remote_root_folder_properties.result_or do |error| - format_and_log_error(error, { folder: @storage.group_folder }) - return add_error(:remote_folder_properties, error.code, options: { group: @storage.group }).fail! - end + info "Preparing the remote group folder #{@storage.group_folder}" - ensure_root_folder_permissions.result_or do |error| - format_and_log_error(error, { folder: @storage.group_folder }) - return add_error(:ensure_root_folder_permissions, error.code, - options: { group: @storage.group, username: @storage.username }).fail! - end + remote_folders = remote_root_folder_map(@storage.group_folder).on_failure { return _1 }.result + info "Found #{remote_folders.count} remote folders" + + ensure_root_folder_permissions(@storage.group_folder, @storage.group, @storage.username).on_failure { return _1 } ensure_folders_exist(remote_folders).on_success { hide_inactive_folders(remote_folders) } end @@ -137,6 +134,7 @@ module Storages end end + # rubocop:disable Metrics/AbcSize def set_folders_permissions(remote_admins, project_storage) admin_permissions = remote_admins.to_set.map do |username| [username, ALL_PERMISSIONS] @@ -148,18 +146,22 @@ module Storages hash[token.origin_user_id] = PERMISSIONS_MAP.values_at(*(PERMISSIONS_KEYS & permissions)).sum end + folder = project_storage.managed_project_folder_path + command_params = { - path: project_storage.managed_project_folder_path, + path: folder, permissions: { users: admin_permissions.to_h.merge(users_permissions), groups: { "#{@storage.group}": NO_PERMISSIONS } } } - set_permissions.call(storage: @storage, **command_params).result_or do |error| - format_and_log_error(error, folder: project_storage.managed_project_folder_path) + set_permissions.call(storage: @storage, **command_params).on_failure do |service_result| + format_and_log_error(service_result.errors, folder:) + add_error(:set_folder_permission, service_result.errors, options: { folder: }) end end + # rubocop:enable Metrics/AbcSize def project_tokens(project_storage) project_tokens = client_tokens_scope.where.not(id: admin_client_tokens_scope).order(:id) @@ -171,108 +173,118 @@ module Storages end end + # rubocop:disable Metrics/AbcSize def hide_inactive_folders(remote_folders) - log_message "Hiding folders related to inactive projects" + info "Hiding folders related to inactive projects" project_folder_ids = active_project_storages_scope.pluck(:project_folder_id).compact + remote_folders.except("/#{@storage.group_folder}/").each do |(path, attrs)| next if project_folder_ids.include?(attrs["fileid"]) - log_message "Hiding project folder #{path}" - command_params = { - path:, - permissions: { - users: { "#{@storage.username}": ALL_PERMISSIONS }, - groups: { "#{@storage.group}": NO_PERMISSIONS } - } - } + info "Hiding folder #{path} as it does not belong to any active project" + command_params = { path:, + permissions: { + users: { "#{@storage.username}": ALL_PERMISSIONS }, + groups: { "#{@storage.group}": NO_PERMISSIONS } + } } set_permissions.call(storage: @storage, **command_params).on_failure do |service_result| format_and_log_error(service_result.errors, folder: path, context: "hide_folder") - add_error(:hide_inactive_folders, service_result.errors, options: { folder: path }) + add_error(:hide_inactive_folders, service_result.errors, options: { path: }) end end end + # rubocop:enable Metrics/AbcSize def ensure_folders_exist(remote_folders) - log_message "Ensuring project folders exist and are correctly named." + info "Ensuring that automatically managed project folders exist and are correctly named." id_folder_map = remote_folders.to_h { |folder, properties| [properties["fileid"], folder] } active_project_storages_scope.includes(:project).map do |project_storage| unless id_folder_map.key?(project_storage.project_folder_id) - log_message "#{project_storage.managed_project_folder_path} does not exist. Creating..." - next create_folder_stuff(project_storage) + info "#{project_storage.managed_project_folder_path} does not exist. Creating..." + next create_remote_folder(project_storage) end - current_path = id_folder_map[project_storage.project_folder_id] - if current_path != project_storage.managed_project_folder_path - log_message "#{current_path} is misnamed. Renaming to #{project_storage.managed_project_folder_path}" - target_folder_name = name_from_path(project_storage.managed_project_folder_path) - rename_folder(project_storage.project_folder_id, target_folder_name).on_failure do |service_result| - format_and_log_error(service_result.errors, - folder_id: project_storage.project_folder_id, - folder_name: target_folder_name) - - return add_error(:rename_folder, service_result.errors).fail! - end - end + rename_folder(project_storage, id_folder_map[project_storage.project_folder_id])&.on_failure { return _1 } end # We processed every folder successfully ServiceResult.success end - def name_from_path(path) - path.split("/").last + # @param project_storage [Storages::ProjectStorage] Storages::ProjectStorage that the remote folder might need renaming + # @param current_path [String] current name of the remote project storage folder + # @return [ServiceResult, nil] + def rename_folder(project_storage, current_path) + return if current_path == project_storage.managed_project_folder_path + + name = project_storage.managed_project_folder_name + file_id = project_storage.project_folder_id + + info "#{current_path} is misnamed. Renaming to #{name}" + rename_file.call(storage: @storage, auth_strategy:, file_id:, name:).on_failure do |service_result| + format_and_log_error(service_result.errors, folder_id: file_id, folder_name: name) + + add_error(:rename_project_folder, service_result.errors, + options: { project_folder_name: name, project_folder_id: file_id }).fail! + end end - def rename_folder(folder_id, folder_name) - rename_file.call(storage: @storage, auth_strategy:, file_id: folder_id, name: folder_name) - end - - def create_folder_stuff(project_storage) + def create_remote_folder(project_storage) folder_name = project_storage.managed_project_folder_path parent_location = Peripherals::ParentFolder.new("/") created_folder = create_folder.call(storage: @storage, auth_strategy:, folder_name:, parent_location:) - .result_or do |error| - format_and_log_error(error, folder_name:) + .on_failure do |service_result| + format_and_log_error(service_result.errors, folder_name:) - return add_error(:create_folder, error, options: { folder_name:, parent_location: }) - end + return add_error(:create_folder, service_result.errors, options: { folder_name:, parent_location: }) + end.result - project_folder_id = created_folder.id - last_project_folder = LastProjectFolder - .find_by(project_storage_id: project_storage.id, mode: project_storage.project_folder_mode) + last_project_folder = LastProjectFolder.find_by( + project_storage_id: project_storage.id, mode: project_storage.project_folder_mode + ) - audit_last_project_folder(last_project_folder, project_folder_id) - project_storage.project_folder_id + audit_last_project_folder(last_project_folder, created_folder.id) end def audit_last_project_folder(last_project_folder, project_folder_id) ApplicationRecord.transaction do - last_project_folder.update!(origin_folder_id: project_folder_id) - project_storage.update!(project_folder_id:) - project_storage.project_folder_id + success = last_project_folder.update(origin_folder_id: project_folder_id) && + last_project_folder.project_storage.update(project_folder_id:) + + raise ActiveRecord::Rollback unless success end end - def ensure_root_folder_permissions - log_message "Setting base permissions for user #{@storage.username} on the #{@storage.group_folder} folder" + # @param group_folder [string] name of the Group Folder in Nextcloud. + # @param username [String] username for the integration user + # @param group [String] group that the user should be part of + # @return [ServiceResult] + def ensure_root_folder_permissions(group_folder, username, group) + info "Setting needed permissions for user #{username} and group #{group} on #{group_folder} group folder" + command_params = { - path: @storage.group_folder, + path: group_folder, permissions: { - users: { @storage.username.to_sym => ALL_PERMISSIONS }, - groups: { @storage.group.to_sym => PERMISSIONS_MAP[:read_files] } + users: { username.to_sym => ALL_PERMISSIONS }, + groups: { group.to_sym => PERMISSIONS_MAP[:read_files] } } } - set_permissions.call(storage: @storage, **command_params) + set_permissions.call(storage: @storage, **command_params).on_failure do |service_result| + format_and_log_error(service_result.errors, { folder: group_folder }) + add_error(:ensure_root_folder_permissions, service_result.errors, options: { group:, username: }).fail! + end end - ### Base Queries/Commands - def remote_root_folder_properties - log_message "Retrieving already existing folders under #{@storage.group_folder}" - file_ids.call(storage: @storage, path: @storage.group_folder) + def remote_root_folder_map(group_folder) + info "Retrieving already existing folders under #{group_folder}" + file_ids.call(storage: @storage, path: group_folder).on_failure do |service_result| + format_and_log_error(service_result.errors, { folder: group_folder }) + add_error(:remote_folders, service_result.errors, options: { group_folder:, username: @storage.username }).fail! + end end def remote_group_users @@ -313,8 +325,8 @@ module Storages logger.error error_message end - def log_message(message) - logger.debug(message) + def info(message) + logger.info(message) end def with_logging(&) diff --git a/modules/storages/app/workers/storages/automatically_managed_storage_sync_job.rb b/modules/storages/app/workers/storages/automatically_managed_storage_sync_job.rb index ef942bd6790..205f7de9dcf 100644 --- a/modules/storages/app/workers/storages/automatically_managed_storage_sync_job.rb +++ b/modules/storages/app/workers/storages/automatically_managed_storage_sync_job.rb @@ -66,7 +66,7 @@ module Storages raise "Unknown Storage Type" end - sync_result.on_failure { raise Errors::IntegrationJobError, sync_result.errors.to_s } + sync_result.on_failure { raise Errors::IntegrationJobError, sync_result.errors.full_messages.join(", ") } sync_result.on_success { OpenProject::Notifications.send(OpenProject::Events::STORAGE_TURNED_HEALTHY, storage:) } end end diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 18500700dd8..733bbd741ab 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -1,14 +1,22 @@ --- en: services: + attributes: + storages/nextcloud_group_folder_properties_sync_service: + remote_folders: 'Reading contents of the group folder:' + ensure_root_folder_permissions: 'Setting Basic Permissions:' + create_folder: 'Managed Project Folder Creation:' + rename_project_folder: 'Renaming managed project Folder:' + hide_inactive_folders: 'Hide Inactive Folders Step:' errors: models: storages/nextcloud_group_folder_properties_sync_service: - unauthorized: "Please check if your AMPF password is correct" + unauthorized: "OpenProject could not sync with Nextcloud. Please check you storage and Nextcloud configuration" error: "An unexpected error occurred. Please ensure that you Nextcloud instance is reachable and check OpenProject worker logs for more information" attributes: - remote_folder_properties: - not_found: "The Group Folder %{group} wasn't found. Please check your Nextcloud setup." + remote_folders: + not_found: "%{group_folder} wasn't found. Please check your Nextcloud setup." + not_allowed: "The %{username} doesn't have access to the %{group_folder}. Please check the folder permissions on Nextcloud" activerecord: attributes: @@ -24,6 +32,7 @@ en: errors: messages: not_linked_to_project: is not linked to project. + invalid_url: is not a valid URL. models: storages/file_link: attributes: diff --git a/modules/storages/spec/common/storages/peripherals/registry_spec.rb b/modules/storages/spec/common/storages/peripherals/registry_spec.rb index 3cc5797e9b5..a7d4f988107 100644 --- a/modules/storages/spec/common/storages/peripherals/registry_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/registry_spec.rb @@ -372,15 +372,17 @@ RSpec.describe Storages::Peripherals::Registry, :webmock do context "when forbidden values are given as folder" do it "raises an ArgumentError on nil" do - expect do - registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: nil, permissions:) - end.to raise_error(ArgumentError) + 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 "raises an ArgumentError on empty string" do - expect do - registry.resolve("nextcloud.commands.set_permissions").call(path: "", permissions:) - end.to raise_error(ArgumentError) + 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 diff --git a/modules/storages/spec/services/storages/nextcloud_group_folder_properties_sync_service_spec.rb b/modules/storages/spec/services/storages/nextcloud_group_folder_properties_sync_service_spec.rb index efeb5a13824..ec71a0255b2 100644 --- a/modules/storages/spec/services/storages/nextcloud_group_folder_properties_sync_service_spec.rb +++ b/modules/storages/spec/services/storages/nextcloud_group_folder_properties_sync_service_spec.rb @@ -701,8 +701,9 @@ RSpec.describe Storages::NextcloudGroupFolderPropertiesSyncService, :webmock do result = described_class.new(storage).call expect(result).to be_failure - expect(result.errors[:remote_folder_properties]) - .to contain_exactly(I18n.t("#{prefix}.attributes.remote_folder_properties.not_found", group: storage.group)) + expect(result.errors[:remote_folders]) + .to contain_exactly(I18n.t("#{prefix}.attributes.remote_folders.not_found", + group_folder: storage.group_folder)) end end diff --git a/modules/storages/spec/workers/storages/automatically_managed_storage_sync_job_spec.rb b/modules/storages/spec/workers/storages/automatically_managed_storage_sync_job_spec.rb index 81a961f2b4c..d29b77a6433 100644 --- a/modules/storages/spec/workers/storages/automatically_managed_storage_sync_job_spec.rb +++ b/modules/storages/spec/workers/storages/automatically_managed_storage_sync_job_spec.rb @@ -91,10 +91,13 @@ RSpec.describe Storages::AutomaticallyManagedStorageSyncJob, type: :job do allow(Storages::HealthStatusMailerJob).to receive(:set).and_return(job) allow(job).to receive(:perform_later) + errors = ActiveModel::Errors.new(Storages::NextcloudGroupFolderPropertiesSyncService.new(managed_nextcloud)) + errors.add(:group_folder, :not_found, group_folder: managed_nextcloud.group_folder) + allow(Storages::NextcloudGroupFolderPropertiesSyncService) .to receive(:call) .with(managed_nextcloud) - .and_return(ServiceResult.failure(errors: Storages::StorageError.new(code: :not_found))) + .and_return(ServiceResult.failure(errors:)) Timecop.freeze("2023-03-14T15:17:00Z") do expect do @@ -103,16 +106,19 @@ RSpec.describe Storages::AutomaticallyManagedStorageSyncJob, type: :job do end.to( change(managed_nextcloud, :health_changed_at).to(Time.now.utc) .and(change(managed_nextcloud, :health_status).from("pending").to("unhealthy")) - .and(change(managed_nextcloud, :health_reason).from(nil).to("not_found")) + .and(change(managed_nextcloud, :health_reason).from(nil).to(/wasn't found/)) ) end end context "when Storages::Errors::IntegrationJobError is raised" do before do + errors = ActiveModel::Errors.new(Storages::NextcloudGroupFolderPropertiesSyncService.new(managed_nextcloud)) + errors.add(:base, :error) + allow(Storages::NextcloudGroupFolderPropertiesSyncService) .to receive(:call).with(managed_nextcloud) - .and_return(ServiceResult.failure(errors: Storages::StorageError.new(code: :custom_error))) + .and_return(ServiceResult.failure(errors:)) allow(OpenProject::Notifications).to receive(:send) end @@ -130,7 +136,7 @@ RSpec.describe Storages::AutomaticallyManagedStorageSyncJob, type: :job do expect(OpenProject::Notifications).to have_received(:send).with( OpenProject::Events::STORAGE_TURNED_UNHEALTHY, storage: managed_nextcloud, - reason: "custom_error" + reason: /unexpected error occurred/ ) end end