CLDC-768: Add 2FA for Admin Users (#146)

* Add 2FA to admin panel sign in

* Test OTP code expiry

* Validate phone number presence

* Resend view

* Autocomplete 2fa code

* Update resend text

.env.example

@@ -1,3 +1,5 @@
 DB_USERNAME=postgres-user
 DB_PASSWORD=postgres-password
-GOVUK_NOTIFY_API_KEY=<notify-key-here-if-testing-emails>
+
+GOVUK_NOTIFY_API_KEY=<notify-key-here-if-testing-emails-or-admin-users>
+OTP_SECRET_ENCRYPTION_KEY="<Generate this using bundle exec rake secret>"

Gemfile

@@ -37,6 +37,9 @@ gem "json-schema"
 # Authentication
 # Point at branch until devise is compatible with Turbo, see https://github.com/heartcombo/devise/pull/5340
 gem "devise", github: "baarkerlounger/devise", branch: "dluhc-fixes"
+# Two-factor Authentication for devise models. Pointing at fork until this is merged for Rails 6 compatibility
+# https://github.com/Houdini/two_factor_authentication/pull/204
+gem "two_factor_authentication", github: "baarkerlounger/two_factor_authentication"
 # UK postcode parsing and validation
 gem "uk_postcode"
 # Get rich data from postcode lookups. Wraps postcodes.io

Gemfile.lock

@@ -18,6 +18,17 @@ GIT
       responders
       warden (~> 1.2.3)
 
+GIT
+  remote: https://github.com/baarkerlounger/two_factor_authentication.git
+  revision: a7522becd7222f1aa4ddf73d7caf19f05bdb4dac
+  specs:
+    two_factor_authentication (2.2.0)
+      devise
+      encryptor
+      rails (>= 3.1.1)
+      randexp
+      rotp (>= 4.0.0)
+
 GIT
   remote: https://github.com/tagliala/activeadmin.git
   revision: d1492c54e76871d95f3a7ff20e445b48f455d4cb
@@ -157,6 +168,7 @@ GEM
     dotenv-rails (2.7.6)
       dotenv (= 2.7.6)
       railties (>= 3.2)
+    encryptor (3.0.0)
     erubi (1.10.0)
     excon (0.90.0)
     factory_bot (6.2.0)
@@ -310,6 +322,7 @@ GEM
       zeitwerk (~> 2.5)
     rainbow (3.1.1)
     rake (13.0.6)
+    randexp (0.1.7)
     ransack (2.5.0)
       activerecord (>= 5.2.4)
       activesupport (>= 5.2.4)
@@ -325,6 +338,7 @@ GEM
     roo (2.8.3)
       nokogiri (~> 1)
       rubyzip (>= 1.3.0, < 3.0.0)
+    rotp (6.2.0)
     rspec-core (3.10.2)
       rspec-support (~> 3.10.0)
     rspec-expectations (3.10.2)
@@ -472,6 +486,7 @@ DEPENDENCIES
   scss_lint-govuk
   selenium-webdriver
   simplecov
+  two_factor_authentication!
   tzinfo-data
   uk_postcode
   view_component

app/admin/admin_users.rb

@@ -1,5 +1,5 @@
 ActiveAdmin.register AdminUser do
-  permit_params :email, :password, :password_confirmation
+  permit_params :email, :phone, :password, :password_confirmation
 
   controller do
     def update_resource(object, attributes)
@@ -12,6 +12,7 @@ ActiveAdmin.register AdminUser do
     selectable_column
     id_column
     column :email
+    column "Phone Number", :phone
     column :current_sign_in_at
     column :sign_in_count
     column :created_at
@@ -19,6 +20,7 @@ ActiveAdmin.register AdminUser do
   end
 
   filter :email
+  filter :phone
   filter :current_sign_in_at
   filter :sign_in_count
   filter :created_at
@@ -26,6 +28,7 @@ ActiveAdmin.register AdminUser do
   form do |f|
     f.inputs do
       f.input :email
+      f.input :phone
       f.input :password
       f.input :password_confirmation
     end

app/controllers/auth/two_factor_authentication_controller.rb

@@ -0,0 +1,5 @@
+class Auth::TwoFactorAuthenticationController < Devise::TwoFactorAuthenticationController
+  def show_resend
+    render "devise/two_factor_authentication/resend"
+  end
+end

app/models/admin_user.rb

@@ -1,5 +1,18 @@
 class AdminUser < ApplicationRecord
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
-  devise :database_authenticatable, :recoverable, :rememberable, :validatable
+  devise :two_factor_authenticatable, :database_authenticatable, :recoverable,
+         :rememberable, :validatable
+
+  has_one_time_password(encrypted: true)
+
+  validates :phone, presence: true, numericality: true
+
+  MFA_SMS_TEMPLATE_ID = "bf309d93-804e-4f95-b1f4-bd513c48ecb0".freeze
+
+  def send_two_factor_authentication_code(code)
+    template_id = MFA_SMS_TEMPLATE_ID
+    personalisation = { otp: code }
+    Sms.send(phone, template_id, personalisation)
+  end
 end

app/services/sms.rb

@@ -0,0 +1,15 @@
+require "notifications/client"
+
+class Sms
+  def self.notify_client
+    Notifications::Client.new(ENV["GOVUK_NOTIFY_API_KEY"])
+  end
+
+  def self.send(phone_number, template_id, args)
+    notify_client.send_sms(
+      phone_number: phone_number,
+      template_id: template_id,
+      personalisation: args,
+    )
+  end
+end

app/views/devise/two_factor_authentication/resend.html.erb

@@ -0,0 +1,23 @@
+<% content_for :title, "Resend security code" %>
+
+<% content_for :before_content do %>
+  <%= govuk_back_link(
+    text: 'Back',
+    href: 'javascript:history.back()',
+  ) %>
+<% end %>
+
+<%= form_with(url: resend_code_admin_user_two_factor_authentication_path, html: { method: :get }) do |f| %>
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+
+      <h1 class="govuk-heading-l">
+        <%= content_for(:title) %>
+      </h1>
+
+      <p class="govuk-body">Text messages sometimes take a few minutes to arrive. If you do not receive the text message, you can request a new one.</p>
+
+      <%= f.govuk_submit "Resend security code" %>
+    </div>
+  </div>
+<% end %>

app/views/devise/two_factor_authentication/show.html.erb

@@ -0,0 +1,27 @@
+<% content_for :title, "Check your phone" %>
+
+<%= form_with(url: "/admin/two-factor-authentication", html: { method: :put }) do |f| %>
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+
+      <h1 class="govuk-heading-l">
+        <%= content_for(:title) %>
+      </h1>
+
+      <p class="govuk-body">We’ve sent you a text message with a security code.</p>
+
+      <%= f.govuk_number_field :code,
+          label: { text: "Security code" },
+          width: 5,
+          autocomplete: 'one-time-code',
+          autofocus: true
+      %>
+
+      <%= f.govuk_submit "Submit" %>
+    </div>
+  </div>
+<% end %>
+
+<p class="govuk-body">
+  <%= govuk_link_to "Not received a text message?", admin_two_factor_authentication_resend_path %>
+</p>

config/initializers/devise.rb

@@ -309,4 +309,15 @@ Devise.setup do |config|
   # When set to false, does not sign a user in automatically after their password is
   # changed. Defaults to true, so a user is signed in automatically after changing a password.
   # config.sign_in_after_change_password = true
+
+  # 2FA
+  config.max_login_attempts = 3 # Maximum second factor attempts count.
+  config.allowed_otp_drift_seconds = 30 # Allowed TOTP time drift between client and server.
+  config.otp_length = 6 # TOTP code length
+  config.direct_otp_valid_for = 5.minutes # Time before direct OTP becomes invalid
+  config.direct_otp_length = 6 # Direct OTP code length
+  config.remember_otp_session_for_seconds = 1.day # Time before browser has to perform 2fA again. Default is 0.
+  config.otp_secret_encryption_key = ENV["OTP_SECRET_ENCRYPTION_KEY"]
+  config.second_factor_resource_id = "id" # Field or method name used to set value for 2fA remember cookie
+  config.delete_cookie_on_logout = false # Delete cookie when user signs out, to force 2fA again on login
 end

config/locales/en.yml

@@ -122,3 +122,10 @@ en:
         message: "Net income is lower than expected based on the main tenant’s working situation. Are you sure this is correct?"
       in_soft_max_range:
         message: "Net income is higher than expected based on the main tenant’s working situation. Are you sure this is correct?"
+  devise:
+    two_factor_authentication:
+      success: "Two factor authentication successful."
+      attempt_failed: "Attempt failed."
+      max_login_attempts_reached: "Access completely denied as you have reached your attempts limit"
+      contact_administrator: "Please contact your system administrator."
+      code_has_been_sent: "Your security code has been sent."

config/routes.rb

@@ -1,5 +1,22 @@
 Rails.application.routes.draw do
-  devise_for :admin_users, ActiveAdmin::Devise.config
+  devise_for :admin_users, {
+    path: :admin,
+    controllers: {
+      sessions: "active_admin/devise/sessions",
+      passwords: "active_admin/devise/passwords",
+      unlocks: "active_admin/devise/unlocks",
+      registrations: "active_admin/devise/registrations",
+      confirmations: "active_admin/devise/confirmations",
+      two_factor_authentication: "auth/two_factor_authentication",
+    },
+    path_names: { sign_in: "login", sign_out: "logout", two_factor_authentication: "two-factor-authentication" },
+    sign_out_via: %i[delete get],
+  }
+
+  devise_scope :admin_user do
+    get "admin/two-factor-authentication/resend", to: "auth/two_factor_authentication#show_resend"
+  end
+
   devise_for :users, controllers: {
     passwords: "auth/passwords",
     sessions: "auth/sessions",

db/migrate/20211203135623_two_factor_authentication_add_to_admin_users.rb

@@ -0,0 +1,16 @@
+class TwoFactorAuthenticationAddToAdminUsers < ActiveRecord::Migration[6.1]
+  def change
+    change_table :admin_users, bulk: true do |t|
+      t.column :second_factor_attempts_count, :integer, default: 0
+      t.column :encrypted_otp_secret_key, :string
+      t.column :encrypted_otp_secret_key_iv, :string
+      t.column :encrypted_otp_secret_key_salt, :string
+      t.column :direct_otp, :string
+      t.column :direct_otp_sent_at, :datetime
+      t.column :totp_timestamp, :timestamp
+      t.column :phone, :string
+
+      t.index :encrypted_otp_secret_key, unique: true
+    end
+  end
+end

db/schema.rb

@@ -23,6 +23,15 @@ ActiveRecord::Schema.define(version: 2022_01_31_123638) do
     t.datetime "remember_created_at"
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
+    t.integer "second_factor_attempts_count", default: 0
+    t.string "encrypted_otp_secret_key"
+    t.string "encrypted_otp_secret_key_iv"
+    t.string "encrypted_otp_secret_key_salt"
+    t.string "direct_otp"
+    t.datetime "direct_otp_sent_at"
+    t.datetime "totp_timestamp"
+    t.string "phone"
+    t.index ["encrypted_otp_secret_key"], name: "index_admin_users_on_encrypted_otp_secret_key", unique: true
   end
 
   create_table "case_logs", force: :cascade do |t|

db/seeds.rb

@@ -30,4 +30,4 @@ User.create!(
   role: "data_coordinator",
 )
 
-AdminUser.create!(email: "admin@example.com", password: "password")
+AdminUser.create!(email: "admin@example.com", password: "password", phone: "000000000")

spec/controllers/admin/admin_users_controller_spec.rb

@@ -22,7 +22,7 @@ describe Admin::AdminUsersController, type: :controller do
   end
 
   describe "Create admin users" do
-    let(:params) { { admin_user: { email: "test2@example.com", password: "pAssword1" } } }
+    let(:params) { { admin_user: { email: "test2@example.com", password: "pAssword1", phone: "07566126368" } } }
 
     it "creates a new admin user" do
       expect { post :create, session: valid_session, params: params }.to change(AdminUser, :count).by(1)

spec/factories/admin_user.rb

@@ -2,6 +2,7 @@ FactoryBot.define do
   factory :admin_user do
     sequence(:email) { |i| "admin#{i}@example.com" }
     password { "pAssword1" }
+    phone { "07563867654" }
     created_at { Time.zone.now }
     updated_at { Time.zone.now }
   end

spec/features/admin_panel_spec.rb

@@ -0,0 +1,75 @@
+require "rails_helper"
+
+RSpec.describe "Admin Panel" do
+  let!(:admin) { FactoryBot.create(:admin_user) }
+  let(:notify_client) { instance_double(Notifications::Client) }
+  let(:mfa_template_id) { AdminUser::MFA_SMS_TEMPLATE_ID }
+  let(:otp) { "999111" }
+
+  before do
+    allow(Sms).to receive(:notify_client).and_return(notify_client)
+    allow(notify_client).to receive(:send_sms).and_return(true)
+  end
+
+  context "with a valid 2FA code" do
+    before do
+      allow(SecureRandom).to receive(:random_number).and_return(otp)
+      visit("/admin")
+      fill_in("admin_user[email]", with: admin.email)
+      fill_in("admin_user[password]", with: admin.password)
+    end
+
+    it "authenticates successfully" do
+      expect(notify_client).to receive(:send_sms).with(
+        hash_including(phone_number: admin.phone, template_id: mfa_template_id),
+      )
+      click_button("Login")
+      fill_in("code", with: otp)
+      click_button("Submit")
+      expect(page).to have_content("Dashboard")
+      expect(page).to have_content("Two factor authentication successful.")
+    end
+
+    context "but it is more than 5 minutes old" do
+      it "does not authenticate successfully" do
+        click_button("Login")
+        admin.update!(direct_otp_sent_at: 10.minutes.ago)
+        fill_in("code", with: otp)
+        click_button("Submit")
+        expect(page).to have_content("Check your phone")
+      end
+    end
+  end
+
+  context "with an invalid 2FA code" do
+    it "does not authenticate successfully" do
+      visit("/admin")
+      fill_in("admin_user[email]", with: admin.email)
+      fill_in("admin_user[password]", with: admin.password)
+      click_button("Login")
+      fill_in("code", with: otp)
+      click_button("Submit")
+      expect(page).to have_content("Check your phone")
+    end
+  end
+
+  context "when the 2FA code needs to be resent" do
+    before do
+      visit("/admin")
+      fill_in("admin_user[email]", with: admin.email)
+      fill_in("admin_user[password]", with: admin.password)
+      click_button("Login")
+    end
+
+    it "displays the resend view" do
+      click_link("Not received a text message?")
+      expect(page).to have_button("Resend security code")
+    end
+
+    it "send a new OTP code and redirects back to the 2FA view" do
+      click_link("Not received a text message?")
+      expect { click_button("Resend security code") }.to(change { admin.reload.direct_otp })
+      expect(page).to have_current_path("/admin/two-factor-authentication")
+    end
+  end
+end

spec/models/admin_user_spec.rb

@@ -0,0 +1,52 @@
+require "rails_helper"
+
+RSpec.describe AdminUser, type: :model do
+  describe "#new" do
+    it "requires a phone number" do
+      expect {
+        described_class.create!(
+          email: "admin_test@example.com",
+          password: "password123",
+        )
+      }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    it "requires a numerical phone number" do
+      expect {
+        described_class.create!(
+          email: "admin_test@example.com",
+          password: "password123",
+          phone: "string",
+        )
+      }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+  end
+
+  it "requires an email" do
+    expect {
+      described_class.create!(
+        password: "password123",
+        phone: "075752137",
+      )
+    }.to raise_error(ActiveRecord::RecordInvalid)
+  end
+
+  it "requires a password" do
+    expect {
+      described_class.create!(
+        email: "admin_test@example.com",
+        phone: "075752137",
+      )
+    }.to raise_error(ActiveRecord::RecordInvalid)
+  end
+
+  it "can be created" do
+    expect {
+      described_class.create!(
+        email: "admin_test@example.com",
+        password: "password123",
+        phone: "075752137",
+      )
+    }.to change(described_class, :count).by(1)
+  end
+end