From 6dee7763ea3cebd9cedb26b0cbfd37317687dcba Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Wed, 25 Sep 2024 10:38:05 +0200 Subject: [PATCH 1/2] [#58018] no group folder content validation if ampf is disabled - https://community.openproject.org/work_packages/58018 - added new validation rule, if userless authentication fails --- .../nextcloud_connection_validator.rb | 43 +++- modules/storages/config/locales/en.yml | 1 + .../nextcloud_connection_validator_spec.rb | 227 +++++++++--------- 3 files changed, 146 insertions(+), 125 deletions(-) diff --git a/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb b/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb index c275c310b732..5c64da713a48 100644 --- a/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb +++ b/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb @@ -39,30 +39,38 @@ def initialize(storage:) @storage = storage end - # rubocop:disable Metrics/AbcSize def validate maybe_is_not_configured - .or { host_url_not_found } - .or { missing_dependencies } - .or { version_mismatch } - .or { with_unexpected_content } - .or { group_folder_not_found } - .or { capabilities_request_failed_with_unknown_error } - .or { files_request_failed_with_unknown_error } + .or { has_base_configuration_error? } + .or { has_ampf_configuration_error? } .value_or(ConnectionValidation.new(type: :healthy, error_code: :none, timestamp: Time.current, description: nil)) end - # rubocop:enable Metrics/AbcSize - private + def has_base_configuration_error? + host_url_not_found + .or { missing_dependencies } + .or { version_mismatch } + .or { with_unexpected_content } + .or { capabilities_request_failed_with_unknown_error } + end + + def has_ampf_configuration_error? + return None() unless @storage.automatic_management_enabled? + + userless_access_denied + .or { group_folder_not_found } + .or { files_request_failed_with_unknown_error } + end + def capabilities @capabilities ||= Peripherals::Registry - .resolve("#{@storage.short_provider_type}.queries.capabilities") - .call(storage: @storage, auth_strategy: noop) + .resolve("#{@storage.short_provider_type}.queries.capabilities") + .call(storage: @storage, auth_strategy: noop) end def files @@ -156,6 +164,15 @@ def version_mismatch # rubocop:enable Metrics/AbcSize + def userless_access_denied + return None() if files.result != :unauthorized + + Some(ConnectionValidation.new(type: :error, + error_code: :err_userless_access_denied, + timestamp: Time.current, + description: I18n.t("storages.health.connection_validation.userless_access_denied"))) + end + def group_folder_not_found return None() if files.result != :not_found @@ -195,6 +212,7 @@ def capabilities_request_failed_with_unknown_error Rails.logger.error( "Connection validation failed with unknown error:\n\t" \ "storage: ##{@storage.id} #{@storage.name}\n\t" \ + "request: Nextcloud capabilities\n\t" \ "status: #{capabilities.result}\n\t" \ "response: #{capabilities.error_payload}" ) @@ -211,6 +229,7 @@ def files_request_failed_with_unknown_error Rails.logger.error( "Connection validation failed with unknown error:\n\t" \ "storage: ##{@storage.id} #{@storage.name}\n\t" \ + "request: Group folder content\n\t" \ "status: #{files.result}\n\t" \ "response: #{files.error_payload}" ) diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index 12e36098ad22..0471043a59f9 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -227,6 +227,7 @@ en: nextcloud: Unexpected content found in the managed group folder. one_drive: Unexpected content found in the drive. unknown_error: The connection could not be validated. An unknown error occurred. Please check the server logs for further information. + userless_access_denied: The configured app password is invalid. label_error: Error label_healthy: Healthy label_pending: Pending diff --git a/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb b/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb index f9ddf0a3b403..7ceefc484563 100644 --- a/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb @@ -47,38 +47,18 @@ end end - context "if nextcloud host url could not be found" do - let(:storage) { create(:nextcloud_storage_configured) } - let(:capabilities_response) { build_failure(code: :not_found, payload: nil) } - - it "returns a validation failure" do - expect(subject.type).to eq(:error) - expect(subject.error_code).to eq(:err_host_not_found) - expect(subject.description) - .to eq("No Nextcloud server found at the configured host url. Please check the configuration.") - end - end - - context "if request returns a capabilities response" do - let(:storage) { create(:nextcloud_storage_configured, :as_automatically_managed) } + context "if storage is not configured for automatic folder management" do + let(:storage) { create(:nextcloud_storage_configured, :as_not_automatically_managed) } let(:app_enabled) { true } let(:app_version) { Storages::SemanticVersion.parse("2.6.3") } - let(:group_folder_enabled) { true } - let(:group_folder_version) { Storages::SemanticVersion.parse("17.0.1") } - let(:project_folder_id) { "1337" } + # let(:group_folder_enabled) { true } + # let(:group_folder_version) { Storages::SemanticVersion.parse("17.0.1") } let(:capabilities_response) do ServiceResult.success(result: Storages::NextcloudCapabilities.new( app_enabled?: app_enabled, app_version:, - group_folder_enabled?: group_folder_enabled, - group_folder_version: - )) - end - let(:files_response) do - ServiceResult.success(result: Storages::StorageFiles.new( - [], - Storages::StorageFile.new(id: "root", name: "root"), - [] + group_folder_enabled?: false, + group_folder_version: nil )) end @@ -87,6 +67,17 @@ expect(subject.error_code).to eq(:none) end + context "if nextcloud host url could not be found" do + let(:capabilities_response) { build_failure(code: :not_found, payload: nil) } + + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_host_not_found) + expect(subject.description) + .to eq("No Nextcloud server found at the configured host url. Please check the configuration.") + end + end + context "if nextcloud server instance is badly configured" do context "with missing integration app" do let(:app_enabled) { false } @@ -110,122 +101,132 @@ end end - context "with disabled/not installed group folder app" do - let(:group_folder_enabled) { false } + context "if capabilities query returns an unhandled error" do + let(:capabilities_response) { build_failure(code: :error, payload: nil) } + + before do + allow(Rails.logger).to receive(:error) + end it "returns a validation failure" do expect(subject.type).to eq(:error) - expect(subject.error_code).to eq(:err_missing_dependencies) - expect(subject.description).to eq("A required dependency is missing on the file storage. " \ - "Please add the following dependency: Group folders.") + expect(subject.error_code).to eq(:err_unknown) + expect(subject.description).to eq("The connection could not be validated. An unknown error occurred. " \ + "Please check the server logs for further information.") end - context "if storage is not automatically_managed" do - let(:storage) { create(:nextcloud_storage_configured) } - - it "does not check group_folder app" do - expect(subject.type).to eq(:healthy) - expect(subject.error_code).to eq(:none) - end + it "logs the error message" do + described_class.new(storage:).validate + expect(Rails.logger).to have_received(:error).with(/Connection validation failed with unknown error/) end end + end + end - context "with outdated group folder app" do - let(:group_folder_version) { Storages::SemanticVersion.parse("11.0.1") } + context "if storage is configured for automatic folder management" do + let(:storage) { create(:nextcloud_storage_configured, :as_automatically_managed) } + let(:group_folder_enabled) { true } + let(:group_folder_version) { Storages::SemanticVersion.parse("17.0.1") } + let(:capabilities_response) do + ServiceResult.success(result: Storages::NextcloudCapabilities.new( + app_enabled?: true, + app_version: Storages::SemanticVersion.parse("2.6.3"), + group_folder_enabled?: group_folder_enabled, + group_folder_version: + )) + end + let(:project_folder_id) { "1337" } + let(:project_storage) do + create(:project_storage, + :as_automatically_managed, + project_folder_id:, + storage:, + project: create(:project)) + end - it "returns a validation failure" do - expect(subject.type).to eq(:error) - expect(subject.error_code).to eq(:err_unexpected_version) - expect(subject.description) - .to eq("The Group Folder version is not supported. Please update your Nextcloud server.") - end + before { project_storage } - context "if storage is not automatically_managed" do - let(:storage) { create(:nextcloud_storage_configured) } + context "with disabled/not installed group folder app" do + let(:group_folder_enabled) { false } - it "does not check group_folder app" do - expect(subject.type).to eq(:healthy) - expect(subject.error_code).to eq(:none) - end - end + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_missing_dependencies) + expect(subject.description).to eq("A required dependency is missing on the file storage. " \ + "Please add the following dependency: Group folders.") end end - context "with configured ProjectStorage" do - let(:storage) { create(:nextcloud_storage_configured, :as_automatically_managed) } - let(:project_storage) do - create(:project_storage, - :as_automatically_managed, - project_folder_id:, - storage:, - project: create(:project)) - end + context "with outdated group folder app" do + let(:group_folder_version) { Storages::SemanticVersion.parse("11.0.1") } - before { project_storage } + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_unexpected_version) + expect(subject.description) + .to eq("The Group Folder version is not supported. Please update your Nextcloud server.") + end + end - context "if the request returns not_found" do - let(:files_response) { build_failure(code: :not_found, payload: nil) } + context "if userless authentication fails" do + let(:files_response) { build_failure(code: :unauthorized, payload: nil) } - it "returns a validation failure" do - expect(subject.type).to eq(:error) - expect(subject.error_code).to eq(:err_group_folder_not_found) - expect(subject.description).to eq("The group folder could not be found.") - end + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_userless_access_denied) + expect(subject.description).to eq("The configured app password is invalid.") end + end - context "if the request returns an error" do - let(:files_response) do - Storages::Peripherals::StorageInteraction::Nextcloud::Util.error(:error) - end + context "if the files request returns not_found" do + let(:files_response) { build_failure(code: :not_found, payload: nil) } - it "returns a validation failure" do - expect(subject.type).to eq(:error) - expect(subject.error_code).to eq(:err_unknown) - expect(subject.description).to eq("The connection could not be validated. An unknown error occurred. " \ - "Please check the server logs for further information.") - end + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_group_folder_not_found) + expect(subject.description).to eq("The group folder could not be found.") end + end - context "if the request returns unexpected files" do - let(:files_response) do - ServiceResult.success(result: Storages::StorageFiles.new( - [ - Storages::StorageFile.new(id: project_folder_id, name: "I am your father"), - Storages::StorageFile.new(id: "noooooooooo", name: "testimony_of_luke_skywalker.md") - ], - Storages::StorageFile.new(id: "root", name: "root"), - [] - )) - end + context "if the files request returns an unknown error" do + let(:files_response) do + Storages::Peripherals::StorageInteraction::Nextcloud::Util.error(:error) + end - it "returns a validation failure" do - expect(subject.type).to eq(:warning) - expect(subject.error_code).to eq(:wrn_unexpected_content) - expect(subject.description).to eq("Unexpected content found in the managed group folder.") - end + before do + allow(Rails.logger).to receive(:error) end - end - end - context "if query returns an unhandled error" do - let(:storage) { create(:nextcloud_storage_configured) } - let(:capabilities_response) { build_failure(code: :error, payload: nil) } - let(:files_response) { build_failure(code: :error, payload: nil) } + it "returns a validation failure" do + expect(subject.type).to eq(:error) + expect(subject.error_code).to eq(:err_unknown) + expect(subject.description).to eq("The connection could not be validated. An unknown error occurred. " \ + "Please check the server logs for further information.") + end - before do - allow(Rails.logger).to receive(:error) + it "logs the error message" do + described_class.new(storage:).validate + expect(Rails.logger).to have_received(:error).with(/Connection validation failed with unknown error/) + end end - it "returns a validation failure" do - expect(subject.type).to eq(:error) - expect(subject.error_code).to eq(:err_unknown) - expect(subject.description).to eq("The connection could not be validated. An unknown error occurred. " \ - "Please check the server logs for further information.") - end + context "if the files request returns unexpected files" do + let(:files_response) do + ServiceResult.success(result: Storages::StorageFiles.new( + [ + Storages::StorageFile.new(id: project_folder_id, name: "I am your father"), + Storages::StorageFile.new(id: "noooooooooo", name: "testimony_of_luke_skywalker.md") + ], + Storages::StorageFile.new(id: "root", name: "root"), + [] + )) + end - it "logs the error message" do - described_class.new(storage:).validate - expect(Rails.logger).to have_received(:error).with(/Connection validation failed with unknown error/) + it "returns a validation failure" do + expect(subject.type).to eq(:warning) + expect(subject.error_code).to eq(:wrn_unexpected_content) + expect(subject.description).to eq("Unexpected content found in the managed group folder.") + end end end From aff09c10a8e008487b6fac5c12b5fff20e9b0e34 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Wed, 25 Sep 2024 11:43:59 +0200 Subject: [PATCH 2/2] [#58018] removed commented lines --- .../storages/peripherals/nextcloud_connection_validator_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb b/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb index 7ceefc484563..f95786a23f04 100644 --- a/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb +++ b/modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb @@ -51,8 +51,6 @@ let(:storage) { create(:nextcloud_storage_configured, :as_not_automatically_managed) } let(:app_enabled) { true } let(:app_version) { Storages::SemanticVersion.parse("2.6.3") } - # let(:group_folder_enabled) { true } - # let(:group_folder_version) { Storages::SemanticVersion.parse("17.0.1") } let(:capabilities_response) do ServiceResult.success(result: Storages::NextcloudCapabilities.new( app_enabled?: app_enabled,