home/gitlabhq
2
0
Fork 0
mirror of https://github.com/wahyd4/gitlabhq.git synced 2026-08-21 10:36:04 +10:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch '2fa' into 'master'

Browse Source 
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
This commit is contained in:
Dmitriy Zaporozhets
2015-05-12 09:41:27 +00:00
parent 8e4dcbb8fb 22badc1313
commit 4a373be861
26 changed files with 566 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG
+1 -1
View File
@@ -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.
Gemfile
+5
View File
@@ -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"
Gemfile.lock
+17
View File
@@ -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
app/controllers/application_controller.rb
+1 -1
View File
@@ -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)
app/controllers/passwords_controller.rb
+21
View File
@@ -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
app/controllers/profiles/two_factor_auths_controller.rb
+49
View File
@@ -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
app/controllers/sessions_controller.rb
+53 -2
View File
@@ -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
app/models/user.rb
+13 -2
View File
@@ -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
app/views/devise/sessions/two_factor.html.haml
+10
View File
@@ -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"
app/views/layouts/nav/_profile.html.haml
+1 -1
View File
@@ -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
app/views/profiles/accounts/show.html.haml
+29 -2
View File
@@ -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 @@
&nbsp;
.loading-gif.hide
%p
%i.fa.fa-spinner.fa-spin
= icon('spinner spin')
Saving new username
%p.light
= user_url(@user)
app/views/profiles/two_factor_auths/_codes.html.haml
+11
View File
@@ -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'
app/views/profiles/two_factor_auths/codes.html.haml
+5
View File
@@ -0,0 +1,5 @@
- page_title 'Recovery Codes', 'Two-factor Authentication'
%h3.page-title Two-factor Authentication Recovery codes
%hr
= render 'codes'
app/views/profiles/two_factor_auths/create.html.haml
+6
View File
@@ -0,0 +1,6 @@
- page_title 'Two-factor Authentication', 'Account'
.alert.alert-success
Congratulations! You have enabled Two-factor Authentication!
= render 'codes'
app/views/profiles/two_factor_auths/new.html.haml
+23
View File
@@ -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'
config/application.rb
+1 -1
View File
@@ -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
config/initializers/devise.rb
+5
View File
@@ -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"
config/routes.rb
+5
View File
@@ -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
db/migrate/20150327223628_add_devise_two_factor_to_users.rb
+8
View File
@@ -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
db/migrate/20150331183602_add_devise_two_factor_backupable_to_users.rb
+5
View File
@@ -0,0 +1,5 @@
class AddDeviseTwoFactorBackupableToUsers < ActiveRecord::Migration
def change
add_column :users, :otp_backup_codes, :text
end
end
db/schema.rb
+5
View File
@@ -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
spec/controllers/profiles/two_factor_auths_controller_spec.rb
+126
View File
@@ -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
spec/factories.rb
+7
View File
@@ -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'
spec/features/login_spec.rb
+101
View File
@@ -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
spec/features/password_reset_spec.rb
+53
View File
@@ -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
spec/models/user_spec.rb
+5
View File
@@ -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
#