Skip to content

Commit

Permalink
Better error message when trying to post to missing strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
oliverguenther committed Oct 17, 2022
1 parent 596a7e1 commit e416c2f
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 11 deletions.
12 changes: 11 additions & 1 deletion app/contracts/authentication/omniauth_auth_hash_contract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,35 @@ module Authentication
class OmniauthAuthHashContract
include ActiveModel::Validations

attr_reader :strategy
attr_reader :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?

errors.add(:base, I18n.t(:error_omniauth_invalid_auth))
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
Expand Down
27 changes: 17 additions & 10 deletions app/services/authentication/omniauth_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,15 @@ def initialize(strategy:, auth_hash:, controller:)
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)
inspect_response(Logger::DEBUG)

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
Expand All @@ -80,16 +78,15 @@ def inspect_response(log_level)
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

##
Expand Down Expand Up @@ -221,7 +218,7 @@ def build_omniauth_hash_to_user_attributes
# 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

Expand All @@ -241,5 +238,15 @@ def auth_uid
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
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down

0 comments on commit e416c2f

Please sign in to comment.