Add a method to remove parameters from urls, so we can redirect without risk of infinite redirect. Fix a bunch of redirects to login afer being foced to log out. Add missing migrations
This commit is contained in:
@@ -40,7 +40,6 @@ module Authentication
|
||||
end
|
||||
|
||||
def after_authentication_url
|
||||
session[:return_to_after_authenticating]
|
||||
session.delete(:return_to_after_authenticating) || root_url
|
||||
end
|
||||
|
||||
|
||||
@@ -246,9 +246,7 @@ class OidcController < ApplicationController
|
||||
|
||||
# Store the current URL (which contains all OAuth params) for redirect after login
|
||||
# Remove prompt=login to prevent infinite re-auth loop
|
||||
return_url = request.url.sub(/&prompt=login(?=&|$)|\?prompt=login&?/, '\1')
|
||||
# Fix any resulting URL issues (like ?& or & at end)
|
||||
return_url = return_url.gsub("?&", "?").gsub(/[?&]$/, "")
|
||||
return_url = remove_query_param(request.url, "prompt")
|
||||
session[:return_to_after_authenticating] = return_url
|
||||
|
||||
redirect_to signin_path, alert: "Please sign in to continue"
|
||||
@@ -263,15 +261,19 @@ class OidcController < ApplicationController
|
||||
# Calculate session age
|
||||
session_age_seconds = Time.current.to_i - Current.session.created_at.to_i
|
||||
|
||||
if session_age_seconds > max_age_seconds
|
||||
if session_age_seconds >= max_age_seconds
|
||||
# Session is too old - require re-authentication
|
||||
# Store return URL in session (creates new session cookie)
|
||||
# Store the return URL in Rails session, then destroy the Session record
|
||||
|
||||
# Destroy session and clear cookie to force fresh login
|
||||
# Store return URL before destroying anything
|
||||
# Remove max_age from return URL to prevent infinite re-auth loop
|
||||
return_url = remove_query_param(request.url, "max_age")
|
||||
session[:return_to_after_authenticating] = return_url
|
||||
|
||||
# Destroy the Session record and clear its cookie
|
||||
Current.session&.destroy!
|
||||
cookies.delete(:session_id)
|
||||
|
||||
session[:return_to_after_authenticating] = request.url
|
||||
Current.session = nil
|
||||
|
||||
redirect_to signin_path, alert: "Please sign in to continue"
|
||||
return
|
||||
@@ -1113,6 +1115,23 @@ class OidcController < ApplicationController
|
||||
end
|
||||
end
|
||||
|
||||
# Remove a query parameter from a URL using proper URI parsing
|
||||
# More robust than regex - handles URL encoding, edge cases, etc.
|
||||
def remove_query_param(url, param_name)
|
||||
uri = URI.parse(url)
|
||||
return url unless uri.query
|
||||
|
||||
# Parse query string into hash
|
||||
params = CGI.parse(uri.query)
|
||||
params.delete(param_name)
|
||||
|
||||
# Rebuild query string (empty string if no params left)
|
||||
uri.query = params.any? ? URI.encode_www_form(params) : nil
|
||||
uri.to_s
|
||||
rescue URI::InvalidURIError
|
||||
url
|
||||
end
|
||||
|
||||
def send_backchannel_logout_notifications(user)
|
||||
# Find all active OIDC consents for this user
|
||||
consents = OidcUserConsent.where(user: user).includes(:application)
|
||||
|
||||
@@ -86,17 +86,8 @@ class SessionsController < ApplicationController
|
||||
end
|
||||
|
||||
# Sign in successful (password only)
|
||||
# Preserve the return_to_after_authenticating value across session boundary
|
||||
# (e.g., when max_age flow destroys the session and creates a temporary one)
|
||||
preserved_return_url = session[:return_to_after_authenticating]
|
||||
|
||||
start_new_session_for user, acr: "1"
|
||||
|
||||
# Restore the return URL if it was lost during session recreation
|
||||
if preserved_return_url.present? && session[:return_to_after_authenticating].blank?
|
||||
session[:return_to_after_authenticating] = preserved_return_url
|
||||
end
|
||||
|
||||
# Use status: :see_other to ensure browser makes a GET request
|
||||
# This prevents Turbo from converting it to a TURBO_STREAM request
|
||||
redirect_to after_authentication_url, notice: "Signed in successfully.", allow_other_host: true, status: :see_other
|
||||
@@ -134,12 +125,7 @@ class SessionsController < ApplicationController
|
||||
if session[:totp_redirect_url].present?
|
||||
session[:return_to_after_authenticating] = session.delete(:totp_redirect_url)
|
||||
end
|
||||
# Preserve return URL across session boundary for max_age flow
|
||||
preserved_return_url = session[:return_to_after_authenticating]
|
||||
start_new_session_for user, acr: "2"
|
||||
if preserved_return_url.present? && session[:return_to_after_authenticating].blank?
|
||||
session[:return_to_after_authenticating] = preserved_return_url
|
||||
end
|
||||
redirect_to after_authentication_url, notice: "Signed in successfully.", allow_other_host: true
|
||||
return
|
||||
end
|
||||
@@ -151,12 +137,7 @@ class SessionsController < ApplicationController
|
||||
if session[:totp_redirect_url].present?
|
||||
session[:return_to_after_authenticating] = session.delete(:totp_redirect_url)
|
||||
end
|
||||
# Preserve return URL across session boundary for max_age flow
|
||||
preserved_return_url = session[:return_to_after_authenticating]
|
||||
start_new_session_for user, acr: "2"
|
||||
if preserved_return_url.present? && session[:return_to_after_authenticating].blank?
|
||||
session[:return_to_after_authenticating] = preserved_return_url
|
||||
end
|
||||
redirect_to after_authentication_url, notice: "Signed in successfully using backup code.", allow_other_host: true
|
||||
return
|
||||
end
|
||||
|
||||
@@ -106,12 +106,7 @@ class TotpController < ApplicationController
|
||||
session[:return_to_after_authenticating] = session.delete(:totp_redirect_url)
|
||||
end
|
||||
|
||||
# Preserve return URL across session boundary for max_age flow
|
||||
preserved_return_url = session[:return_to_after_authenticating]
|
||||
start_new_session_for @user
|
||||
if preserved_return_url.present? && session[:return_to_after_authenticating].blank?
|
||||
session[:return_to_after_authenticating] = preserved_return_url
|
||||
end
|
||||
redirect_to after_authentication_url, notice: "Two-factor authentication enabled. Signed in successfully.", allow_other_host: true
|
||||
end
|
||||
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
class AddSkipConsentToApplications < ActiveRecord::Migration[8.1]
|
||||
def change
|
||||
add_column :applications, :skip_consent, :boolean, default: false, null: false
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,5 @@
|
||||
class AddClaimsRequestsToOidcUserConsents < ActiveRecord::Migration[8.1]
|
||||
def change
|
||||
add_column :oidc_user_consents, :claims_requests, :json, default: {}, null: false
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,5 @@
|
||||
class AddClaimsRequestsToOidcAuthorizationCodes < ActiveRecord::Migration[8.1]
|
||||
def change
|
||||
add_column :oidc_authorization_codes, :claims_requests, :json, default: {}, null: false
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user