Better error message when trying to post to missing strategy

app/contracts/authentication/omniauth_auth_hash_contract.rb

@@ -30,18 +30,26 @@ module Authentication
   class OmniauthAuthHashContract
     include ActiveModel::Validations
 
-    attr_reader :auth_hash
+    attr_reader :strategy, :auth_hash
 
-    def initialize(auth_hash)
+    def initialize(strategy, auth_hash)
+      @strategy = strategy
       @auth_hash = auth_hash
     end
 
+    validate :validate_strategy
     validate :validate_auth_hash
     validate :validate_auth_hash_not_expired
     validate :validate_authorization_callback
 
     private
 
+    def validate_strategy
+      return if strategy
+
+      errors.add(:base, I18n.t(:error_omniauth_missing_strategy))
+    end
+
     def validate_auth_hash
       return if auth_hash&.valid?
 
@@ -49,6 +57,7 @@ module Authentication
     end
 
     def validate_auth_hash_not_expired
+      return unless auth_hash.present?
       return unless auth_hash['timestamp']
 
       if auth_hash['timestamp'] < Time.now - 30.minutes

app/services/authentication/omniauth_service.rb

@@ -44,7 +44,7 @@ module Authentication
       self.strategy = strategy
       self.auth_hash = auth_hash
       self.controller = controller
-      self.contract = ::Authentication::OmniauthAuthHashContract.new(auth_hash)
+      self.contract = ::Authentication::OmniauthAuthHashContract.new(strategy, auth_hash)
     end
 
     def call(additional_user_params = nil)
@@ -52,9 +52,7 @@ module Authentication
 
       unless contract.validate
         result = ServiceResult.failure(errors: contract.errors)
-        Rails.logger.error do
-          "[OmniAuth strategy #{strategy.name}] Failed to process omniauth response for #{auth_uid}: #{result.message}"
-        end
+        omniauth_log_line(:error) { "Failed to process omniauth response for #{auth_uid}: #{result.message}" }
         inspect_response(Logger::ERROR)
 
         return result
@@ -80,16 +78,15 @@ module Authentication
       case strategy
       when ::OmniAuth::Strategies::SAML
         ::OpenProject::AuthSaml::Inspector.inspect_response(auth_hash) do |message|
-          Rails.logger.add log_level, message
+          omniauth_log_line(log_level) { message }
         end
       else
-        Rails.logger.add(log_level) do
-          "[OmniAuth strategy #{strategy.name}] Returning from omniauth with hash " \
-            "#{auth_hash&.to_hash.inspect} Valid? #{auth_hash&.valid?}"
+        omniauth_log_line(log_level) do
+          "Returning from omniauth with hash #{auth_hash&.to_hash.inspect} Valid? #{auth_hash&.valid?}"
         end
       end
     rescue StandardError => e
-      OpenProject.logger.error "[OmniAuth strategy #{strategy&.name}] Failed to inspect OmniAuth response: #{e.message}"
+      omniauth_log_line(:error) { "Failed to inspect OmniAuth response: #{e.message}" }
     end
 
     ##
@@ -221,7 +218,7 @@ module Authentication
       # overriding existing attributes
       attribute_map.compact!
 
-      Rails.logger.debug { "Mapped auth_hash user attributes #{attribute_map.inspect}" }
+      omniauth_log_line(:debug) { "Mapped auth_hash user attributes #{attribute_map.inspect}" }
       attribute_map
     end
 
@@ -241,5 +238,15 @@ module Authentication
       hash = (auth_hash || {})
       hash.dig(:info, :uid) || hash.dig(:uid) || 'unknown'
     end
+
+    ##
+    # Prepare a log message for the strategy
+    def omniauth_log_line(level, &block)
+      Rails.logger.public_send(level) do
+        strategy_tag = strategy ? "[OmniAuth strategy #{strategy.name}]" : "[OmniAuth no matching strategy found]"
+        message = block.call
+        "#{strategy_tag} #{message}"
+      end
+    end
   end
 end

config/locales/en.yml

@@ -1342,6 +1342,7 @@ en:
   error_no_type_in_project: "No type is associated to this project. Please check the Project settings."
   error_omniauth_registration_timed_out: "The registration via an external authentication provider timed out. Please try again."
   error_omniauth_invalid_auth: "The authentication information returned from the identity provider was invalid. Please contact your administrator for further help."
+  error_omniauth_missing_strategy: "No strategy matched the request path, this is likely a configuration error."
   error_password_change_failed: 'An error occurred when trying to change the password.'
   error_scm_command_failed: "An error occurred when trying to access the repository: %{value}"
   error_scm_not_found: "The entry or revision was not found in the repository."

spec/contracts/authentication/omniauth_auth_hash_contract_spec.rb

@@ -29,6 +29,7 @@
 require 'spec_helper'
 
 describe Authentication::OmniauthAuthHashContract do
+  let(:strategy) { double('Strategy', name: 'saml') } # rubocop:disable RSpec/VerifiedDoubles
   let(:auth_hash) do
     OmniAuth::AuthHash.new(
       provider: 'google',
@@ -40,7 +41,7 @@ describe Authentication::OmniauthAuthHashContract do
     )
   end
 
-  let(:instance) { described_class.new(auth_hash) }
+  let(:instance) { described_class.new(strategy, auth_hash) }
 
   subject do
     instance.validate