From cbc48023a4ec24e21da7ce7a761479168a3d4694 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 9 Nov 2016 11:56:49 +0100 Subject: [PATCH 01/28] Update to new ruby syntax and styleguide --- lib/two_factor_authentication.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/two_factor_authentication.rb b/lib/two_factor_authentication.rb index 59ffa039..1ecac79e 100644 --- a/lib/two_factor_authentication.rb +++ b/lib/two_factor_authentication.rb @@ -1,10 +1,10 @@ require 'two_factor_authentication/version' require 'devise' require 'active_support/concern' -require "active_model" -require "active_record" -require "active_support/core_ext/class/attribute_accessors" -require "cgi" +require 'active_model' +require 'active_record' +require 'active_support/core_ext/class/attribute_accessors' +require 'cgi' module Devise mattr_accessor :max_login_attempts @@ -33,8 +33,8 @@ module Devise end module TwoFactorAuthentication - NEED_AUTHENTICATION = 'need_two_factor_authentication' - REMEMBER_TFA_COOKIE_NAME = "remember_tfa" + NEED_AUTHENTICATION = 'need_two_factor_authentication'.freeze + REMEMBER_TFA_COOKIE_NAME = 'remember_tfa'.freeze autoload :Schema, 'two_factor_authentication/schema' module Controllers @@ -42,7 +42,10 @@ module Controllers end end -Devise.add_module :two_factor_authenticatable, :model => 'two_factor_authentication/models/two_factor_authenticatable', :controller => :two_factor_authentication, :route => :two_factor_authentication +Devise.add_module :two_factor_authenticatable, + model: 'two_factor_authentication/models/two_factor_authenticatable', + controller: :two_factor_authentication, + route: :two_factor_authentication require 'two_factor_authentication/orm/active_record' require 'two_factor_authentication/routes' From 8e5eff38502e6b11d28f9760c1fff2deb6703d21 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 9 Nov 2016 14:29:29 +0100 Subject: [PATCH 02/28] Update for style guides --- .../models/two_factor_authenticatable.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 7d4a3306..19b64ed1 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -16,7 +16,8 @@ def has_one_time_password(options = {}) ::Devise::Models.config( self, :max_login_attempts, :allowed_otp_drift_seconds, :otp_length, :remember_otp_session_for_seconds, :otp_secret_encryption_key, - :direct_otp_length, :direct_otp_valid_for, :totp_timestamp) + :direct_otp_length, :direct_otp_valid_for, :totp_timestamp + ) end module InstanceMethodsOnActivation @@ -36,7 +37,7 @@ def authenticate_totp(code, options = {}) totp_secret = options[:otp_secret_key] || otp_secret_key digits = options[:otp_length] || self.class.otp_length drift = options[:drift] || self.class.allowed_otp_drift_seconds - raise "authenticate_totp called with no otp_secret_key set" if totp_secret.nil? + fails 'authenticate_totp called with no otp_secret_key set' if totp_secret.nil? totp = ROTP::TOTP.new(totp_secret, digits: digits) new_timestamp = totp.verify_with_drift_and_prior(code, drift, totp_timestamp) return false unless new_timestamp @@ -47,7 +48,7 @@ def authenticate_totp(code, options = {}) def provisioning_uri(account = nil, options = {}) totp_secret = options[:otp_secret_key] || otp_secret_key options[:digits] ||= options[:otp_length] || self.class.otp_length - raise "provisioning_uri called with no otp_secret_key set" if totp_secret.nil? + fail 'provisioning_uri called with no otp_secret_key set' if totp_secret.nil? account ||= email if respond_to?(:email) ROTP::TOTP.new(totp_secret, options).provisioning_uri(account) end @@ -62,7 +63,7 @@ def send_new_otp(options = {}) end def send_two_factor_authentication_code(code) - raise NotImplementedError.new("No default implementation - please define in your class.") + fail NotImplementedError.new('No default implementation - please define in your class.') end def max_login_attempts? @@ -77,8 +78,8 @@ def totp_enabled? respond_to?(:otp_secret_key) && !otp_secret_key.nil? end - def confirm_totp_secret(secret, code, options = {}) - return false unless authenticate_totp(code, {otp_secret_key: secret}) + def confirm_totp_secret(secret, code, _options = {}) + return false unless authenticate_totp(code, otp_secret_key: secret) self.otp_secret_key = secret true end From 6ead3f0cf5d544c7c0b0f21dfe6b67766adfd335 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 9 Nov 2016 17:47:43 +0100 Subject: [PATCH 03/28] Add new and create method to add a totp code via QR --- .../two_factor_authentication_controller.rb | 33 ++++++++++++++++++- .../two_factor_authentication/new.html.erb | 21 ++++++++++++ config/locales/en.yml | 1 + lib/two_factor_authentication/routes.rb | 7 ++-- two_factor_authentication.gemspec | 1 + 5 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 app/views/devise/two_factor_authentication/new.html.erb diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index 6d2b4873..a1d0a977 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -1,12 +1,29 @@ +require 'rqrcode' + class Devise::TwoFactorAuthenticationController < DeviseController prepend_before_action :authenticate_scope! before_action :prepare_and_validate, :handle_two_factor_authentication + before_action :set_temp_secret, only: [:new, :create] def show end + def new + end + + def create + return render :new if params[:code].nil? || params[:totp_secret].nil? + + if resource.confirm_totp_secret(params[:totp_secret], params[:code]) + after_two_factor_success_for(resource) + else + set_flash_message :notice, :confirm_failed, now: true + render :new + end + end + def update - render :show and return if params[:code].nil? + return render :show if params[:code].nil? if resource.authenticate_otp(params[:code]) after_two_factor_success_for(resource) @@ -22,6 +39,20 @@ def resend_code private + def set_temp_secret + @totp_secret = resource.generate_totp_secret + provisioning_uri = resource.provisioning_uri(nil, otp_secret_key: @totp_secret) + @qr = RQRCode::QRCode.new(provisioning_uri).as_png( + resize_gte_to: false, + resize_exactly_to: false, + fill: 'white', + color: 'black', + size: 250, + border_modules: 4, + module_px_size: 6, + ).to_data_url + end + def after_two_factor_success_for(resource) set_remember_two_factor_cookie(resource) diff --git a/app/views/devise/two_factor_authentication/new.html.erb b/app/views/devise/two_factor_authentication/new.html.erb new file mode 100644 index 00000000..0c4935fe --- /dev/null +++ b/app/views/devise/two_factor_authentication/new.html.erb @@ -0,0 +1,21 @@ +
+

Enable two-factor authentication

+ +

<%= flash[:notice] %>

+ + <%= form_tag([resource_name, :two_factor_authentication]) do %> + <%= image_tag @qr %> + + <%= text_field_tag :code, nil, placeholder: 'Enter code', autocomplete: 'off' %> + + <%= hidden_field_tag :totp_secret, @totp_secret %> + + <%= submit_tag 'Confirm and activate' %> + <% end %> + +

+ <%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path %> +

+ <%= link_to 'Skip', skip_user_two_factor_authentication_path%> +

+
\ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index 37ff1dfc..14d941f6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3,6 +3,7 @@ en: two_factor_authentication: success: "Two factor authentication successful." attempt_failed: "Attempt failed." + confirm_failed: "Your code did not match, or expired after scanning. Remove the old barcode from your app, and try again. Since this process is time-sensitive, make sure your device's date and time is set to 'automatic'." 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 authentication code has been sent." diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb index 543059a2..2c57eb5e 100644 --- a/lib/two_factor_authentication/routes.rb +++ b/lib/two_factor_authentication/routes.rb @@ -3,8 +3,11 @@ class Mapper protected def devise_two_factor_authentication(mapping, controllers) - resource :two_factor_authentication, :only => [:show, :update, :resend_code], :path => mapping.path_names[:two_factor_authentication], :controller => controllers[:two_factor_authentication] do - collection { get "resend_code" } + resource :two_factor_authentication, only: [:show, :new, :create, :update, :resend_code], path: mapping.path_names[:two_factor_authentication], controller: controllers[:two_factor_authentication] do + collection do + get 'resend_code' + get 'skip' + end end end end diff --git a/two_factor_authentication.gemspec b/two_factor_authentication.gemspec index d606d6ed..8dae3f3b 100644 --- a/two_factor_authentication.gemspec +++ b/two_factor_authentication.gemspec @@ -29,6 +29,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'randexp' s.add_runtime_dependency 'rotp', '>= 3.2.0' s.add_runtime_dependency 'encryptor' + s.add_runtime_dependency 'rqrcode', '>= 0.10.1' s.add_development_dependency 'bundler' s.add_development_dependency 'rake' From 20a4348556fede723b1de1023fb080a475e0896e Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 9 Nov 2016 17:48:14 +0100 Subject: [PATCH 04/28] Add issuer to config to set issuer via provisioning_uri --- lib/two_factor_authentication.rb | 3 +++ .../models/two_factor_authenticatable.rb | 3 ++- lib/two_factor_authentication/routes.rb | 1 - 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/two_factor_authentication.rb b/lib/two_factor_authentication.rb index 1ecac79e..5f866c67 100644 --- a/lib/two_factor_authentication.rb +++ b/lib/two_factor_authentication.rb @@ -30,6 +30,9 @@ module Devise mattr_accessor :second_factor_resource_id @@second_factor_resource_id = 'id' + + mattr_accessor :issuer_name + @@issuer_name = '' end module TwoFactorAuthentication diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 19b64ed1..3e02cf91 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -16,7 +16,7 @@ def has_one_time_password(options = {}) ::Devise::Models.config( self, :max_login_attempts, :allowed_otp_drift_seconds, :otp_length, :remember_otp_session_for_seconds, :otp_secret_encryption_key, - :direct_otp_length, :direct_otp_valid_for, :totp_timestamp + :direct_otp_length, :direct_otp_valid_for, :totp_timestamp, :issuer_name ) end @@ -50,6 +50,7 @@ def provisioning_uri(account = nil, options = {}) options[:digits] ||= options[:otp_length] || self.class.otp_length fail 'provisioning_uri called with no otp_secret_key set' if totp_secret.nil? account ||= email if respond_to?(:email) + options[:issuer] ||= options[:issuer_name] || self.class.issuer_name ROTP::TOTP.new(totp_secret, options).provisioning_uri(account) end diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb index 2c57eb5e..1a8a5df2 100644 --- a/lib/two_factor_authentication/routes.rb +++ b/lib/two_factor_authentication/routes.rb @@ -6,7 +6,6 @@ def devise_two_factor_authentication(mapping, controllers) resource :two_factor_authentication, only: [:show, :new, :create, :update, :resend_code], path: mapping.path_names[:two_factor_authentication], controller: controllers[:two_factor_authentication] do collection do get 'resend_code' - get 'skip' end end end From de4e6a1f619c39802ec403622dbc0dd9c9f7af3c Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Thu, 10 Nov 2016 09:22:01 +0100 Subject: [PATCH 05/28] Add db column for otp enabled status --- lib/generators/active_record/templates/migration.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/generators/active_record/templates/migration.rb b/lib/generators/active_record/templates/migration.rb index 251ef402..0b154efe 100644 --- a/lib/generators/active_record/templates/migration.rb +++ b/lib/generators/active_record/templates/migration.rb @@ -1,5 +1,6 @@ class TwoFactorAuthenticationAddTo<%= table_name.camelize %> < ActiveRecord::Migration def change + add_column :<%= table_name %>, :otp_enabled, :boolean, default: false add_column :<%= table_name %>, :second_factor_attempts_count, :integer, default: 0 add_column :<%= table_name %>, :encrypted_otp_secret_key, :string add_column :<%= table_name %>, :encrypted_otp_secret_key_iv, :string From a4df525569ab7c966f2bffa2dbf1e3c39d31a0eb Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Thu, 10 Nov 2016 09:22:44 +0100 Subject: [PATCH 06/28] Change routing by adding verify path and using edit/update to disable otp --- .../two_factor_authentication_controller.rb | 24 +++++++-- .../two_factor_authentication/edit.html.erb | 16 ++++++ .../two_factor_authentication/new.html.erb | 50 +++++++++++++------ .../two_factor_authentication/show.html.erb | 3 +- config/locales/en.yml | 2 + .../hooks/two_factor_authenticatable.rb | 2 +- .../models/two_factor_authenticatable.rb | 10 ++-- lib/two_factor_authentication/routes.rb | 3 +- 8 files changed, 86 insertions(+), 24 deletions(-) create mode 100644 app/views/devise/two_factor_authentication/edit.html.erb diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index a1d0a977..307c0da7 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -9,11 +9,16 @@ def show end def new + if resource.totp_enabled? + return redirect_to({ action: :edit }, notice: I18n.t('devise.two_factor_authentication.totp_already_enabled')) + end + end + + def edit end def create return render :new if params[:code].nil? || params[:totp_secret].nil? - if resource.confirm_totp_secret(params[:totp_secret], params[:code]) after_two_factor_success_for(resource) else @@ -23,6 +28,16 @@ def create end def update + return render :edit if params[:code].nil? + if resource.remove_totp(params[:code]) + redirect_to after_two_factor_success_path_for(resource) + else + set_flash_message :notice, :remove_failed, now: true + render :edit + end + end + + def verify return render :show if params[:code].nil? if resource.authenticate_otp(params[:code]) @@ -34,7 +49,11 @@ def update def resend_code resource.send_new_otp - redirect_to send("#{resource_name}_two_factor_authentication_path"), notice: I18n.t('devise.two_factor_authentication.code_has_been_sent') + + respond_to do |format| + format.html { redirect_to send("#{resource_name}_two_factor_authentication_path"), notice: I18n.t('devise.two_factor_authentication.code_has_been_sent') } + format.json { head :no_content, status: :ok } + end end private @@ -55,7 +74,6 @@ def set_temp_secret def after_two_factor_success_for(resource) set_remember_two_factor_cookie(resource) - warden.session(resource_name)[TwoFactorAuthentication::NEED_AUTHENTICATION] = false bypass_sign_in(resource, scope: resource_name) set_flash_message :notice, :success diff --git a/app/views/devise/two_factor_authentication/edit.html.erb b/app/views/devise/two_factor_authentication/edit.html.erb new file mode 100644 index 00000000..657bab7c --- /dev/null +++ b/app/views/devise/two_factor_authentication/edit.html.erb @@ -0,0 +1,16 @@ +

Disable two-factor authentication

+ +

<%= flash[:notice] %>

+ +<%= form_tag([resource_name, :two_factor_authentication], method: 'PUT') do %> + <%= image_tag @qr %> + + <%= text_field_tag :code, nil, placeholder: 'Enter code', autocomplete: 'off' %> + + <%= hidden_field_tag :totp_secret, @totp_secret %> + + <%= submit_tag 'Confirm and deactivate' %> +<% end %> + +

+<%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path %> diff --git a/app/views/devise/two_factor_authentication/new.html.erb b/app/views/devise/two_factor_authentication/new.html.erb index 0c4935fe..0c5105b4 100644 --- a/app/views/devise/two_factor_authentication/new.html.erb +++ b/app/views/devise/two_factor_authentication/new.html.erb @@ -1,21 +1,41 @@ -
-

Enable two-factor authentication

+

Enable two-factor authentication

-

<%= flash[:notice] %>

+

<%= flash[:notice] %>

- <%= form_tag([resource_name, :two_factor_authentication]) do %> - <%= image_tag @qr %> +

Authentication with an app

- <%= text_field_tag :code, nil, placeholder: 'Enter code', autocomplete: 'off' %> +
Get the app
+

+ Download and install one of the following apps for your phone or table:
+ - Google Authenticator
+ - Duo Mobile
+ - Authy
+ - Windows Phone Authenticator +

- <%= hidden_field_tag :totp_secret, @totp_secret %> +

Scan this barcode

+<%= image_tag @qr %> +

+ Open the authentication app and:
+ - Tap the "+" icon in the top-right of the app
+ - Scan the image to the left, using your phone's camera
+
+ Can't scan this barcode?
+ Instead of scanning, use your authentication app's "Manual entry" or equivalent option and provide the following time-based key.
+
+ <%= @totp_secret %>
+
+ Your app will then generate a 6-digit verification code, which you use below. +

- <%= submit_tag 'Confirm and activate' %> - <% end %> +

Authentication via code

+ +<%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path, remote: true %>

+ +<%= form_tag([resource_name, :two_factor_authentication]) do %> + <%= text_field_tag :code, nil, placeholder: 'Enter code', autocomplete: 'off' %> + <%= hidden_field_tag :totp_secret, @totp_secret %> + + <%= submit_tag 'Confirm and activate' %> +<% end %> -

- <%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path %> -

- <%= link_to 'Skip', skip_user_two_factor_authentication_path%> -

-
\ No newline at end of file diff --git a/app/views/devise/two_factor_authentication/show.html.erb b/app/views/devise/two_factor_authentication/show.html.erb index 552fd7d8..5bca76f3 100644 --- a/app/views/devise/two_factor_authentication/show.html.erb +++ b/app/views/devise/two_factor_authentication/show.html.erb @@ -6,7 +6,7 @@

<%= flash[:notice] %>

-<%= form_tag([resource_name, :two_factor_authentication], :method => :put) do %> +<%= form_tag(verify_user_two_factor_authentication_path, method: :put) do %> <%= text_field_tag :code %> <%= submit_tag "Submit" %> <% end %> @@ -16,4 +16,5 @@ <% else %> <%= link_to "Send me a code instead", resend_code_user_two_factor_authentication_path, action: :get %> <% end %> +
<%= link_to "Sign out", destroy_user_session_path, :method => :delete %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 14d941f6..1324360f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4,6 +4,8 @@ en: success: "Two factor authentication successful." attempt_failed: "Attempt failed." confirm_failed: "Your code did not match, or expired after scanning. Remove the old barcode from your app, and try again. Since this process is time-sensitive, make sure your device's date and time is set to 'automatic'." + remove_failed: "Your code did not match, please try again." 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 authentication code has been sent." + totp_already_enabled: "Totp is already enabled. Redirected to edit." \ No newline at end of file diff --git a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb index 159ae144..8a333af1 100644 --- a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb @@ -7,7 +7,7 @@ if user.respond_to?(:need_two_factor_authentication?) && !bypass_by_cookie if auth.session(options[:scope])[TwoFactorAuthentication::NEED_AUTHENTICATION] = user.need_two_factor_authentication?(auth.request) - user.send_new_otp unless user.totp_enabled? + user.send_new_otp if user.otp_enabled && !user.totp_enabled? end end end diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 3e02cf91..d77052f2 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -37,7 +37,7 @@ def authenticate_totp(code, options = {}) totp_secret = options[:otp_secret_key] || otp_secret_key digits = options[:otp_length] || self.class.otp_length drift = options[:drift] || self.class.allowed_otp_drift_seconds - fails 'authenticate_totp called with no otp_secret_key set' if totp_secret.nil? + fail 'authenticate_totp called with no otp_secret_key set' if totp_secret.nil? totp = ROTP::TOTP.new(totp_secret, digits: digits) new_timestamp = totp.verify_with_drift_and_prior(code, drift, totp_timestamp) return false unless new_timestamp @@ -81,8 +81,12 @@ def totp_enabled? def confirm_totp_secret(secret, code, _options = {}) return false unless authenticate_totp(code, otp_secret_key: secret) - self.otp_secret_key = secret - true + update!(otp_secret_key: secret) + end + + def remove_totp(code, _options = {}) + return false unless authenticate_totp(code) + update!(otp_secret_key: nil) end def generate_totp_secret diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb index 1a8a5df2..8fdd1dc1 100644 --- a/lib/two_factor_authentication/routes.rb +++ b/lib/two_factor_authentication/routes.rb @@ -3,8 +3,9 @@ class Mapper protected def devise_two_factor_authentication(mapping, controllers) - resource :two_factor_authentication, only: [:show, :new, :create, :update, :resend_code], path: mapping.path_names[:two_factor_authentication], controller: controllers[:two_factor_authentication] do + resource :two_factor_authentication, only: [:show, :new, :edit, :create, :update, :resend_code], path: mapping.path_names[:two_factor_authentication], controller: controllers[:two_factor_authentication] do collection do + put 'verify' get 'resend_code' end end From c2ab97a069a37c43db6259435aeaa253e4d06f23 Mon Sep 17 00:00:00 2001 From: Kerim Haccou Date: Thu, 10 Nov 2016 12:14:00 +0100 Subject: [PATCH 07/28] Update creating and updating otp on user --- .../devise/two_factor_authentication_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index 307c0da7..7fc6f490 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -19,7 +19,7 @@ def edit def create return render :new if params[:code].nil? || params[:totp_secret].nil? - if resource.confirm_totp_secret(params[:totp_secret], params[:code]) + if resource.confirm_otp(params[:totp_secret], params[:code]) after_two_factor_success_for(resource) else set_flash_message :notice, :confirm_failed, now: true @@ -29,7 +29,8 @@ def create def update return render :edit if params[:code].nil? - if resource.remove_totp(params[:code]) + if resource.authenticate_otp(params[:code]) + resource.disable_otp redirect_to after_two_factor_success_path_for(resource) else set_flash_message :notice, :remove_failed, now: true From 6a7ec621ef6c8a1980931f0f29aed71279cab90c Mon Sep 17 00:00:00 2001 From: Kerim Haccou Date: Thu, 10 Nov 2016 12:15:11 +0100 Subject: [PATCH 08/28] Add routing to new_user_two_factor_authentication if otp is not enabled but is tfa is needed --- lib/two_factor_authentication/controllers/helpers.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/two_factor_authentication/controllers/helpers.rb b/lib/two_factor_authentication/controllers/helpers.rb index f8a084d4..79fe0e3c 100644 --- a/lib/two_factor_authentication/controllers/helpers.rb +++ b/lib/two_factor_authentication/controllers/helpers.rb @@ -30,7 +30,12 @@ def handle_failed_second_factor(scope) def two_factor_authentication_path_for(resource_or_scope = nil) scope = Devise::Mapping.find_scope!(resource_or_scope) - change_path = "#{scope}_two_factor_authentication_path" + user = warden.user(scope: scope, run_callbacks: false) + if user.otp_enabled? + change_path = "#{scope}_two_factor_authentication_path" + else + change_path = "new_#{scope}_two_factor_authentication_path" + end send(change_path) end From 5bb1c3f13e7d7f9f470d060fe678cac0d75b67f4 Mon Sep 17 00:00:00 2001 From: Kerim Haccou Date: Thu, 10 Nov 2016 12:24:14 +0100 Subject: [PATCH 09/28] Rename set_qr method and remove unused settings --- .../devise/two_factor_authentication_controller.rb | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index 7fc6f490..d2e55385 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -3,7 +3,7 @@ class Devise::TwoFactorAuthenticationController < DeviseController prepend_before_action :authenticate_scope! before_action :prepare_and_validate, :handle_two_factor_authentication - before_action :set_temp_secret, only: [:new, :create] + before_action :set_qr, only: [:new, :create] def show end @@ -59,18 +59,10 @@ def resend_code private - def set_temp_secret + def set_qr @totp_secret = resource.generate_totp_secret provisioning_uri = resource.provisioning_uri(nil, otp_secret_key: @totp_secret) - @qr = RQRCode::QRCode.new(provisioning_uri).as_png( - resize_gte_to: false, - resize_exactly_to: false, - fill: 'white', - color: 'black', - size: 250, - border_modules: 4, - module_px_size: 6, - ).to_data_url + @qr = RQRCode::QRCode.new(provisioning_uri).as_png(size: 250).to_data_url end def after_two_factor_success_for(resource) From 6cbcf549e64d05b7616ccf4e2a1dadf94e0f1598 Mon Sep 17 00:00:00 2001 From: Kerim Haccou Date: Thu, 10 Nov 2016 12:24:32 +0100 Subject: [PATCH 10/28] Add enable, confirm and disable methods --- .../models/two_factor_authenticatable.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index d77052f2..09861e9e 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -79,14 +79,24 @@ def totp_enabled? respond_to?(:otp_secret_key) && !otp_secret_key.nil? end + def confirm_otp(secret, code) + if direct_otp && authenticate_direct_otp(code) + return enable_otp + end + confirm_totp_secret(secret, code) + end + def confirm_totp_secret(secret, code, _options = {}) return false unless authenticate_totp(code, otp_secret_key: secret) - update!(otp_secret_key: secret) + update!(otp_secret_key: secret, otp_enabled: true) + end + + def enable_otp + update!(otp_enabled: true) end - def remove_totp(code, _options = {}) - return false unless authenticate_totp(code) - update!(otp_secret_key: nil) + def disable_otp + update!(otp_secret_key: nil, otp_enabled: false) end def generate_totp_secret From f3517e35821d928521340cd9c43ee89daf0ecb0f Mon Sep 17 00:00:00 2001 From: Kerim Haccou Date: Thu, 10 Nov 2016 12:24:48 +0100 Subject: [PATCH 11/28] Updat readme for new functionalities --- README.md | 71 ++++++++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index dabdea41..bd1df09c 100644 --- a/README.md +++ b/README.md @@ -10,10 +10,12 @@ * Support for 2 types of OTP codes 1. Codes delivered directly to the user 2. TOTP (Google Authenticator) codes based on a shared secret (HMAC) +* Option to enable or disable otp * Configurable OTP code digit length * Configurable max login attempts * Customizable logic to determine if a user needs two factor authentication * Configurable period where users won't be asked for 2FA again +* Configurable issuer name for display in TOTP apps * Option to encrypt the TOTP secret in the database, with iv and salt ## Configuration @@ -43,6 +45,7 @@ Where MODEL is your model name (e.g. User or Admin). This generator will add `:two_factor_authenticatable` to your model's Devise options and create a migration in `db/migrate/`, which will add the following columns to your table: +- `:otp_enabled` - `:second_factor_attempts_count` - `:encrypted_otp_secret_key` - `:encrypted_otp_secret_key_iv` @@ -64,14 +67,15 @@ devise :database_authenticatable, :registerable, :recoverable, :rememberable, Then create your migration file using the Rails generator, such as: ``` -rails g migration AddTwoFactorFieldsToUsers second_factor_attempts_count:integer encrypted_otp_secret_key:string:index encrypted_otp_secret_key_iv:string encrypted_otp_secret_key_salt:string direct_otp:string direct_otp_sent_at:datetime totp_timestamp:timestamp +rails g migration AddTwoFactorFieldsToUsers otp_enabled:boolean second_factor_attempts_count:integer encrypted_otp_secret_key:string:index encrypted_otp_secret_key_iv:string encrypted_otp_secret_key_salt:string direct_otp:string direct_otp_sent_at:datetime totp_timestamp:timestamp ``` Open your migration file (it will be in the `db/migrate` directory and will be named something like `20151230163930_add_two_factor_fields_to_users.rb`), and -add `unique: true` to the `add_index` line so that it looks like this: +add `unique: true` to the `add_index` line and add default: false to otp_enabled, so that it looks like this: ```ruby +add_column :users, :otp_enabled, :boolean, default: false add_index :users, :encrypted_otp_secret_key, unique: true ``` Save the file. @@ -97,14 +101,18 @@ config.direct_otp_length = 6 # Direct OTP code length config.remember_otp_session_for_seconds = 30.days # 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.issuer_name = 'App name' # The name shown in TOTP apps ``` The `otp_secret_encryption_key` must be a random key that is not stored in the DB, and is not checked in to your repo. It is recommended to store it in an environment variable, and you can generate it with `bundle exec rake secret`. -Override the method in your model in order to send direct OTP codes. This is -automatically called when a user logs in unless they have TOTP enabled (see -below): +#### Enabling two factor authentication +By default when users login and otp is not enabled, the user is asked to enable two factor authentication. + +The user has the option to choose between using the app (for example [Google Authenticator](https://support.google.com/accounts/answer/1066447?hl=en)), or receiving direct OTP codes. + +Override the method in your model in order to send direct OTP codes: ```ruby def send_two_factor_authentication_code(code) @@ -112,6 +120,9 @@ def send_two_factor_authentication_code(code) end ``` +Once the user has confirmed by entering the code from his app or direct code, +two factor authentication is enabled. + ### Customisation and Usage By default, second factor authentication is required for each user. You can @@ -126,36 +137,23 @@ end In the example above, two factor authentication will not be required for local users. -This gem is compatible with [Google Authenticator](https://support.google.com/accounts/answer/1066447?hl=en). -To enable this a shared secret must be generated by invoking the following -method on your model: - -```ruby -user.generate_totp_secret -``` +#### Overriding the views -This must then be shared via a provisioning uri: +The default views that show the forms can be overridden by adding the following files +in either ERB or haml: -```ruby -user.provisioning_uri # This assumes a user model with an email attribute -``` +- `new.html.erb` Enabling two factor authentication +- `edit.html.erb` Disabling two factor authentication +- `show.html.erb` Verifying OTP code after login -This provisioning uri can then be turned in to a QR code if desired so that -users may add the app to Google Authenticator easily. Once this is done, they -may retrieve a one-time password directly from the Google Authenticator app. - -#### Overriding the view - -The default view that shows the form can be overridden by adding a -file named `show.html.erb` (or `show.html.haml` if you prefer HAML) inside `app/views/devise/two_factor_authentication/` and customizing it. -Below is an example using ERB: +Below is an example for show using ERB: ```html

Hi, you received a code by email, please enter it below, thanks!

-<%= form_tag([resource_name, :two_factor_authentication], :method => :put) do %> +<%= form_tag(verify_user_two_factor_authentication_path, method: :put) do %> <%= text_field_tag :code %> <%= submit_tag "Log in!" %> <% end %> @@ -164,22 +162,15 @@ Below is an example using ERB: ``` -#### Enable TOTP support for existing users +#### Disable OTP by users -If you have existing users that need to be provided with a OTP secret key, so -they can use TOTP, create a rake task. It could look like this one below: +If you want to give the users to option to disable OTP, you must add a route to edit_#{scope}_two_factor_authentication_path. In this view the user has to confirm with a code to disable his two factor authentication. -```ruby -desc 'rake task to update users with otp secret key' -task :update_users_with_otp_secret_key => :environment do - User.find_each do |user| - user.generate_totp_secret - user.save! - puts "Rake[:update_users_with_otp_secret_key] => OTP secret key set to '#{key}' for User '#{user.email}'" - end -end -``` -Then run the task with `bundle exec rake update_users_with_otp_secret_key` +#### Filtering sensitive parameters from the logs + +To prevent two-factor authentication codes from leaking if your application logs get breached, you'll want to filter sensitive parameters from the Rails logs. Add the following to config/initializers/filter_parameter_logging.rb: + +Rails.application.config.filter_parameters += [:totp_secret] #### Adding the TOTP encryption option to an existing app From 4b71f78b84e8d72431217c28f9d8e2d72117f57a Mon Sep 17 00:00:00 2001 From: Kerim Haccou Date: Thu, 10 Nov 2016 12:50:56 +0100 Subject: [PATCH 12/28] Add view generator and documentate in readme --- README.md | 6 +++++- .../views_generator.rb | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 lib/generators/two_factor_authentication/views_generator.rb diff --git a/README.md b/README.md index bd1df09c..de29fa8a 100644 --- a/README.md +++ b/README.md @@ -147,8 +147,12 @@ in either ERB or haml: - `show.html.erb` Verifying OTP code after login inside `app/views/devise/two_factor_authentication/` and customizing it. -Below is an example for show using ERB: +Or you can use the generator: + +`bundle exec rails g two_factor_authentication:views` + +Below is an example for show using ERB: ```html

Hi, you received a code by email, please enter it below, thanks!

diff --git a/lib/generators/two_factor_authentication/views_generator.rb b/lib/generators/two_factor_authentication/views_generator.rb new file mode 100644 index 00000000..ebe0c18b --- /dev/null +++ b/lib/generators/two_factor_authentication/views_generator.rb @@ -0,0 +1,20 @@ +require 'generators/devise/views_generator' + +module TwoFactorAuthenticatable + module Generators + class ViewsGenerator < Rails::Generators::Base + namespace 'two_factor_authentication:views' + + desc 'Copies all Devise Two Factor Authenticatable views to your application.' + + argument :scope, :required => false, :default => nil, + :desc => "The scope to copy views to" + + include ::Devise::Generators::ViewPathTemplates + source_root File.expand_path("../../../../app/views/devise", __FILE__) + def copy_views + view_directory :two_factor_authentication + end + end + end +end From 7522c8bb11727c1dab11b46d50e166adb35d1159 Mon Sep 17 00:00:00 2001 From: Kerim Haccou Date: Thu, 10 Nov 2016 14:13:34 +0100 Subject: [PATCH 13/28] Update test for new database field --- .rspec | 2 ++ .../models/two_factor_authenticatable.rb | 14 +++++++++++--- .../two_factor_authentication_generator_spec.rb | 1 + spec/rails_app/app/models/guest_user.rb | 2 +- .../20161110120108_add_enable_otp_to_user.rb | 5 +++++ spec/rails_app/db/schema.rb | 13 +++++++------ spec/support/authenticated_model_helper.rb | 1 + 7 files changed, 28 insertions(+), 10 deletions(-) create mode 100644 .rspec create mode 100644 spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb diff --git a/.rspec b/.rspec new file mode 100644 index 00000000..b3eb8b49 --- /dev/null +++ b/.rspec @@ -0,0 +1,2 @@ +--color +--format documentation \ No newline at end of file diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 09861e9e..71dc19c6 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -88,15 +88,23 @@ def confirm_otp(secret, code) def confirm_totp_secret(secret, code, _options = {}) return false unless authenticate_totp(code, otp_secret_key: secret) - update!(otp_secret_key: secret, otp_enabled: true) + update_attributes( + otp_secret_key: secret, + otp_enabled: true + ) end def enable_otp - update!(otp_enabled: true) + update_attributes( + otp_enabled: true + ) end def disable_otp - update!(otp_secret_key: nil, otp_enabled: false) + update_attributes( + otp_secret_key: nil, + otp_enabled: false + ) end def generate_totp_secret diff --git a/spec/generators/active_record/two_factor_authentication_generator_spec.rb b/spec/generators/active_record/two_factor_authentication_generator_spec.rb index 5a8989d0..648e13d2 100644 --- a/spec/generators/active_record/two_factor_authentication_generator_spec.rb +++ b/spec/generators/active_record/two_factor_authentication_generator_spec.rb @@ -26,6 +26,7 @@ it { is_expected.to exist } it { is_expected.to be_a_migration } it { is_expected.to contain /def change/ } + it { is_expected.to contain /add_column :users, :otp_enabled, :boolean, default: false/ } it { is_expected.to contain /add_column :users, :second_factor_attempts_count, :integer, default: 0/ } it { is_expected.to contain /add_column :users, :encrypted_otp_secret_key, :string/ } it { is_expected.to contain /add_column :users, :encrypted_otp_secret_key_iv, :string/ } diff --git a/spec/rails_app/app/models/guest_user.rb b/spec/rails_app/app/models/guest_user.rb index 8003624c..e6365758 100644 --- a/spec/rails_app/app/models/guest_user.rb +++ b/spec/rails_app/app/models/guest_user.rb @@ -5,7 +5,7 @@ class GuestUser define_model_callbacks :create attr_accessor :direct_otp, :direct_otp_sent_at, :otp_secret_key, :email, - :second_factor_attempts_count, :totp_timestamp + :second_factor_attempts_count, :totp_timestamp, :otp_enabled def update_attributes(attrs) attrs.each do |key, value| diff --git a/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb b/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb new file mode 100644 index 00000000..17dcc594 --- /dev/null +++ b/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb @@ -0,0 +1,5 @@ +class AddEnableOtpToUser < ActiveRecord::Migration + def change + add_column :users, :otp_enabled, :boolean, default: false + end +end diff --git a/spec/rails_app/db/schema.rb b/spec/rails_app/db/schema.rb index 5f011618..23d5d32f 100644 --- a/spec/rails_app/db/schema.rb +++ b/spec/rails_app/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160209032439) do +ActiveRecord::Schema.define(version: 20161110120108) do create_table "admins", force: :cascade do |t| t.string "email", default: "", null: false @@ -32,23 +32,24 @@ add_index "admins", ["reset_password_token"], name: "index_admins_on_reset_password_token", unique: true create_table "users", force: :cascade do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false + t.string "email", default: "", null: false + t.string "encrypted_password", default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0, null: false + t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.integer "second_factor_attempts_count", default: 0 t.string "nickname", limit: 64 t.string "encrypted_otp_secret_key" t.string "encrypted_otp_secret_key_iv" t.string "encrypted_otp_secret_key_salt" + t.boolean "otp_enabled", default: false end add_index "users", ["email"], name: "index_users_on_email", unique: true diff --git a/spec/support/authenticated_model_helper.rb b/spec/support/authenticated_model_helper.rb index 42696e68..0d1c47e2 100644 --- a/spec/support/authenticated_model_helper.rb +++ b/spec/support/authenticated_model_helper.rb @@ -50,6 +50,7 @@ def create_table_for_nonencrypted_user t.string 'direct_otp' t.datetime 'direct_otp_sent_at' t.timestamp 'totp_timestamp' + t.boolean 'otp_enabled', default: false end end end From 157668eeb40f55c6cdd4eef439b374e6ecf3d876 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Thu, 10 Nov 2016 14:41:10 +0100 Subject: [PATCH 14/28] Remove issuer option --- README.md | 2 -- lib/two_factor_authentication.rb | 3 --- .../models/two_factor_authenticatable.rb | 3 +-- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/README.md b/README.md index de29fa8a..ae7276f0 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,6 @@ * Configurable max login attempts * Customizable logic to determine if a user needs two factor authentication * Configurable period where users won't be asked for 2FA again -* Configurable issuer name for display in TOTP apps * Option to encrypt the TOTP secret in the database, with iv and salt ## Configuration @@ -101,7 +100,6 @@ config.direct_otp_length = 6 # Direct OTP code length config.remember_otp_session_for_seconds = 30.days # 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.issuer_name = 'App name' # The name shown in TOTP apps ``` The `otp_secret_encryption_key` must be a random key that is not stored in the DB, and is not checked in to your repo. It is recommended to store it in an diff --git a/lib/two_factor_authentication.rb b/lib/two_factor_authentication.rb index 5f866c67..1ecac79e 100644 --- a/lib/two_factor_authentication.rb +++ b/lib/two_factor_authentication.rb @@ -30,9 +30,6 @@ module Devise mattr_accessor :second_factor_resource_id @@second_factor_resource_id = 'id' - - mattr_accessor :issuer_name - @@issuer_name = '' end module TwoFactorAuthentication diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 71dc19c6..052fc87e 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -16,7 +16,7 @@ def has_one_time_password(options = {}) ::Devise::Models.config( self, :max_login_attempts, :allowed_otp_drift_seconds, :otp_length, :remember_otp_session_for_seconds, :otp_secret_encryption_key, - :direct_otp_length, :direct_otp_valid_for, :totp_timestamp, :issuer_name + :direct_otp_length, :direct_otp_valid_for, :totp_timestamp ) end @@ -50,7 +50,6 @@ def provisioning_uri(account = nil, options = {}) options[:digits] ||= options[:otp_length] || self.class.otp_length fail 'provisioning_uri called with no otp_secret_key set' if totp_secret.nil? account ||= email if respond_to?(:email) - options[:issuer] ||= options[:issuer_name] || self.class.issuer_name ROTP::TOTP.new(totp_secret, options).provisioning_uri(account) end From 3bdcddd3f630fd8eaefe026b91ca1d1c92e04468 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Thu, 10 Nov 2016 14:50:34 +0100 Subject: [PATCH 15/28] Update controller spec for verify route instead of update --- spec/controllers/two_factor_authentication_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/two_factor_authentication_controller_spec.rb b/spec/controllers/two_factor_authentication_controller_spec.rb index 100876ad..443e193d 100644 --- a/spec/controllers/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/two_factor_authentication_controller_spec.rb @@ -9,7 +9,7 @@ context 'after user enters valid OTP code' do it 'returns true' do controller.current_user.send_new_otp - post :update, code: controller.current_user.direct_otp + post :verify, code: controller.current_user.direct_otp expect(subject.is_fully_authenticated?).to eq true end end @@ -24,7 +24,7 @@ context 'when user enters an invalid OTP' do it 'returns false' do - post :update, code: '12345' + post :verify, code: '12345' expect(subject.is_fully_authenticated?).to eq false end From f376e55059a0338d37f0cdec616ac1bee93a2e5b Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Thu, 10 Nov 2016 15:04:24 +0100 Subject: [PATCH 16/28] Fix test for added otp_enabled attribute --- .../models/two_factor_authenticatable.rb | 27 +++++++++++-------- .../two_factor_authenticatable_spec.rb | 6 ++--- spec/rails_app/app/models/encrypted_user.rb | 3 ++- spec/support/authenticated_model_helper.rb | 2 +- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 052fc87e..e9651e48 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -87,23 +87,28 @@ def confirm_otp(secret, code) def confirm_totp_secret(secret, code, _options = {}) return false unless authenticate_totp(code, otp_secret_key: secret) - update_attributes( - otp_secret_key: secret, - otp_enabled: true - ) + # update_attributes( + # otp_secret_key: secret, + # otp_enabled: true + # ) + self.otp_secret_key = secret + self.otp_enabled = true end def enable_otp - update_attributes( - otp_enabled: true - ) + # update_attributes( + # otp_enabled: true + # ) + self.otp_enabled = true end def disable_otp - update_attributes( - otp_secret_key: nil, - otp_enabled: false - ) + # update_attributes( + # otp_secret_key: nil, + # otp_enabled: false + # ) + self.otp_secret_key = nil + self.otp_enabled = false end def generate_totp_secret diff --git a/spec/features/two_factor_authenticatable_spec.rb b/spec/features/two_factor_authenticatable_spec.rb index ebde2d80..45d6afaa 100644 --- a/spec/features/two_factor_authenticatable_spec.rb +++ b/spec/features/two_factor_authenticatable_spec.rb @@ -56,7 +56,7 @@ end context "when logged in" do - let(:user) { create_user } + let(:user) { create_user('encrypted', otp_enabled: true) } background do login_as user @@ -144,7 +144,7 @@ logout reset_session! - user2 = create_user() + user2 = create_user('encrypted', otp_enabled: true) login_as(user2) sms_sign_in @@ -168,7 +168,7 @@ def sms_sign_in logout reset_session! - user2 = create_user() + user2 = create_user('encrypted', otp_enabled: true) set_tfa_cookie(tfa_cookie1) login_as(user2) visit dashboard_path diff --git a/spec/rails_app/app/models/encrypted_user.rb b/spec/rails_app/app/models/encrypted_user.rb index 292004a3..4e9033d6 100644 --- a/spec/rails_app/app/models/encrypted_user.rb +++ b/spec/rails_app/app/models/encrypted_user.rb @@ -9,7 +9,8 @@ class EncryptedUser :encrypted_otp_secret_key_salt, :email, :second_factor_attempts_count, - :totp_timestamp + :totp_timestamp, + :otp_enabled has_one_time_password(encrypted: true) end diff --git a/spec/support/authenticated_model_helper.rb b/spec/support/authenticated_model_helper.rb index 0d1c47e2..ba0c10f5 100644 --- a/spec/support/authenticated_model_helper.rb +++ b/spec/support/authenticated_model_helper.rb @@ -50,7 +50,7 @@ def create_table_for_nonencrypted_user t.string 'direct_otp' t.datetime 'direct_otp_sent_at' t.timestamp 'totp_timestamp' - t.boolean 'otp_enabled', default: false + t.boolean 'otp_enabled', default: true end end end From a228c0364073fc309eb6ce845628d3432cb312d1 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Thu, 10 Nov 2016 18:19:05 +0100 Subject: [PATCH 17/28] Add feature and controller tests --- .../two_factor_authentication_controller.rb | 3 + .../two_factor_authentication/new.html.erb | 5 +- config/locales/en.yml | 3 +- ...o_factor_authentication_controller_spec.rb | 126 +++++++++++++++++- .../two_factor_authenticatable_spec.rb | 40 ++++++ 5 files changed, 173 insertions(+), 4 deletions(-) diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index d2e55385..2e396d40 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -6,6 +6,9 @@ class Devise::TwoFactorAuthenticationController < DeviseController before_action :set_qr, only: [:new, :create] def show + unless resource.totp_enabled? + return redirect_to({ action: :new }, notice: I18n.t('devise.two_factor_authentication.totp_not_enabled')) + end end def new diff --git a/app/views/devise/two_factor_authentication/new.html.erb b/app/views/devise/two_factor_authentication/new.html.erb index 0c5105b4..fedf0ed8 100644 --- a/app/views/devise/two_factor_authentication/new.html.erb +++ b/app/views/devise/two_factor_authentication/new.html.erb @@ -23,14 +23,15 @@ Can't scan this barcode?
Instead of scanning, use your authentication app's "Manual entry" or equivalent option and provide the following time-based key.

- <%= @totp_secret %>
+ <%= @totp_secret %>

Your app will then generate a 6-digit verification code, which you use below.

Authentication via code

-<%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path, remote: true %>

+<%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path, remote: true %> +

<%= form_tag([resource_name, :two_factor_authentication]) do %> <%= text_field_tag :code, nil, placeholder: 'Enter code', autocomplete: 'off' %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 1324360f..0f1ef204 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -8,4 +8,5 @@ en: 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 authentication code has been sent." - totp_already_enabled: "Totp is already enabled. Redirected to edit." \ No newline at end of file + totp_already_enabled: "Two factor authentication is already enabled." + totp_not_enabled: "Two factor authentication is not enabled. Activate first." \ No newline at end of file diff --git a/spec/controllers/two_factor_authentication_controller_spec.rb b/spec/controllers/two_factor_authentication_controller_spec.rb index 443e193d..d8bf4874 100644 --- a/spec/controllers/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/two_factor_authentication_controller_spec.rb @@ -1,9 +1,133 @@ require 'spec_helper' describe Devise::TwoFactorAuthenticationController, type: :controller do + describe 'Enabling otp' do + before do + sign_in create_user('not_encrypted', otp_enabled: false) + end + + describe 'with direct code' do + context 'when user has not entered any OTP yet' do + it 'returns false' do + get :show + + expect(subject.current_user.otp_enabled).to eq false + end + end + + context 'when users enters valid OTP code' do + it 'returns true' do + controller.current_user.send_new_otp + post :create, code: controller.current_user.direct_otp, totp_secret: 'secret' + + expect(subject.current_user.otp_enabled).to eq true + end + end + + context 'when user enters an invalid OTP' do + it 'return false' do + post :create, code: '12345', totp_secret: 'secret' + + expect(subject.current_user.otp_enabled).to eq false + end + end + end + + describe 'with totp app' do + context 'when user has not entered any OTP yet' do + it 'returns false' do + get :show + + expect(subject.current_user.otp_enabled).to eq false + end + end + + context 'when users enters valid TOTP code' do + it 'returns true' do + secret = controller.current_user.generate_totp_secret + totp = ROTP::TOTP.new(secret) + post :create, code: totp.now, totp_secret: secret + + expect(subject.current_user.otp_enabled).to eq true + end + end + + context 'when user enters an invalid OTP' do + it 'return false' do + post :create, code: '12345', totp_secret: 'secret' + + expect(subject.current_user.otp_enabled).to eq false + end + end + end + end + + describe 'Disabling otp' do + before do + sign_in create_user('not_encrypted', otp_enabled: true) + secret = controller.current_user.generate_totp_secret + controller.current_user.update(otp_secret_key: secret) + end + + describe 'with direct code' do + context 'when user has not entered any OTP yet' do + it 'returns true' do + get :edit + + expect(subject.current_user.otp_enabled).to eq true + end + end + + context 'when users enters valid OTP code' do + it 'returns false' do + controller.current_user.send_new_otp + post :update, code: controller.current_user.direct_otp + + expect(subject.current_user.otp_enabled).to eq false + end + end + + context 'when user enters an invalid OTP' do + it 'return true' do + post :update, code: '12345' + + expect(subject.current_user.otp_enabled).to eq true + end + end + end + + describe 'with totp app' do + context 'when user has not entered any OTP yet' do + it 'returns true' do + get :edit + + expect(subject.current_user.otp_enabled).to eq true + end + end + + context 'when users enters valid TOTP code' do + it 'returns true' do + secret = controller.current_user.otp_secret_key + totp = ROTP::TOTP.new(secret) + post :update, code: totp.now + + expect(subject.current_user.otp_enabled).to eq false + end + end + + context 'when user enters an invalid OTP' do + it 'return false' do + post :update, code: '12345' + + expect(subject.current_user.otp_enabled).to eq true + end + end + end + end + describe 'is_fully_authenticated? helper' do before do - sign_in + sign_in create_user('not_encrypted', otp_enabled: true) end context 'after user enters valid OTP code' do diff --git a/spec/features/two_factor_authenticatable_spec.rb b/spec/features/two_factor_authenticatable_spec.rb index 45d6afaa..f02f41ae 100644 --- a/spec/features/two_factor_authenticatable_spec.rb +++ b/spec/features/two_factor_authenticatable_spec.rb @@ -55,6 +55,46 @@ expect(page).to have_content("You are signed out") end + context "when logged in without otp enabled" do + let(:user) { create_user('encrypted', otp_enabled: false) } + + background do + login_as user + end + + scenario "is redirected to TFA activation when path requires authentication" do + visit dashboard_path + "?A=param%20a&B=param%20b" + + expect(page).to_not have_content("Your Personal Dashboard") + expect(page).to have_xpath('//img') + end + + scenario "can enable TFA with a TOTP code" do + visit new_user_two_factor_authentication_path + + secret = find(:css, 'i#totp_secret').text + totp = ROTP::TOTP.new(secret) + fill_in "code", with: totp.now + click_button "Confirm and activate" + + expect(page).to have_content("You are signed in as Marissa") + expect(page).to have_content("Welcome Home") + end + + scenario "can enable TFA with a direct code" do + visit new_user_two_factor_authentication_path + + click_link "Send me a code instead" + + visit new_user_two_factor_authentication_path + fill_in 'code', with: SMSProvider.last_message.body + click_button "Confirm and activate" + + expect(page).to have_content("You are signed in as Marissa") + expect(page).to have_content("Welcome Home") + end + end + context "when logged in" do let(:user) { create_user('encrypted', otp_enabled: true) } From 497f0c38d966208886d70421418be7abe6f9a190 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Fri, 11 Nov 2016 10:26:28 +0100 Subject: [PATCH 18/28] Refactor routes resources --- lib/two_factor_authentication/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb index 8fdd1dc1..b6b200ce 100644 --- a/lib/two_factor_authentication/routes.rb +++ b/lib/two_factor_authentication/routes.rb @@ -3,7 +3,7 @@ class Mapper protected def devise_two_factor_authentication(mapping, controllers) - resource :two_factor_authentication, only: [:show, :new, :edit, :create, :update, :resend_code], path: mapping.path_names[:two_factor_authentication], controller: controllers[:two_factor_authentication] do + resource :two_factor_authentication, except: [:index, :destroy], path: mapping.path_names[:two_factor_authentication], controller: controllers[:two_factor_authentication] do collection do put 'verify' get 'resend_code' From f49878238a50b9745df8b33aca66bf7905bb3921 Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 30 Nov 2016 13:19:30 +0100 Subject: [PATCH 19/28] Fix test for new otp_enabled field --- spec/features/two_factor_authenticatable_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/features/two_factor_authenticatable_spec.rb b/spec/features/two_factor_authenticatable_spec.rb index f02f41ae..9d86c41e 100644 --- a/spec/features/two_factor_authenticatable_spec.rb +++ b/spec/features/two_factor_authenticatable_spec.rb @@ -18,7 +18,6 @@ it 'sends code via SMS after sign in' do visit new_user_session_path complete_sign_in_form_for(user) - expect(page).to have_content 'Enter the code that was sent to you' expect(SMSProvider.messages.size).to eq(1) @@ -44,8 +43,8 @@ end end - it_behaves_like 'sends and authenticates code', create_user('not_encrypted') - it_behaves_like 'sends and authenticates code', create_user, 'encrypted' + it_behaves_like 'sends and authenticates code', create_user('not_encrypted', otp_enabled: true) + it_behaves_like 'sends and authenticates code', create_user('encrypted', otp_enabled: true), 'encrypted' end scenario "must be logged in" do From de509bc09f97b7ba0808cbbdfe4ca38701d08d2f Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 30 Nov 2016 13:19:58 +0100 Subject: [PATCH 20/28] Add notice for successful disable of tfa --- .../devise/two_factor_authentication_controller.rb | 11 +++++------ config/locales/en.yml | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index 2e396d40..9f908098 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -6,13 +6,13 @@ class Devise::TwoFactorAuthenticationController < DeviseController before_action :set_qr, only: [:new, :create] def show - unless resource.totp_enabled? + unless resource.otp_enabled return redirect_to({ action: :new }, notice: I18n.t('devise.two_factor_authentication.totp_not_enabled')) end end def new - if resource.totp_enabled? + if resource.otp_enabled return redirect_to({ action: :edit }, notice: I18n.t('devise.two_factor_authentication.totp_already_enabled')) end end @@ -22,7 +22,7 @@ def edit def create return render :new if params[:code].nil? || params[:totp_secret].nil? - if resource.confirm_otp(params[:totp_secret], params[:code]) + if resource.confirm_otp(params[:totp_secret], params[:code]) && resource.save after_two_factor_success_for(resource) else set_flash_message :notice, :confirm_failed, now: true @@ -32,9 +32,8 @@ def create def update return render :edit if params[:code].nil? - if resource.authenticate_otp(params[:code]) - resource.disable_otp - redirect_to after_two_factor_success_path_for(resource) + if resource.authenticate_otp(params[:code]) && resource.disable_otp + redirect_to after_two_factor_success_path_for(resource), notice: I18n.t('devise.two_factor_authentication.remove_success') else set_flash_message :notice, :remove_failed, now: true render :edit diff --git a/config/locales/en.yml b/config/locales/en.yml index 0f1ef204..efa25ecd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4,6 +4,7 @@ en: success: "Two factor authentication successful." attempt_failed: "Attempt failed." confirm_failed: "Your code did not match, or expired after scanning. Remove the old barcode from your app, and try again. Since this process is time-sensitive, make sure your device's date and time is set to 'automatic'." + remove_success: "Two factor authentication successful disabled" remove_failed: "Your code did not match, please try again." max_login_attempts_reached: "Access completely denied as you have reached your attempts limit" contact_administrator: "Please contact your system administrator." From db9cb88d14fd52adceccfce9fdbab728400270bc Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 30 Nov 2016 13:20:39 +0100 Subject: [PATCH 21/28] Save changes after enabling and disabling otp --- .../models/two_factor_authenticatable.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index e9651e48..ebd90beb 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -100,6 +100,7 @@ def enable_otp # otp_enabled: true # ) self.otp_enabled = true + save end def disable_otp @@ -109,6 +110,7 @@ def disable_otp # ) self.otp_secret_key = nil self.otp_enabled = false + save end def generate_totp_secret From f6e182dc3695c56b15b651ba112d5a0e574570be Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Wed, 30 Nov 2016 13:29:37 +0100 Subject: [PATCH 22/28] Remove commented code --- .../models/two_factor_authenticatable.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index ebd90beb..67c84d48 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -87,27 +87,16 @@ def confirm_otp(secret, code) def confirm_totp_secret(secret, code, _options = {}) return false unless authenticate_totp(code, otp_secret_key: secret) - # update_attributes( - # otp_secret_key: secret, - # otp_enabled: true - # ) self.otp_secret_key = secret self.otp_enabled = true end def enable_otp - # update_attributes( - # otp_enabled: true - # ) self.otp_enabled = true save end def disable_otp - # update_attributes( - # otp_secret_key: nil, - # otp_enabled: false - # ) self.otp_secret_key = nil self.otp_enabled = false save From 17bd00a2d2570d617334d27f880d7c9b5af5a78a Mon Sep 17 00:00:00 2001 From: Benjamin Wols Date: Thu, 1 Dec 2016 15:17:49 +0100 Subject: [PATCH 23/28] Update edit view --- app/views/devise/two_factor_authentication/edit.html.erb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/views/devise/two_factor_authentication/edit.html.erb b/app/views/devise/two_factor_authentication/edit.html.erb index 657bab7c..d0794e2e 100644 --- a/app/views/devise/two_factor_authentication/edit.html.erb +++ b/app/views/devise/two_factor_authentication/edit.html.erb @@ -3,14 +3,10 @@

<%= flash[:notice] %>

<%= form_tag([resource_name, :two_factor_authentication], method: 'PUT') do %> - <%= image_tag @qr %> - <%= text_field_tag :code, nil, placeholder: 'Enter code', autocomplete: 'off' %> - <%= hidden_field_tag :totp_secret, @totp_secret %> - <%= submit_tag 'Confirm and deactivate' %> <% end %>

-<%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path %> +<%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path, remote: true %> From 24fe7d95d0fd44b8b6c4a266127cf626902745b4 Mon Sep 17 00:00:00 2001 From: Greg Myers Date: Mon, 29 Mar 2021 15:15:12 +0100 Subject: [PATCH 24/28] Update readme --- README.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 0fe9a63f..c758ec14 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ devise :database_authenticatable, :registerable, :recoverable, :rememberable, Then create your migration file using the Rails generator, such as: -``` +```bash rails g migration AddTwoFactorFieldsToUsers otp_enabled:boolean second_factor_attempts_count:integer encrypted_otp_secret_key:string:index encrypted_otp_secret_key_iv:string encrypted_otp_secret_key_salt:string direct_otp:string direct_otp_sent_at:datetime totp_timestamp:timestamp ``` @@ -207,7 +207,7 @@ steps: 1. Generate a migration to add the necessary columns to your model's table: - ``` + ```bash rails g migration AddEncryptionFieldsToUsers encrypted_otp_secret_key:string:index encrypted_otp_secret_key_iv:string encrypted_otp_secret_key_salt:string ``` @@ -228,7 +228,7 @@ steps: For example: `has_one_time_password(encrypted: true)` 4. Generate a migration to populate the new encryption fields: - ``` + ```bash rails g migration PopulateEncryptedOtpFields ``` @@ -256,7 +256,7 @@ steps: ``` 5. Generate a migration to remove the `:otp_secret_key` column: - ``` + ```bash rails g migration RemoveOtpSecretKeyFromUsers otp_secret_key:string ``` @@ -269,7 +269,7 @@ use these steps: 2. Roll back the last 3 migrations (assuming you haven't added any new ones after them): - ``` + ```bash bundle exec rake db:rollback STEP=3 ``` @@ -299,7 +299,7 @@ Make sure you are passing the 2FA secret codes securely and checking for them up For example, a simple account_controller.rb may look something like this: - ``` + ```ruby require 'json' class AccountController < ApplicationController @@ -363,7 +363,7 @@ config.otp_secret_encryption_key = ENV['OTP_SECRET_ENCRYPTION_KEY'] to set up TOTP for Google Authenticator for user: - ``` + ```ruby current_user.otp_secret_key = current_user.generate_totp_secret current_user.save! ``` @@ -374,16 +374,16 @@ rails c access relies on setting env var: OTP_SECRET_ENCRYPTION_KEY ) to check if user has input the correct code (from the QR display page) before saving the user model: - ``` + ```ruby current_user.authenticate_totp('123456') ``` additional note: - - ``` + + ```ruby current_user.otp_secret_key ``` - + This returns the OTP secret key in plaintext for the user (if you have set the env var) in the console the string used for generating the QR given to the user for their Google Auth is something like: @@ -395,7 +395,7 @@ this returns true or false with an allowed_otp_drift_seconds 'grace period' to set TOTP to DISABLED for a user account: - ``` + ```ruby current_user.second_factor_attempts_count=nil current_user.encrypted_otp_secret_key=nil current_user.encrypted_otp_secret_key_iv=nil From 30bce037c281f7590f3a22c126319f4a9440e6f0 Mon Sep 17 00:00:00 2001 From: Greg Myers Date: Mon, 29 Mar 2021 15:23:20 +0100 Subject: [PATCH 25/28] Remigrate the DB --- Gemfile | 2 +- .../db/migrate/20161110120108_add_enable_otp_to_user.rb | 2 +- spec/rails_app/db/schema.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 810ce296..ec4c2b45 100644 --- a/Gemfile +++ b/Gemfile @@ -19,7 +19,7 @@ gem "rails", rails if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.2.0') gem "test-unit", "~> 3.0" end - +gem 'sprockets-rails', '~> 2.0' group :test, :development do gem 'sqlite3' end diff --git a/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb b/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb index 17dcc594..eaa8b461 100644 --- a/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb +++ b/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb @@ -1,4 +1,4 @@ -class AddEnableOtpToUser < ActiveRecord::Migration +class AddEnableOtpToUser < ActiveRecord::Migration[4.2] def change add_column :users, :otp_enabled, :boolean, default: false end diff --git a/spec/rails_app/db/schema.rb b/spec/rails_app/db/schema.rb index 82524c1b..48f5e385 100644 --- a/spec/rails_app/db/schema.rb +++ b/spec/rails_app/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2016_02_09_032439) do +ActiveRecord::Schema.define(version: 2016_11_10_120108) do create_table "admins", force: :cascade do |t| t.string "email", default: "", null: false From 00b273dfd9a7168ab81eb65771d1fffe8c567ae8 Mon Sep 17 00:00:00 2001 From: Greg Myers Date: Mon, 29 Mar 2021 15:25:04 +0100 Subject: [PATCH 26/28] Support rails migration versions --- lib/generators/active_record/templates/migration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/generators/active_record/templates/migration.rb b/lib/generators/active_record/templates/migration.rb index 53d36df2..a4655b4d 100644 --- a/lib/generators/active_record/templates/migration.rb +++ b/lib/generators/active_record/templates/migration.rb @@ -1,4 +1,4 @@ -class TwoFactorAuthenticationAddTo<%= table_name.camelize %> < ActiveRecord::Migration +class TwoFactorAuthenticationAddTo<%= table_name.camelize %> < ActiveRecord::Migration[4.2] disable_ddl_transaction! def change From e935492d21d57452f70ccbbf4edcfc44b6c2a5b3 Mon Sep 17 00:00:00 2001 From: Greg Myers Date: Mon, 29 Mar 2021 15:38:29 +0100 Subject: [PATCH 27/28] Fixes #169 rotp returning url encoded at --- .../models/two_factor_authenticatable_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb b/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb index 6fb4f505..6e58fd1d 100644 --- a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb +++ b/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb @@ -138,7 +138,7 @@ def instance.send_two_factor_authentication_code(code) it "returns uri with user's email" do expect(instance.provisioning_uri). - to match(%r{otpauth://totp/houdini@example.com\?secret=\w{32}}) + to match(%r{otpauth://totp/houdini%40example.com\?secret=\w{32}}) end it 'returns uri with issuer option' do From e6844bcc78e041b10e37b335e42910e5992eb166 Mon Sep 17 00:00:00 2001 From: Greg Myers Date: Mon, 29 Mar 2021 15:46:33 +0100 Subject: [PATCH 28/28] Refactor to upgrade specs to rails 4 and 5 compatibility --- ...o_factor_authentication_controller_spec.rb | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/spec/controllers/two_factor_authentication_controller_spec.rb b/spec/controllers/two_factor_authentication_controller_spec.rb index 11ef48a9..57b259e2 100644 --- a/spec/controllers/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/two_factor_authentication_controller_spec.rb @@ -1,6 +1,14 @@ require 'spec_helper' describe Devise::TwoFactorAuthenticationController, type: :controller do + def post_params(action, params) + if Rails::VERSION::MAJOR >= 5 + post action, params: params + else + post action, params + end + end + describe 'Enabling otp' do before do sign_in create_user('not_encrypted', otp_enabled: false) @@ -18,7 +26,7 @@ context 'when users enters valid OTP code' do it 'returns true' do controller.current_user.send_new_otp - post :create, code: controller.current_user.direct_otp, totp_secret: 'secret' + post_params :create, code: controller.current_user.direct_otp, totp_secret: 'secret' expect(subject.current_user.otp_enabled).to eq true end @@ -26,7 +34,7 @@ context 'when user enters an invalid OTP' do it 'return false' do - post :create, code: '12345', totp_secret: 'secret' + post_params :create, code: '12345', totp_secret: 'secret' expect(subject.current_user.otp_enabled).to eq false end @@ -46,7 +54,7 @@ it 'returns true' do secret = controller.current_user.generate_totp_secret totp = ROTP::TOTP.new(secret) - post :create, code: totp.now, totp_secret: secret + post_params :create, code: totp.now, totp_secret: secret expect(subject.current_user.otp_enabled).to eq true end @@ -54,7 +62,7 @@ context 'when user enters an invalid OTP' do it 'return false' do - post :create, code: '12345', totp_secret: 'secret' + post_params :create, code: '12345', totp_secret: 'secret' expect(subject.current_user.otp_enabled).to eq false end @@ -81,7 +89,7 @@ context 'when users enters valid OTP code' do it 'returns false' do controller.current_user.send_new_otp - post :update, code: controller.current_user.direct_otp + post_params :update, code: controller.current_user.direct_otp expect(subject.current_user.otp_enabled).to eq false end @@ -89,7 +97,7 @@ context 'when user enters an invalid OTP' do it 'return true' do - post :update, code: '12345' + post_params :update, code: '12345' expect(subject.current_user.otp_enabled).to eq true end @@ -109,7 +117,7 @@ it 'returns true' do secret = controller.current_user.otp_secret_key totp = ROTP::TOTP.new(secret) - post :update, code: totp.now + post_params :update, code: totp.now expect(subject.current_user.otp_enabled).to eq false end @@ -117,7 +125,7 @@ context 'when user enters an invalid OTP' do it 'return false' do - post :update, code: '12345' + post_params :update, code: '12345' expect(subject.current_user.otp_enabled).to eq true end @@ -126,14 +134,6 @@ end describe 'is_fully_authenticated? helper' do - def post_code(code, action=:update) - if Rails::VERSION::MAJOR >= 5 - post :update, params: { code: code } - else - post :update, code: code - end - end - before do sign_in create_user('not_encrypted', otp_enabled: true) end @@ -141,7 +141,7 @@ def post_code(code, action=:update) context 'after user enters valid OTP code' do it 'returns true' do controller.current_user.send_new_otp - post_code controller.current_user.direct_otp, :verify + post_params :verify, code: controller.current_user.direct_otp expect(subject.is_fully_authenticated?).to eq true end end @@ -156,7 +156,7 @@ def post_code(code, action=:update) context 'when user enters an invalid OTP' do it 'returns false' do - post_code '12345', :verify + post_params :verify, code: '12345' expect(subject.is_fully_authenticated?).to eq false end