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

10 files changed, 81 insertions, 18 deletions

diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index 145f27b67dd..8450ba31021 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -24,7 +24,7 @@ class PasswordsController < Devise::PasswordsController
     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?)
+      if resource.errors.empty? && resource.try(:two_factor_enabled?)
         resource.unlock_access! if unlockable?(resource)
 
         # Since we are not signing this user in, we use the :updated_not_active
diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb
index e7579c652fb..03845f1e1ec 100644
--- a/app/controllers/profiles/two_factor_auths_controller.rb
+++ b/app/controllers/profiles/two_factor_auths_controller.rb
@@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
   def create
     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!
       current_user.save!
 
@@ -30,7 +30,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
   def destroy
     current_user.update_attributes({
-      otp_required_for_login:    false,
+      two_factor_enabled:        false,
       encrypted_otp_secret:      nil,
       encrypted_otp_secret_iv:   nil,
       encrypted_otp_secret_salt: nil,
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 4d976fe6630..7577fc96d6d 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -57,7 +57,7 @@ class SessionsController < Devise::SessionsController
   def authenticate_with_two_factor
     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 valid_otp_attempt?(user)
diff --git a/app/models/user.rb b/app/models/user.rb
index 982c05212ce..a2e2d220b3a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -172,6 +172,9 @@ class User < ActiveRecord::Base
   after_create :post_create_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
 
@@ -297,6 +300,18 @@ class User < ActiveRecord::Base
     @reset_token
   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
     namespace_name = self.username
     existing_namespace = Namespace.by_path(namespace_name)
@@ -704,8 +719,4 @@ class User < ActiveRecord::Base
   def can_be_removed?
     !solo_owned_groups.present?
   end
-
-  # User's Dashboard preference
-  # Note: When adding an option, it MUST go on the end of the array.
-  enum dashboard: [:projects, :stars]
 end
diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml
index f7195ac3326..48cd22fc34b 100644
--- a/app/views/admin/users/show.html.haml
+++ b/app/views/admin/users/show.html.haml
@@ -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
                   %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
               %span.light Can create groups:
               %strong
diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml
index ed009c86568..378dfa2dce0 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -36,7 +36,7 @@
       .panel-heading
         Two-factor Authentication
       .panel-body
-        - if current_user.otp_required_for_login
+        - if current_user.two_factor_enabled?
           .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?' }
diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb
index 65415f21e55..aa09f1a758d 100644
--- a/spec/controllers/profiles/two_factor_auths_controller_spec.rb
+++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb
@@ -40,11 +40,11 @@ describe Profiles::TwoFactorAuthsController do
         expect(user).to receive(:valid_otp?).with(pin).and_return(true)
       end
 
-      it 'sets otp_required_for_login' do
+      it 'sets two_factor_enabled' do
         go
 
         user.reload
-        expect(user.otp_required_for_login).to eq true
+        expect(user).to be_two_factor_enabled
       end
 
       it 'presents plaintext codes for the user to save' do
@@ -109,13 +109,13 @@ describe Profiles::TwoFactorAuthsController do
     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).to be_two_factor_enabled
       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).not_to be_two_factor_enabled
       expect(user.otp_backup_codes).to be_nil
       expect(user.encrypted_otp_secret).to be_nil
     end
diff --git a/spec/factories.rb b/spec/factories.rb
index 9373a7af024..578a2e4dc69 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -30,7 +30,7 @@ FactoryGirl.define do
 
     trait :two_factor do
       before(:create) do |user|
-        user.otp_required_for_login = true
+        user.two_factor_enabled = true
         user.otp_secret = User.generate_otp_secret(32)
       end
     end
diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb
index f97b69713ce..7f5cb30cb94 100644
--- a/spec/features/admin/admin_users_spec.rb
+++ b/spec/features/admin/admin_users_spec.rb
@@ -63,15 +63,35 @@ describe "Admin::Users", feature: true  do
   end
 
   describe "GET /admin/users/:id" do
-    before do
+    it "should have user info" do
       visit admin_users_path
-      click_link "#{@user.name}"
-    end
+      click_link @user.name
 
-    it "should have user info" do
       expect(page).to have_content(@user.email)
       expect(page).to have_content(@user.name)
     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
 
   describe "GET /admin/users/:id/edit" do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index f3e278e5c5f..fa7680fbbec 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -210,6 +210,30 @@ describe User do
     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
     it "should have authentication token" do
       user = create(:user)