Merge pull request #16858 from opf/implementation/57709-use-authentic…

…ation-in-add_user_to_group_command [#57709] rework add user command
opf · Oct 7, 2024 · b3b8af8 · b3b8af8
2 parents d4d2057 + ba8e921
commit b3b8af8
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 93 deletions.
diff --git a/...pp/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command.rb b/...pp/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
@@ -113,10 +113,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
@@ -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
-        <?xml version="1.0"?>
-        <ocs>
-          <meta>
-            <status>ok</status>
-            <statuscode>100</statuscode>
-            <message>OK</message>
-            <totalitems></totalitems>
-            <itemsperpage></itemsperpage>
-          </meta>
-          <data/>
-        </ocs>
-      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
@@ -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",