Merge pull request #16803 from opf/bug/58018-nextcloud-connection-val…

…idation-fails-on-disabled-ampf-configuration

[#58018] no group folder content validation if ampf is disabled

modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb

@@ -39,35 +39,43 @@ 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}.queries.capabilities")
-                     .call(storage: @storage, auth_strategy: noop)
+                            .resolve("#{@storage}.queries.capabilities")
+                            .call(storage: @storage, auth_strategy: noop)
       end
 
       def files
         @files ||= Peripherals::Registry
-                     .resolve("#{@storage.short_provider_type}.queries.files")
+                     .resolve("#{@storage}.queries.files")
                      .call(storage: @storage, auth_strategy: userless, folder: ParentFolder.new(@storage.group_folder))
       end
 
@@ -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}"
         )

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

modules/storages/spec/common/storages/peripherals/nextcloud_connection_validator_spec.rb

@@ -34,7 +34,7 @@
 RSpec.describe Storages::Peripherals::NextcloudConnectionValidator do
   before do
     Storages::Peripherals::Registry.stub("#{storage}.queries.capabilities", ->(_) { capabilities_response })
-    Storages::Peripherals::Registry.stub("#{storage.short_provider_type}.queries.files", ->(_) { files_response })
+    Storages::Peripherals::Registry.stub("#{storage}.queries.files", ->(_) { files_response })
   end
 
   subject { described_class.new(storage:).validate }
@@ -49,38 +49,16 @@
     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(: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
 
@@ -89,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 }
@@ -112,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