Add rails encryption for totp - allow configuration of encryption secrets from env, or derive them from SECRET_KEY_BASE. Don't leak email address via web_authn, rate limit web_authn, escape oidc state value, require password for changing email address, allow settings the hmac secret for token prefix generation

This commit is contained in:
Dan Milne
2025-12-31 10:33:56 +11:00
parent cc7beba9de
commit bb5aa2e6d6
7 changed files with 56 additions and 12 deletions

View File

@@ -169,7 +169,7 @@ class OidcController < ApplicationController
# Redirect back to client with authorization code
redirect_uri = "#{redirect_uri}?code=#{code}"
redirect_uri += "&state=#{state}" if state.present?
redirect_uri += "&state=#{CGI.escape(state)}" if state.present?
redirect_to redirect_uri, allow_other_host: true
return
end
@@ -224,7 +224,7 @@ class OidcController < ApplicationController
if params[:deny].present?
session.delete(:oauth_params)
error_uri = "#{oauth_params['redirect_uri']}?error=access_denied"
error_uri += "&state=#{oauth_params['state']}" if oauth_params['state']
error_uri += "&state=#{CGI.escape(oauth_params['state'])}" if oauth_params['state']
redirect_to error_uri, allow_other_host: true
return
end
@@ -276,7 +276,7 @@ class OidcController < ApplicationController
# Redirect back to client with authorization code
redirect_uri = "#{oauth_params['redirect_uri']}?code=#{code}"
redirect_uri += "&state=#{oauth_params['state']}" if oauth_params['state']
redirect_uri += "&state=#{CGI.escape(oauth_params['state'])}" if oauth_params['state']
redirect_to redirect_uri, allow_other_host: true
end
@@ -724,7 +724,7 @@ class OidcController < ApplicationController
if validated_uri
redirect_uri = validated_uri
redirect_uri += "?state=#{state}" if state.present?
redirect_uri += "?state=#{CGI.escape(state)}" if state.present?
redirect_to redirect_uri, allow_other_host: true
else
# Invalid redirect URI - log warning and go to default

View File

@@ -19,13 +19,21 @@ class ProfilesController < ApplicationController
else
render :show, status: :unprocessable_entity
end
else
# Updating email
elsif params[:user][:email_address].present?
# Updating email - requires current password (security: prevents account takeover)
unless @user.authenticate(params[:user][:current_password])
@user.errors.add(:current_password, "is required to change email")
render :show, status: :unprocessable_entity
return
end
if @user.update(email_params)
redirect_to profile_path, notice: "Email updated successfully."
else
render :show, status: :unprocessable_entity
end
else
render :show, status: :unprocessable_entity
end
end

View File

@@ -2,6 +2,11 @@ class WebauthnController < ApplicationController
before_action :set_webauthn_credential, only: [:destroy]
skip_before_action :require_authentication, only: [:check]
# Rate limit check endpoint to prevent enumeration attacks
rate_limit to: 10, within: 1.minute, only: [:check], with: -> {
render json: { error: "Too many requests. Try again later." }, status: :too_many_requests
}
# GET /webauthn/new
def new
@webauthn_credential = WebauthnCredential.new
@@ -131,25 +136,27 @@ class WebauthnController < ApplicationController
# GET /webauthn/check
# Check if user has WebAuthn credentials (for login page detection)
# Security: Returns identical responses for non-existent users to prevent enumeration
def check
email = params[:email]&.strip&.downcase
if email.blank?
render json: { has_webauthn: false, error: "Email is required" }
render json: { has_webauthn: false, requires_webauthn: false }
return
end
user = User.find_by(email_address: email)
# Security: Return identical response for non-existent users
# Combined with rate limiting (10/min), this prevents account enumeration
if user.nil?
render json: { has_webauthn: false, message: "User not found" }
render json: { has_webauthn: false, requires_webauthn: false }
return
end
# Only return minimal necessary info - no user_id or preferred_method
render json: {
has_webauthn: user.can_authenticate_with_webauthn?,
user_id: user.id,
preferred_method: user.preferred_authentication_method,
requires_webauthn: user.require_webauthn?
}
end