Merge pull request #240 from mssola/disable

Implemented the "Disable users" feature

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

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.enabled.page(params[:page])
+    @admin_count = User.admins.count
   end
 
   # PATCH/PUT /admin/user/1/toggle_admin

app/controllers/auth/registrations_controller.rb

@@ -3,6 +3,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController
 
   before_action :check_admin, only: [:new, :create]
   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.update_attributes(enabled: false)
+      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

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
 

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 }
 

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')
@@ -51,4 +54,17 @@ def toggle_admin!
     team = Registry.first.global_namespace.team
     admin ? team.owners << self : team.owners.delete(self)
   end
+
+  ##
+  # Disabling users.
+
+  # 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

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.enabled.count
 
   .col-md-3.col-xs-6
     .panel-overview

app/views/admin/users/index.html.slim

@@ -9,19 +9,22 @@
         col.col-40
         col.col-10
         col.col-10
+        col.col-10
+        col.col-10
         thead
           tr
             th Name
             th Email
             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)

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();

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 = '/';
+}
+

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')

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

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

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

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

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

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

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

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

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.update_attributes(enabled: false)
+    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