From 2d713fd0399ba4a335999d70e65c9f1649e7c775 Mon Sep 17 00:00:00 2001 From: Pierre Rioux Date: Tue, 13 Aug 2024 09:47:00 -0400 Subject: [PATCH] More adjustments of OIDC Just silence some internal errors when invalid OIDC requests come in. --- BrainPortal/app/controllers/nh_sessions_controller.rb | 11 +++++------ BrainPortal/app/controllers/sessions_controller.rb | 9 ++++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/BrainPortal/app/controllers/nh_sessions_controller.rb b/BrainPortal/app/controllers/nh_sessions_controller.rb index 4e8144762..1acc64924 100644 --- a/BrainPortal/app/controllers/nh_sessions_controller.rb +++ b/BrainPortal/app/controllers/nh_sessions_controller.rb @@ -139,12 +139,12 @@ def orcid #:nodoc: # a user's identity. def nh_oidc code = params[:code].presence.try(:strip) - state = params[:state].presence || 'wrong' + state = params[:state].presence # Some initial simple validations - oidc = OidcConfig.find_by_state(state) if state + oidc = OidcConfig.find_by_state(state) if state if !code || !oidc || state != oidc_current_state(oidc) - cb_error "#{oidc.name} session is out of sync with CBRAIN" + cb_error "#{oidc&.name || 'OIDC'} session is out of sync with CBRAIN" end # Query an OpenID provider; this returns all the info we need at the same time. @@ -195,9 +195,8 @@ def nh_oidc redirect_to signin_path rescue => ex clean_bt = Rails.backtrace_cleaner.clean(ex.backtrace || []) - oidc ||= OidcConfig.new.tap { |oidc| oidc.name = 'OIDC' } - Rails.logger.error "#{oidc.name} auth failed: #{ex.class} #{ex.message} at #{clean_bt[0]}" - flash[:error] = "The #{oidc.name} authentication failed" + Rails.logger.error "#{oidc&.name || 'OIDC'} auth failed: #{ex.class} #{ex.message} at #{clean_bt[0]}" + flash[:error] = "The #{oidc&.name || 'OIDC'} authentication failed" redirect_to signin_path end diff --git a/BrainPortal/app/controllers/sessions_controller.rb b/BrainPortal/app/controllers/sessions_controller.rb index e40a2a140..984bbac80 100644 --- a/BrainPortal/app/controllers/sessions_controller.rb +++ b/BrainPortal/app/controllers/sessions_controller.rb @@ -148,12 +148,12 @@ def destroy #:nodoc: # a user's identity. def oidc code = params[:code].presence.try(:strip) - state = params[:state].presence || 'wrong' + state = params[:state].presence # Some initial simple validations oidc = OidcConfig.find_by_state(state) if state if !code || !oidc || state != oidc_current_state(oidc) - cb_error "#{oidc.name} session is out of sync with CBRAIN" + cb_error "#{oidc&.name || 'OIDC'} session is out of sync with CBRAIN" end # Query OpenID provider; this returns all the info we need at the same time. @@ -204,9 +204,8 @@ def oidc redirect_to new_session_path rescue => ex clean_bt = Rails.backtrace_cleaner.clean(ex.backtrace || []) - oidc ||= OidcConfig.new.tap { |oidc| oidc.name = 'OIDC' } - Rails.logger.error "#{oidc.name} auth failed: #{ex.class} #{ex.message} at #{clean_bt[0]}" - flash[:error] = "The #{oidc.name} authentication failed" + Rails.logger.error "#{oidc&.name || 'OIDC'} auth failed: #{ex.class} #{ex.message} at #{clean_bt[0]}" + flash[:error] = "The #{oidc&.name || 'OIDC'} authentication failed" redirect_to new_session_path end