From 26a767e0d3eb44a7d8c29dde3b1a3b9439f34240 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= <msabate@suse.com>
Date: Fri, 31 Jul 2015 10:57:50 +0200
Subject: [PATCH 1/7] Implemented the "Disable users" feature

Fixes #196
---
 app/controllers/admin/users_controller.rb     |  3 +-
 .../auth/registrations_controller.rb          | 35 ++++++++++++++-
 app/models/user.rb                            | 24 +++++++++++
 app/views/admin/dashboard/index.html.slim     |  2 +-
 app/views/admin/users/index.html.slim         | 15 ++++++-
 app/views/admin/users/toggle_admin.js.erb     |  4 +-
 app/views/auth/registrations/disabled.js.erb  | 14 ++++++
 app/views/devise/registrations/edit.html.slim | 13 ++++++
 config/routes.rb                              |  1 +
 .../20150729153854_add_enabled_to_users.rb    |  5 +++
 db/schema.rb                                  |  3 +-
 .../auth/registrations_controller_spec.rb     | 43 ++++++++++++++++++-
 spec/factories/teams_factory.rb               |  2 -
 spec/features/admin/users_spec.rb             | 25 +++++++++++
 spec/features/auth/login_feature_spec.rb      | 10 +++++
 spec/features/auth/profile_feature_spec.rb    | 18 +++++++-
 spec/models/user_spec.rb                      | 35 +++++++++++++++
 17 files changed, 241 insertions(+), 11 deletions(-)
 create mode 100644 app/views/auth/registrations/disabled.js.erb
 create mode 100644 db/migrate/20150729153854_add_enabled_to_users.rb
 create mode 100644 spec/features/admin/users_spec.rb

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 8c333e187..9758c6c45 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -2,7 +2,8 @@ class Admin::UsersController < Admin::BaseController
   respond_to :html, :js
 
   def index
-    @users = User.all.page(params[:page])
+    @users = User.where(enabled: true).page(params[:page])
+    @admin_count = User.admins.count
   end
 
   # PATCH/PUT /admin/user/1/toggle_admin
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index a07e49393..e5516901e 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -1,8 +1,9 @@
 class Auth::RegistrationsController < Devise::RegistrationsController
   layout 'authentication', except: :edit
 
-  before_action :check_admin, only: [:new, :create]
+  before_action :check_admin, only: [:new, :create, :disable]
   before_action :configure_sign_up_params, only: [:create]
+  before_action :authenticate_user!, only: [:disable]
 
   # Re-implemented so the template has the auxiliary variables regarding if
   # there are more users on the system or this is the first user to be created.
@@ -25,6 +26,11 @@ def create
     end
   end
 
+  def edit
+    @admin_count = User.admins.count
+    super
+  end
+
   def update
     success =
     if password_update?
@@ -46,6 +52,19 @@ def update
     end
   end
 
+  # Disable a user.
+  def disable
+    user = User.find(params[:id])
+
+    if can_disable?(user)
+      render nothing: true, status: 403
+    else
+      user.disable!
+      sign_out user if current_user == user
+      render template: 'auth/registrations/disabled', locals: { user: user, path: request.fullpath }
+    end
+  end
+
   # Devise does not allow to disable routes on purpose. Ideally, though we
   # could still be able to disable the `destroy` method through the
   # `routes.rb` file as described in the wiki (by disabling all the routes and
@@ -76,4 +95,18 @@ def password_update?
     !user[:current_password].blank? || !user[:password].blank? ||
       !user[:password_confirmation].blank?
   end
+
+  # Returns whether the given user can be disabled or not. The following rules
+  # apply:
+  #   1. A user can disable himself unless it's the last admin on the system.
+  #   2. The admin user is the only one that can disable other users.
+  def can_disable?(user)
+    if current_user == user
+      # An admin cannot disable himself if he's the only admin in the system.
+      current_user.admin? && User.admins.count == 1
+    else
+      # Only admin users can disable other users.
+      !current_user.admin?
+    end
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 99cc4d1dd..bf7a8ab4e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -42,6 +42,11 @@ def self.find_from_event(event)
     actor
   end
 
+  # Returns all the enabled admins in the system.
+  def self.admins
+    User.where(enabled: true, admin: true)
+  end
+
   # Toggle the 'admin' attribute for this user. It will also update the
   # registry accordingly.
   def toggle_admin!
@@ -51,4 +56,23 @@ def toggle_admin!
     team = Registry.first.global_namespace.team
     admin ? team.owners << self : team.owners.delete(self)
   end
+
+  ##
+  # Disabling users.
+
+  # Disable this user and remove all its associations with teams.
+  def disable!
+    return unless update_attributes(enabled: false)
+    TeamUser.where(user: id).delete_all
+  end
+
+  # This method is picked up by Devise before signing in a user.
+  def active_for_authentication?
+    super && enabled?
+  end
+
+  # The flashy message to be shown for disabled users that try to login.
+  def inactive_message
+    'Sorry, this account has been disabled.'
+  end
 end
diff --git a/app/views/admin/dashboard/index.html.slim b/app/views/admin/dashboard/index.html.slim
index 9a87c221e..0be9f76e9 100644
--- a/app/views/admin/dashboard/index.html.slim
+++ b/app/views/admin/dashboard/index.html.slim
@@ -19,7 +19,7 @@
           .col-xs-4
             i.fa.fa-user.users.fa-2x
           .col-xs-8
-            span.fa-3x= User.count
+            span.fa-3x= User.where(enabled: true).count
 
   .col-md-3.col-xs-6
     .panel-overview
diff --git a/app/views/admin/users/index.html.slim b/app/views/admin/users/index.html.slim
index f9368ba36..9d7a26284 100644
--- a/app/views/admin/users/index.html.slim
+++ b/app/views/admin/users/index.html.slim
@@ -9,6 +9,8 @@
         col.col-40
         col.col-10
         col.col-10
+        col.col-10
+        col.col-10
         thead
           tr
             th Name
@@ -16,12 +18,13 @@
             th Admin
             th Namespaces
             th Teams
+            th Enabled
         tbody
           - @users.each do |user|
             tr[id="user_#{user.id}"]
               td= user.username
               td= user.email
-              td
+              td.admin-btn
                 - if current_user.id == user.id
                   i.fa.fa-lg[class="fa-toggle-#{user.admin? ? 'on': 'off'}"]
                 - else
@@ -34,4 +37,14 @@
 
               td= user.teams.reduce(0){ |sum, t| sum += t.namespaces.count}
               td= user.teams.count
+              td.enabled-btn
+                - if current_user.id == user.id && @admin_count == 1
+                  i.fa.fa-lg.fa-toggle-on
+                - else
+                  a[class="btn btn-default"
+                    data-remote="true"
+                    data-method="put"
+                    rel="nofollow"
+                    href=url_for(disable_user_path(user))]
+                      i.fa.fa-lg[class="fa-toggle-#{user.enabled? ? 'on': 'off'}"]
   .panel-footer= paginate(@users)
diff --git a/app/views/admin/users/toggle_admin.js.erb b/app/views/admin/users/toggle_admin.js.erb
index 8dfab35fd..4fca734e1 100644
--- a/app/views/admin/users/toggle_admin.js.erb
+++ b/app/views/admin/users/toggle_admin.js.erb
@@ -1,5 +1,5 @@
-$('#user_<%= user.id %> td a i').addClass("fa-toggle-<%= user.admin? ? 'on' : 'off' %>");
-$('#user_<%= user.id %> td a i').removeClass("fa-toggle-<%= user.admin? ? 'off' : 'on' %>");
+$('#user_<%= user.id %> .admin-btn a i').addClass("fa-toggle-<%= user.admin? ? 'on' : 'off' %>");
+$('#user_<%= user.id %> .admin-btn a i').removeClass("fa-toggle-<%= user.admin? ? 'off' : 'on' %>");
 
 $('#notice p').html("User '<%= user.username %>' is <%= user.admin? ? 'now' : 'no longer' %> an admin");
 $('#notice').fadeIn();
diff --git a/app/views/auth/registrations/disabled.js.erb b/app/views/auth/registrations/disabled.js.erb
new file mode 100644
index 000000000..bb97ef08e
--- /dev/null
+++ b/app/views/auth/registrations/disabled.js.erb
@@ -0,0 +1,14 @@
+
+if (window.location.pathname == '/admin/users') {
+  // We are on the admin panel.
+  $("#user_<%= user.id %>").fadeOut('normal', function() {
+    $("#user_<%= user.id %>").remove();
+  });
+
+  $('#alert p').html("User '<%= user.username %>' has been disabled.");
+  $('#alert').fadeIn();
+} else {
+  // The user profile page.
+  window.location.pathname = '/';
+}
+
diff --git a/app/views/devise/registrations/edit.html.slim b/app/views/devise/registrations/edit.html.slim
index d479ceabf..529a96b4c 100644
--- a/app/views/devise/registrations/edit.html.slim
+++ b/app/views/devise/registrations/edit.html.slim
@@ -39,3 +39,16 @@
         .form-group
           .actions
             = f.submit('Change', class: 'btn btn-primary', disabled: true)
+
+  - unless current_user.admin? && @admin_count == 1
+    .panel.panel-default
+      .panel-heading
+        h5
+          ' Disable account
+      .panel-body
+        = form_tag(disable_user_path(current_user), method: :put, remote: true, id: 'disable-form') do
+            .form-group
+              p
+                | By disabling the account, you won't be able to access Portus with it, and
+                  any affiliations with any team will be lost.
+              = submit_tag('Disable', class: 'btn btn-primary btn-danger')
diff --git a/config/routes.rb b/config/routes.rb
index 75b2914e0..5395e86c7 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -21,6 +21,7 @@
 
   devise_scope :user do
     root to: 'auth/sessions#new'
+    put 'disable_user/:id', to: 'auth/registrations#disable', as: :disable_user
   end
 
   namespace :v2, module: 'api/v2', defaults: { format: :json } do
diff --git a/db/migrate/20150729153854_add_enabled_to_users.rb b/db/migrate/20150729153854_add_enabled_to_users.rb
new file mode 100644
index 000000000..a539c9b91
--- /dev/null
+++ b/db/migrate/20150729153854_add_enabled_to_users.rb
@@ -0,0 +1,5 @@
+class AddEnabledToUsers < ActiveRecord::Migration
+  def change
+    add_column :users, :enabled, :bool, default: true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 36d0e28aa..142474f10 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150722194840) do
+ActiveRecord::Schema.define(version: 20150729153854) do
 
   create_table "activities", force: :cascade do |t|
     t.integer  "trackable_id",   limit: 4
@@ -124,6 +124,7 @@
     t.datetime "created_at"
     t.datetime "updated_at"
     t.boolean  "admin",                  limit: 1,   default: false
+    t.boolean  "enabled",                limit: 1,   default: true
   end
 
   add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
diff --git a/spec/controllers/auth/registrations_controller_spec.rb b/spec/controllers/auth/registrations_controller_spec.rb
index f15c34c97..3b9f3874e 100644
--- a/spec/controllers/auth/registrations_controller_spec.rb
+++ b/spec/controllers/auth/registrations_controller_spec.rb
@@ -109,7 +109,6 @@
   end
 
   describe 'DELETE #destroy' do
-
     let!(:user) { create(:admin) }
 
     before :each do
@@ -123,4 +122,46 @@
     end
   end
 
+  describe 'PUT #disable_user' do
+    let!(:admin) { create(:admin) }
+    let!(:user) { create(:user) }
+
+    before :each do
+      request.env['devise.mapping'] = Devise.mappings[:user]
+    end
+
+    it 'does not allow to disable the only admin' do
+      sign_in admin
+      put :disable, id: admin.id
+      expect(response.status).to be 403
+    end
+
+    it 'does not allow a regular user to disable another' do
+      sign_in user
+      put :disable, id: admin.id
+      expect(response.status).to be 403
+    end
+
+    it 'allows a user to disable himself' do
+      sign_in user
+      put :disable, id: user.id, format: :erb
+      expect(response.status).to be 200
+      expect(User.find(user.id).enabled?).to be false
+    end
+
+    it 'allows the admin to disable a regular user' do
+      sign_in admin
+      put :disable, id: user.id, format: :erb
+      expect(response.status).to be 200
+      expect(User.find(user.id).enabled?).to be false
+    end
+
+    it 'allows an admin to disable another admin' do
+      admin2 = create(:admin)
+      sign_in admin
+      put :disable, id: admin2.id, format: :erb
+      expect(response.status).to be 200
+      expect(User.find(admin2.id).enabled?).to be false
+    end
+  end
 end
diff --git a/spec/factories/teams_factory.rb b/spec/factories/teams_factory.rb
index 48b34d047..fb8f1fc80 100644
--- a/spec/factories/teams_factory.rb
+++ b/spec/factories/teams_factory.rb
@@ -1,8 +1,6 @@
 FactoryGirl.define do
-
   factory :team do
     sequence(:name) { |n| "team_name#{n}" }
     owners { |t| [t.association(:user)] }
   end
-
 end
diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb
new file mode 100644
index 000000000..1f71fb8cd
--- /dev/null
+++ b/spec/features/admin/users_spec.rb
@@ -0,0 +1,25 @@
+require 'rails_helper'
+
+feature 'Admin - Users panel' do
+  let!(:registry) { create(:registry) }
+  let!(:admin) { create(:admin) }
+  let!(:user) { create(:user) }
+
+  before do
+    login_as admin, scope: :user
+    visit admin_users_path
+  end
+
+  describe 'disable users' do
+    scenario 'allows the admin to disable other users', js: true do
+      expect(page).to have_css("#user_#{user.id}")
+      find("#user_#{user.id} .enabled-btn").click
+
+      wait_for_effect_on("#user_#{user.id}")
+
+      expect(page).to_not have_css("#user_#{user.id}")
+      wait_for_effect_on('#alert')
+      expect(page).to have_content("User '#{user.username}' has been disabled.")
+    end
+  end
+end
diff --git a/spec/features/auth/login_feature_spec.rb b/spec/features/auth/login_feature_spec.rb
index e43bbd75e..2a86fd606 100644
--- a/spec/features/auth/login_feature_spec.rb
+++ b/spec/features/auth/login_feature_spec.rb
@@ -40,4 +40,14 @@
     expect(current_url).to eq root_url
   end
 
+  scenario 'A disabled user cannot login' do
+    user.disable!
+    fill_in 'user_username', with: user.username
+    fill_in 'user_password', with: user.password
+    click_button 'Login'
+
+    expect(page).to have_content(user.inactive_message)
+    expect(page).to have_content('Login')
+    expect(current_url).to eq new_user_session_url
+  end
 end
diff --git a/spec/features/auth/profile_feature_spec.rb b/spec/features/auth/profile_feature_spec.rb
index db447aaf7..e88dedc94 100644
--- a/spec/features/auth/profile_feature_spec.rb
+++ b/spec/features/auth/profile_feature_spec.rb
@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 feature 'Update password feature' do
-  let!(:user) { create(:user) }
+  let!(:user) { create(:admin) }
 
   before do
     login_as user, scope: :user
@@ -53,4 +53,20 @@
     expect(current_path).to eq edit_user_registration_path
     expect(User.first.valid_password?('12341234')).to be true
   end
+
+  # Disabling user
+
+  scenario 'It disables the current user', js: true, focus: true do
+    create(:admin)
+    visit edit_user_registration_path
+
+    click_button 'Disable'
+    wait_until { current_path == root_path }
+    expect(current_path).to eq root_path
+    expect(page).to have_content('Login')
+  end
+
+  scenario 'The "disable" pannel does not exists if it\'s the only admin', js: true do
+    expect(page).to_not have_css('#disable-form')
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index f59933048..589ca72f9 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -53,6 +53,17 @@
 
   end
 
+  describe 'admins' do
+    let!(:admin1) { create(:admin) }
+    let!(:admin2) { create(:admin, enabled: false) }
+
+    it 'computes the right amount of admin users' do
+      admins = User.admins
+      expect(admins.count).to be 1
+      expect(admins.first.id).to be admin1.id
+    end
+  end
+
   describe '#toggle_admin' do
     let!(:registry) { create(:registry) }
     let!(:user) { create(:user) }
@@ -77,4 +88,28 @@
       expect(owners.first.id).to be(admin.id)
     end
   end
+
+  describe 'disabling' do
+    let!(:admin) { create(:admin) }
+    let!(:user) { create(:user) }
+    let!(:team) { create(:team, owners: [admin], viewers: [user]) }
+
+    it 'invalidates a user and removes all its associations with teams' do
+      expect(User.count).to eq 2
+      expect(TeamUser.count).to eq 2
+      expect(TeamUser.where(user: user.id)).to_not be_empty
+
+      user.disable!
+
+      expect(User.count).to eq 2
+      expect(TeamUser.count).to eq 1
+      expect(TeamUser.where(user: user.id)).to be_empty
+    end
+
+    it 'interacts with Devise as expected' do
+      expect(user.active_for_authentication?).to be true
+      user.disable!
+      expect(user.active_for_authentication?).to be false
+    end
+  end
 end

From 36a013b8e82e2b0d512a1d0e2d60ea33d4fa35b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= <msabate@suse.com>
Date: Fri, 31 Jul 2015 11:00:46 +0200
Subject: [PATCH 2/7] Updated Changelog file

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 67f9767cb..6f3128173 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,7 @@ pull requests: [#193](https://github.com/SUSE/Portus/pull/193),
 PR [#212](https://github.com/SUSE/Portus/pull/212).
 - Added the appliance tests. See PR [#208](https://github.com/SUSE/Portus/pull/208).
 - Star/Unstar repositories. See PR [#230](https://github.com/SUSE/Portus/pull/230).
+- Now users can be disabled. See PR [#240](https://github.com/SUSE/Portus/pull/240).
 - And some minor fixes here and there.
 
 ## 1.0.1

From fddfe7792b80cbaafd9650896df61ac29cdc4fc2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= <msabate@suse.com>
Date: Fri, 31 Jul 2015 11:19:39 +0200
Subject: [PATCH 3/7] Removed unneeded hook for the `disabled` method

---
 app/controllers/auth/registrations_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index e5516901e..75566ef21 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -1,7 +1,7 @@
 class Auth::RegistrationsController < Devise::RegistrationsController
   layout 'authentication', except: :edit
 
-  before_action :check_admin, only: [:new, :create, :disable]
+  before_action :check_admin, only: [:new, :create]
   before_action :configure_sign_up_params, only: [:create]
   before_action :authenticate_user!, only: [:disable]
 

From 6ae62eb8d54088e2b564d4907069b58994520903 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= <msabate@suse.com>
Date: Fri, 31 Jul 2015 15:25:33 +0200
Subject: [PATCH 4/7] Make sure that disabled users cannot login through a
 docker client

---
 spec/api/v2/token_spec.rb | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/spec/api/v2/token_spec.rb b/spec/api/v2/token_spec.rb
index 02e3dc7be..1eec1d218 100644
--- a/spec/api/v2/token_spec.rb
+++ b/spec/api/v2/token_spec.rb
@@ -22,6 +22,13 @@
     end
 
     context 'as invalid user' do
+      let(:valid_request) do
+        {
+          service: registry.hostname,
+          account: 'account',
+          scope: 'repository:foo_namespace/me:push'
+        }
+      end
 
       it 'denies access when the password is wrong' do
         get v2_token_url, { service: registry.hostname, account: 'account', scope: 'repository:foo/me:push' }, invalid_auth_header
@@ -38,6 +45,11 @@
         expect(response.status).to eq 401
       end
 
+      it 'denies access to a disabled user' do
+        user.update_attributes(enabled: false)
+        get v2_token_url, valid_request, valid_auth_header
+        expect(response.status).to eq 401
+      end
     end
 
     context 'as the special portus user' do

From 0c5b0fc12d130e64cda41cd4c9a6c1e414cae1b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= <msabate@suse.com>
Date: Fri, 31 Jul 2015 15:32:44 +0200
Subject: [PATCH 5/7] Transformed the User.admins method into a scope

I've also taken the opportunity to add the `enabled` scope, which can be
convenient.
---
 app/models/user.rb                        | 8 +++-----
 app/views/admin/dashboard/index.html.slim | 2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index bf7a8ab4e..f92137d6d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -14,6 +14,9 @@ class User < ActiveRecord::Base
   has_many :teams, through: :team_users
   has_many :stars
 
+  scope :enabled, -> { where enabled: true }
+  scope :admins,  -> { where enabled: true, admin: true }
+
   def private_namespace_available
     return unless Namespace.exists?(name: username)
     errors.add(:username, 'cannot be used as name for private namespace')
@@ -42,11 +45,6 @@ def self.find_from_event(event)
     actor
   end
 
-  # Returns all the enabled admins in the system.
-  def self.admins
-    User.where(enabled: true, admin: true)
-  end
-
   # Toggle the 'admin' attribute for this user. It will also update the
   # registry accordingly.
   def toggle_admin!
diff --git a/app/views/admin/dashboard/index.html.slim b/app/views/admin/dashboard/index.html.slim
index 0be9f76e9..b1df07956 100644
--- a/app/views/admin/dashboard/index.html.slim
+++ b/app/views/admin/dashboard/index.html.slim
@@ -19,7 +19,7 @@
           .col-xs-4
             i.fa.fa-user.users.fa-2x
           .col-xs-8
-            span.fa-3x= User.where(enabled: true).count
+            span.fa-3x= User.enabled.count
 
   .col-md-3.col-xs-6
     .panel-overview

From bf68c0b8e42d36e405b73ef29209535ed8bd6e5a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= <msabate@suse.com>
Date: Fri, 31 Jul 2015 18:41:00 +0200
Subject: [PATCH 6/7] Don't show disabled users in the team page

---
 app/controllers/auth/registrations_controller.rb |  2 +-
 app/controllers/teams_controller.rb              |  2 +-
 app/models/team_user.rb                          |  2 ++
 app/models/user.rb                               |  6 ------
 spec/controllers/teams_controller_spec.rb        | 12 ++++++++++++
 spec/features/auth/login_feature_spec.rb         |  2 +-
 spec/features/auth/profile_feature_spec.rb       |  2 +-
 spec/models/team_spec.rb                         |  1 -
 spec/models/team_user_spec.rb                    | 15 +++++++++++++++
 spec/models/user_spec.rb                         | 14 +-------------
 10 files changed, 34 insertions(+), 24 deletions(-)
 create mode 100644 spec/models/team_user_spec.rb

diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 75566ef21..f9335c09f 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -59,7 +59,7 @@ def disable
     if can_disable?(user)
       render nothing: true, status: 403
     else
-      user.disable!
+      user.update_attributes(enabled: false)
       sign_out user if current_user == user
       render template: 'auth/registrations/disabled', locals: { user: user, path: request.fullpath }
     end
diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb
index d967d9fc6..e9eac3a2d 100644
--- a/app/controllers/teams_controller.rb
+++ b/app/controllers/teams_controller.rb
@@ -13,7 +13,7 @@ def index
   # GET /teams/1.json
   def show
     authorize @team
-    @team_users = @team.team_users.page(params[:users_page]).per(10)
+    @team_users = @team.team_users.enabled.page(params[:users_page]).per(10)
     @team_namespaces = @team.namespaces.page(params[:namespaces_page]).per(15)
   end
 
diff --git a/app/models/team_user.rb b/app/models/team_user.rb
index 577480962..d725d8243 100644
--- a/app/models/team_user.rb
+++ b/app/models/team_user.rb
@@ -7,6 +7,8 @@
 class TeamUser < ActiveRecord::Base
   enum role: [:viewer, :contributor, :owner]
 
+  scope :enabled, -> { joins(:user).where('users.enabled' => true) }
+
   validates :team, presence: true
   validates :user, presence: true, uniqueness: { scope: :team }
 
diff --git a/app/models/user.rb b/app/models/user.rb
index f92137d6d..2a106ad06 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -58,12 +58,6 @@ def toggle_admin!
   ##
   # Disabling users.
 
-  # Disable this user and remove all its associations with teams.
-  def disable!
-    return unless update_attributes(enabled: false)
-    TeamUser.where(user: id).delete_all
-  end
-
   # This method is picked up by Devise before signing in a user.
   def active_for_authentication?
     super && enabled?
diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb
index 7af87582e..29453a4cd 100644
--- a/spec/controllers/teams_controller_spec.rb
+++ b/spec/controllers/teams_controller_spec.rb
@@ -39,6 +39,18 @@
 
       expect(response.status).to eq 401
     end
+
+    it 'does not display disabled users' do
+      user = create(:user, enabled: false)
+      TeamUser.create(team: team, user: user, role: TeamUser.roles['viewer'])
+      sign_in owner
+
+      get :show, id: team.id
+
+      expect(response.status).to eq 200
+      expect(TeamUser.count).to be 2
+      expect(assigns(:team_users).count).to be 1
+    end
   end
 
   describe 'as a portus user' do
diff --git a/spec/features/auth/login_feature_spec.rb b/spec/features/auth/login_feature_spec.rb
index 2a86fd606..2cc6c783f 100644
--- a/spec/features/auth/login_feature_spec.rb
+++ b/spec/features/auth/login_feature_spec.rb
@@ -41,7 +41,7 @@
   end
 
   scenario 'A disabled user cannot login' do
-    user.disable!
+    user.update_attributes(enabled: false)
     fill_in 'user_username', with: user.username
     fill_in 'user_password', with: user.password
     click_button 'Login'
diff --git a/spec/features/auth/profile_feature_spec.rb b/spec/features/auth/profile_feature_spec.rb
index e88dedc94..0bf7a5b9d 100644
--- a/spec/features/auth/profile_feature_spec.rb
+++ b/spec/features/auth/profile_feature_spec.rb
@@ -56,7 +56,7 @@
 
   # Disabling user
 
-  scenario 'It disables the current user', js: true, focus: true do
+  scenario 'It disables the current user', js: true do
     create(:admin)
     visit edit_user_registration_path
 
diff --git a/spec/models/team_spec.rb b/spec/models/team_spec.rb
index ab3a65149..284aaeae5 100644
--- a/spec/models/team_spec.rb
+++ b/spec/models/team_spec.rb
@@ -1,7 +1,6 @@
 require 'rails_helper'
 
 describe Team do
-
   it { should validate_presence_of(:name) }
   it { should validate_presence_of(:owners) }
   it { should have_many(:namespaces) }
diff --git a/spec/models/team_user_spec.rb b/spec/models/team_user_spec.rb
new file mode 100644
index 000000000..5682aa887
--- /dev/null
+++ b/spec/models/team_user_spec.rb
@@ -0,0 +1,15 @@
+require 'rails_helper'
+
+describe TeamUser do
+  let!(:user1) { create(:user) }
+  let!(:user2) { create(:user) }
+  let!(:team)  { create(:team, owners: [user1, user2]) }
+
+  it 'does not return disabled team members' do
+    id = team.id
+    expect(team.team_users.count).to be 2
+    user2.update_attributes(enabled: false)
+    team = Team.find(id)
+    expect(team.team_users.enabled.count).to be 1
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 589ca72f9..a5bc5be2b 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -94,21 +94,9 @@
     let!(:user) { create(:user) }
     let!(:team) { create(:team, owners: [admin], viewers: [user]) }
 
-    it 'invalidates a user and removes all its associations with teams' do
-      expect(User.count).to eq 2
-      expect(TeamUser.count).to eq 2
-      expect(TeamUser.where(user: user.id)).to_not be_empty
-
-      user.disable!
-
-      expect(User.count).to eq 2
-      expect(TeamUser.count).to eq 1
-      expect(TeamUser.where(user: user.id)).to be_empty
-    end
-
     it 'interacts with Devise as expected' do
       expect(user.active_for_authentication?).to be true
-      user.disable!
+      user.update_attributes(enabled: false)
       expect(user.active_for_authentication?).to be false
     end
   end

From bc8bf565676125302b33729e114254fbfdfb5062 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= <msabate@suse.com>
Date: Mon, 3 Aug 2015 11:24:25 +0200
Subject: [PATCH 7/7] admin: Use the `enabled` scope

---
 app/controllers/admin/users_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 9758c6c45..884e39e48 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -2,7 +2,7 @@ class Admin::UsersController < Admin::BaseController
   respond_to :html, :js
 
   def index
-    @users = User.where(enabled: true).page(params[:page])
+    @users = User.enabled.page(params[:page])
     @admin_count = User.admins.count
   end