Commit e13b523c authored by Douwe Maan's avatar Douwe Maan

Merge branch 'rs-dev-issue-2415' into 'master'

Show user 2FA status on Admin::Users#show

| Enabled | Disabled |
| ------- | -------- |
| ![Screen_Shot_2015-06-19_at_2.58.42_PM](https://gitlab.com/gitlab-org/gitlab-ce/uploads/b81f6a992366105c14e03852385b28f0/Screen_Shot_2015-06-19_at_2.58.42_PM.png) | ![Screen_Shot_2015-06-19_at_2.58.30_PM](https://gitlab.com/gitlab-org/gitlab-ce/uploads/7bb8fd11b6c27615eef19154ffabe2de/Screen_Shot_2015-06-19_at_2.58.30_PM.png) |

Closes internal https://dev.gitlab.org/gitlab/gitlabhq/issues/2415

See merge request !851
parents 6c0db429 b6318297
...@@ -24,7 +24,7 @@ class PasswordsController < Devise::PasswordsController ...@@ -24,7 +24,7 @@ class PasswordsController < Devise::PasswordsController
super do |resource| super do |resource|
# TODO (rspeicher): In Devise master (> 3.4.1), we can set # TODO (rspeicher): In Devise master (> 3.4.1), we can set
# `Devise.sign_in_after_reset_password = false` and avoid this mess. # `Devise.sign_in_after_reset_password = false` and avoid this mess.
if resource.errors.empty? && resource.try(:otp_required_for_login?) if resource.errors.empty? && resource.try(:two_factor_enabled?)
resource.unlock_access! if unlockable?(resource) resource.unlock_access! if unlockable?(resource)
# Since we are not signing this user in, we use the :updated_not_active # Since we are not signing this user in, we use the :updated_not_active
......
...@@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController ...@@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
def create def create
if current_user.valid_otp?(params[:pin_code]) if current_user.valid_otp?(params[:pin_code])
current_user.otp_required_for_login = true current_user.two_factor_enabled = true
@codes = current_user.generate_otp_backup_codes! @codes = current_user.generate_otp_backup_codes!
current_user.save! current_user.save!
...@@ -30,7 +30,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController ...@@ -30,7 +30,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
def destroy def destroy
current_user.update_attributes({ current_user.update_attributes({
otp_required_for_login: false, two_factor_enabled: false,
encrypted_otp_secret: nil, encrypted_otp_secret: nil,
encrypted_otp_secret_iv: nil, encrypted_otp_secret_iv: nil,
encrypted_otp_secret_salt: nil, encrypted_otp_secret_salt: nil,
......
...@@ -57,7 +57,7 @@ class SessionsController < Devise::SessionsController ...@@ -57,7 +57,7 @@ class SessionsController < Devise::SessionsController
def authenticate_with_two_factor def authenticate_with_two_factor
user = self.resource = find_user user = self.resource = find_user
return unless user && user.otp_required_for_login return unless user && user.two_factor_enabled?
if user_params[:otp_attempt].present? && session[:otp_user_id] if user_params[:otp_attempt].present? && session[:otp_user_id]
if valid_otp_attempt?(user) if valid_otp_attempt?(user)
......
...@@ -172,6 +172,9 @@ class User < ActiveRecord::Base ...@@ -172,6 +172,9 @@ class User < ActiveRecord::Base
after_create :post_create_hook after_create :post_create_hook
after_destroy :post_destroy_hook after_destroy :post_destroy_hook
# User's Dashboard preference
# Note: When adding an option, it MUST go on the end of the array.
enum dashboard: [:projects, :stars]
alias_attribute :private_token, :authentication_token alias_attribute :private_token, :authentication_token
...@@ -297,6 +300,18 @@ class User < ActiveRecord::Base ...@@ -297,6 +300,18 @@ class User < ActiveRecord::Base
@reset_token @reset_token
end end
# Check if the user has enabled Two-factor Authentication
def two_factor_enabled?
otp_required_for_login
end
# Set whether or not Two-factor Authentication is enabled for the current user
#
# setting - Boolean
def two_factor_enabled=(setting)
self.otp_required_for_login = setting
end
def namespace_uniq def namespace_uniq
namespace_name = self.username namespace_name = self.username
existing_namespace = Namespace.by_path(namespace_name) existing_namespace = Namespace.by_path(namespace_name)
...@@ -704,8 +719,4 @@ class User < ActiveRecord::Base ...@@ -704,8 +719,4 @@ class User < ActiveRecord::Base
def can_be_removed? def can_be_removed?
!solo_owned_groups.present? !solo_owned_groups.present?
end end
# User's Dashboard preference
# Note: When adding an option, it MUST go on the end of the array.
enum dashboard: [:projects, :stars]
end end
...@@ -50,6 +50,14 @@ ...@@ -50,6 +50,14 @@
= link_to remove_email_admin_user_path(@user, email), data: { confirm: "Are you sure you want to remove #{email.email}?" }, method: :delete, class: "btn-xs btn btn-remove pull-right", title: 'Remove secondary email', id: "remove_email_#{email.id}" do = link_to remove_email_admin_user_path(@user, email), data: { confirm: "Are you sure you want to remove #{email.email}?" }, method: :delete, class: "btn-xs btn btn-remove pull-right", title: 'Remove secondary email', id: "remove_email_#{email.id}" do
%i.fa.fa-times %i.fa.fa-times
%li.two-factor-status
%span.light Two-factor Authentication:
%strong{class: @user.two_factor_enabled? ? 'cgreen' : 'cred'}
- if @user.two_factor_enabled?
Enabled
- else
Disabled
%li %li
%span.light Can create groups: %span.light Can create groups:
%strong %strong
......
...@@ -36,7 +36,7 @@ ...@@ -36,7 +36,7 @@
.panel-heading .panel-heading
Two-factor Authentication Two-factor Authentication
.panel-body .panel-body
- if current_user.otp_required_for_login - if current_user.two_factor_enabled?
.pull-right .pull-right
= link_to 'Disable Two-factor Authentication', profile_two_factor_auth_path, method: :delete, class: 'btn btn-close btn-sm', = link_to 'Disable Two-factor Authentication', profile_two_factor_auth_path, method: :delete, class: 'btn btn-close btn-sm',
data: { confirm: 'Are you sure?' } data: { confirm: 'Are you sure?' }
......
...@@ -40,11 +40,11 @@ describe Profiles::TwoFactorAuthsController do ...@@ -40,11 +40,11 @@ describe Profiles::TwoFactorAuthsController do
expect(user).to receive(:valid_otp?).with(pin).and_return(true) expect(user).to receive(:valid_otp?).with(pin).and_return(true)
end end
it 'sets otp_required_for_login' do it 'sets two_factor_enabled' do
go go
user.reload user.reload
expect(user.otp_required_for_login).to eq true expect(user).to be_two_factor_enabled
end end
it 'presents plaintext codes for the user to save' do it 'presents plaintext codes for the user to save' do
...@@ -109,13 +109,13 @@ describe Profiles::TwoFactorAuthsController do ...@@ -109,13 +109,13 @@ describe Profiles::TwoFactorAuthsController do
let!(:codes) { user.generate_otp_backup_codes! } let!(:codes) { user.generate_otp_backup_codes! }
it 'clears all 2FA-related fields' do it 'clears all 2FA-related fields' do
expect(user.otp_required_for_login).to eq true expect(user).to be_two_factor_enabled
expect(user.otp_backup_codes).not_to be_nil expect(user.otp_backup_codes).not_to be_nil
expect(user.encrypted_otp_secret).not_to be_nil expect(user.encrypted_otp_secret).not_to be_nil
delete :destroy delete :destroy
expect(user.otp_required_for_login).to eq false expect(user).not_to be_two_factor_enabled
expect(user.otp_backup_codes).to be_nil expect(user.otp_backup_codes).to be_nil
expect(user.encrypted_otp_secret).to be_nil expect(user.encrypted_otp_secret).to be_nil
end end
......
...@@ -30,7 +30,7 @@ FactoryGirl.define do ...@@ -30,7 +30,7 @@ FactoryGirl.define do
trait :two_factor do trait :two_factor do
before(:create) do |user| before(:create) do |user|
user.otp_required_for_login = true user.two_factor_enabled = true
user.otp_secret = User.generate_otp_secret(32) user.otp_secret = User.generate_otp_secret(32)
end end
end end
......
...@@ -63,15 +63,35 @@ describe "Admin::Users", feature: true do ...@@ -63,15 +63,35 @@ describe "Admin::Users", feature: true do
end end
describe "GET /admin/users/:id" do describe "GET /admin/users/:id" do
before do it "should have user info" do
visit admin_users_path visit admin_users_path
click_link "#{@user.name}" click_link @user.name
end
it "should have user info" do
expect(page).to have_content(@user.email) expect(page).to have_content(@user.email)
expect(page).to have_content(@user.name) expect(page).to have_content(@user.name)
end end
describe 'Two-factor Authentication status' do
it 'shows when enabled' do
@user.update_attribute(:two_factor_enabled, true)
visit admin_user_path(@user)
expect_two_factor_status('Enabled')
end
it 'shows when disabled' do
visit admin_user_path(@user)
expect_two_factor_status('Disabled')
end
def expect_two_factor_status(status)
page.within('.two-factor-status') do
expect(page).to have_content(status)
end
end
end
end end
describe "GET /admin/users/:id/edit" do describe "GET /admin/users/:id/edit" do
......
...@@ -210,6 +210,30 @@ describe User do ...@@ -210,6 +210,30 @@ describe User do
end end
end end
describe '#two_factor_enabled' do
it 'returns two-factor authentication status' do
enabled = build_stubbed(:user, two_factor_enabled: true)
disabled = build_stubbed(:user)
expect(enabled).to be_two_factor_enabled
expect(disabled).not_to be_two_factor_enabled
end
end
describe '#two_factor_enabled=' do
it 'enables two-factor authentication' do
user = build_stubbed(:user, two_factor_enabled: false)
expect { user.two_factor_enabled = true }.
to change { user.two_factor_enabled? }.to(true)
end
it 'disables two-factor authentication' do
user = build_stubbed(:user, two_factor_enabled: true)
expect { user.two_factor_enabled = false }.
to change { user.two_factor_enabled? }.to(false)
end
end
describe 'authentication token' do describe 'authentication token' do
it "should have authentication token" do it "should have authentication token" do
user = create(:user) user = create(:user)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment