Merge pull request #16859 from opf/implementation/57707-use-authentic…

…ation-in-group_users_query

[#57707] rework group users query

modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command.rb

@@ -50,7 +50,7 @@ def call(auth_strategy:, user:, group:)
                 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) })
+                                     form: { "groupid" => group })
 
                 handle_response(response)
               end

modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/group_users_query.rb

@@ -34,51 +34,67 @@ module StorageInteraction
       module Nextcloud
         class GroupUsersQuery
           include TaggedLogging
-          using ServiceResultRefinements
 
-          def self.call(storage:, group: storage.group)
-            new(storage).call(group:)
+          def self.call(storage:, auth_strategy:, group:)
+            new(storage).call(auth_strategy:, group:)
           end
 
           def initialize(storage)
             @storage = storage
-            @username = storage.username
-            @password = storage.password
           end
 
-          # rubocop:disable Metrics/AbcSize
-          def call(group:)
+          def call(auth_strategy:, group:)
             with_tagged_logger do
-              url = UrlBuilder.url(@storage.uri, "ocs/v1.php/cloud/groups", CGI.escapeURIComponent(group))
+              Authentication[auth_strategy].call(storage: @storage, http_options:) do |http|
+                url = UrlBuilder.url(@storage.uri, "ocs/v1.php/cloud/groups", group)
+                info "Requesting user list for group #{group} via url #{url} "
 
-              info "Requesting user list for group #{group} via url #{url} "
-              response = OpenProject.httpx
-                                    .basic_auth(@username, @password)
-                                    .with(headers: { "OCS-APIRequest" => "true" })
-                                    .get(url)
+                handle_response(http.get(url))
+              end
+            end
+          end
 
-              error_data = StorageErrorData.new(source: self.class, payload: response)
+          private
 
-              case response
-              in { status: 200..299 }
-                group_users = Nokogiri::XML(response.body.to_s).xpath("/ocs/data/users/element").map(&:text)
-                info "#{group_users.size} users found"
-                ServiceResult.success(result: group_users)
-              in { status: 405 }
-                Util.error(:not_allowed, "Outbound request method not allowed", error_data)
-              in { status: 401 }
-                Util.error(:unauthorized, "Outbound request not authorized", error_data)
-              in { status: 404 }
-                Util.error(:not_found, "Outbound request destination not found", error_data)
-              in { status: 409 }
-                Util.error(:conflict, error_text_from_response(response), error_data)
-              else
-                Util.error(:error, "Outbound request failed", error_data)
-              end
+          def http_options
+            Util.ocs_api_request
+          end
+
+          def handle_response(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(:unauthorized, "Outbound request not authorized", error_data)
+            in { status: 404 }
+              Util.error(:not_found, "Outbound request destination not found", error_data)
+            in { status: 409 }
+              Util.error(:conflict, error_text_from_response(response), error_data)
+            else
+              Util.error(:error, "Outbound request failed", error_data)
             end
           end
 
-          # rubocop:enable Metrics/AbcSize
+          def handle_success_response(response)
+            error_data = StorageErrorData.new(source: self.class, payload: response)
+            xml = Nokogiri::XML(response.body.to_s)
+            statuscode = xml.xpath("/ocs/meta/statuscode").text
+
+            case statuscode
+            when "100"
+              group_users = xml.xpath("/ocs/data/users/element").map(&:text)
+              info "#{group_users.size} users found"
+              ServiceResult.success(result: group_users)
+            when "404"
+              Util.error(:group_does_not_exist, "Group does not exist", error_data)
+            else
+              Util.error(:error, "Unknown response body", error_data)
+            end
+          end
         end
       end
     end

modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/util.rb

@@ -62,20 +62,6 @@ def error(code, log_message = nil, data = nil)
               )
             end
 
-            def token(user:, configuration:, &)
-              connection_manager = OAuthClients::ConnectionManager.new(user:, configuration:)
-              connection_manager.get_access_token.match(
-                on_success: lambda do |token|
-                  connection_manager.request_with_token_refresh(token) { yield token }
-                end,
-                on_failure: lambda do |_|
-                  error(:unauthorized,
-                        "Query could not be created! No access token found!",
-                        StorageErrorData.new(source: connection_manager))
-                end
-              )
-            end
-
             def error_text_from_response(response)
               response.xml.xpath("//s:message").text
             end

modules/storages/app/services/storages/nextcloud_managed_folder_sync_service.rb

@@ -308,7 +308,7 @@ def build_set_permissions_input_data(file_id, user_permissions)
 
     def remote_group_users
       info "Retrieving users that a part of the #{@storage.group} group"
-      group_users.call(storage: @storage, group: @storage.group)
+      group_users.call(storage: @storage, auth_strategy:, group: @storage.group)
     end
 
     ### Model Scopes

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

@@ -56,56 +56,6 @@
     end
   end
 
-  describe "#group_users_query" do
-    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>
-            <users>
-            <element>admin</element>
-            <element>OpenProject</element>
-            <element>reader</element>
-            <element>TestUser</element>
-            <element>TestUser34</element>
-            </users>
-          </data>
-        </ocs>
-      XML
-    end
-    let(:expected_response) do
-      {
-        status: 200,
-        body: expected_response_body,
-        headers: {}
-      }
-    end
-
-    before do
-      stub_request(:get, "https://example.com/ocs/v1.php/cloud/groups/#{storage.group}")
-        .with(
-          headers: {
-            "Authorization" => "Basic T3BlblByb2plY3Q6T3BlblByb2plY3RTZWN1cmVQYXNzd29yZA==",
-            "OCS-APIRequest" => "true"
-          }
-        )
-        .to_return(expected_response)
-    end
-
-    it "responds with a strings array with group users" do
-      result = registry.resolve("nextcloud.queries.group_users").call(storage:)
-      expect(result).to be_success
-      expect(result.result).to eq(%w[admin OpenProject reader TestUser TestUser34])
-    end
-  end
-
   describe "#delete_folder_command" do
     let(:auth_strategy) { Storages::Peripherals::StorageInteraction::AuthenticationStrategies::BasicAuth.strategy }
 

modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command_spec.rb

@@ -0,0 +1,123 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+require_module_spec_helper
+
+RSpec.describe Storages::Peripherals::StorageInteraction::Nextcloud::AddUserToGroupCommand, :webmock do
+  include NextcloudGroupUserHelper
+
+  let(:storage) { create(:nextcloud_storage_with_local_connection, :as_automatically_managed, username: "vcr") }
+  let(:auth_strategy) { Storages::Peripherals::Registry.resolve("nextcloud.authentication.userless").call }
+
+  describe "basic command setup" do
+    it "is registered as commands.add_user_to_group" do
+      expect(Storages::Peripherals::Registry
+               .resolve("#{storage}.commands.add_user_to_group")).to eq(described_class)
+    end
+
+    it "responds to #call with correct parameters" do
+      expect(described_class).to respond_to(:call)
+
+      method = described_class.method(:call)
+      expect(method.parameters).to contain_exactly(%i[keyreq storage],
+                                                   %i[keyreq auth_strategy],
+                                                   %i[keyreq user],
+                                                   %i[keyreq group])
+    end
+  end
+
+  shared_examples_for "failing request" do |error_code:|
+    it "returns a failure" do
+      result = described_class.call(storage:, auth_strategy:, user:, group:)
+      expect(result).to be_failure
+
+      error = result.errors
+      expect(error.code).to eq(error_code)
+      expect(error.data.source).to eq(described_class)
+    end
+  end
+
+  context "if group exists", vcr: "nextcloud/add_user_to_group_success" do
+    let(:user) { "[email protected]" }
+    let(:group) { "Sith Assassins" }
+
+    before do
+      create_group(auth, storage, group)
+    end
+
+    after do
+      remove_group(auth, storage, group)
+    end
+
+    it "returns a success" do
+      members = group_members(group)
+      expect(members).not_to include(user)
+
+      result = described_class.call(storage:, auth_strategy:, user:, group:)
+      expect(result).to be_success
+
+      members = group_members(group)
+      expect(members).to include(user)
+    end
+  end
+
+  context "if target group does not exist", vcr: "nextcloud/add_user_to_group_not_existing_group" do
+    let(:user) { "[email protected]" }
+    let(:group) { "Sith Assassins" }
+
+    it_behaves_like "failing request", error_code: :group_does_not_exist
+  end
+
+  context "if user does not exist", vcr: "nextcloud/add_user_to_group_not_existing_user" do
+    let(:user) { "this is not the user you are looking for" }
+    let(:group) { "Sith Assassins" }
+
+    before do
+      create_group(auth, storage, group)
+    end
+
+    after do
+      remove_group(auth, storage, group)
+    end
+
+    it_behaves_like "failing request", error_code: :user_does_not_exist
+  end
+
+  private
+
+  def auth = Storages::Peripherals::StorageInteraction::Authentication[auth_strategy]
+
+  def group_members(group)
+    Storages::Peripherals::StorageInteraction::Nextcloud::GroupUsersQuery
+      .call(storage:, auth_strategy:, group:)
+      .result
+  end
+end