New version pulling in lessons from implementing on real aps

ABOUT_MFA.md

@@ -0,0 +1,89 @@
+# About this branch
+
+This branch contains the changes we would like rails-template to make to an app
+if they user enabled "Devise + 2FA"
+
+## two-factor authentication (2FA) background
+
+There are 3 main ways two-factor authentication (2FA) can be used in an app:
+
+1. Optional two-factor authentication (2FA): something that users can opt-in to to improve the security of
+   their account
+   - Users can disable two-factor authentication (2FA) if they choose to
+2. two-factor authentication (2FA) required for **all** users
+   - All users **must** setup two-factor authentication (2FA) as part of registration flow
+   - All users can **never** disable two-factor authentication (2FA)
+3. two-factor authentication (2FA) required for some users e.g. all users with the `admin` role
+   - Users can register without two-factor authentication (2FA)
+   - Users must set up two-factor authentication (2FA) when they become part of the group which requires it
+     e.g. they are given a new role
+   - Sign in flow must check whether the user requires two-factor authentication (2FA) and enforce it if
+     required
+
+This template implements _Optional two-factor authentication (2FA)_ (option 1).
+
+We choose this because it is the simplest and is a starting point for
+implementing the other kinds of two-factor authentication (2FA) integration. The other kinds of two-factor authentication (2FA)
+integration require more deep integration with your app and have a lot more
+scope for being different between applications e.g. is two-factor authentication (2FA) required for role,
+maybe a whole different user model etc. We don't think we could do a good job of
+the more complex kinds of integration from a template.
+
+The goal of this template is to provide your team with an _"two-factor authentication (2FA) starting
+point"_, not necessarily a complete two-factor authentication (2FA) feature that you don't have to modify in
+any way. If your app needs _Optional two-factor authentication (2FA)_ then the code here is very close to
+being complete. If you need required two-factor authentication (2FA) then you will need to do some work to
+integrate it with your app.
+
+## The underlying two-factor authentication (2FA) gem
+
+This code installs and configures the
+[devise-two-factor](https://github.com/tinfoil/devise-two-factor) gem.
+devise-two-factor is quite bare bones so this code adds endpoints to help users
+manage their two-factor authentication (2FA).
+
+# TODO
+
+- UX Challenges
+  - I've been working on this from the tech stuff up to the UX stuff. It might
+    benefit from somebody starting from UX (user-flows etc. and working down).
+    We are somewhat constrained here by not wanting to rewrite big chunks of
+    devise.
+  - We currently ask for the OTP code on the devise sign-in screen? Does it need
+    to be a second screen after sign-in?
+    - ++ current way easiest to implement, it's what the gem encourages
+    - ?? is it even possible to do it any other way?
+      - the devise action which gets the username and pass would need to stash
+        them somewhere waiting for the OTP code
+        - put them in the session? and then ask for the 2fa? putting creds in
+          session like that feels bad for security
+    - i think this would be a pretty big change to how devise sign-in works,
+      maybe more than we want?
+    - ?? maybe we could do it client side? but browser would need to get a
+      signal about that user requiring two-factor authentication (2FA) in a way that wouldn't allow an
+      attacker to enumerate which users require two-factor authentication (2FA) and which don't
+  - The terminology around two-factor authentication (2FA) is confusing and I don't think I'm currently
+    doing a good job of explaining it in plain english in the UI copy. This is
+    probably something we want designer input on.
+  - I don't think I'm warning users properly that "Reset my two-factor authentication (2FA)" destroys their
+    existing OTP secret so they can't actually sign in if they don't complete
+    the reset
+- After we agree that this branch is good:
+  - Should these changes be applied by a whole separate template repo or by the
+    main rails-template repo?
+
+# ============= Content for README ================
+
+### About two-factor authentication (2FA)
+
+- We only support one two-factor authentication (2FA) device per user (`devise-two-factor` does not support
+  multiple devices). If you need multiple devices, you probably want to fork
+  `devise-two-factor`.
+
+### two-factor authentication (2FA) Backup codes
+
+- Users can create _OTP Backup codes_ which they can use instead of the OTP
+  value to sign in.
+- Each backup code can be used **only once**
+- Backup codes do not depend on the current OTP secret. Resetting the OTP secret
+  does not invalidate the backup codes!

Gemfile

@@ -29,7 +29,6 @@ group :development do
 
   # Required in Rails 5 by ActiveSupport::EventedFileUpdateChecker
   gem "listen"
-  gem "overcommit", ">= 0.37.0", require: false
 end
 
 group :development, :test do
@@ -52,3 +51,7 @@ gem "axe-matchers"
 end
 
 gem "devise", "~> 4.8"
+gem "devise-two-factor", "~> 5.0"
+gem "rqrcode-rails3"
+gem "turbo-rails"
+gem "stimulus-rails", "~> 1.1"

Gemfile.lock

@@ -98,6 +98,7 @@ GEM
       regexp_parser (>= 1.5, < 3.0)
       xpath (~> 3.2)
     childprocess (4.1.0)
+    chunky_png (1.4.0)
     coderay (1.1.3)
     coercible (1.0.0)
       descendants_tracker (~> 0.0.1)
@@ -111,6 +112,11 @@ GEM
       railties (>= 4.1.0)
       responders
       warden (~> 1.2.3)
+    devise-two-factor (5.0.0)
+      activesupport (~> 7.0)
+      devise (~> 4.0)
+      railties (~> 7.0)
+      rotp (~> 6.0)
     diff-lcs (1.5.0)
     digest (3.1.0)
     docile (1.4.0)
@@ -134,7 +140,6 @@ GEM
     i18n (1.10.0)
       concurrent-ruby (~> 1.0)
     ice_nine (0.11.2)
-    iniparse (1.5.0)
     io-wait (0.2.1)
     launchy (2.5.0)
       addressable (~> 2.7)
@@ -184,10 +189,6 @@ GEM
       racc (~> 1.4)
     okcomputer (1.18.4)
     orm_adapter (0.5.0)
-    overcommit (0.58.0)
-      childprocess (>= 0.6.3, < 5)
-      iniparse (~> 1.4)
-      rexml (~> 3.2)
     parallel (1.21.0)
     parser (3.1.1.0)
       ast (~> 2.4.1)
@@ -250,6 +251,13 @@ GEM
       actionpack (>= 5.0)
       railties (>= 5.0)
     rexml (3.2.5)
+    rotp (6.2.0)
+    rqrcode (2.1.1)
+      chunky_png (~> 1.0)
+      rqrcode_core (~> 1.0)
+    rqrcode-rails3 (0.1.7)
+      rqrcode (>= 0.4.2)
+    rqrcode_core (1.2.0)
     rspec-core (3.11.0)
       rspec-support (~> 3.11.0)
     rspec-expectations (3.11.0)
@@ -329,11 +337,17 @@ GEM
       actionpack (>= 5.2)
       activesupport (>= 5.2)
       sprockets (>= 3.0.0)
+    stimulus-rails (1.1.0)
+      railties (>= 6.0.0)
     strscan (3.0.1)
     thor (1.2.1)
     thread_safe (0.3.6)
     tilt (2.0.10)
     timeout (0.2.0)
+    turbo-rails (1.1.1)
+      actionpack (>= 6.0.0)
+      activejob (>= 6.0.0)
+      railties (>= 6.0.0)
     tzinfo (2.0.4)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.1.0)
@@ -368,6 +382,7 @@ DEPENDENCIES
   bundler-audit
   capybara
   devise (~> 4.8)
+  devise-two-factor (~> 5.0)
   dotenv-rails
   factory_bot_rails
   faker
@@ -377,13 +392,13 @@ DEPENDENCIES
   lograge
   mock_redis
   okcomputer
-  overcommit (>= 0.37.0)
   pg (~> 1.1)
   pry-byebug
   pry-rails
   puma (~> 4.1)
   rack-canonical-host (~> 0.2.3)
   rails (= 7.0.2.2)
+  rqrcode-rails3
   rspec-rails
   rubocop (>= 0.70.0)
   rubocop-performance
@@ -395,6 +410,8 @@ DEPENDENCIES
   sentry-ruby
   shakapacker
   simplecov
+  stimulus-rails (~> 1.1)
+  turbo-rails
   webdrivers
 
 RUBY VERSION

README.md

@@ -1,7 +1,7 @@
-# Playground for MFA on devise
+# Playground for two-factor authentication (2FA) on devise
 
-This is a Rails app created with [Ackama rails template](). [PR #1](https://github.com/ackama/rails-template-demo-devise-2fa/pull/1) in this repo is us experimenting with what an MFA feature on devise should look like. This repo exists solely to host that PR.
+This is a Rails app created with [Ackama rails template](). [PR #1](https://github.com/ackama/rails-template-demo-devise-2fa/pull/1) in this repo is us experimenting with what an two-factor authentication (2FA) feature on devise should look like. This repo exists solely to host that PR.
 
 Eventually the code changes in PR #1 will become part of [rails-template PR #77](https://github.com/ackama/rails-template/pull/77).
 
-This repo is temporary and will go away once we are happy with the shape of the MFA feature. **Do not depend on this repo continuing to exist**.
+This repo is temporary and will go away once we are happy with the shape of the two-factor authentication (2FA) feature. **Do not depend on this repo continuing to exist**.

app/controllers/application_controller.rb

@@ -1,2 +1,17 @@
 class ApplicationController < ActionController::Base
+  before_action :require_multi_factor_authentication!
+
+  protected
+
+  def require_multi_factor_authentication!
+    return unless user_signed_in?
+    return if devise_controller?
+    return if current_user.otp_required_for_login?
+
+    redirect_to new_users_multi_factor_authentication_path, alert: "MFA required"
+  end
+
+  def after_sign_in_path_for(resource)
+    stored_location_for(resource) || dashboards_path
+  end
 end

app/controllers/dashboards_controller.rb

@@ -0,0 +1,7 @@
+# Renders the home page.
+class DashboardsController < ApplicationController
+  before_action :authenticate_user!
+
+  def show
+  end
+end

app/controllers/publics_controller.rb

@@ -0,0 +1,5 @@
+# Renders the home page.
+class PublicsController < ApplicationController
+  def home
+  end
+end

app/controllers/users/devise_controller.rb

@@ -0,0 +1,37 @@
+# TODO: is this still the best way to do this with latest devise?
+module Users
+  class DeviseController < ApplicationController
+    ##
+    # A custom responder for Devise, required for the 'responders' gem
+    # which Devise uses ('respond_with'). Without this, form
+    # submissions do not respond with the correct status, meaning that
+    # Turbo does not update the page.
+    class Responder < ActionController::Responder
+      def to_turbo_stream
+        controller.render(options.merge(formats: :html))
+      rescue ActionView::MissingTemplate => e
+        raise e if get?
+
+        if has_errors? && default_action
+          render rendering_options.merge(formats: :html, status: :unprocessable_entity)
+        else
+          redirect_to navigation_location
+        end
+      end
+    end
+
+    self.responder = Responder
+    respond_to :html, :turbo_stream
+
+    # TODO: consider the utility of a custom authenticaiton layout
+    # layout "authentication"
+
+    before_action :configure_permitted_parameters, if: :devise_controller?
+
+    protected
+
+    def configure_permitted_parameters
+      devise_parameter_sanitizer.permit(:sign_in, keys: [:otp_attempt])
+    end
+  end
+end

app/controllers/users/multi_factor_authentications_controller.rb

@@ -0,0 +1,91 @@
+##
+# This controller allows users to manage their MFA credential(s). This
+# controller is not involved in signing in or out.
+#
+# Conceptually each user has one "two-factor auth" resource which they can
+# manage because devise-two-factor only supports one 2fa code per user
+#
+module Users
+  class MultiFactorAuthenticationsController < ApplicationController
+    before_action :authenticate_user!
+    # Add the environment name to the two-factor authentication (2FA) string so that
+    # you can more easily tell the 2FA codes for different environments
+    # apart in your 2FA app.
+    #
+    # Keep this name short. Some TOTP management applications (e.g. Google
+    # Authenticator) do not let users edit this name and do not show many
+    # characters on screen.
+    #
+    ISSUER = if Rails.env.production?
+               I18n.t("application.name").freeze
+             else
+               "#{I18n.t("application.name")} #{Rails.env}".freeze
+             end
+
+    skip_before_action :require_multi_factor_authentication!, only: %i[new show create]
+    # before_action { authorize :multi_factor_authentication } # no pundit in this demo app
+
+    # Allow template to access issuer and OTP secret
+    helper_method :issuer
+
+    ##
+    # #show is the entry point for a user managing their two-factor authentication (2FA). It displays a
+    # summary of the current state of their two-factor authentication (2FA) setup and buttons/links to take
+    # actions on it.
+    #
+    def show
+    end
+
+    def create_backup_codes
+      @backup_codes = current_user.generate_otp_backup_codes!
+      current_user.save!
+    end
+
+    def destroy_backup_codes
+      current_user.update!(otp_backup_codes: nil)
+      redirect_to users_multi_factor_authentication_path,
+                  notice: t(".success")
+    end
+
+    ##
+    # #new starts the process of setting up two-factor authentication (2FA) for the user
+    #
+    def new
+      @otp_secret = generate_otp_secret
+    end
+
+    ##
+    # #create accepts the form submission from #new and checks that the TOTP
+    # code supplied by the user is valid. If the user gives us a valid TOTP code
+    # then we can we be confident they have correctly setup OTP so we can
+    # require it at next sign in.
+    def create
+      if current_user.validate_and_consume_otp!(otp_param, otp_secret:)
+        current_user.update!(otp_secret:, otp_required_for_login: true)
+        redirect_to users_multi_factor_authentication_path, notice: t(".success")
+      else
+        @otp_secret = otp_secret
+        flash.now[:alert] = t(".invalid_code")
+        render :new
+      end
+    end
+
+    private
+
+    def otp_param
+      params.require(:otp_attempt).gsub(/\A[^\d+]\z/, "")
+    end
+
+    def otp_secret
+      params.require(:otp_secret)
+    end
+
+    def generate_otp_secret
+      User.generate_otp_secret
+    end
+
+    def issuer
+      ISSUER
+    end
+  end
+end