Add pairwise SID with a UUIDv4, a significatant upgrade over User.id.to_s. Complete allowing admin to enforce TOTP per user
This commit is contained in:
@@ -30,13 +30,7 @@ module Admin
|
||||
end
|
||||
|
||||
def update
|
||||
# Prevent changing params for the current user's email and admin status
|
||||
# to avoid locking themselves out
|
||||
update_params = user_params.dup
|
||||
|
||||
if @user == Current.session.user
|
||||
update_params.delete(:admin)
|
||||
end
|
||||
update_params = user_params
|
||||
|
||||
# Only update password if provided
|
||||
update_params.delete(:password) if update_params[:password].blank?
|
||||
@@ -76,7 +70,15 @@ module Admin
|
||||
end
|
||||
|
||||
def user_params
|
||||
params.require(:user).permit(:email_address, :name, :password, :admin, :status, custom_claims: {})
|
||||
# Base attributes that all admins can modify
|
||||
base_params = params.require(:user).permit(:email_address, :name, :password, :status, :totp_required, custom_claims: {})
|
||||
|
||||
# Only allow modifying admin status when editing other users (prevent self-demotion)
|
||||
if params[:id] != Current.session.user.id.to_s
|
||||
base_params[:admin] = params[:user][:admin] if params[:user][:admin].present?
|
||||
end
|
||||
|
||||
base_params
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -365,8 +365,17 @@ class OidcController < ApplicationController
|
||||
scope: auth_code.scope
|
||||
)
|
||||
|
||||
# Generate ID token (JWT)
|
||||
id_token = OidcJwtService.generate_id_token(user, application, nonce: auth_code.nonce)
|
||||
# Find user consent for this application
|
||||
consent = OidcUserConsent.find_by(user: user, application: application)
|
||||
|
||||
unless consent
|
||||
Rails.logger.error "OIDC Security: Token requested without consent record (user: #{user.id}, app: #{application.id})"
|
||||
render json: { error: "invalid_grant", error_description: "Authorization consent not found" }, status: :bad_request
|
||||
return
|
||||
end
|
||||
|
||||
# Generate ID token (JWT) with pairwise SID
|
||||
id_token = OidcJwtService.generate_id_token(user, application, consent: consent, nonce: auth_code.nonce)
|
||||
|
||||
# Return tokens
|
||||
render json: {
|
||||
@@ -457,8 +466,17 @@ class OidcController < ApplicationController
|
||||
token_family_id: refresh_token_record.token_family_id # Keep same family for rotation tracking
|
||||
)
|
||||
|
||||
# Generate new ID token (JWT, no nonce for refresh grants)
|
||||
id_token = OidcJwtService.generate_id_token(user, application)
|
||||
# Find user consent for this application
|
||||
consent = OidcUserConsent.find_by(user: user, application: application)
|
||||
|
||||
unless consent
|
||||
Rails.logger.error "OIDC Security: Refresh token used without consent record (user: #{user.id}, app: #{application.id})"
|
||||
render json: { error: "invalid_grant", error_description: "Authorization consent not found" }, status: :bad_request
|
||||
return
|
||||
end
|
||||
|
||||
# Generate new ID token (JWT with pairwise SID, no nonce for refresh grants)
|
||||
id_token = OidcJwtService.generate_id_token(user, application, consent: consent)
|
||||
|
||||
# Return new tokens
|
||||
render json: {
|
||||
@@ -498,9 +516,13 @@ class OidcController < ApplicationController
|
||||
return
|
||||
end
|
||||
|
||||
# Find user consent for this application to get pairwise SID
|
||||
consent = OidcUserConsent.find_by(user: user, application: access_token.application)
|
||||
subject = consent&.sid || user.id.to_s
|
||||
|
||||
# Return user claims
|
||||
claims = {
|
||||
sub: user.id.to_s,
|
||||
sub: subject,
|
||||
email: user.email_address,
|
||||
email_verified: true,
|
||||
preferred_username: user.email_address,
|
||||
@@ -609,11 +631,19 @@ class OidcController < ApplicationController
|
||||
reset_session
|
||||
end
|
||||
|
||||
# If post_logout_redirect_uri is provided, redirect there
|
||||
# If post_logout_redirect_uri is provided, validate and redirect
|
||||
if post_logout_redirect_uri.present?
|
||||
redirect_uri = post_logout_redirect_uri
|
||||
redirect_uri += "?state=#{state}" if state.present?
|
||||
redirect_to redirect_uri, allow_other_host: true
|
||||
validated_uri = validate_logout_redirect_uri(post_logout_redirect_uri)
|
||||
|
||||
if validated_uri
|
||||
redirect_uri = validated_uri
|
||||
redirect_uri += "?state=#{state}" if state.present?
|
||||
redirect_to redirect_uri, allow_other_host: true
|
||||
else
|
||||
# Invalid redirect URI - log warning and go to default
|
||||
Rails.logger.warn "OIDC Logout: Invalid post_logout_redirect_uri attempted: #{post_logout_redirect_uri}"
|
||||
redirect_to root_path
|
||||
end
|
||||
else
|
||||
# Default redirect to home page
|
||||
redirect_to root_path
|
||||
@@ -685,4 +715,54 @@ class OidcController < ApplicationController
|
||||
[params[:client_id], params[:client_secret]]
|
||||
end
|
||||
end
|
||||
|
||||
def validate_logout_redirect_uri(uri)
|
||||
return nil unless uri.present?
|
||||
|
||||
begin
|
||||
parsed_uri = URI.parse(uri)
|
||||
|
||||
# Only allow HTTP/HTTPS schemes (prevent javascript:, data:, etc.)
|
||||
return nil unless parsed_uri.is_a?(URI::HTTP) || parsed_uri.is_a?(URI::HTTPS)
|
||||
|
||||
# Only allow HTTPS in production
|
||||
return nil if Rails.env.production? && parsed_uri.scheme != 'https'
|
||||
|
||||
# Check if URI matches any registered OIDC application's redirect URIs
|
||||
# According to OIDC spec, post_logout_redirect_uri should be pre-registered
|
||||
Application.oidc.active.find_each do |app|
|
||||
# Check if this URI matches any of the app's registered redirect URIs
|
||||
if app.parsed_redirect_uris.any? { |registered_uri| logout_uri_matches?(uri, registered_uri) }
|
||||
return uri
|
||||
end
|
||||
end
|
||||
|
||||
# No matching application found
|
||||
nil
|
||||
rescue URI::InvalidURIError
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
# Check if logout URI matches a registered redirect URI
|
||||
# More lenient than exact match - allows same host/path with different query params
|
||||
def logout_uri_matches?(provided, registered)
|
||||
# Exact match is always valid
|
||||
return true if provided == registered
|
||||
|
||||
# Parse both URIs to compare components
|
||||
begin
|
||||
provided_parsed = URI.parse(provided)
|
||||
registered_parsed = URI.parse(registered)
|
||||
|
||||
# Match if scheme, host, port, and path are the same
|
||||
# (allows different query params which is common for logout redirects)
|
||||
provided_parsed.scheme == registered_parsed.scheme &&
|
||||
provided_parsed.host == registered_parsed.host &&
|
||||
provided_parsed.port == registered_parsed.port &&
|
||||
provided_parsed.path == registered_parsed.path
|
||||
rescue URI::InvalidURIError
|
||||
false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -6,7 +6,18 @@ class SessionsController < ApplicationController
|
||||
|
||||
def new
|
||||
# Redirect to signup if this is first run
|
||||
redirect_to signup_path if User.count.zero?
|
||||
if User.count.zero?
|
||||
respond_to do |format|
|
||||
format.html { redirect_to signup_path }
|
||||
format.json { render json: { error: "No users exist. Please complete initial setup." }, status: :service_unavailable }
|
||||
end
|
||||
return
|
||||
end
|
||||
|
||||
respond_to do |format|
|
||||
format.html # render HTML login page
|
||||
format.json { render json: { error: "Authentication required" }, status: :unauthorized }
|
||||
end
|
||||
end
|
||||
|
||||
def create
|
||||
@@ -33,8 +44,22 @@ class SessionsController < ApplicationController
|
||||
return
|
||||
end
|
||||
|
||||
# Check if TOTP is required
|
||||
if user.totp_enabled?
|
||||
# Check if TOTP is required or enabled
|
||||
if user.totp_required? || user.totp_enabled?
|
||||
# If TOTP is required but not yet set up, redirect to setup
|
||||
if user.totp_required? && !user.totp_enabled?
|
||||
# Store user ID in session for TOTP setup
|
||||
session[:pending_totp_setup_user_id] = user.id
|
||||
# Preserve the redirect URL through TOTP setup
|
||||
if params[:rd].present?
|
||||
validated_url = validate_redirect_url(params[:rd])
|
||||
session[:totp_redirect_url] = validated_url if validated_url
|
||||
end
|
||||
redirect_to new_totp_path, alert: "Your administrator requires two-factor authentication. Please set it up now to continue."
|
||||
return
|
||||
end
|
||||
|
||||
# TOTP is enabled, proceed to verification
|
||||
# Store user ID in session temporarily for TOTP verification
|
||||
session[:pending_totp_user_id] = user.id
|
||||
# Preserve the redirect URL through TOTP verification (after validation)
|
||||
@@ -275,12 +300,12 @@ class SessionsController < ApplicationController
|
||||
redirect_domain = uri.host.downcase
|
||||
return nil unless redirect_domain.present?
|
||||
|
||||
# Check against our ForwardAuthRules
|
||||
matching_rule = ForwardAuthRule.active.find do |rule|
|
||||
rule.matches_domain?(redirect_domain)
|
||||
# Check against our forward auth applications
|
||||
matching_app = Application.forward_auth.active.find do |app|
|
||||
app.matches_domain?(redirect_domain)
|
||||
end
|
||||
|
||||
matching_rule ? url : nil
|
||||
matching_app ? url : nil
|
||||
|
||||
rescue URI::InvalidURIError
|
||||
nil
|
||||
|
||||
@@ -5,6 +5,9 @@ class TotpController < ApplicationController
|
||||
|
||||
# GET /totp/new - Show QR code to set up TOTP
|
||||
def new
|
||||
# Check if user is being forced to set up TOTP by admin
|
||||
@totp_setup_required = session[:pending_totp_setup_user_id].present?
|
||||
|
||||
# Generate TOTP secret but don't save yet
|
||||
@totp_secret = ROTP::Base32.random
|
||||
@provisioning_uri = ROTP::TOTP.new(@totp_secret, issuer: "Clinch").provisioning_uri(@user.email_address)
|
||||
@@ -30,8 +33,16 @@ class TotpController < ApplicationController
|
||||
# Store plain codes temporarily in session for display after redirect
|
||||
session[:temp_backup_codes] = plain_codes
|
||||
|
||||
# Redirect to backup codes page with success message
|
||||
redirect_to backup_codes_totp_path, notice: "Two-factor authentication has been enabled successfully! Save these backup codes now."
|
||||
# Check if this was a required setup from login
|
||||
if session[:pending_totp_setup_user_id].present?
|
||||
session.delete(:pending_totp_setup_user_id)
|
||||
# Mark that user should be auto-signed in after viewing backup codes
|
||||
session[:auto_signin_after_forced_totp] = true
|
||||
redirect_to backup_codes_totp_path, notice: "Two-factor authentication has been enabled successfully! Save these backup codes, then you'll be signed in."
|
||||
else
|
||||
# Regular setup from profile
|
||||
redirect_to backup_codes_totp_path, notice: "Two-factor authentication has been enabled successfully! Save these backup codes now."
|
||||
end
|
||||
else
|
||||
redirect_to new_totp_path, alert: "Invalid verification code. Please try again."
|
||||
end
|
||||
@@ -43,6 +54,12 @@ class TotpController < ApplicationController
|
||||
if session[:temp_backup_codes].present?
|
||||
@backup_codes = session[:temp_backup_codes]
|
||||
session.delete(:temp_backup_codes) # Clear after use
|
||||
|
||||
# Check if this was a forced TOTP setup during login
|
||||
@auto_signin_pending = session[:auto_signin_after_forced_totp].present?
|
||||
if @auto_signin_pending
|
||||
session.delete(:auto_signin_after_forced_totp)
|
||||
end
|
||||
else
|
||||
# This will be shown after password verification for existing users
|
||||
# Since we can't display BCrypt hashes, redirect to regenerate
|
||||
@@ -81,6 +98,18 @@ class TotpController < ApplicationController
|
||||
redirect_to backup_codes_totp_path, notice: "New backup codes have been generated. Save them now!"
|
||||
end
|
||||
|
||||
# POST /totp/complete_setup - Complete forced TOTP setup and sign in
|
||||
def complete_setup
|
||||
# Sign in the user after they've saved their backup codes
|
||||
# This is only used when admin requires TOTP and user just set it up during login
|
||||
if session[:totp_redirect_url].present?
|
||||
session[:return_to_after_authenticating] = session.delete(:totp_redirect_url)
|
||||
end
|
||||
|
||||
start_new_session_for @user
|
||||
redirect_to after_authentication_url, notice: "Two-factor authentication enabled. Signed in successfully.", allow_other_host: true
|
||||
end
|
||||
|
||||
# DELETE /totp - Disable TOTP (requires password)
|
||||
def destroy
|
||||
unless @user.authenticate(params[:password])
|
||||
@@ -88,6 +117,12 @@ class TotpController < ApplicationController
|
||||
return
|
||||
end
|
||||
|
||||
# Prevent disabling if admin requires TOTP
|
||||
if @user.totp_required?
|
||||
redirect_to profile_path, alert: "Two-factor authentication is required by your administrator and cannot be disabled."
|
||||
return
|
||||
end
|
||||
|
||||
@user.disable_totp!
|
||||
redirect_to profile_path, notice: "Two-factor authentication has been disabled."
|
||||
end
|
||||
@@ -99,7 +134,8 @@ class TotpController < ApplicationController
|
||||
end
|
||||
|
||||
def redirect_if_totp_enabled
|
||||
if @user.totp_enabled?
|
||||
# Allow setup if admin requires it, even if already enabled (for regeneration)
|
||||
if @user.totp_enabled? && !session[:pending_totp_setup_user_id].present?
|
||||
redirect_to profile_path, alert: "Two-factor authentication is already enabled."
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user