diff --git a/Gemfile b/Gemfile index 1b4f56a..af5ad62 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ source "https://rubygems.org" # Bundle edge Rails instead: gem "rails", github: "rails/rails", branch: "main" -gem "rails", "~> 8.1.0" +gem "rails", "~> 8.1.1" # The modern asset pipeline for Rails [https://github.com/rails/propshaft] gem "propshaft" # Use sqlite3 as the database for Active Record diff --git a/Gemfile.lock b/Gemfile.lock index 1473cc0..e5c8528 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,29 +3,29 @@ GEM specs: action_text-trix (2.1.15) railties - actioncable (8.1.0) - actionpack (= 8.1.0) - activesupport (= 8.1.0) + actioncable (8.1.1) + actionpack (= 8.1.1) + activesupport (= 8.1.1) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (8.1.0) - actionpack (= 8.1.0) - activejob (= 8.1.0) - activerecord (= 8.1.0) - activestorage (= 8.1.0) - activesupport (= 8.1.0) + actionmailbox (8.1.1) + actionpack (= 8.1.1) + activejob (= 8.1.1) + activerecord (= 8.1.1) + activestorage (= 8.1.1) + activesupport (= 8.1.1) mail (>= 2.8.0) - actionmailer (8.1.0) - actionpack (= 8.1.0) - actionview (= 8.1.0) - activejob (= 8.1.0) - activesupport (= 8.1.0) + actionmailer (8.1.1) + actionpack (= 8.1.1) + actionview (= 8.1.1) + activejob (= 8.1.1) + activesupport (= 8.1.1) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (8.1.0) - actionview (= 8.1.0) - activesupport (= 8.1.0) + actionpack (8.1.1) + actionview (= 8.1.1) + activesupport (= 8.1.1) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -33,36 +33,36 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (8.1.0) + actiontext (8.1.1) action_text-trix (~> 2.1.15) - actionpack (= 8.1.0) - activerecord (= 8.1.0) - activestorage (= 8.1.0) - activesupport (= 8.1.0) + actionpack (= 8.1.1) + activerecord (= 8.1.1) + activestorage (= 8.1.1) + activesupport (= 8.1.1) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (8.1.0) - activesupport (= 8.1.0) + actionview (8.1.1) + activesupport (= 8.1.1) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (8.1.0) - activesupport (= 8.1.0) + activejob (8.1.1) + activesupport (= 8.1.1) globalid (>= 0.3.6) - activemodel (8.1.0) - activesupport (= 8.1.0) - activerecord (8.1.0) - activemodel (= 8.1.0) - activesupport (= 8.1.0) + activemodel (8.1.1) + activesupport (= 8.1.1) + activerecord (8.1.1) + activemodel (= 8.1.1) + activesupport (= 8.1.1) timeout (>= 0.4.0) - activestorage (8.1.0) - actionpack (= 8.1.0) - activejob (= 8.1.0) - activerecord (= 8.1.0) - activesupport (= 8.1.0) + activestorage (8.1.1) + actionpack (= 8.1.1) + activejob (= 8.1.1) + activerecord (= 8.1.1) + activesupport (= 8.1.1) marcel (~> 1.0) - activesupport (8.1.0) + activesupport (8.1.1) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) @@ -112,14 +112,14 @@ GEM cbor (~> 0.5.9) openssl-signature_algorithm (~> 1.0) crass (1.0.6) - date (3.4.1) + date (3.5.0) debug (1.11.0) irb (~> 1.10) reline (>= 0.3.8) dotenv (3.1.8) drb (2.2.3) ed25519 (1.4.0) - erb (5.1.1) + erb (5.1.3) erubi (1.13.1) ffi (1.17.2-aarch64-linux-gnu) ffi (1.17.2-aarch64-linux-musl) @@ -140,14 +140,14 @@ GEM activesupport (>= 6.0.0) railties (>= 6.0.0) io-console (0.8.1) - irb (1.15.2) + irb (1.15.3) pp (>= 0.6.0) rdoc (>= 4.0.0) reline (>= 0.4.2) jbuilder (2.14.1) actionview (>= 7.0.0) activesupport (>= 7.0.0) - json (2.15.1) + json (2.15.2) jwt (3.1.2) base64 kamal (2.8.1) @@ -200,7 +200,7 @@ GEM net-smtp (0.5.1) net-protocol net-ssh (7.3.0) - nio4r (2.7.4) + nio4r (2.7.5) nokogiri (1.18.10-aarch64-linux-gnu) racc (~> 1.4) nokogiri (1.18.10-aarch64-linux-musl) @@ -238,7 +238,7 @@ GEM puma (7.1.0) nio4r (~> 2.0) racc (1.8.1) - rack (3.2.3) + rack (3.2.4) rack-session (2.1.1) base64 (>= 0.1.0) rack (>= 3.0.0) @@ -246,20 +246,20 @@ GEM rack (>= 1.3) rackup (2.2.1) rack (>= 3) - rails (8.1.0) - actioncable (= 8.1.0) - actionmailbox (= 8.1.0) - actionmailer (= 8.1.0) - actionpack (= 8.1.0) - actiontext (= 8.1.0) - actionview (= 8.1.0) - activejob (= 8.1.0) - activemodel (= 8.1.0) - activerecord (= 8.1.0) - activestorage (= 8.1.0) - activesupport (= 8.1.0) + rails (8.1.1) + actioncable (= 8.1.1) + actionmailbox (= 8.1.1) + actionmailer (= 8.1.1) + actionpack (= 8.1.1) + actiontext (= 8.1.1) + actionview (= 8.1.1) + activejob (= 8.1.1) + activemodel (= 8.1.1) + activerecord (= 8.1.1) + activestorage (= 8.1.1) + activesupport (= 8.1.1) bundler (>= 1.15.0) - railties (= 8.1.0) + railties (= 8.1.1) rails-dom-testing (2.3.0) activesupport (>= 5.0.0) minitest @@ -267,9 +267,9 @@ GEM rails-html-sanitizer (1.6.2) loofah (~> 2.21) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.1.0) - actionpack (= 8.1.0) - activesupport (= 8.1.0) + railties (8.1.1) + actionpack (= 8.1.1) + activesupport (= 8.1.1) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) @@ -277,8 +277,8 @@ GEM tsort (>= 0.2) zeitwerk (~> 2.6) rainbow (3.1.1) - rake (13.3.0) - rdoc (6.15.0) + rake (13.3.1) + rdoc (6.15.1) erb psych (>= 4.0.0) tsort @@ -373,7 +373,7 @@ GEM thruster (0.1.16-aarch64-linux) thruster (0.1.16-arm64-darwin) thruster (0.1.16-x86_64-linux) - timeout (0.4.3) + timeout (0.4.4) tpm-key_attestation (0.14.1) bindata (~> 2.4) openssl (> 2.0) @@ -387,7 +387,7 @@ GEM unicode-display_width (3.2.0) unicode-emoji (~> 4.1) unicode-emoji (4.1.0) - uri (1.0.4) + uri (1.1.0) useragent (0.16.11) web-console (4.2.1) actionview (>= 6.0.0) @@ -438,7 +438,7 @@ DEPENDENCIES propshaft public_suffix (~> 6.0) puma (>= 5.0) - rails (~> 8.1.0) + rails (~> 8.1.1) rotp (~> 6.3) rqrcode (~> 3.1) rubocop-rails-omakase diff --git a/app/controllers/totp_controller.rb b/app/controllers/totp_controller.rb index dfa15c3..37d2553 100644 --- a/app/controllers/totp_controller.rb +++ b/app/controllers/totp_controller.rb @@ -24,9 +24,12 @@ class TotpController < ApplicationController if totp.verify(code, drift_behind: 30, drift_ahead: 30) # Save the secret and generate backup codes @user.totp_secret = totp_secret - @user.backup_codes = generate_backup_codes + plain_codes = @user.send(:generate_backup_codes) # Use private method from User model @user.save! + # 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." else @@ -36,8 +39,15 @@ class TotpController < ApplicationController # GET /totp/backup_codes - Show backup codes (requires password) def backup_codes - # This will be shown after password verification - @backup_codes = @user.parsed_backup_codes + # Check if we have temporary codes from TOTP setup + if session[:temp_backup_codes].present? + @backup_codes = session[:temp_backup_codes] + session.delete(:temp_backup_codes) # Clear after use + else + # This will be shown after password verification for existing users + # Since we can't display BCrypt hashes, redirect to regenerate + redirect_to regenerate_backup_codes_totp_path + end end # POST /totp/verify_password - Verify password before showing backup codes @@ -49,6 +59,28 @@ class TotpController < ApplicationController end end + # GET /totp/regenerate_backup_codes - Regenerate backup codes (requires password) + def regenerate_backup_codes + # This will be shown after password verification + end + + # POST /totp/regenerate_backup_codes - Actually regenerate backup codes + def create_new_backup_codes + unless @user.authenticate(params[:password]) + redirect_to regenerate_backup_codes_totp_path, alert: "Incorrect password." + return + end + + # Generate new backup codes and store BCrypt hashes + plain_codes = @user.send(:generate_backup_codes) + @user.save! + + # Store plain codes temporarily in session for display + session[:temp_backup_codes] = plain_codes + + redirect_to backup_codes_totp_path, notice: "New backup codes have been generated. Save them now!" + end + # DELETE /totp - Disable TOTP (requires password) def destroy unless @user.authenticate(params[:password]) @@ -77,8 +109,4 @@ class TotpController < ApplicationController redirect_to profile_path, alert: "Two-factor authentication is not enabled." end end - - def generate_backup_codes - Array.new(10) { SecureRandom.alphanumeric(8).upcase }.to_json - end end diff --git a/app/javascript/controllers/modal_controller.js b/app/javascript/controllers/modal_controller.js index 4d5beca..5ee48b9 100644 --- a/app/javascript/controllers/modal_controller.js +++ b/app/javascript/controllers/modal_controller.js @@ -3,6 +3,9 @@ import { Controller } from "@hotwired/stimulus" // Generic modal controller for showing/hiding modal dialogs export default class extends Controller { static targets = ["dialog"] + static values = { + refreshOnClose: { type: Boolean, default: false } + } show(event) { // If called from a button with data-modal-id, find and show that modal @@ -11,6 +14,8 @@ export default class extends Controller { const modal = document.getElementById(modalId); if (modal) { modal.classList.remove("hidden"); + // Store the refresh preference from the button + this.refreshOnCloseValue = event.currentTarget?.dataset?.refreshOnClose === "true"; } } else if (this.hasDialogTarget) { // Otherwise show the dialog target @@ -22,11 +27,20 @@ export default class extends Controller { } hide() { - if (this.hasDialogTarget) { + // Find the currently visible modal to hide it + const visibleModal = document.querySelector('[data-controller="modal"] .fixed.inset-0:not(.hidden)'); + if (visibleModal) { + visibleModal.classList.add("hidden"); + } else if (this.hasDialogTarget) { this.dialogTarget.classList.add("hidden"); } else { this.element.classList.add("hidden"); } + + // Refresh page if requested + if (this.refreshOnCloseValue) { + window.location.reload(); + } } // Close modal when clicking backdrop diff --git a/app/models/user.rb b/app/models/user.rb index abed8bf..8cbbabd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -66,19 +66,53 @@ class User < ApplicationRecord def verify_backup_code(code) return false unless backup_codes.present? - codes = JSON.parse(backup_codes) - if codes.include?(code) - codes.delete(code) - update(backup_codes: codes.to_json) + # Rate limiting: prevent brute force attacks on backup codes + if rate_limit_backup_code_verification? + Rails.logger.warn "Rate limit exceeded for backup code verification - User ID: #{id}" + return false + end + + # backup_codes is now an Array (JSON column), no need to parse + # Find the matching hash by comparing with BCrypt + matching_hash = backup_codes.find do |hashed_code| + BCrypt::Password.new(hashed_code) == code + end + + if matching_hash + # Remove the used hash from the array (single-use property) + backup_codes.delete(matching_hash) + save! # Save the updated array + + # Log successful backup code usage for security monitoring + Rails.logger.info "Backup code used successfully - User ID: #{id}, IP: #{Current.session&.client_ip}" true else + # Increment failed attempt counter and log for security monitoring + increment_backup_code_failed_attempts + Rails.logger.warn "Failed backup code attempt - User ID: #{id}, IP: #{Current.session&.client_ip}" false end end - def parsed_backup_codes - return [] unless backup_codes.present? - JSON.parse(backup_codes) + # Rate limiting for backup code verification to prevent brute force attacks + def rate_limit_backup_code_verification? + # Use Rails.cache to track failed attempts + cache_key = "backup_code_failed_attempts_#{id}" + attempts = Rails.cache.read(cache_key) || 0 + + if attempts >= 5 # Allow max 5 failed attempts per hour + true + else + # Don't increment here - increment only on failed attempts + false + end + end + + # Increment failed attempt counter + def increment_backup_code_failed_attempts + cache_key = "backup_code_failed_attempts_#{id}" + attempts = Rails.cache.read(cache_key) || 0 + Rails.cache.write(cache_key, attempts + 1, expires_in: 1.hour) end # WebAuthn methods @@ -152,6 +186,16 @@ class User < ApplicationRecord private def generate_backup_codes - Array.new(10) { SecureRandom.alphanumeric(8).upcase }.to_json + # Generate plain codes for user to see/save + plain_codes = Array.new(10) { SecureRandom.alphanumeric(8).upcase } + + # Store BCrypt hashes of the codes + hashed_codes = plain_codes.map { |code| BCrypt::Password.create(code) } + + # Return plain codes for display (will be shown to user once) + # Store only hashes in the database (as Array for JSON column) + self.backup_codes = hashed_codes + + plain_codes end end diff --git a/app/views/admin/applications/_form.html.erb b/app/views/admin/applications/_form.html.erb index 9e50c98..e253afd 100644 --- a/app/views/admin/applications/_form.html.erb +++ b/app/views/admin/applications/_form.html.erb @@ -1,4 +1,4 @@ -<%= form_with(model: [:admin, application], class: "space-y-6") do |form| %> +<%= form_with(model: [:admin, application], class: "space-y-6", data: { controller: "application-form" }) do |form| %> <% if application.errors.any? %>
Application type cannot be changed after creation.
<% end %>