Merge pull request opf#16411 from opf/implementation/55975-use-authentication-in-set_permission_command

[#55975] introduced auth strategy for set permissions command

Author: Kharonus

Diff for: 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 for: 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 for: 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 for: 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] })
@@ -212,6 +212,7 @@ def admin_remote_identities_scope
     end
 
     def root_folder = Peripherals::ParentFolder.new("/")
+
     def auth_strategy = userless.call
 
     # @param attribute [Symbol] attribute to which the error will be tied to

Diff for: 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
-        <?xml version="1.0"?>
-        <d:propertyupdate xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns">
-          <d:set>
-            <d:prop>
-              <nc:acl-list>
-                <nc:acl>
-                  <nc:acl-mapping-type>group</nc:acl-mapping-type>
-                  <nc:acl-mapping-id>OpenProject</nc:acl-mapping-id>
-                  <nc:acl-mask>31</nc:acl-mask>
-                  <nc:acl-permissions>0</nc:acl-permissions>
-                </nc:acl>
-                <nc:acl>
-                  <nc:acl-mapping-type>user</nc:acl-mapping-type>
-                  <nc:acl-mapping-id>OpenProject</nc:acl-mapping-id>
-                  <nc:acl-mask>31</nc:acl-mask>
-                  <nc:acl-permissions>31</nc:acl-permissions>
-                </nc:acl>
-                <nc:acl>
-                  <nc:acl-mapping-type>user</nc:acl-mapping-type>
-                  <nc:acl-mapping-id>Obi-Wan</nc:acl-mapping-id>
-                  <nc:acl-mask>31</nc:acl-mask>
-                  <nc:acl-permissions>31</nc:acl-permissions>
-                </nc:acl>
-                <nc:acl>
-                  <nc:acl-mapping-type>user</nc:acl-mapping-type>
-                  <nc:acl-mapping-id>Qui-Gon</nc:acl-mapping-id>
-                  <nc:acl-mask>31</nc:acl-mask>
-                  <nc:acl-permissions>31</nc:acl-permissions>
-                </nc:acl>
-              </nc:acl-list>
-            </d:prop>
-          </d:set>
-        </d:propertyupdate>
-      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
-              <?xml version="1.0"?>
-              <d:multistatus
-                xmlns:d="DAV:"
-                xmlns:s="http://sabredav.org/ns"
-                xmlns:oc="http://owncloud.org/ns"
-                xmlns:nc="http://nextcloud.org/ns">
-                <d:response>
-                  <d:href>/remote.php/dav/files/OpenProject/OpenProject/Project%231</d:href>
-                  <d:propstat>
-                    <d:prop>
-                      <nc:acl-list/>
-                    </d:prop>
-                    <d:status>HTTP/1.1 200 OK</d:status>
-                  </d:propstat>
-                </d:response>
-              </d:multistatus>
-            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
-              <?xml version="1.0" encoding="utf-8"?>
-              <d:error
-                xmlns:d="DAV:"
-                xmlns:s="http://sabredav.org/ns">
-                <s:exception>Sabre\DAV\Exception\NotAuthenticated</s:exception>
-                <s:message>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</s:message>
-              </d:error>
-            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
-              <?xml version="1.0" encoding="utf-8"?>
-              <d:error
-                xmlns:d="DAV:"
-                xmlns:s="http://sabredav.org/ns">
-                <s:exception>Sabre\DAV\Exception\NotFound</s:exception>
-                <s:message>File with name /OpenProject/JediProject could not be located</s:message>
-              </d:error>
-            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}" }