[#43473] Support valid Nextcloud installations using index.php paths

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

app/services/oauth_clients/connection_manager.rb

@@ -231,8 +231,8 @@ module OAuthClients
         scheme: oauth_client_scheme,
         host: oauth_client_host,
         port: oauth_client_port,
-        authorization_endpoint: File.join(oauth_client_path, "/apps/oauth2/authorize"),
-        token_endpoint: File.join(oauth_client_path, "/apps/oauth2/api/v1/token")
+        authorization_endpoint: File.join(oauth_client_path, "/index.php/apps/oauth2/authorize"),
+        token_endpoint: File.join(oauth_client_path, "/index.php/apps/oauth2/api/v1/token")
       )
     end
 

modules/storages/app/services/storages/oauth_applications/create_service.rb

@@ -47,7 +47,7 @@ module Storages::OAuthApplications
         .new(::Doorkeeper::Application.new, user:)
         .call({
                 name: "#{storage.name} (#{I18n.t("storages.provider_types.#{storage.provider_type}.name")})",
-                redirect_uri: File.join(storage.host, "apps/integration_openproject/oauth-redirect"),
+                redirect_uri: File.join(storage.host, "index.php/apps/integration_openproject/oauth-redirect"),
                 scopes: 'api_v3',
                 confidential: true,
                 owner: storage.creator,

modules/storages/app/services/storages/storages/update_service.rb

@@ -41,7 +41,7 @@ module Storages::Storages
          .new(application, user:)
          .call({
                  name: "#{storage.name} (#{I18n.t("storages.provider_types.#{storage.provider_type}.name")})",
-                 redirect_uri: File.join(storage.host, "apps/integration_openproject/oauth-redirect")
+                 redirect_uri: File.join(storage.host, "index.php/apps/integration_openproject/oauth-redirect")
                })
         service_call.add_dependent!(persist_service_result)
       end

modules/storages/app/validator/nextcloud_compatible_host_validator.rb

@@ -27,6 +27,7 @@
 #++
 class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
   MINIMAL_NEXTCLOUD_VERSION = 22
+  AUTHORIZATION_HEADER = "Bearer TESTBEARERTOKEN".freeze
 
   def validate_each(contract, attribute, value)
     return unless contract.model.changed_attributes.include?(attribute)
@@ -53,8 +54,8 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
   # 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)
+    response = request_config_check(value)
+    error_type = check_config_check_response(response)
 
     if error_type
       contract.errors.add(attribute, error_type)
@@ -65,12 +66,19 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
   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 :not_nextcloud_server unless json_response_with_version?(response)
     return :minimal_nextcloud_version_unmet unless major_version_sufficient?(response)
+
+    nil
   end
 
-  def check_host_response(response)
-    :setup_incomplete if response.code == '302' && response.header['location']&.include?("/index.php")
+  def check_config_check_response(response)
+    return :cannot_be_connected_to if response.is_a? StandardError
+    return :op_application_not_installed if response.is_a? Net::HTTPRedirection
+    return :cannot_be_connected_to unless response.is_a? Net::HTTPSuccess
+    return :authorization_header_missing if read_authorization_header(response) != AUTHORIZATION_HEADER
+
+    nil
   end
 
   def message(host, response_or_exception, error_type)
@@ -133,14 +141,15 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
     make_request(request)
   end
 
-  def request_host(host)
-    uri = URI.parse(host)
+  def request_config_check(host)
+    uri = URI.parse(File.join(host, 'index.php/apps/integration_openproject/check-config'))
     request = Net::HTTP::Get.new(uri)
+    request["Authorization"] = AUTHORIZATION_HEADER
 
     make_request(request)
   end
 
-  def json_response?(response)
+  def json_response_with_version?(response)
     (
       response['content-type'].split(';').first.strip.downcase == 'application/json' \
       && read_version(response)
@@ -152,4 +161,10 @@ class NextcloudCompatibleHostValidator < ActiveModel::EachValidator
   def read_version(response)
     JSON.parse(response.body).dig('ocs', 'data', 'version', 'major')
   end
+
+  def read_authorization_header(response)
+    JSON.parse(response.body)['authorization_header']
+  rescue JSON::ParserError
+    nil
+  end
 end

modules/storages/config/locales/en.yml

@@ -26,10 +26,13 @@ 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.
+              op_application_not_installed: >
+                appears to not have the app "OpenProject integration" installed. Please install it first and then try
+                again.
+              authorization_header_missing: >
+                is not fully set up. The Nextcloud instance does not receive the "Authorization" header, which is 
+                necessary for a Bearer token based authorization of API requests. Please double check your HTTP
+                server configuration.
         storages/file_link:
           attributes:
             origin_id:

modules/storages/lib/api/v3/file_links/storage_url_helper.rb

@@ -31,11 +31,11 @@ module API::V3::FileLinks::StorageUrlHelper
   def storage_url_open_file(file_link, open_location: false)
     location_flag = ActiveModel::Type::Boolean.new.cast(open_location) ? 0 : 1
 
-    "#{file_link.storage.host}/f/#{file_link.origin_id}?openfile=#{location_flag}"
+    "#{file_link.storage.host}/index.php/f/#{file_link.origin_id}?openfile=#{location_flag}"
   end
 
   def storage_url_open(storage)
-    "#{storage.host}/apps/files"
+    "#{storage.host}/index.php/apps/files"
   end
 
   # rubocop:disable Metrics/AbcSize
@@ -54,7 +54,7 @@ module API::V3::FileLinks::StorageUrlHelper
     download_token = direct_download_token(body: direct_download_response.result)
     return download_token if download_token.failure?
 
-    url = "#{storage.host}/apps/integration_openproject/direct/#{download_token.result}/#{file_link.origin_name}"
+    url = "#{storage.host}/index.php/apps/integration_openproject/direct/#{download_token.result}/#{file_link.origin_name}"
     ServiceResult.success(result: url)
   end
 

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

@@ -36,7 +36,7 @@ describe Storages::Storages::BaseContract, :storage_server_helpers, webmock: tru
 
   it 'checks the storage url only when changed' do
     capabilities_request = mock_server_capabilities_response(storage_host)
-    host_request = mock_server_host_response(storage_host)
+    host_request = mock_server_config_check_response(storage_host)
     contract.valid?
     expect(capabilities_request).to have_been_made.once
     expect(host_request).to have_been_made.once

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

@@ -40,7 +40,7 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
   before do
     if storage_host.present?
       mock_server_capabilities_response(storage_host)
-      mock_server_host_response(storage_host)
+      mock_server_config_check_response(storage_host)
     end
   end
 
@@ -133,16 +133,25 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
         let(:capabilities_response_code) { nil } # use default
         let(:capabilities_response_headers) { nil } # use default
         let(:capabilities_response_major_version) { 22 }
+        let(:check_config_response_body) { nil } # use default
+        let(:check_config_response_code) { nil } # use default
+        let(:check_config_response_headers) { nil } # use default
 
         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: capabilities_response_code,
                                             response_headers: capabilities_response_headers,
                                             response_body: capabilities_response_body,
                                             response_nextcloud_major_version: capabilities_response_major_version)
+
+          mock_server_config_check_response(storage_host,
+                                            response_code: check_config_response_code,
+                                            response_headers: check_config_response_headers,
+                                            response_body: check_config_response_body)
         end
 
         context 'when connection fails' do
@@ -193,17 +202,16 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
           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') } }
+        context 'when Nextcloud instance is missing the "OpenProject integration" app' do
+          let(:check_config_response_code) { 302 }
 
-          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: :op_application_not_installed
+        end
+
+        context 'when Nextcloud instance is misconfigured and strips AUTHORIZATION header from HTTP request' do
+          let(:check_config_response_body) { { authorization_header: '' }.to_json }
 
-          include_examples 'contract is invalid', host: :setup_incomplete
+          include_examples 'contract is invalid', host: :authorization_header_missing
         end
       end
     end

modules/storages/spec/controller/storages_controller_spec.rb

@@ -43,7 +43,7 @@ describe ::Storages::Admin::StoragesController, webmock: true, type: :controller
   before do
     login_as admin
     mock_server_capabilities_response(host)
-    mock_server_host_response(host)
+    mock_server_config_check_response(host)
     post :create, params:
   end
 

modules/storages/spec/features/admin_storages_spec.rb

@@ -53,7 +53,7 @@ describe 'Admin storages', :storage_server_helpers, type: :feature, js: true do
     # 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")
+    mock_server_config_check_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/lib/api/v3/file_links/file_link_representer_rendering_spec.rb

@@ -125,7 +125,7 @@ describe ::API::V3::FileLinks::FileLinkRepresenter, 'rendering' do
     describe 'originOpen' do
       it_behaves_like 'has an untitled link' do
         let(:link) { 'originOpen' }
-        let(:href) { "#{storage.host}/f/#{file_link.origin_id}?openfile=1" }
+        let(:href) { "#{storage.host}/index.php/f/#{file_link.origin_id}?openfile=1" }
       end
     end
 
@@ -139,7 +139,7 @@ describe ::API::V3::FileLinks::FileLinkRepresenter, 'rendering' do
     describe 'originOpenLocation' do
       it_behaves_like 'has an untitled link' do
         let(:link) { 'originOpenLocation' }
-        let(:href) { "#{storage.host}/f/#{file_link.origin_id}?openfile=0" }
+        let(:href) { "#{storage.host}/index.php/f/#{file_link.origin_id}?openfile=0" }
       end
     end
 

modules/storages/spec/support/storage_server_helpers.rb

@@ -64,18 +64,30 @@ module StorageServerHelpers
     )
   end
 
-  def mock_server_host_response(nextcloud_host,
-                                response_code: nil,
-                                response_headers: nil)
+  def mock_server_config_check_response(nextcloud_host,
+                                        response_code: nil,
+                                        response_headers: nil,
+                                        response_body: nil)
     response_code ||= 200
-    response_headers ||= {}
+    response_headers ||= {
+      'Content-Type' => 'application/json; charset=utf-8'
+    }
+
+    response_body ||=
+      %{
+        {
+          "user_id": "",
+          "authorization_header": "Bearer TESTBEARERTOKEN"
+        }
+      }
 
     stub_request(
       :get,
-      nextcloud_host
+      File.join(nextcloud_host, 'index.php/apps/integration_openproject/check-config')
     ).to_return(
       status: response_code,
-      headers: response_headers
+      headers: response_headers,
+      body: response_body
     )
   end
 end

spec/services/oauth_clients/connection_manager_spec.rb

@@ -157,7 +157,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
           refresh_token: "UwFp...1FROJ",
           user_id: "admin"
         }.to_json
-        stub_request(:any, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 200, body: response_body)
       end
 
@@ -169,7 +169,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with known error', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 400, body: { error: error_message }.to_json)
       end
 
@@ -197,7 +197,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with known reply invalid_grant', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 400, body: { error: "invalid_grant" }.to_json)
       end
 
@@ -211,7 +211,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with unknown reply', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 400, body: { error: "invalid_requesttt" }.to_json)
       end
 
@@ -225,7 +225,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with reply including JSON syntax error', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(
             status: 400,
             headers: { 'Content-Type' => 'application/json; charset=utf-8' },
@@ -243,7 +243,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with 500 reply without body', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 500)
       end
 
@@ -257,7 +257,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with bad HTTP response', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/apps/oauth2/api/v1/token')).to_raise(Net::HTTPBadResponse)
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token')).to_raise(Net::HTTPBadResponse)
       end
 
       it 'returns an unspecific error message' do
@@ -270,7 +270,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with timeout returns internal error', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/apps/oauth2/api/v1/token')).to_timeout
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token')).to_timeout
       end
 
       it 'returns an unspecific error message' do
@@ -303,7 +303,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
           refresh_token: "xUwFp...1FROJ",
           user_id: "admin"
         }.to_json
-        stub_request(:any, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 200, body: response_body)
         oauth_client_token
       end
@@ -327,7 +327,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
           refresh_token: "xUwFp...1FROJ",
           user_id: "admin"
         }.to_json
-        stub_request(:any, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 200, body: response_body)
 
         oauth_client_token
@@ -343,7 +343,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
 
     context 'with server error from OAuth2 provider' do
       before do
-        stub_request(:any, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 400, body: { error: "invalid_request" }.to_json)
         oauth_client_token
       end
@@ -358,7 +358,7 @@ describe ::OAuthClients::ConnectionManager, type: :model do
     context 'with successful response but invalid data' do
       before do
         # Simulate timeout
-        stub_request(:any, File.join(host, '/apps/oauth2/api/v1/token'))
+        stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_timeout
         oauth_client_token
       end