Validate storage host to not redirect to "/index.php"

If the storage is a Nextcloud instance and it redirects
to a new path containing "/index.php" then it is a strong
indicator that the Nextcloud server was not fully set up.
It usually means that the Apache is not fully configured
and thus the Bearer authorization tokens get stripped
from API calls.

https://community.openproject.org/work_packages/43297

modules/storages/app/validator/nextcloud_compatible_host_validator.rb

@@ -31,23 +31,48 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
   def validate_each(contract, attribute, value)
     return unless contract.model.changed_attributes.include?(attribute)
 
+    validate_capabilities(contract, attribute, value)
+    validate_setup_completeness(contract, attribute, value) if contract.errors.empty?
+  end
+
+  private
+
+  def validate_capabilities(contract, attribute, value)
     response = request_capabilities(value)
-    error_type = check_response(response)
+    error_type = check_capabilities_response(response)
+
     if error_type
       contract.errors.add(attribute, error_type)
       Rails.logger.info(message(value, response, error_type))
     end
   end
 
-  private
+  # Apparently some Nextcloud installations do not use mod_rewrite. Then requesting its app root (the storage host name)
+  # the response is a redirect to a URI containing 'index.php' in its path. If that is the case that installation
+  # is not compatible with our integration. It is missing support for Bearer token based authorization. Apparently
+  # Apache strips that part of the request header by default.
+  # https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/oauth2.html
+  def validate_setup_completeness(contract, attribute, value)
+    response = request_host(value)
+    error_type = check_host_response(response)
+
+    if error_type
+      contract.errors.add(attribute, error_type)
+      Rails.logger.info(message(value, response, error_type))
+    end
+  end
 
-  def check_response(response)
+  def check_capabilities_response(response)
     return :cannot_be_connected_to if response.is_a? StandardError
     return :cannot_be_connected_to unless response.is_a? Net::HTTPSuccess
     return :not_nextcloud_server unless json_response?(response)
     return :minimal_nextcloud_version_unmet unless major_version_sufficient?(response)
   end
 
+  def check_host_response(response)
+    :setup_incomplete if response.code == '302' && response.header['location']&.include?("/index.php")
+  end
+
   def message(host, response_or_exception, error_type)
     if response_or_exception.is_a?(Net::HTTPResponse)
       response = response_or_exception
@@ -80,11 +105,8 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
     true
   end
 
-  def request_capabilities(host)
-    uri = URI.parse(File.join(host, '/ocs/v2.php/cloud/capabilities'))
-    request = Net::HTTP::Get.new(uri)
-    request["Ocs-Apirequest"] = "true"
-    request["Accept"] = "application/json"
+  def make_request(request)
+    uri = request.uri
 
     req_options = {
       max_retries: 0,
@@ -102,6 +124,22 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
     end
   end
 
+  def request_capabilities(host)
+    uri = URI.parse(File.join(host, '/ocs/v2.php/cloud/capabilities'))
+    request = Net::HTTP::Get.new(uri)
+    request["Ocs-Apirequest"] = "true"
+    request["Accept"] = "application/json"
+
+    make_request(request)
+  end
+
+  def request_host(host)
+    uri = URI.parse(host)
+    request = Net::HTTP::Get.new(uri)
+
+    make_request(request)
+  end
+
   def json_response?(response)
     (
       response['content-type'].split(';').first.strip.downcase == 'application/json' \

modules/storages/config/locales/en.yml

@@ -26,6 +26,10 @@ en:
               cannot_be_connected_to: "can not be connected to."
               minimal_nextcloud_version_unmet: "does not meet minimal version requirements (must be Nextcloud 23 or higher)"
               not_nextcloud_server: "is not a Nextcloud server"
+              setup_incomplete: >
+                is not fully set up. That Nextcloud instance still redirects to a URL containing 'index.php'. That is a 
+                strong indicator that the server does not have mod_rewrite, mod_headers and/or mod_env fully configured,
+                which is necessary for a Bearer token based authorization of API requests.
         storages/file_link:
           attributes:
             origin_id:

modules/storages/spec/contracts/storages/storages/base_contract_spec.rb

@@ -35,13 +35,16 @@ describe Storages::Storages::BaseContract, :storage_server_helpers, with_flag: {
   let(:contract) { described_class.new(storage, current_user) }
 
   it 'checks the storage url only when changed' do
-    request = mock_server_capabilities_response(storage_host)
+    capabilities_request = mock_server_capabilities_response(storage_host)
+    host_request = mock_server_host_response(storage_host)
     contract.valid?
-    expect(request).to have_been_made.once
+    expect(capabilities_request).to have_been_made.once
+    expect(host_request).to have_been_made.once
 
     WebMock.reset_executed_requests!
     storage.save
     contract.valid?
-    expect(request).not_to have_been_made
+    expect(capabilities_request).not_to have_been_made
+    expect(host_request).not_to have_been_made
   end
 end

modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb

@@ -38,7 +38,10 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
   let(:storage_creator) { current_user }
 
   before do
-    mock_server_capabilities_response(storage_host) if storage_host
+    if storage_host.present?
+      mock_server_capabilities_response(storage_host)
+      mock_server_host_response(storage_host)
+    end
   end
 
   it_behaves_like 'contract is valid for active admins and invalid for regular users'
@@ -114,20 +117,20 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
       end
 
       context 'when provider_type is nextcloud' do
-        let(:host_response_body) { nil } # use default
-        let(:host_response_code) { nil } # use default
-        let(:host_response_headers) { nil } # use default
-        let(:host_response_major_version) { 23 }
+        let(:capabilities_response_body) { nil } # use default
+        let(:capabilities_response_code) { nil } # use default
+        let(:capabilities_response_headers) { nil } # use default
+        let(:capabilities_response_major_version) { 23 }
 
         before do
           # simulate host value changed to have GET request sent to check host URL validity
           storage.host_will_change!
           # simulate http response returned upon GET request
           mock_server_capabilities_response(storage_host,
-                                            response_code: host_response_code,
-                                            response_headers: host_response_headers,
-                                            response_body: host_response_body,
-                                            response_nextcloud_major_version: host_response_major_version)
+                                            response_code: capabilities_response_code,
+                                            response_headers: capabilities_response_headers,
+                                            response_body: capabilities_response_body,
+                                            response_nextcloud_major_version: capabilities_response_major_version)
         end
 
         context 'when connection fails' do
@@ -139,19 +142,19 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
         end
 
         context 'when response code is a 404 NOT FOUND' do
-          let(:host_response_code) { 404 }
+          let(:capabilities_response_code) { 404 }
 
           include_examples 'contract is invalid', host: :cannot_be_connected_to
         end
 
         context 'when response code is a 500 PERMISSION DENIED' do
-          let(:host_response_code) { 500 }
+          let(:capabilities_response_code) { 500 }
 
           include_examples 'contract is invalid', host: :cannot_be_connected_to
         end
 
         context 'when response content type is not application/json' do
-          let(:host_response_headers) do
+          let(:capabilities_response_headers) do
             {
               'Content-Type' => 'text/html'
             }
@@ -161,22 +164,35 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
         end
 
         context 'when response is unparsable JSON' do
-          let(:host_response_body) { '{' }
+          let(:capabilities_response_body) { '{' }
 
           include_examples 'contract is invalid', host: :not_nextcloud_server
         end
 
         context 'when response is valid JSON but not the expected data' do
-          let(:host_response_body) { '{}' }
+          let(:capabilities_response_body) { '{}' }
 
           include_examples 'contract is invalid', host: :not_nextcloud_server
         end
 
         context 'when Nextcloud version is below the required minimal version which is 23' do
-          let(:host_response_major_version) { 22 }
+          let(:capabilities_response_major_version) { 22 }
 
           include_examples 'contract is invalid', host: :minimal_nextcloud_version_unmet
         end
+
+        context 'when Nextcloud instance is not fully set up' do
+          let(:host_response_code) { 302 }
+          let(:host_response_header) { { Location: File.join(storage_host, '/index.php/login') } }
+
+          before do
+            mock_server_host_response(storage_host,
+                                      response_code: host_response_code,
+                                      response_headers: host_response_header)
+          end
+
+          include_examples 'contract is invalid', host: :setup_incomplete
+        end
       end
     end
   end

modules/storages/spec/features/admin_storages_spec.rb

@@ -53,6 +53,7 @@ describe 'Admin storages', :storage_server_helpers, with_flag: { storages_module
     # Test the happy path for a valid storage server (host).
     # Mock a valid response (=200) for example.com, so the host validation should succeed
     mock_server_capabilities_response("https://example.com")
+    mock_server_host_response("https://example.com")
     page.find('#storages_storage_name').set("NC 1")
     page.find('#storages_storage_host').set("https://example.com")
     page.find('button[type=submit]', text: "Save and continue setup").click

modules/storages/spec/support/storage_server_helpers.rb

@@ -63,6 +63,21 @@ module StorageServerHelpers
       body: response_body
     )
   end
+
+  def mock_server_host_response(nextcloud_host,
+                                response_code: nil,
+                                response_headers: nil)
+    response_code ||= 200
+    response_headers ||= {}
+
+    stub_request(
+      :get,
+      nextcloud_host
+    ).to_return(
+      status: response_code,
+      headers: response_headers
+    )
+  end
 end
 
 RSpec.configure do |c|