Skip to content

Commit

Permalink
feat: two factor authentication
Browse files Browse the repository at this point in the history
  • Loading branch information
kjellberg committed Apr 1, 2024
1 parent da4d8f4 commit a38aef8
Show file tree
Hide file tree
Showing 15 changed files with 147 additions and 19 deletions.
2 changes: 1 addition & 1 deletion app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def edit
end

def update
if @account.update(account_params)
if @account.update(account_permitted_parameters)
redirect_to edit_account_path, notice: I18n.t("accounts.update.success")
else
render :edit, status: :unprocessable_entity
Expand Down
23 changes: 12 additions & 11 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,15 @@ class ApplicationController < ActionController::Base

before_action :authenticate_user!
before_action :ensure_onboarded, unless: :devise_controller?
before_action :configure_permitted_parameters, if: :devise_controller?

protected

def configure_permitted_parameters
devise_parameter_sanitizer.permit(:sign_in, keys: [:otp_attempt])
end

private

# Strong parameters for account.
# Used for account creation and update.
def account_params
def account_permitted_parameters
params.require(:account).permit(:name)
end

private

# Automatically include account_id in all URL options if it is already present in the params.
# This is used to ensure that all routes are scoped to the current team. Personal account
# routes are not affected.
Expand All @@ -32,8 +25,16 @@ def ensure_onboarded
redirect_to onboarding_path if user_signed_in? && !current_user.onboarded?
end

# Redirect to dashboard after sign in
# Override the method to change the sign-in redirect path
def after_sign_in_path_for(resource)
dashboard_path
end

# Override the method to change the sign-out redirect path
def after_sign_out_path_for(resource_or_scope)
# Generate the root path without default URL options
uri = URI.parse(root_url)
uri.query = nil # Remove any query parameters
uri.to_s
end
end
2 changes: 1 addition & 1 deletion app/controllers/users/onboarding_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def new
end

def create
@account = current_user.build_personal_account(account_params)
@account = current_user.build_personal_account(account_permitted_parameters)
@account.personal = true

if current_user.save
Expand Down
44 changes: 44 additions & 0 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,46 @@
class Users::SessionsController < Devise::SessionsController
# prepend_before_action :configure_permitted_parameters, if: :devise_controller?
prepend_before_action :otp_authentication, only: :create

def new
# New login attempts should reset the otp_user_id
session.delete(:otp_user_id)
super
end

# Override the default Devise create method and check if the user has enabled 2FA
# If 2FA is enabled, render the OTP form page. Otherwise, proceed with the default login flow.
def otp_authentication
devise_parameter_sanitizer.permit(:sign_in, keys: [:otp_attempt])

if sign_in_params[:email]
show_otp_code_form
elsif session[:otp_user_id]
validate_otp_code
end
end

def show_otp_code_form
# Reset the session if the user is trying to login again.
session.delete(:otp_user_id)

self.resource = User.find_by(email: sign_in_params[:email])
if resource.valid_password?(sign_in_params[:password]) && resource.otp_required_for_login?
session[:otp_user_id] = resource.id
render :otp, status: :unprocessable_entity
end
end

def validate_otp_code
self.resource = User.find(session[:otp_user_id])
if resource.validate_and_consume_otp!(sign_in_params[:otp_attempt])

set_flash_message!(:notice, :signed_in)
sign_in(resource_name, resource)
redirect_to after_sign_in_path_for(resource)
else
resource.errors.add(:otp_attempt, :invalid)
render :otp, status: :unprocessable_entity
end
end
end
14 changes: 9 additions & 5 deletions app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
class User < ApplicationRecord
# Include default devise modules.
devise :registerable, :recoverable, :rememberable, :validatable, :confirmable, :lockable, :timeoutable, :trackable, :two_factor_authenticatable # , :omniauthable
# Default devise modules.
devise :registerable, :recoverable, :rememberable, :validatable, :confirmable, :lockable, :timeoutable, :trackable

# Access the users personal account.
# Enable two-factor authentication.
devise :two_factor_authenticatable

# User belongs to a personal account.
belongs_to :personal_account, class_name: "Account", optional: true, dependent: :destroy
validates_associated :personal_account

# User can have many team accounts.
has_many :account_users, dependent: :destroy
has_many :accounts, through: :account_users

validates_associated :personal_account

def onboarded?
personal_account.present? && personal_account.persisted?
end
Expand Down
1 change: 1 addition & 0 deletions app/views/users/sessions/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<%= simple_form_for(resource, as: resource_name, url: session_path(resource_name)) do |f| %>
<%= f.input :email, placeholder: t(".form.email.placeholder"), required: true, autofocus: true, input_html: { autocomplete: "email" } %>
<%= f.input :password, placeholder: t(".form.password.placeholder"), required: true, input_html: { autocomplete: "current-password" } %>

<%= f.input :remember_me, label: t(".form.remember_me.label"), as: :boolean, wrapper: :inline_checkbox if devise_mapping.rememberable? %>

<div class="flex justify-between items-center">
Expand Down
18 changes: 18 additions & 0 deletions app/views/users/sessions/otp.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<% title "Two factor authentication" %>

<%= fullscreen do |screen| %>
<%= screen.with_form(
title: t(".heading.title", app_name: Kiqr::Config.app_name),
description: t(".heading.description")
) do %>
<main class="flex flex-col gap-4 w-full max-w-xl">
<%= simple_form_for(resource, as: resource_name, url: new_user_session_path, method: :post) do |f| %>
<%= f.input :otp_attempt, label: t(".form.otp.label"), placeholder: t(".form.otp.placeholder") %>

<div class="flex justify-between items-center">
<%= f.button :submit, t(".form.submit_button") %>
</div>
<% end %>
</main>
<% end %>
<% end %>
1 change: 1 addition & 0 deletions config/credentials/test.key
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
aecc9f0ffc88c629a51ee97520c76963
1 change: 1 addition & 0 deletions config/credentials/test.yml.enc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nl6YcBTnI/xYrRDaLXfnCNI8HV9AhUXELHeFfAZ7A7StirCmjsNU1RN64n0XHs9nwaANuWrVpuEmyw1nSkpKBkkaOTY/OFNIjua7gULz43Z0u+yuSxNDXVD1REVWmtKfAioQUWy564MbLNDcbic8zlJJsv3zgEN5cNe6/nLBDov8aG9LFyJg948ZO5QBj2N41dZl5MjesyPIPfFpYtzEwNdrGlHxYtGmmAcl9nRIMBAN6T+26RAkQoxIDPwOk+N384Wxisc5AvenRj+GCLgCZoC9pbNdWYnnZck/yxB8/rA9slCdmTrPxsFTdP2O8ho6LLe23HthKzqVkEwgNXUTiDHeGV76Z83u27tNBclGjxuCtE8GZZsNmq8onexj2PLxy8VlZ4iQhxPu9k7cPEm7t+te476FvSWwpSJUyq3H1+L7OOdMhhVYEEjie8Ue+vmifiZKbxDGMBIEi9/tY3yNmLdHBT/JnXr3AegJIS5+UHh4J5Cl8naTWYrXU2E7/o8x8Hl2Rr4uWXSpuKPIdd0cqzayITl+mDrXfEH0OE/n4WcMYWn5fAnf90AfFgSZ32+WufkisKl7E1cWWDOFpTh+16bz1v7clAazhv9oKmCOOko=--MHfJClNqfFA2siBd--fWeUh7gKNWGNnxnxulvNZQ==
2 changes: 1 addition & 1 deletion config/initializers/filter_parameter_logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
# Use this to limit dissemination of sensitive information.
# See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors.
Rails.application.config.filter_parameters += [
:passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn
:passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :otp_attempt
]
9 changes: 9 additions & 0 deletions config/locales/kiqr.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ en:
submit_button: "Sign in"
no_account: "Don't have an account?"
sign_up: "Sign up!"
otp:
heading:
title: "Two-factor authentication"
description: "Enter the code from your authenticator app."
form:
otp:
label: "One-time password"
placeholder: "Enter the code"
submit_button: "Verify"
registrations:
new:
heading:
Expand Down
13 changes: 13 additions & 0 deletions lib/templates/rails/credentials/credentials.yml.tt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<% unless options.skip_secret_key_base? -%>
# Used as the base secret for all MessageVerifiers in Rails, including the one protecting cookies.
secret_key_base: <%= secret_key_base %>

<% end -%>
# The new encryption features in Active Record. This will enable the new `encrypts` method to encrypt attributes.
# This is a required configuration if you want to enable two factor authentication.
#
# Read more: https://guides.rubyonrails.org/active_record_encryption.html
active_record_encryption:
primary_key: <%= SecureRandom.alphanumeric(32) %>
deterministic_key: <%= SecureRandom.alphanumeric(32) %>
key_derivation_salt: <%= SecureRandom.alphanumeric(32) %>
17 changes: 17 additions & 0 deletions test/controllers/users/sessions_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
require "test_helper"

class Users::SessionsControllerTest < ActionDispatch::IntegrationTest
test "require otp if otp is enabled" do
user = create(:user, :otp_enabled)
post user_session_path, params: {user: {email: user.email, password: user.password}}
assert_response :unprocessable_entity
assert_template "users/sessions/otp"
end

test "can sign in if otp is disabled" do
user = create(:user)
post user_session_path, params: {user: {email: user.email, password: user.password}}
assert_response :redirect
assert_redirected_to dashboard_path
end
end
5 changes: 5 additions & 0 deletions test/factories/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,10 @@
trait :unconfirmed do
confirmed_at { nil }
end

trait :otp_enabled do
otp_required_for_login { true }
otp_secret { User.generate_otp_secret }
end
end
end
14 changes: 14 additions & 0 deletions test/system/signin_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@ class SigninTest < ApplicationSystemTestCase
assert_current_path dashboard_path
end

test "signs in with otp code" do
user = create(:user, :otp_enabled)

visit new_user_session_path
fill_in "user[email]", with: user.email
fill_in "user[password]", with: user.password
click_on "commit"

fill_in "user[otp_attempt]", with: user.current_otp
click_on "commit"

assert_current_path dashboard_path
end

test "displays unconfirmed email message" do
unconfirmed_user = create(:user, :unconfirmed)

Expand Down

0 comments on commit a38aef8

Please sign in to comment.