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/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/README.md b/README.md index f4f91d1a..c758ec14 100644 --- a/README.md +++ b/README.md @@ -8,8 +8,9 @@ ## Features * 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) + 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 @@ -43,6 +44,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` @@ -63,15 +65,16 @@ 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 +```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 ``` 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. @@ -103,9 +106,12 @@ 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) @@ -113,6 +119,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 @@ -127,36 +136,27 @@ 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: +#### Overriding the views -```ruby -user.generate_totp_secret -``` - -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. +inside `app/views/devise/two_factor_authentication/` and customizing it. -#### Overriding the view +Or you can use the generator: -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: +`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!

-<%= 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 %> @@ -189,6 +189,16 @@ User.find_each do |user| do end ``` +#### Disable OTP by users + +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. + +#### 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 If you've already been using this gem, and want to start encrypting the OTP @@ -197,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 ``` @@ -218,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 ``` @@ -246,7 +256,7 @@ steps: ``` 5. Generate a migration to remove the `:otp_secret_key` column: - ``` + ```bash rails g migration RemoveOtpSecretKeyFromUsers otp_secret_key:string ``` @@ -259,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 ``` @@ -289,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 @@ -353,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! ``` @@ -364,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: @@ -385,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 @@ -399,6 +409,3 @@ to set TOTP to DISABLED for a user account: current_user.direct_otp? => false current_user.totp_enabled? => false ``` - - - diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index 0e7aed84..ca5e490f 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -1,14 +1,48 @@ +require 'rqrcode' require 'devise/version' class Devise::TwoFactorAuthenticationController < DeviseController prepend_before_action :authenticate_scope! before_action :prepare_and_validate, :handle_two_factor_authentication + before_action :set_qr, only: [:new, :create] def show + 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.otp_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_otp(params[:totp_secret], params[:code]) && resource.save + 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 :edit if params[:code].nil? + 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 + end + end + + def verify + return render :show if params[:code].nil? if resource.authenticate_otp(params[:code]) after_two_factor_success_for(resource) @@ -19,14 +53,23 @@ 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 + 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(size: 250).to_data_url + end + def after_two_factor_success_for(resource) set_remember_two_factor_cookie(resource) - warden.session(resource_name)[TwoFactorAuthentication::NEED_AUTHENTICATION] = false # For compatability with devise versions below v4.2.0 # https://github.com/plataformatec/devise/commit/2044fffa25d781fcbaf090e7728b48b65c854ccb 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..d0794e2e --- /dev/null +++ b/app/views/devise/two_factor_authentication/edit.html.erb @@ -0,0 +1,12 @@ +

Disable two-factor authentication

+ +

<%= flash[:notice] %>

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

+<%= link_to 'Send me a code instead', resend_code_user_two_factor_authentication_path, remote: true %> 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..fedf0ed8 --- /dev/null +++ b/app/views/devise/two_factor_authentication/new.html.erb @@ -0,0 +1,42 @@ +

Enable two-factor authentication

+ +

<%= flash[:notice] %>

+ +

Authentication with an app

+ +
Get the app
+

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

+ +

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. +

+ +

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 %> + diff --git a/app/views/devise/two_factor_authentication/show.html.erb b/app/views/devise/two_factor_authentication/show.html.erb index 9a3ae060..041c079f 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, '', autofocus: true %> <%= submit_tag "Submit" %> <% end %> @@ -16,4 +16,6 @@ <% else %> <%= link_to "Send me a code instead", send("resend_code_#{resource_name}_two_factor_authentication_path"), action: :get %> <% end %> + +
<%= link_to "Sign out", send("destroy_#{resource_name}_session_path"), :method => :delete %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 37ff1dfc..efa25ecd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3,6 +3,11 @@ 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'." + 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." code_has_been_sent: "Your authentication code has been sent." + 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/lib/generators/active_record/templates/migration.rb b/lib/generators/active_record/templates/migration.rb index f14a733d..a4655b4d 100644 --- a/lib/generators/active_record/templates/migration.rb +++ b/lib/generators/active_record/templates/migration.rb @@ -1,7 +1,8 @@ -class TwoFactorAuthenticationAddTo<%= table_name.camelize %> < ActiveRecord::Migration +class TwoFactorAuthenticationAddTo<%= table_name.camelize %> < ActiveRecord::Migration[4.2] disable_ddl_transaction! 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 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 diff --git a/lib/two_factor_authentication.rb b/lib/two_factor_authentication.rb index 7b1bbbc1..495625cd 100644 --- a/lib/two_factor_authentication.rb +++ b/lib/two_factor_authentication.rb @@ -1,9 +1,9 @@ require 'two_factor_authentication/version' require 'devise' require 'active_support/concern' -require "active_model" -require "active_support/core_ext/class/attribute_accessors" -require "cgi" +require 'active_model' +require 'active_support/core_ext/class/attribute_accessors' +require 'cgi' module Devise mattr_accessor :max_login_attempts @@ -35,8 +35,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 @@ -44,7 +44,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' if defined?(ActiveRecord::Base) require 'two_factor_authentication/routes' diff --git a/lib/two_factor_authentication/controllers/helpers.rb b/lib/two_factor_authentication/controllers/helpers.rb index 64e8377c..0554f886 100644 --- a/lib/two_factor_authentication/controllers/helpers.rb +++ b/lib/two_factor_authentication/controllers/helpers.rb @@ -35,7 +35,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 diff --git a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb index 3ff03415..61baefea 100644 --- a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb @@ -7,6 +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 if user.send_new_otp_after_login? 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 6d73a0fb..cb4505a8 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -37,10 +37,10 @@ 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? + 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( - without_spaces(code), + without_spaces(code), drift_ahead: drift, drift_behind: drift, after: totp_timestamp ) return false unless new_timestamp @@ -51,7 +51,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 @@ -66,11 +66,11 @@ def send_new_otp(options = {}) end def send_new_otp_after_login? - !totp_enabled? + otp_enabled && !totp_enabled? 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? @@ -85,10 +85,28 @@ 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_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) self.otp_secret_key = secret - true + self.otp_enabled = true + end + + def enable_otp + self.otp_enabled = true + save + end + + def disable_otp + self.otp_secret_key = nil + self.otp_enabled = false + save end def generate_totp_secret diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb index 543059a2..b6b200ce 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, except: [:index, :destroy], path: mapping.path_names[:two_factor_authentication], controller: controllers[:two_factor_authentication] do + collection do + put 'verify' + get 'resend_code' + end end end end diff --git a/spec/controllers/two_factor_authentication_controller_spec.rb b/spec/controllers/two_factor_authentication_controller_spec.rb index d578d0bb..57b259e2 100644 --- a/spec/controllers/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/two_factor_authentication_controller_spec.rb @@ -1,23 +1,147 @@ require 'spec_helper' describe Devise::TwoFactorAuthenticationController, type: :controller do - describe 'is_fully_authenticated? helper' do - def post_code(code) - if Rails::VERSION::MAJOR >= 5 - post :update, params: { code: code } - else - post :update, code: code + 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) + 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_params :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_params :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_params :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_params :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_params :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_params :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_params :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_params :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 it 'returns true' do controller.current_user.send_new_otp - post_code controller.current_user.direct_otp + post_params :verify, code: controller.current_user.direct_otp expect(subject.is_fully_authenticated?).to eq true end end @@ -32,7 +156,7 @@ def post_code(code) context 'when user enters an invalid OTP' do it 'returns false' do - post_code '12345' + post_params :verify, code: '12345' expect(subject.is_fully_authenticated?).to eq false end diff --git a/spec/features/two_factor_authenticatable_spec.rb b/spec/features/two_factor_authenticatable_spec.rb index d433d636..dbb51be0 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 @@ -55,8 +54,48 @@ 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 } + let(:user) { create_user('encrypted', otp_enabled: true) } background do login_as user @@ -144,7 +183,7 @@ logout reset_session! - user2 = create_user() + user2 = create_user('encrypted', otp_enabled: true) login_as(user2) sms_sign_in @@ -168,7 +207,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/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/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 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/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..eaa8b461 --- /dev/null +++ b/spec/rails_app/db/migrate/20161110120108_add_enable_otp_to_user.rb @@ -0,0 +1,5 @@ +class AddEnableOtpToUser < ActiveRecord::Migration[4.2] + 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 9c0d0f18..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 @@ -47,6 +47,7 @@ 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 t.index ["email"], name: "index_users_on_email", unique: true t.index ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/spec/support/authenticated_model_helper.rb b/spec/support/authenticated_model_helper.rb index 8138dc58..245e6170 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: true end end end diff --git a/two_factor_authentication.gemspec b/two_factor_authentication.gemspec index 9580f117..2bce16ad 100644 --- a/two_factor_authentication.gemspec +++ b/two_factor_authentication.gemspec @@ -30,6 +30,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'randexp' s.add_runtime_dependency 'rotp', '>= 4.0.0' s.add_runtime_dependency 'encryptor' + s.add_runtime_dependency 'rqrcode', '>= 0.10.1' s.add_development_dependency 'bundler' s.add_development_dependency 'rake'