diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-05-12 09:41:27 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-05-12 09:41:27 +0000 |
commit | 4a373be8617814f74fa1bfa99740daecc4fe8278 (patch) | |
tree | a84b923215c43efa5a82eed984e00c4e7d318493 | |
parent | 8e4dcbb8fb4823a464dfdd8b62075df124ca5bc6 (diff) | |
parent | 22badc13136369e202dc6df06a62456110879ee4 (diff) | |
download | gitlab-ce-4a373be8617814f74fa1bfa99740daecc4fe8278.tar.gz |
Merge branch '2fa' into 'master'
Two-factor authentication
Implement's Two-factor authentication using tokens.
- [X] Authentication logic
- [X] Enable/disable 2FA feature
- [x] Make 2-step login process if 2FA enabled
- [x] Backup codes
- [x] Backup code removed after being used
- [x] Check backup codes for mysql db (mention mysql limitation if applied)
- [x] Add tests
- [x] Test if https://github.com/tinfoil/devise-two-factor#disabling-automatic-login-after-password-resets applies, and address if so
- [x] Wait for fixed version of `attr_encrypted` or fork and use forked version - https://github.com/attr-encrypted/attr_encrypted/issues/155
Fixes http://feedback.gitlab.com/forums/176466-general/suggestions/4516817-implement-two-factor-authentication-2fa
See merge request !474
26 files changed, 566 insertions, 10 deletions
diff --git a/CHANGELOG b/CHANGELOG index 54c42d30dce..6c544fc9398 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -27,7 +27,7 @@ v 7.11.0 (unreleased) - When use change branches link at MR form - save source branch selection instead of target one - Improve handling of large diffs - Added GitLab Event header for project hooks - - + - Add Two-factor authentication (2FA) for GitLab logins - Show Atom feed buttons everywhere where applicable. - Add project activity atom feed. - Don't crash when an MR from a fork has a cross-reference comment from the target project on one of its commits. @@ -34,6 +34,11 @@ gem 'omniauth-bitbucket' gem 'doorkeeper', '2.1.3' gem "rack-oauth2", "~> 1.0.5" +# Two-factor authentication +gem 'devise-two-factor' +gem 'rqrcode-rails3' +gem 'attr_encrypted', '1.3.4' + # Browser detection gem "browser" diff --git a/Gemfile.lock b/Gemfile.lock index f5d3011376a..14b9a6848ab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -46,6 +46,8 @@ GEM ast (2.0.0) astrolabe (1.3.0) parser (>= 2.2.0.pre.3, < 3.0) + attr_encrypted (1.3.4) + encryptor (>= 1.3.0) attr_required (1.0.0) autoprefixer-rails (5.1.11) execjs @@ -136,6 +138,13 @@ GEM warden (~> 1.2.3) devise-async (0.9.0) devise (~> 3.2) + devise-two-factor (1.0.1) + activemodel + activesupport + attr_encrypted (~> 1.3.2) + devise (~> 3.2.4) + rails + rotp (~> 1.6.1) diff-lcs (1.2.5) diffy (3.0.3) docile (1.1.5) @@ -147,6 +156,7 @@ GEM email_spec (1.5.0) launchy (~> 2.1) mail (~> 2.2) + encryptor (1.3.0) enumerize (0.7.0) activesupport (>= 3.2) equalizer (0.0.8) @@ -482,7 +492,11 @@ GEM rest-client (1.6.7) mime-types (>= 1.16) rinku (1.7.3) + rotp (1.6.1) rouge (1.7.7) + rqrcode (0.4.2) + rqrcode-rails3 (0.1.7) + rqrcode (>= 0.4.2) rspec (2.99.0) rspec-core (~> 2.99.0) rspec-expectations (~> 2.99.0) @@ -670,6 +684,7 @@ DEPENDENCIES annotate (~> 2.6.0.beta2) asana (~> 0.0.6) asciidoctor (= 0.1.4) + attr_encrypted (= 1.3.4) awesome_print better_errors binding_of_caller @@ -691,6 +706,7 @@ DEPENDENCIES default_value_for (~> 3.0.0) devise (= 3.2.4) devise-async (= 0.9.0) + devise-two-factor diffy (~> 3.0.3) doorkeeper (= 2.1.3) dropzonejs-rails @@ -762,6 +778,7 @@ DEPENDENCIES redis-rails request_store rerun (~> 0.10.0) + rqrcode-rails3 rspec-rails (= 2.99) rubocop (= 0.28.0) rugments diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eee10d6c22a..8ce881c7414 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -252,7 +252,7 @@ class ApplicationController < ActionController::Base end def configure_permitted_parameters - devise_parameter_sanitizer.sanitize(:sign_in) { |u| u.permit(:username, :email, :password, :login, :remember_me) } + devise_parameter_sanitizer.for(:sign_in) { |u| u.permit(:username, :email, :password, :login, :remember_me, :otp_attempt) } end def hexdigest(string) diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index dcbbe5baa4b..88459d4080a 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -15,4 +15,25 @@ class PasswordsController < Devise::PasswordsController respond_with(resource) end end + + # After a user resets their password, prompt for 2FA code if enabled instead + # of signing in automatically + # + # See http://git.io/vURrI + def update + super do |resource| + # TODO (rspeicher): In Devise master (> 3.4.1), we can set + # `Devise.sign_in_after_reset_password = false` and avoid this mess. + if resource.errors.empty? && resource.try(:otp_required_for_login?) + resource.unlock_access! if unlockable?(resource) + + # Since we are not signing this user in, we use the :updated_not_active + # message which only contains "Your password was changed successfully." + set_flash_message(:notice, :updated_not_active) if is_flashing_format? + + # Redirect to sign in so they can enter 2FA code + respond_with(resource, location: new_session_path(resource)) and return + end + end + end end diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb new file mode 100644 index 00000000000..30ee6891733 --- /dev/null +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -0,0 +1,49 @@ +class Profiles::TwoFactorAuthsController < Profiles::ApplicationController + def new + unless current_user.otp_secret + current_user.otp_secret = User.generate_otp_secret + current_user.save! + end + + @qr_code = build_qr_code + end + + def create + if current_user.valid_otp?(params[:pin_code]) + current_user.otp_required_for_login = true + @codes = current_user.generate_otp_backup_codes! + current_user.save! + + render 'create' + else + @error = 'Invalid pin code' + @qr_code = build_qr_code + render 'new' + end + end + + def codes + @codes = current_user.generate_otp_backup_codes! + current_user.save! + end + + def destroy + current_user.update_attributes({ + otp_required_for_login: false, + encrypted_otp_secret: nil, + encrypted_otp_secret_iv: nil, + encrypted_otp_secret_salt: nil, + otp_backup_codes: nil + }) + + redirect_to profile_account_path + end + + private + + def build_qr_code + issuer = "GitLab | #{current_user.email}" + uri = current_user.otp_provisioning_uri(current_user.email, issuer: issuer) + RQRCode::render_qrcode(uri, :svg, level: :m, unit: 3) + end +end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 3f11d7afe6f..d4ff0d97561 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,12 @@ class SessionsController < Devise::SessionsController + prepend_before_action :authenticate_with_two_factor, only: [:create] + + # This action comes from DeviseController, but because we call `sign_in` + # manually inside `authenticate_with_two_factor`, not skipping this action + # would cause a "You are already signed in." error message to be shown upon + # successful login. + skip_before_action :require_no_authentication, only: [:create] + def new redirect_path = if request.referer.present? && (params['redirect_to_referer'] == 'yes') @@ -14,7 +22,7 @@ class SessionsController < Devise::SessionsController # Prevent a 'you are already signed in' message directly after signing: # we should never redirect to '/users/sign_in' after signing in successfully. - unless redirect_path == '/users/sign_in' + unless redirect_path == new_user_session_path store_location_for(:redirect, redirect_path) end @@ -27,11 +35,54 @@ class SessionsController < Devise::SessionsController def create super do |resource| - # User has successfully signed in, so clear any unused reset tokens + # User has successfully signed in, so clear any unused reset token if resource.reset_password_token.present? resource.update_attributes(reset_password_token: nil, reset_password_sent_at: nil) end end end + + private + + def user_params + params.require(:user).permit(:login, :password, :remember_me, :otp_attempt) + end + + def find_user + if user_params[:login] + User.by_login(user_params[:login]) + elsif user_params[:otp_attempt] && session[:otp_user_id] + User.find(session[:otp_user_id]) + end + end + + def authenticate_with_two_factor + user = self.resource = find_user + + return unless user && user.otp_required_for_login + + if user_params[:otp_attempt].present? && session[:otp_user_id] + if valid_otp_attempt?(user) + # Remove any lingering user data from login + session.delete(:otp_user_id) + + sign_in(user) and return + else + flash.now[:alert] = 'Invalid two-factor code.' + render :two_factor and return + end + else + if user && user.valid_password?(user_params[:password]) + # Save the user's ID to session so we can ask for a one-time password + session[:otp_user_id] = user.id + render :two_factor and return + end + end + end + + def valid_otp_attempt?(user) + user.valid_otp?(user_params[:otp_attempt]) || + user.invalidate_otp_backup_code!(user_params[:otp_attempt]) + end end diff --git a/app/models/user.rb b/app/models/user.rb index a70cbaa518b..d088d2d8630 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,6 +50,11 @@ # bitbucket_access_token :string(255) # bitbucket_access_token_secret :string(255) # location :string(255) +# encrypted_otp_secret :string(255) +# encrypted_otp_secret_iv :string(255) +# encrypted_otp_secret_salt :string(255) +# otp_required_for_login :boolean +# otp_backup_codes :text # public_email :string(255) default(""), not null # @@ -70,8 +75,14 @@ class User < ActiveRecord::Base default_value_for :hide_no_password, false default_value_for :theme_id, gitlab_config.default_theme - devise :database_authenticatable, :lockable, :async, - :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :confirmable, :registerable + devise :two_factor_authenticatable, + otp_secret_encryption_key: File.read(Rails.root.join('.secret')).chomp + + devise :two_factor_backupable, otp_number_of_backup_codes: 10 + serialize :otp_backup_codes, JSON + + devise :lockable, :async, :recoverable, :rememberable, :trackable, + :validatable, :omniauthable, :confirmable, :registerable attr_accessor :force_random_password diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml new file mode 100644 index 00000000000..22b2c1a186b --- /dev/null +++ b/app/views/devise/sessions/two_factor.html.haml @@ -0,0 +1,10 @@ +%div + .login-box + .login-heading + %h3 Two-factor Authentication + .login-body + = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f| + = f.text_field :otp_attempt, class: 'form-control', placeholder: 'Two-factor authentication code', required: true, autofocus: true + %p.help-block.hint If you've lost your phone, you may enter one of your recovery codes. + .prepend-top-20 + = f.submit "Verify code", class: "btn btn-save" diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 31d8ed3ed86..ac37fd4c1c1 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -4,7 +4,7 @@ = icon('user fw') %span Profile - = nav_link(controller: :accounts) do + = nav_link(controller: [:accounts, :two_factor_auths]) do = link_to profile_account_path, title: 'Account', data: {placement: 'right'} do = icon('gear fw') %span diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 1c3a3d68aca..6ac60b01f85 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -26,6 +26,33 @@ %span You don`t have one yet. Click generate to fix it. = f.submit 'Generate', class: "btn success btn-build-token" + - unless current_user.ldap_user? + %fieldset + - if current_user.otp_required_for_login + %legend.text-success + = icon('check') + Two-factor Authentication enabled + %div + .pull-right + = link_to 'Disable Two-factor Authentication', profile_two_factor_auth_path, method: :delete, class: 'btn btn-close btn-sm', + data: { confirm: 'Are you sure?' } + %p + If you lose your recovery codes you can + %strong + = succeed ',' do + = link_to 'generate new ones', codes_profile_two_factor_auth_path, method: :post, data: { confirm: 'Are you sure?' } + invalidating all previous codes. + + - else + %legend Two-factor Authentication + %div + %p + Increase your account's security by enabling two-factor authentication (2FA). + %p + Each time you log in you’ll be required to provide your username and + password as usual, plus a randomly-generated code from your phone. + %div + = link_to 'Enable Two-factor Authentication', new_profile_two_factor_auth_path, class: 'btn btn-success' - if show_profile_social_tab? %fieldset @@ -38,7 +65,7 @@ class: "btn btn-lg #{'active' if oauth_active?(provider)}" - if oauth_active?(provider) = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'btn btn-lg' do - %i.fa.fa-close + = icon('close') - if show_profile_username_tab? %fieldset.update-username @@ -52,7 +79,7 @@ .loading-gif.hide %p - %i.fa.fa-spinner.fa-spin + = icon('spinner spin') Saving new username %p.light = user_url(@user) diff --git a/app/views/profiles/two_factor_auths/_codes.html.haml b/app/views/profiles/two_factor_auths/_codes.html.haml new file mode 100644 index 00000000000..1b1395eaa17 --- /dev/null +++ b/app/views/profiles/two_factor_auths/_codes.html.haml @@ -0,0 +1,11 @@ +%p.slead + Should you ever lose your phone, each of these recovery codes can be used one + time each to regain access to your account. Please save them in a safe place. + +.codes.well + %ul + - @codes.each do |code| + %li + %span.monospace= code + += link_to 'Proceed', profile_account_path, class: 'btn btn-success' diff --git a/app/views/profiles/two_factor_auths/codes.html.haml b/app/views/profiles/two_factor_auths/codes.html.haml new file mode 100644 index 00000000000..addf356697a --- /dev/null +++ b/app/views/profiles/two_factor_auths/codes.html.haml @@ -0,0 +1,5 @@ +- page_title 'Recovery Codes', 'Two-factor Authentication' + +%h3.page-title Two-factor Authentication Recovery codes +%hr += render 'codes' diff --git a/app/views/profiles/two_factor_auths/create.html.haml b/app/views/profiles/two_factor_auths/create.html.haml new file mode 100644 index 00000000000..e330aadac13 --- /dev/null +++ b/app/views/profiles/two_factor_auths/create.html.haml @@ -0,0 +1,6 @@ +- page_title 'Two-factor Authentication', 'Account' + +.alert.alert-success + Congratulations! You have enabled Two-factor Authentication! + += render 'codes' diff --git a/app/views/profiles/two_factor_auths/new.html.haml b/app/views/profiles/two_factor_auths/new.html.haml new file mode 100644 index 00000000000..fe03a259a12 --- /dev/null +++ b/app/views/profiles/two_factor_auths/new.html.haml @@ -0,0 +1,23 @@ +- page_title 'Two-factor Authentication', 'Account' + +%h2.page-title Two-Factor Authentication (2FA) +%p + Download the Google Authenticator application from App Store for iOS or + Google Play for Android and scan this code. + +%hr + += form_tag profile_two_factor_auth_path, method: :post, class: 'form-horizontal' do |f| + - if @error + .alert.alert-danger + = @error + .form-group + .col-sm-2 + .col-sm-10 + = raw @qr_code + .form-group + = label_tag :pin_code, nil, class: "control-label" + .col-sm-10 + = text_field_tag :pin_code, nil, class: "form-control", required: true, autofocus: true + .form-actions + = submit_tag 'Submit', class: 'btn btn-success' diff --git a/config/application.rb b/config/application.rb index fa399533e52..7e899cc3b5b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,7 +31,7 @@ module Gitlab config.encoding = "utf-8" # Configure sensitive parameters which will be filtered from the log file. - config.filter_parameters.push(:password, :password_confirmation, :private_token) + config.filter_parameters.push(:password, :password_confirmation, :private_token, :otp_attempt) # Enable escaping HTML in JSON. config.active_support.escape_html_entities_in_json = true diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 8f8c4169740..091548348b1 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -1,6 +1,11 @@ # Use this hook to configure devise mailer, warden hooks and so forth. The first # four configuration values can also be set straight in your models. Devise.setup do |config| + config.warden do |manager| + manager.default_strategies(scope: :user).unshift :two_factor_authenticatable + manager.default_strategies(scope: :user).unshift :two_factor_backupable + end + # ==> Mailer Configuration # Configure the class responsible to send e-mails. config.mailer = "DeviseMailer" diff --git a/config/routes.rb b/config/routes.rb index 4b38dede7b4..bf2cb6421c5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -226,6 +226,11 @@ Gitlab::Application.routes.draw do resources :keys resources :emails, only: [:index, :create, :destroy] resource :avatar, only: [:destroy] + resource :two_factor_auth, only: [:new, :create, :destroy] do + member do + post :codes + end + end end end diff --git a/db/migrate/20150327223628_add_devise_two_factor_to_users.rb b/db/migrate/20150327223628_add_devise_two_factor_to_users.rb new file mode 100644 index 00000000000..11b026ee8f3 --- /dev/null +++ b/db/migrate/20150327223628_add_devise_two_factor_to_users.rb @@ -0,0 +1,8 @@ +class AddDeviseTwoFactorToUsers < ActiveRecord::Migration + def change + add_column :users, :encrypted_otp_secret, :string + add_column :users, :encrypted_otp_secret_iv, :string + add_column :users, :encrypted_otp_secret_salt, :string + add_column :users, :otp_required_for_login, :boolean + end +end diff --git a/db/migrate/20150331183602_add_devise_two_factor_backupable_to_users.rb b/db/migrate/20150331183602_add_devise_two_factor_backupable_to_users.rb new file mode 100644 index 00000000000..913958db7c5 --- /dev/null +++ b/db/migrate/20150331183602_add_devise_two_factor_backupable_to_users.rb @@ -0,0 +1,5 @@ +class AddDeviseTwoFactorBackupableToUsers < ActiveRecord::Migration + def change + add_column :users, :otp_backup_codes, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 04abf9bb9a6..3e5810d7408 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -492,6 +492,11 @@ ActiveRecord::Schema.define(version: 20150502064022) do t.string "bitbucket_access_token" t.string "bitbucket_access_token_secret" t.string "location" + t.string "encrypted_otp_secret" + t.string "encrypted_otp_secret_iv" + t.string "encrypted_otp_secret_salt" + t.boolean "otp_required_for_login" + t.text "otp_backup_codes" t.string "public_email", default: "", null: false end diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb new file mode 100644 index 00000000000..f05d1f5fbe1 --- /dev/null +++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb @@ -0,0 +1,126 @@ +require 'spec_helper' + +describe Profiles::TwoFactorAuthsController do + before do + # `user` should be defined within the action-specific describe blocks + sign_in(user) + + allow(subject).to receive(:current_user).and_return(user) + end + + describe 'GET new' do + let(:user) { create(:user) } + + it 'generates otp_secret' do + expect { get :new }.to change { user.otp_secret } + end + + it 'assigns qr_code' do + code = double('qr code') + expect(subject).to receive(:build_qr_code).and_return(code) + + get :new + expect(assigns[:qr_code]).to eq code + end + end + + describe 'POST create' do + let(:user) { create(:user) } + let(:pin) { 'pin-code' } + + def go + post :create, pin_code: pin + end + + context 'with valid pin' do + before do + expect(user).to receive(:valid_otp?).with(pin).and_return(true) + end + + it 'sets otp_required_for_login' do + go + + user.reload + expect(user.otp_required_for_login).to eq true + end + + it 'presents plaintext codes for the user to save' do + expect(user).to receive(:generate_otp_backup_codes!).and_return(%w(a b c)) + + go + + expect(assigns[:codes]).to match_array %w(a b c) + end + + it 'renders create' do + go + expect(response).to render_template(:create) + end + end + + context 'with invalid pin' do + before do + expect(user).to receive(:valid_otp?).with(pin).and_return(false) + end + + it 'assigns error' do + go + expect(assigns[:error]).to eq 'Invalid pin code' + end + + it 'assigns qr_code' do + code = double('qr code') + expect(subject).to receive(:build_qr_code).and_return(code) + + go + expect(assigns[:qr_code]).to eq code + end + + it 'renders new' do + go + expect(response).to render_template(:new) + end + end + end + + describe 'POST codes' do + let(:user) { create(:user, :two_factor) } + + it 'presents plaintext codes for the user to save' do + expect(user).to receive(:generate_otp_backup_codes!).and_return(%w(a b c)) + + post :codes + expect(assigns[:codes]).to match_array %w(a b c) + end + + it 'persists the generated codes' do + post :codes + + user.reload + expect(user.otp_backup_codes).not_to be_empty + end + end + + describe 'DELETE destroy' do + let(:user) { create(:user, :two_factor) } + let!(:codes) { user.generate_otp_backup_codes! } + + it 'clears all 2FA-related fields' do + expect(user.otp_required_for_login).to eq true + expect(user.otp_backup_codes).not_to be_nil + expect(user.encrypted_otp_secret).not_to be_nil + + delete :destroy + + expect(user.otp_required_for_login).to eq false + expect(user.otp_backup_codes).to be_nil + expect(user.encrypted_otp_secret).to be_nil + end + + it 'redirects to profile_account_path' do + delete :destroy + + expect(response).to redirect_to(profile_account_path) + end + end +end diff --git a/spec/factories.rb b/spec/factories.rb index 19f2935f30e..26e8a795fa4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -28,6 +28,13 @@ FactoryGirl.define do admin true end + trait :two_factor do + before(:create) do |user| + user.otp_required_for_login = true + user.otp_secret = User.generate_otp_secret + end + end + factory :omniauth_user do ignore do extern_uid '123456' diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb new file mode 100644 index 00000000000..046a9f6191d --- /dev/null +++ b/spec/features/login_spec.rb @@ -0,0 +1,101 @@ +require 'spec_helper' + +feature 'Login' do + describe 'with two-factor authentication' do + context 'with valid username/password' do + let(:user) { create(:user, :two_factor) } + + before do + login_with(user) + expect(page).to have_content('Two-factor Authentication') + end + + def enter_code(code) + fill_in 'Two-factor authentication code', with: code + click_button 'Verify code' + end + + it 'does not show a "You are already signed in." error message' do + enter_code(user.current_otp) + expect(page).not_to have_content('You are already signed in.') + end + + context 'using one-time code' do + it 'allows login with valid code' do + enter_code(user.current_otp) + expect(current_path).to eq root_path + end + + it 'blocks login with invalid code' do + enter_code('foo') + expect(page).to have_content('Invalid two-factor code') + end + + it 'allows login with invalid code, then valid code' do + enter_code('foo') + expect(page).to have_content('Invalid two-factor code') + + enter_code(user.current_otp) + expect(current_path).to eq root_path + end + end + + context 'using backup code' do + let(:codes) { user.generate_otp_backup_codes! } + + before do + expect(codes.size).to eq 10 + + # Ensure the generated codes get saved + user.save + end + + context 'with valid code' do + it 'allows login' do + enter_code(codes.sample) + expect(current_path).to eq root_path + end + + it 'invalidates the used code' do + expect { enter_code(codes.sample) }. + to change { user.reload.otp_backup_codes.size }.by(-1) + end + end + + context 'with invalid code' do + it 'blocks login' do + code = codes.sample + expect(user.invalidate_otp_backup_code!(code)).to eq true + + user.save! + expect(user.reload.otp_backup_codes.size).to eq 9 + + enter_code(code) + expect(page).to have_content('Invalid two-factor code.') + end + end + end + end + end + + describe 'without two-factor authentication' do + let(:user) { create(:user) } + + it 'allows basic login' do + login_with(user) + expect(current_path).to eq root_path + end + + it 'does not show a "You are already signed in." error message' do + login_with(user) + expect(page).not_to have_content('You are already signed in.') + end + + it 'blocks invalid login' do + user = create(:user, password: 'not-the-default') + + login_with(user) + expect(page).to have_content('Invalid email or password.') + end + end +end diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb new file mode 100644 index 00000000000..a34efce09ef --- /dev/null +++ b/spec/features/password_reset_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +feature 'Password reset' do + def forgot_password + click_on 'Forgot your password?' + fill_in 'Email', with: user.email + click_button 'Reset password' + user.reload + end + + def get_reset_token + mail = ActionMailer::Base.deliveries.last + body = mail.body.encoded + body.scan(/reset_password_token=(.+)\"/).flatten.first + end + + def reset_password(password = 'password') + visit edit_user_password_path(reset_password_token: get_reset_token) + + fill_in 'New password', with: password + fill_in 'Confirm new password', with: password + click_button 'Change your password' + end + + describe 'with two-factor authentication' do + let(:user) { create(:user, :two_factor) } + + it 'requires login after password reset' do + visit root_path + + forgot_password + reset_password + + expect(page).to have_content("Your password was changed successfully.") + expect(page).not_to have_content("You are now signed in.") + expect(current_path).to eq new_user_session_path + end + end + + describe 'without two-factor authentication' do + let(:user) { create(:user) } + + it 'automatically logs in after password reset' do + visit root_path + + forgot_password + reset_password + + expect(current_path).to eq root_path + expect(page).to have_content("Your password was changed successfully. You are now signed in.") + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 771709c127a..0dddcd5bda2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -50,6 +50,11 @@ # bitbucket_access_token :string(255) # bitbucket_access_token_secret :string(255) # location :string(255) +# encrypted_otp_secret :string(255) +# encrypted_otp_secret_iv :string(255) +# encrypted_otp_secret_salt :string(255) +# otp_required_for_login :boolean +# otp_backup_codes :text # public_email :string(255) default(""), not null # |