diff --git a/app/assets/javascripts/app/controllers/_application_controller/table.coffee b/app/assets/javascripts/app/controllers/_application_controller/table.coffee
index 4971a20ba..2b0b43c0d 100644
--- a/app/assets/javascripts/app/controllers/_application_controller/table.coffee
+++ b/app/assets/javascripts/app/controllers/_application_controller/table.coffee
@@ -502,6 +502,8 @@ class App.ControllerTable extends App.Controller
tableBody = []
objectsToShow = @objectsOfPage(@shownPage)
for object in objectsToShow
+ objectActions = []
+
if object
position++
if @groupBy
@@ -509,7 +511,14 @@ class App.ControllerTable extends App.Controller
if groupLastName isnt groupByName
groupLastName = groupByName
tableBody.push @renderTableGroupByRow(object, position, groupByName)
- tableBody.push @renderTableRow(object, position)
+ for action in @actions
+ # Check if the available key is used, it can be a Boolean or a function which should be called.
+ if !action.available? || action.available == true
+ objectActions.push action
+ else if typeof action.available is 'function' && action.available(object) == true
+ objectActions.push action
+
+ tableBody.push @renderTableRow(object, position, objectActions)
tableBody
renderTableGroupByRow: (object, position, groupByName) =>
@@ -531,7 +540,7 @@ class App.ControllerTable extends App.Controller
columnsLength: @columnsLength
)
- renderTableRow: (object, position) =>
+ renderTableRow: (object, position, actions) =>
App.view('generic/table_row')(
headers: @headers
attributes: @attributesList
@@ -541,7 +550,7 @@ class App.ControllerTable extends App.Controller
sortable: @dndCallback
position: position
object: object
- actions: @actions
+ actions: actions
)
tableHeadersHasChanged: =>
diff --git a/app/assets/javascripts/app/controllers/user.coffee b/app/assets/javascripts/app/controllers/user.coffee
index 0242b590e..0044f1753 100644
--- a/app/assets/javascripts/app/controllers/user.coffee
+++ b/app/assets/javascripts/app/controllers/user.coffee
@@ -9,6 +9,7 @@ class User extends App.ControllerSubContent
constructor: ->
super
+
@render()
show: =>
@@ -95,6 +96,17 @@ class User extends App.ControllerSubContent
container: @el.closest('.content')
)
+
+ callbackLoginAttribute = (value, object, attribute, attributes) ->
+ attribute.prefixIcon = null
+ attribute.title = null
+
+ if object.maxLoginFailedReached()
+ attribute.title = App.i18n.translateContent('The user is locked, because of too many failed login attempts.')
+ attribute.prefixIcon = 'lock'
+
+ value
+
users = []
for user_id in user_ids
user = App.User.find(user_id)
@@ -129,7 +141,7 @@ class User extends App.ControllerSubContent
)
800
)
- }
+ },
{
name: 'delete'
display: 'Delete'
@@ -138,7 +150,33 @@ class User extends App.ControllerSubContent
callback: (id) =>
@navigate "#system/data_privacy/#{id}"
},
+ {
+ name: 'unlock'
+ display: 'Unlock'
+ icon: 'lock-open'
+ class: 'unlock'
+ available: (user) ->
+ user.maxLoginFailedReached()
+ callback: (id) =>
+ @ajax(
+ id: "user_unlock_#{id}"
+ type: 'PUT'
+ url: "#{@apiPath}/users/unlock/#{id}"
+ success: =>
+ App.User.full(id,
+ => @notify(
+ type: 'success'
+ msg: App.i18n.translateContent('User successfully unlocked!')
+
+ @renderResult(user_ids)
+ ),
+ true)
+ )
+ }
]
+ callbackAttributes: {
+ login: [ callbackLoginAttribute ]
+ }
bindRow:
events:
'click': edit
diff --git a/app/assets/javascripts/app/models/user.coffee b/app/assets/javascripts/app/models/user.coffee
index 8f1f6aff8..6c1a4beb1 100644
--- a/app/assets/javascripts/app/models/user.coffee
+++ b/app/assets/javascripts/app/models/user.coffee
@@ -121,6 +121,9 @@ class App.User extends App.Model
return true
false
+ maxLoginFailedReached: ->
+ return @login_failed > (App.Config.get('password_max_login_failed') || 10)
+
imageUrl: ->
return if !@image
# set image url
diff --git a/app/assets/javascripts/app/views/generic/table_row.jst.eco b/app/assets/javascripts/app/views/generic/table_row.jst.eco
index c12bd3802..a5ec74a2d 100644
--- a/app/assets/javascripts/app/views/generic/table_row.jst.eco
+++ b/app/assets/javascripts/app/views/generic/table_row.jst.eco
@@ -66,6 +66,9 @@
<% if header.raw: %>
<%- header.raw %>
<% else: %>
+ <% if header.prefixIcon: %>
+ <%- @Icon(header.prefixIcon) %>
+ <% end %>
<% if header.class || header.data: %>
class="<%= header.class %>"<% end %> <% if header.data: %><% for data_key, data_header of header.data: %>data-<%- data_key %>="<%= data_header %>" <% end %><% end %>>
<% end %>
diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss
index 97b867b27..87ea77794 100644
--- a/app/assets/stylesheets/zammad.scss
+++ b/app/assets/stylesheets/zammad.scss
@@ -1353,6 +1353,10 @@ td .icon-trash {
//fill: hsl(240,1%,77%);
}
+td .prefix-icon > .icon {
+ vertical-align: top;
+}
+
.table-checkbox,
.table-radio {
padding: 0 !important;
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index bfe1e4b7b..ce28c6d91 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -3,7 +3,7 @@
class UsersController < ApplicationController
include ChecksUserAttributesByCurrentUserPermission
- prepend_before_action -> { authorize! }, only: %i[import_example import_start search history]
+ prepend_before_action -> { authorize! }, only: %i[import_example import_start search history unlock]
prepend_before_action :authentication_check, except: %i[create password_reset_send password_reset_verify image email_verify email_verify_send]
prepend_before_action :authentication_check_only, only: %i[create]
@@ -338,6 +338,24 @@ class UsersController < ApplicationController
render json: user.history_get(true)
end
+ # @path [PUT] /users/unlock/{id}
+ #
+ # @summary Unlocks the User record matching the identifier.
+ # @notes The requester have 'admin.user' permissions to be able to unlock a user.
+ #
+ # @parameter id(required) [Integer] The identifier matching the requested User record.
+ #
+ # @response_message 200 Unlocked User record.
+ # @response_message 403 Forbidden / Invalid session.
+ def unlock
+ user = User.find(params[:id])
+
+ user.with_lock do
+ user.update!(login_failed: 0)
+ end
+ render json: { message: 'ok' }, status: :ok
+ end
+
=begin
Resource:
diff --git a/app/models/user.rb b/app/models/user.rb
index 753439337..c028e1f6c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -49,7 +49,7 @@ class User < ApplicationModel
before_validation :check_name, :check_email, :check_login, :ensure_uniq_email, :ensure_password, :ensure_roles, :ensure_identifier
before_validation :check_mail_delivery_failed, on: :update
before_create :check_preferences_default, :validate_preferences, :validate_ooo, :domain_based_assignment, :set_locale
- before_update :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute
+ before_update :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed_after_password_change, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute
before_destroy :destroy_longer_required_objects, :destroy_move_dependency_ownership
after_commit :update_caller_id
@@ -1203,7 +1203,7 @@ raise 'Minimum one user need to have admin permissions'
end
# reset login_failed if password is changed
- def reset_login_failed
+ def reset_login_failed_after_password_change
return true if !will_save_change_to_attribute?('password')
self.login_failed = 0
diff --git a/app/policies/controllers/users_controller_policy.rb b/app/policies/controllers/users_controller_policy.rb
index 72cd3c8e7..0539f27c0 100644
--- a/app/policies/controllers/users_controller_policy.rb
+++ b/app/policies/controllers/users_controller_policy.rb
@@ -1,6 +1,6 @@
# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
class Controllers::UsersControllerPolicy < Controllers::ApplicationControllerPolicy
- permit! %i[import_example import_start], to: 'admin.user'
+ permit! %i[import_example import_start unlock], to: 'admin.user'
permit! %i[search history create update], to: ['ticket.agent', 'admin.user']
end
diff --git a/config/routes/user.rb b/config/routes/user.rb
index b1f918102..ea4bd2abb 100644
--- a/config/routes/user.rb
+++ b/config/routes/user.rb
@@ -29,6 +29,7 @@ Zammad::Application.routes.draw do
match api_path + '/users/:id', to: 'users#update', via: :put, as: 'api_v1_update_user'
match api_path + '/users/:id', to: 'users#destroy', via: :delete, as: 'api_v1_delete_user'
match api_path + '/users/image/:hash', to: 'users#image', via: :get
+ match api_path + '/users/unlock/:id', to: 'users#unlock', via: :put
match api_path + '/users/email_verify', to: 'users#email_verify', via: :post
match api_path + '/users/email_verify_send', to: 'users#email_verify_send', via: :post
diff --git a/db/migrate/20210731132202_setting_update_password_max_login_failed.rb b/db/migrate/20210731132202_setting_update_password_max_login_failed.rb
new file mode 100644
index 000000000..abe509115
--- /dev/null
+++ b/db/migrate/20210731132202_setting_update_password_max_login_failed.rb
@@ -0,0 +1,12 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+class SettingUpdatePasswordMaxLoginFailed < ActiveRecord::Migration[6.0]
+ def change
+ return if !Setting.exists?(name: 'system_init_done')
+
+ setting = Setting.find_by(name: 'password_max_login_failed')
+ setting.preferences[:authentication] = true
+ setting.frontend = true
+ setting.save!
+ end
+end
diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb
index 4ce862d11..6f3b761e4 100644
--- a/db/seeds/settings.rb
+++ b/db/seeds/settings.rb
@@ -1869,9 +1869,10 @@ Setting.create_if_not_exists(
},
state: 5,
preferences: {
- permission: ['admin.security'],
+ authentication: true,
+ permission: ['admin.security'],
},
- frontend: false
+ frontend: true
)
Setting.create_if_not_exists(
diff --git a/spec/db/migrate/setting_update_password_max_login_failed_spec.rb b/spec/db/migrate/setting_update_password_max_login_failed_spec.rb
new file mode 100644
index 000000000..cc752e1a5
--- /dev/null
+++ b/spec/db/migrate/setting_update_password_max_login_failed_spec.rb
@@ -0,0 +1,25 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe SettingUpdatePasswordMaxLoginFailed, type: :db_migration do
+ context 'when having old password max login failed setting values' do
+ before do
+ setting.preferences = {
+ permission: ['admin.security'],
+ }
+ setting.frontend = false
+ setting.save!
+ end
+
+ let(:setting) { Setting.find_by(name: 'password_max_login_failed') }
+
+ it 'add authentication to preferences' do
+ expect { migrate }.to change { setting.reload.preferences[:authentication] }.to(true)
+ end
+
+ it 'change frontend flag to true' do
+ expect { migrate }.to change { setting.reload.frontend }.to(true)
+ end
+ end
+end
diff --git a/spec/requests/user_spec.rb b/spec/requests/user_spec.rb
index a402e6fdb..1bc0147da 100644
--- a/spec/requests/user_spec.rb
+++ b/spec/requests/user_spec.rb
@@ -1506,4 +1506,41 @@ RSpec.describe 'User', type: :request do
.to change { Avatar.list('User', user.id) }
end
end
+
+ describe 'PUT /api/v1/users/unlock/{id}' do
+ let(:admin) { create(:admin) }
+ let(:agent) { create(:agent) }
+ let(:customer) { create(:customer, login_failed: 2) }
+
+ def make_request(id)
+ put "/api/v1/users/unlock/#{id}", params: {}, as: :json
+ end
+
+ context 'with authenticated admin user', authenticated_as: :admin do
+ it 'returns success' do
+ make_request(customer.id)
+ expect(response).to have_http_status(:ok)
+ end
+
+ it 'check that login failed was reseted' do
+ expect { make_request(customer.id) }.to change { customer.reload.login_failed }.from(2).to(0)
+ end
+
+ it 'fail with not existing user id' do
+ make_request(99_999)
+ expect(response).to have_http_status(:not_found)
+ end
+ end
+
+ context 'with authenticated agent user', authenticated_as: :agent do
+ it 'fail without admin permission' do
+ make_request(customer.id)
+ expect(response).to have_http_status(:forbidden)
+ end
+
+ it 'check that login failed was not changed' do
+ expect { make_request(customer.id) }.not_to change { customer.reload.login_failed }
+ end
+ end
+ end
end
diff --git a/spec/system/manage/users_spec.rb b/spec/system/manage/users_spec.rb
index 807265796..8072ba604 100644
--- a/spec/system/manage/users_spec.rb
+++ b/spec/system/manage/users_spec.rb
@@ -90,4 +90,24 @@ RSpec.describe 'Manage > Users', type: :system do
end
end
end
+
+ describe 'show/unlock a user', authenticated_as: -> { user } do
+ let(:user) { create(:admin) }
+ let!(:locked_user) { create(:user, login_failed: 6) }
+
+ it 'check marked locked user and execute unlock action' do
+ visit '#manage/users'
+
+ within(:active_content) do
+ row = find("tr[data-id=\"#{locked_user.id}\"]")
+
+ expect(row).to have_css('.icon-lock')
+
+ row.find('.js-action').click
+ row.find('li.unlock').click
+
+ expect(row).to have_no_css('.icon-lock')
+ end
+ end
+ end
end