Skip to content

Commit

Permalink
Multi-factor authentication (#1817)
Browse files Browse the repository at this point in the history
* Initial pass

* Tests for MFA and locale cleanup

* Brakeman

* Update two-factor authentication status styling

* Update app/models/user.rb

Co-authored-by: Zach Gollwitzer <zach@maybe.co>
Signed-off-by: Josh Pigford <josh@joshpigford.com>

* Refactor MFA verification and session handling in tests

---------

Signed-off-by: Josh Pigford <josh@joshpigford.com>
Co-authored-by: Zach Gollwitzer <zach@maybe.co>
  • Loading branch information
Shpigford and zachgoll authored Feb 6, 2025
1 parent 7ba9063 commit 842e376
Show file tree
Hide file tree
Showing 29 changed files with 598 additions and 33 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ gem "redcarpet"
gem "stripe"
gem "intercom-rails"
gem "plaid"
gem "rotp", "~> 6.3"
gem "rqrcode", "~> 2.2"

group :development, :test do
gem "debug", platforms: %i[mri windows]
Expand Down
8 changes: 8 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ GEM
xpath (~> 3.2)
childprocess (5.1.0)
logger (~> 1.5)
chunky_png (1.4.0)
climate_control (1.2.0)
concurrent-ruby (1.3.5)
connection_pool (2.5.0)
Expand Down Expand Up @@ -397,6 +398,11 @@ GEM
reline (0.6.0)
io-console (~> 0.5)
rexml (3.4.0)
rotp (6.3.0)
rqrcode (2.2.0)
chunky_png (~> 1.0)
rqrcode_core (~> 1.0)
rqrcode_core (1.2.0)
rubocop (1.71.0)
json (~> 2.3)
language_server-protocol (>= 3.17.0)
Expand Down Expand Up @@ -561,6 +567,8 @@ DEPENDENCIES
rails (~> 7.2.2)
rails-settings-cached
redcarpet
rotp (~> 6.3)
rqrcode (~> 2.2)
rubocop-rails-omakase
ruby-lsp-rails
selenium-webdriver
Expand Down
53 changes: 53 additions & 0 deletions app/controllers/mfa_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
class MfaController < ApplicationController
layout :determine_layout
skip_authentication only: [ :verify, :verify_code ]

def new
redirect_to root_path if Current.user.otp_required?
Current.user.setup_mfa! unless Current.user.otp_secret.present?
end

def create
if Current.user.verify_otp?(params[:code])
Current.user.enable_mfa!
@backup_codes = Current.user.otp_backup_codes
render :backup_codes
else
Current.user.disable_mfa!
redirect_to new_mfa_path, alert: t(".invalid_code")
end
end

def verify
@user = User.find_by(id: session[:mfa_user_id])
redirect_to new_session_path unless @user
end

def verify_code
@user = User.find_by(id: session[:mfa_user_id])

if @user&.verify_otp?(params[:code])
session.delete(:mfa_user_id)
@session = create_session_for(@user)
redirect_to root_path
else
flash.now[:alert] = t(".invalid_code")
render :verify, status: :unprocessable_entity
end
end

def disable
Current.user.disable_mfa!
redirect_to settings_security_path, notice: t(".success")
end

private

def determine_layout
if action_name.in?(%w[verify verify_code])
"auth"
else
"with_sidebar"
end
end
end
9 changes: 7 additions & 2 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ def new

def create
if user = User.authenticate_by(email: params[:email], password: params[:password])
@session = create_session_for(user)
redirect_to root_path
if user.otp_required?
session[:mfa_user_id] = user.id
redirect_to verify_mfa_path
else
@session = create_session_for(user)
redirect_to root_path
end
else
flash.now[:alert] = t(".invalid_credentials")
render :new, status: :unprocessable_entity
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/settings/securities_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class Settings::SecuritiesController < SettingsController
def show
end
end
4 changes: 4 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ def header_title(page_title)
content_for(:header_title) { page_title }
end

def header_description(page_description)
content_for(:header_description) { page_description }
end

def family_notifications_stream
turbo_stream_from [ Current.family, :notifications ] if Current.family
end
Expand Down
20 changes: 20 additions & 0 deletions app/helpers/mfa_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module MfaHelper
def generate_mfa_qr_code(provisioning_uri)
qr_code = RQRCode::QRCode.new(provisioning_uri).as_svg(
color: "141414",
module_size: 4,
standalone: true,
use_path: true,
svg_attributes: {
width: "228",
height: "228",
viewBox: "0 0 57 57"
}
)

# Whitelist specific SVG attributes and elements that we know are safe
sanitize qr_code,
tags: %w[svg g path rect],
attributes: %w[viewBox height width fill stroke stroke-width d x y class]
end
end
1 change: 1 addition & 0 deletions app/helpers/settings_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module SettingsHelper
{ name: I18n.t("settings.nav.profile_label"), path: :settings_profile_path },
{ name: I18n.t("settings.nav.preferences_label"), path: :settings_preferences_path },
{ name: I18n.t("settings.nav.self_hosting_label"), path: :settings_hosting_path, condition: :self_hosted? },
{ name: I18n.t("settings.nav.security_label"), path: :settings_security_path },
{ name: I18n.t("settings.nav.billing_label"), path: :settings_billing_path },
{ name: I18n.t("settings.nav.accounts_label"), path: :accounts_path },
{ name: I18n.t("settings.nav.imports_label"), path: :imports_path },
Expand Down
57 changes: 57 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,41 @@ def purge
end
end

# MFA
def setup_mfa!
update!(
otp_secret: ROTP::Base32.random(32),
otp_required: false,
otp_backup_codes: []
)
end

def enable_mfa!
update!(
otp_required: true,
otp_backup_codes: generate_backup_codes
)
end

def disable_mfa!
update!(
otp_secret: nil,
otp_required: false,
otp_backup_codes: []
)
end

def verify_otp?(code)
return false if otp_secret.blank?
return true if verify_backup_code?(code)
totp.verify(code, drift_behind: 15)
end

def provisioning_uri
return nil unless otp_secret.present?
totp.provisioning_uri(email)
end

private
def ensure_valid_profile_image
return unless profile_image.attached?
Expand All @@ -133,4 +168,26 @@ def profile_image_size
errors.add(:profile_image, :invalid_file_size, max_megabytes: 10)
end
end

def totp
ROTP::TOTP.new(otp_secret, issuer: "Maybe Finance")
end

def verify_backup_code?(code)
return false if otp_backup_codes.blank?

# Find and remove the used backup code
if (index = otp_backup_codes.index(code))
remaining_codes = otp_backup_codes.dup
remaining_codes.delete_at(index)
update_column(:otp_backup_codes, remaining_codes)
true
else
false
end
end

def generate_backup_codes
8.times.map { SecureRandom.hex(4) }
end
end
29 changes: 29 additions & 0 deletions app/views/mfa/backup_codes.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<%
header_title t(".title")
header_description t(".description")
%>

<% content_for :sidebar do %>
<%= render "settings/nav" %>
<% end %>

<div class="space-y-4">
<h1 class="text-gray-900 text-xl font-medium mb-4"><%= t(".page_title") %></h1>
<%= settings_section title: t(".backup_codes_title"), subtitle: t(".backup_codes_description") do %>
<div class="space-y-6">
<div class="grid grid-cols-2 gap-4">
<% @backup_codes.each do |code| %>
<div class="p-3 bg-gray-100 rounded-lg font-mono text-lg">
<%= code %>
</div>
<% end %>
</div>

<div class="mt-6">
<%= link_to t(".continue"), settings_security_path, class: "w-full btn btn--primary" %>
</div>
</div>
<% end %>

<%= settings_nav_footer %>
</div>
45 changes: 45 additions & 0 deletions app/views/mfa/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<%
header_title t(".title")
header_description t(".description")
%>

<% content_for :sidebar do %>
<%= render "settings/nav" %>
<% end %>

<div class="space-y-4">
<h1 class="text-gray-900 text-xl font-medium mb-4"><%= t(".page_title") %></h1>
<%= settings_section title: t(".scan_title"), subtitle: t(".scan_description") do %>
<div class="space-y-6">
<div>
<%= generate_mfa_qr_code(Current.user.provisioning_uri) %>
</div>

<div>
<h3 class="text-lg font-medium leading-6 text-gray-900"><%= t(".verify_title") %></h3>
<div class="mt-2 text-sm text-gray-500">
<p><%= t(".verify_description") %></p>
</div>
</div>

<%= styled_form_with url: mfa_path, method: :post, class: "mt-5", data: { turbo: false } do |f| %>
<div>
<%= f.text_field :code,
required: true,
autofocus: true,
autocomplete: "one-time-code",
inputmode: "numeric",
pattern: "[0-9]*",
label: t(".code_label"),
placeholder: t(".code_placeholder") %>

<div class="flex justify-end mt-4">
<%= f.submit t(".verify_button"), class: "bg-gray-900 hover:bg-gray-700 cursor-pointer text-white rounded-lg px-3 py-2" %>
</div>
</div>
<% end %>
</div>
<% end %>

<%= settings_nav_footer %>
</div>
14 changes: 14 additions & 0 deletions app/views/mfa/verify.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<%
header_title t(".title")
header_description t(".description")
%>

<%= styled_form_with url: verify_mfa_path, method: :post, class: "space-y-4" do |form| %>
<%= form.text_field :code,
required: true,
autofocus: true,
autocomplete: "one-time-code",
label: t(".page_title") %>

<%= form.submit t(".verify_button") %>
<% end %>
3 changes: 3 additions & 0 deletions app/views/settings/_nav.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
<li>
<%= sidebar_link_to t(".preferences_label"), settings_preferences_path, icon: "bolt" %>
</li>
<li>
<%= sidebar_link_to t(".security_label"), settings_security_path, icon: "shield-check" %>
</li>
<% if self_hosted? %>
<li>
<%= sidebar_link_to t(".self_hosting_label"), settings_hosting_path, icon: "database" %>
Expand Down
45 changes: 45 additions & 0 deletions app/views/settings/securities/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<% content_for :sidebar do %>
<%= render "settings/nav" %>
<% end %>

<div class="space-y-4">
<h1 class="text-gray-900 text-xl font-medium mb-4"><%= t(".page_title") %></h1>
<%= settings_section title: t(".mfa_title"), subtitle: t(".mfa_description") do %>
<div class="space-y-4">
<div class="p-3 shadow-xs bg-white border border-alpha-black-200 rounded-lg flex justify-between items-center">
<div class="flex items-center gap-3">
<div class="w-9 h-9 rounded-full bg-gray-25 flex justify-center items-center">
<%= lucide_icon "shield-check", class: "w-5 h-5 text-gray-500" %>
</div>

<div class="text-sm space-y-1">
<% if Current.user.otp_required? %>
<p class="text-gray-900">Two-factor authentication is <span class="font-medium text-green-600">enabled</span></p>
<p class="text-gray-500">Your account is protected with an additional layer of security.</p>
<% else %>
<p class="text-gray-900">Two-factor authentication is <span class="font-medium text-red-600">disabled</span></p>
<p class="text-gray-500">Enable 2FA to add an extra layer of security to your account.</p>
<% end %>
</div>
</div>

<% if Current.user.otp_required? %>
<%= button_to t(".disable_mfa"), disable_mfa_path,
method: :delete,
class: "btn btn--secondary flex items-center gap-1",
data: { turbo_confirm: {
title: t(".disable_mfa_confirm"),
body: t(".disable_mfa_confirm"),
accept: t(".disable_mfa"),
acceptClass: "w-full bg-red-500 text-white rounded-xl text-center p-[10px] border mb-2"
} } %>
<% else %>
<%= link_to t(".enable_mfa"), new_mfa_path,
class: "btn btn--primary flex items-center gap-1" %>
<% end %>
</div>
</div>
<% end %>

<%= settings_nav_footer %>
</div>
5 changes: 3 additions & 2 deletions config/locales/views/accounts/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,6 @@ en:
success: "%{type} account updated"
email_confirmations:
new:
success_login: "Your email has been confirmed. Please log in with your new email address."
invalid_token: "Invalid or expired confirmation link."
invalid_token: Invalid or expired confirmation link.
success_login: Your email has been confirmed. Please log in with your new email
address.
11 changes: 6 additions & 5 deletions config/locales/views/email_confirmation_mailer/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
en:
email_confirmation_mailer:
confirmation_email:
subject: "Maybe: Confirm your email change"
greeting: "Hello!"
body: "You recently requested to change your email address. Click the button below to confirm this change."
cta: "Confirm email change"
expiry_notice: "This link will expire in %{hours} hours."
body: You recently requested to change your email address. Click the button
below to confirm this change.
cta: Confirm email change
expiry_notice: This link will expire in %{hours} hours.
greeting: Hello!
subject: 'Maybe: Confirm your email change'
Loading

0 comments on commit 842e376

Please sign in to comment.