diff --git a/.rubocop/todo.rspec.yml b/.rubocop/todo.rspec.yml index 1729baba2..c9c01c984 100644 --- a/.rubocop/todo.rspec.yml +++ b/.rubocop/todo.rspec.yml @@ -38,9 +38,6 @@ RSpec/ContextWording: - 'spec/jobs/imap_authentication_migration_cleanup_job_spec.rb' - 'spec/jobs/ticket_article_communicate_email_job_spec.rb' - 'spec/lib/application_handle_info_spec.rb' - - 'spec/lib/auth/developer_spec.rb' - - 'spec/lib/auth/ldap_spec.rb' - - 'spec/lib/auth_spec.rb' - 'spec/lib/core_ext/string_spec.rb' - 'spec/lib/external_credential/google_spec.rb' - 'spec/lib/external_credential/microsoft365_spec.rb' @@ -178,8 +175,6 @@ RSpec/ExampleLength: - 'spec/jobs/migrate_ldap_samaccountname_to_uid_job_spec.rb' - 'spec/jobs/ticket_user_ticket_counter_job_spec.rb' - 'spec/jobs/user_device_log_job_spec.rb' - - 'spec/lib/auth/internal_spec.rb' - - 'spec/lib/auth/ldap_spec.rb' - 'spec/lib/auto_wizard_spec.rb' - 'spec/lib/core_ext/string_spec.rb' - 'spec/lib/external_credential/google_spec.rb' @@ -395,8 +390,6 @@ RSpec/MessageSpies: - 'spec/jobs/app_version_restart_job_spec.rb' - 'spec/jobs/search_index_job_spec.rb' - 'spec/jobs/sla_ticket_rebuild_escalation_job_spec.rb' - - 'spec/lib/auth/developer_spec.rb' - - 'spec/lib/auth/ldap_spec.rb' - 'spec/lib/import/base_factory_examples.rb' - 'spec/lib/import/helper_spec.rb' - 'spec/lib/import/otrs/article/attachment_factory_spec.rb' @@ -472,10 +465,6 @@ RSpec/MultipleExpectations: - 'spec/jobs/migrate_ldap_samaccountname_to_uid_job_spec.rb' - 'spec/jobs/search_index_job_spec.rb' - 'spec/jobs/ticket_user_ticket_counter_job_spec.rb' - - 'spec/lib/auth/developer_spec.rb' - - 'spec/lib/auth/internal_spec.rb' - - 'spec/lib/auth/ldap_spec.rb' - - 'spec/lib/auth_spec.rb' - 'spec/lib/cache_spec.rb' - 'spec/lib/core_ext/string_spec.rb' - 'spec/lib/email_address_validation_spec.rb' @@ -667,7 +656,6 @@ RSpec/VerifiedDoubles: Exclude: - 'spec/db/migrate/issue_2460_fix_corrupted_twitter_ids_spec.rb' - 'spec/jobs/communicate_twitter_job_spec.rb' - - 'spec/lib/auth/ldap_spec.rb' - 'spec/lib/external_sync_spec.rb' - 'spec/lib/import/zendesk/object_attribute/base_examples.rb' - 'spec/lib/import/zendesk/object_attribute/checkbox_spec.rb' diff --git a/app/assets/javascripts/app/models/user.coffee b/app/assets/javascripts/app/models/user.coffee index 6c1a4beb1..d92327242 100644 --- a/app/assets/javascripts/app/models/user.coffee +++ b/app/assets/javascripts/app/models/user.coffee @@ -122,7 +122,7 @@ class App.User extends App.Model false maxLoginFailedReached: -> - return @login_failed > (App.Config.get('password_max_login_failed') || 10) + return @login_failed > (parseInt(App.Config.get('password_max_login_failed'))) imageUrl: -> return if !@image diff --git a/app/controllers/application_controller/authenticates.rb b/app/controllers/application_controller/authenticates.rb index 277414b6e..503649b2f 100644 --- a/app/controllers/application_controller/authenticates.rb +++ b/app/controllers/application_controller/authenticates.rb @@ -66,8 +66,8 @@ module ApplicationController::Authenticates raise Exceptions::Forbidden, 'API password access disabled!' end - user = User.authenticate(username, password) - return authentication_check_prerequesits(user, 'basic_auth', auth_param) if user + auth = Auth.new(username, password) + return authentication_check_prerequesits(auth.user, 'basic_auth', auth_param) if auth.valid? authentication_errors.push('Invalid BasicAuth credentials') end @@ -146,14 +146,6 @@ module ApplicationController::Authenticates raise Exceptions::NotAuthorized, authentication_errors.join(', ') end - def authenticate_with_password - user = User.authenticate(params[:username], params[:password]) - raise_unified_login_error if !user - - session.delete(:switched_from_user_id) - authentication_check_prerequesits(user, 'session', {}) - end - def authentication_check_prerequesits(user, auth_type, auth_param) raise Exceptions::Forbidden, 'Maintenance mode enabled!' if in_maintenance_mode?(user) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f429940ab..50b82f3ba 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -214,6 +214,14 @@ class SessionsController < ApplicationController private + def authenticate_with_password + auth = Auth.new(params[:username], params[:password]) + raise_unified_login_error if !auth.valid? + + session.delete(:switched_from_user_id) + authentication_check_prerequesits(auth.user, 'session', {}) + end + def initiate_session_for(user) request.env['rack.session.options'][:expire_after] = 1.year if params[:remember_me] session[:persistent] = true diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index daaa4ce90..87324b0ae 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -578,8 +578,9 @@ curl http://localhost/api/v1/users/password_change -v -u #{login}:#{password} -H render json: { message: 'failed', notice: ['Current password needed!'] }, status: :ok return end - user = User.authenticate(current_user.login, params[:password_old]) - if !user + + current_password_verified = PasswordHash.verified?(current_user.password, params[:password_old]) + if !current_password_verified render json: { message: 'failed', notice: ['Current password is wrong!'] }, status: :ok return end @@ -596,20 +597,19 @@ curl http://localhost/api/v1/users/password_change -v -u #{login}:#{password} -H return end - user.update!(password: params[:password_new]) + current_user.update!(password: params[:password_new]) - if user.email.present? + if current_user.email.present? NotificationFactory::Mailer.notification( template: 'password_change', - user: user, + user: current_user, objects: { - user: user, - current_user: current_user, + user: current_user, } ) end - render json: { message: 'ok', user_login: user.login }, status: :ok + render json: { message: 'ok', user_login: current_user.login }, status: :ok end =begin diff --git a/app/models/user.rb b/app/models/user.rb index fb1c9cc70..32007b4ad 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -288,54 +288,6 @@ returns =begin -authenticate user - - result = User.authenticate(username, password) - -returns - - result = user_model # user model if authentication was successfully - -=end - - def self.authenticate(username, password) - - # do not authenticate with nothing - return if username.blank? || password.blank? - - user = User.identify(username) - return if !user - - return if !Auth.can_login?(user) - - return user if Auth.valid?(user, password) - - sleep 1 - user.login_failed += 1 - user.save! - nil - end - -=begin - -checks if a user has reached the maximum of failed login tries - - user = User.find(123) - result = user.max_login_failed? - -returns - - result = true | false - -=end - - def max_login_failed? - max_login_failed = Setting.get('password_max_login_failed').to_i || 10 - login_failed > max_login_failed - end - -=begin - tries to find the matching instance by the given identifier. Currently email and login is supported. user = User.indentify('User123') @@ -352,6 +304,8 @@ returns =end def self.identify(identifier) + return if identifier.blank? + # try to find user based on login user = User.find_by(login: identifier.downcase) return user if user diff --git a/db/migrate/20210728103633_move_auth_backends_to_database.rb b/db/migrate/20210728103633_move_auth_backends_to_database.rb new file mode 100644 index 000000000..64a8709d3 --- /dev/null +++ b/db/migrate/20210728103633_move_auth_backends_to_database.rb @@ -0,0 +1,54 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class MoveAuthBackendsToDatabase < ActiveRecord::Migration[6.0] + def change + + # return if it's a new setup + return if !Setting.exists?(name: 'system_init_done') + + Setting.create_if_not_exists( + title: 'Authentication via %s', + name: 'auth_internal', + area: 'Security::Authentication', + description: 'Enables user authentication via %s.', + preferences: { + title_i18n: ['internal database'], + description_i18n: ['internal database'], + permission: ['admin.security'], + }, + state: { + priority: 1, + adapter: 'Auth::Internal', + }, + frontend: false + ) + Setting.create_if_not_exists( + title: 'Authentication via %s', + name: 'auth_developer', + area: 'Security::Authentication', + description: 'Enables user authentication via %s.', + preferences: { + title_i18n: ['developer password'], + description_i18n: ['developer password'], + permission: ['admin.security'], + }, + state: { + priority: 2, + adapter: 'Auth::Developer', + }, + frontend: false + ) + + begin + auth_ldap = Setting.find_by(name: 'auth_ldap') + + auth_ldap.state_initial[:value][:priority] = 3 + auth_ldap.state_current[:value][:priority] = 3 + + auth_ldap.save! + rescue => e + Rails.logger.error "Error while updating 'auth_ldap' Setting priority" + Rails.logger.error e + end + end +end diff --git a/db/migrate/20210729183242_set_user_source_ldap_from_external_sync.rb b/db/migrate/20210729183242_set_user_source_ldap_from_external_sync.rb new file mode 100644 index 000000000..c9f9efff8 --- /dev/null +++ b/db/migrate/20210729183242_set_user_source_ldap_from_external_sync.rb @@ -0,0 +1,16 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class SetUserSourceLdapFromExternalSync < ActiveRecord::Migration[6.0] + def change + return if !Setting.exists?(name: 'system_init_done') + + ldap_user_ids = ExternalSync.where( + source: 'Ldap::User', + object: 'User' + ).pluck(:o_id) + + User.where(id: ldap_user_ids).find_each do |user| + user.update!(source: 'Ldap') + end + end +end diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb index 6674947d9..92b84915a 100644 --- a/db/seeds/settings.rb +++ b/db/seeds/settings.rb @@ -1122,6 +1122,38 @@ Setting.create_if_not_exists( }, frontend: false ) +Setting.create_if_not_exists( + title: 'Authentication via %s', + name: 'auth_internal', + area: 'Security::Authentication', + description: 'Enables user authentication via %s.', + preferences: { + title_i18n: ['internal database'], + description_i18n: ['internal database'], + permission: ['admin.security'], + }, + state: { + priority: 1, + adapter: 'Auth::Backend::Internal', + }, + frontend: false +) +Setting.create_if_not_exists( + title: 'Authentication via %s', + name: 'auth_developer', + area: 'Security::Authentication', + description: 'Enables user authentication via %s.', + preferences: { + title_i18n: ['developer password'], + description_i18n: ['developer password'], + permission: ['admin.security'], + }, + state: { + priority: 2, + adapter: 'Auth::Backend::Developer', + }, + frontend: false +) Setting.create_if_not_exists( title: 'Authentication via %s', name: 'auth_ldap', @@ -1133,7 +1165,8 @@ Setting.create_if_not_exists( permission: ['admin.security'], }, state: { - adapter: 'Auth::Ldap', + priority: 3, + adapter: 'Auth::Backend::Ldap', host: 'localhost', port: 389, bind_dn: 'cn=Manager,dc=example,dc=org', diff --git a/lib/auth.rb b/lib/auth.rb index 3adb45fc5..7038e290f 100644 --- a/lib/auth.rb +++ b/lib/auth.rb @@ -1,115 +1,58 @@ # Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ class Auth - include ApplicationLib -=begin + attr_reader :user, :password, :auth_user -checks if a given user can login. Checks for - - valid user - - active state - - max failed logins + delegate :user, to: :auth_user - result = Auth.can_login?(user) + attr_accessor :increase_login_failed_attempts -returns + # Initializes a Auth object for the given user. + # + # @param username [String] the user name for the user object which needs an authentication. + # + # @example + # auth = Auth.new('master@example.com', 'some+password') + def initialize(username, password) + @lookup_backend_instance = {} - result = true | false + @auth_user = username.present? ? Auth::User.new(username) : nil + @password = password -=end + @increase_login_failed_attempts = false + end - def self.can_login?(user) - return false if !user.is_a?(User) - return false if !user.active? + # Validates the given credentials for the user to the configured auth backends which should + # be performed. + # + # @return [Boolean] true if the user was authenticated, otherwise false. + def valid? + if !auth_user || !auth_user.can_login? + avoid_brute_force_attack - return true if !user.max_login_failed? + return false + end - Rails.logger.info "Max login failed reached for user #{user.login}." + if backends.valid? + auth_user.update_last_login + return true + end + avoid_brute_force_attack + + auth_user.increase_login_failed if increase_login_failed_attempts false end -=begin + private -checks if a given user and password match against multiple auth backends - - valid user - - active state - - max failed logins - - result = Auth.valid?(user, password) - -returns - - result = true | false - -=end - - def self.valid?(user, password) - # try to login against configure auth backends - backends.any? do |config| - next if !backend_validates?( - config: config, - user: user, - password: password, - ) - - Rails.logger.info "Authentication against #{config[:adapter]} for user #{user.login} ok." - - # remember last login date - user.update_last_login - - true - end + # Sleep for a second to avoid brute force attacks. + def avoid_brute_force_attack + sleep 1 end -=begin - -returns a list of all Auth backend configurations - - result = Auth.backends - -returns - - result = [ - { - adapter: 'Auth::Internal', - }, - { - adapter: 'Auth::Developer', - }, - ... - ] - -=end - - def self.backends - - # use std. auth backends - config = [ - { - adapter: 'Auth::Internal', - }, - { - adapter: 'Auth::Developer', - }, - ] - - # added configured backends - Setting.where(area: 'Security::Authentication').each do |setting| - next if setting.state_current[:value].blank? - - config.push setting.state_current[:value] - end - - config + def backends + Auth::Backend.new(self) end - - def self.backend_validates?(config:, user:, password:) - return false if !config[:adapter] - - instance = config[:adapter].constantize.new(config) - - instance.valid?(user, password) - end - private_class_method :backend_validates? end diff --git a/lib/auth/backend.rb b/lib/auth/backend.rb new file mode 100644 index 000000000..181237d23 --- /dev/null +++ b/lib/auth/backend.rb @@ -0,0 +1,41 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Auth + class Backend + + attr_reader :auth + + def initialize(auth) + @auth = auth + end + + def valid? + instances.any? do |instance| + next if !instance.valid? + + Rails.logger.info "Authentication against #{instance.class.name} for user #{auth.user.login} ok." + + true + end + end + + private + + def instances + configs.filter_map do |config| + config[:adapter].constantize.new(config, auth) + rescue => e + Rails.logger.error "Failed to load Auth::Backend from Setting '#{config}'" + Rails.logger.error e + nil + end + end + + def configs + Setting.where(area: 'Security::Authentication') + .map { |setting| setting.state_current[:value] } # extract current Setting value as config + .reject(&:blank?) + .sort { |a, b| a.fetch(:priority, 999) <=> b.fetch(:priority, 999) } # sort by priority and fallback to append if not set + end + end +end diff --git a/lib/auth/backend/base.rb b/lib/auth/backend/base.rb new file mode 100644 index 000000000..532bda2cf --- /dev/null +++ b/lib/auth/backend/base.rb @@ -0,0 +1,40 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Auth + class Backend + class Base + + delegate :user, :password, to: :auth + + attr_reader :config, :auth + + # Base initialization for Auth backend object. + # + # @param config [Hash] backend configuration hash. + # @param auth [Auth] the Auth object for the authentication. + # + # @example + # auth = Auth::Backend::Internal.new('master@example.com', auth) + def initialize(config, auth) + @config = config + @auth = auth + end + + def valid? + return false if !perform? + + authenticated? + end + + private + + def perform? + raise NotImplementedError + end + + def authenticated? + raise NotImplementedError + end + end + end +end diff --git a/lib/auth/backend/developer.rb b/lib/auth/backend/developer.rb new file mode 100644 index 000000000..4c63b5a83 --- /dev/null +++ b/lib/auth/backend/developer.rb @@ -0,0 +1,52 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Auth + class Backend + class Developer < Auth::Backend::Base + + private + + # Special development/test password validation. + # * For the developer mode the password 'test' is allowed for every User. + # * For the test environment the password can be blank if also the user password is currently blank. + # + # @returns [Boolean] true if the validation works, otherwise false. + def authenticated? + if valid_for_developer_mode? || valid_for_test_environment? + Rails.logger.info "System in test/developer mode, authentication for user #{user.login} ok." + return true + end + + false + end + + # Overwrites the default behaviour to check for a allowed environment. + # + # @returns [Boolean] true if the environment is development or test. + def perform? + allowed_environment? + end + + # Check for allowed environments. + # + # @returns [Boolean] true if one allowed environment is active. + def allowed_environment? + Setting.get('developer_mode') == true || Rails.env.test? + end + + # Validate password for test environment. + # + # @returns [Boolean] true if password and user password is blank, otherwise false. + def valid_for_test_environment? + Rails.env.test? && password.blank? && user.password.blank? + end + + # Validate password for test environment. + # + # @returns [Boolean] true if the given password is 'test', otherwise false. + def valid_for_developer_mode? + Setting.get('developer_mode') == true && password == 'test' + end + end + end +end diff --git a/lib/auth/backend/internal.rb b/lib/auth/backend/internal.rb new file mode 100644 index 000000000..459c45c13 --- /dev/null +++ b/lib/auth/backend/internal.rb @@ -0,0 +1,42 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Auth + class Backend + class Internal < Auth::Backend::Base + + private + + # Validation against the internal database. + # + # @returns [Boolean] true if the validation works, otherwise false. + def authenticated? + return true if hash_matches? + + auth.increase_login_failed_attempts = true + + false + end + + # Overwrites the default behaviour to only perform this authentication if an internal password exists. + # + # @returns [Boolean] true if a internal password for the user is present. + def perform? + return false if password.blank? + return false if !user.verified && user.source == 'signup' + + user.password.present? + end + + def hash_matches? + # Because of legacy reason a special check exists and afterwards the + # password will be saved in the current format. + if PasswordHash.legacy?(user.password, password) + user.update!(password: password) + return true + end + + PasswordHash.verified?(user.password, password) + end + end + end +end diff --git a/lib/auth/backend/ldap.rb b/lib/auth/backend/ldap.rb new file mode 100644 index 000000000..bf090aaa4 --- /dev/null +++ b/lib/auth/backend/ldap.rb @@ -0,0 +1,74 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Auth + class Backend + class Ldap < Auth::Backend::Base + + private + + # Validation against the configured ldap integration. + # + # @returns [Boolean] true if the validation works, otherwise false. + def authenticated? + ldap_user = ::Ldap::User.new + + # get from config or fallback to login + # for a list of user attributes which should + # be used for logging in + login_attributes = config[:login_attributes] || %w[login] + + authed = login_attributes.any? do |attribute| + ldap_user.valid?(user[attribute], password) + end + + log_auth_result(authed) + authed + rescue => e + message = "Can't connect to ldap backend #{e}" + Rails.logger.info message + Rails.logger.info e + log( + status: 'failed', + response: message, + ) + false + end + + # Checks the default behaviour and as a addition if the ldap integration is currently active. + # + # @returns [Boolean] true if the ldap integration is active and the default behaviour matches. + def perform? + user.source == 'Ldap' && Setting.get('ldap_integration') + end + + # Logs the auth result + # + # @param authed [Boolean] true if the user is authed, otherwise false. + def log_auth_result(authed) + result = authed ? 'success' : 'failed' + log( + status: result, + ) + end + + # Created the http log for the current authentication. + # + # @param status [String] the status of the ldap authentication. + # @param response [String] the response message. + def log(status:, response: nil) + HttpLog.create( + direction: 'out', + facility: 'ldap', + url: "bind -> #{user.login}", + status: status, + ip: nil, + request: { content: user.login }, + response: { content: response || status }, + method: 'tcp', + created_by_id: 1, + updated_by_id: 1, + ) + end + end + end +end diff --git a/lib/auth/base.rb b/lib/auth/base.rb deleted file mode 100644 index 8cda7595f..000000000 --- a/lib/auth/base.rb +++ /dev/null @@ -1,14 +0,0 @@ -# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ - -class Auth - class Base - - def initialize(config) - @config = config - end - - def valid?(_user, _password) - raise "Missing implementation of method 'valid?' for class '#{self.class.name}'" - end - end -end diff --git a/lib/auth/developer.rb b/lib/auth/developer.rb deleted file mode 100644 index fc07234d1..000000000 --- a/lib/auth/developer.rb +++ /dev/null @@ -1,15 +0,0 @@ -# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ - -class Auth - class Developer < Auth::Base - - def valid?(user, password) - return false if user.blank? - return false if Setting.get('developer_mode') != true - return false if password != 'test' - - Rails.logger.info "System in developer mode, authentication for user #{user.login} ok." - true - end - end -end diff --git a/lib/auth/internal.rb b/lib/auth/internal.rb deleted file mode 100644 index cb9adb766..000000000 --- a/lib/auth/internal.rb +++ /dev/null @@ -1,29 +0,0 @@ -# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ - -class Auth - class Internal < Auth::Base - - def valid?(user, password) - - return false if user.blank? - - if PasswordHash.legacy?(user.password, password) - update_password(user, password) - return true - end - - password_verified = PasswordHash.verified?(user.password, password) - - raise Exceptions::Forbidden, 'Please verify your account before you can login!' if !user.verified && user.source == 'signup' && password_verified - - password_verified - end - - private - - def update_password(user, password) - user.password = PasswordHash.crypt(password) - user.save - end - end -end diff --git a/lib/auth/ldap.rb b/lib/auth/ldap.rb deleted file mode 100644 index 330447ed5..000000000 --- a/lib/auth/ldap.rb +++ /dev/null @@ -1,59 +0,0 @@ -# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ - -class Auth - class Ldap < Auth::Base - - def valid?(user, password) - return false if !Setting.get('ldap_integration') - - ldap_user = ::Ldap::User.new - - # get from config or fallback to login - # for a list of user attributes which should - # be used for logging in - login_attributes = @config[:login_attributes] || %w[login] - - authed = login_attributes.any? do |attribute| - ldap_user.valid?(user[attribute], password) - end - - log_auth_result(user, authed) - authed - rescue => e - message = "Can't connect to ldap backend #{e}" - Rails.logger.info message - Rails.logger.info e - log( - user: user, - status: 'failed', - response: message, - ) - false - end - - private - - def log_auth_result(user, authed) - result = authed ? 'success' : 'failed' - log( - user: user, - status: result, - ) - end - - def log(user:, status:, response: nil) - HttpLog.create( - direction: 'out', - facility: 'ldap', - url: "bind -> #{user.login}", - status: status, - ip: nil, - request: { content: user.login }, - response: { content: response || status }, - method: 'tcp', - created_by_id: 1, - updated_by_id: 1, - ) - end - end -end diff --git a/lib/auth/user.rb b/lib/auth/user.rb new file mode 100644 index 000000000..f5a39d50c --- /dev/null +++ b/lib/auth/user.rb @@ -0,0 +1,52 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Auth + class User < SimpleDelegator + + attr_reader :user + + def initialize(username) + @user = ::User.identify(username) + super(@user) + end + + # Checks if a given user can login. Check for the following criteria: + # * valid user + # * user is active + # * user has not reached the maximum of failed login tries + # + # @return [Boolean] true if the user can login, false otherwise. + def can_login? + return false if !exists? + return false if !active? + + !max_login_failed? + end + + # Increase the current failed login count for the user. + def increase_login_failed + self.login_failed += 1 + save! + end + + private + + # Checks if a user has reached the maximum of failed login tries. + # + # @return [Boolean] true if the user has reached the maximum of failed login tries, otherwise false. + def max_login_failed? + max_login_failed = Setting.get('password_max_login_failed').to_i + return false if login_failed <= max_login_failed + + Rails.logger.info "Max login failed reached for user #{login}." + true + end + + # Checks if a valid user exists. + # + # @return [Boolean] true if a valid user exists, otherwise false. + def exists? + present? && __getobj__.is_a?(::User) + end + end +end diff --git a/lib/sequencer/unit/import/ldap/user/attributes/static.rb b/lib/sequencer/unit/import/ldap/user/attributes/static.rb index cd8aa2828..2649ca4b2 100644 --- a/lib/sequencer/unit/import/ldap/user/attributes/static.rb +++ b/lib/sequencer/unit/import/ldap/user/attributes/static.rb @@ -19,6 +19,9 @@ class Sequencer # because otherwise disabled instances won't get # re-activated if they should get synced again active: true, + + # Set the source to 'Ldap' for the authentication handling. + source: 'Ldap', } end end diff --git a/spec/db/migrate/set_user_source_ldap_from_external_sync_spec.rb b/spec/db/migrate/set_user_source_ldap_from_external_sync_spec.rb new file mode 100644 index 000000000..047661eb9 --- /dev/null +++ b/spec/db/migrate/set_user_source_ldap_from_external_sync_spec.rb @@ -0,0 +1,30 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe SetUserSourceLdapFromExternalSync, type: :db_migration, db_strategy: :reset do + let(:users) { create_list(:user, 2) } + let(:other_user) { create(:user) } + + before do + 2.times do |count| + index = count - 1 + + create(:external_sync, + source: 'Ldap::User', + source_id: "uid=#{users[index].login},ou=People,dc=example,dc=org", + object: 'User', + o_id: users[index].id) + end + end + + context 'when having users from the ldap integration' do + it 'source key for users are filled' do + expect { migrate }.to change { users[0].reload.source }.to('Ldap').and change { users[1].reload.source }.to('Ldap') + end + + it 'other user should not be touched' do + expect { migrate }.not_to change { other_user.reload.source } + end + end +end diff --git a/spec/factories/setting.rb b/spec/factories/setting.rb index d08c96859..0df9ca5ae 100644 --- a/spec/factories/setting.rb +++ b/spec/factories/setting.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory :setting do title { 'ABC API Token' } - name { 'abc_api_token' } + name { Faker::Name.unique.name } area { 'Integration::ABC' } description { 'API Token for ABC to access ABC.' } frontend { false } diff --git a/spec/lib/auth/backend_examples.rb b/spec/lib/auth/backend/backend_examples.rb similarity index 100% rename from spec/lib/auth/backend_examples.rb rename to spec/lib/auth/backend/backend_examples.rb diff --git a/spec/lib/auth/base_spec.rb b/spec/lib/auth/backend/base_spec.rb similarity index 51% rename from spec/lib/auth/base_spec.rb rename to spec/lib/auth/backend/base_spec.rb index 9a8506f04..3299753f2 100644 --- a/spec/lib/auth/base_spec.rb +++ b/spec/lib/auth/backend/base_spec.rb @@ -1,21 +1,15 @@ # Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ require 'rails_helper' -require 'lib/auth/backend_examples' +require 'lib/auth/backend/backend_examples' -RSpec.describe Auth::Base do +RSpec.describe Auth::Backend::Base do let(:user) { create(:user) } - let(:instance) { described_class.new({ adapter: described_class.name }) } + let(:auth) { Auth.new(user.login, 'not_used') } + let(:instance) { described_class.new({ adapter: described_class.name }, auth) } describe '#valid?' do it_behaves_like 'Auth backend' - - it "requires an implementation of the 'valid?' method" do - - expect do - instance.valid?(user, 'password') - end.to raise_error(RuntimeError) - end end end diff --git a/spec/lib/auth/backend/developer_spec.rb b/spec/lib/auth/backend/developer_spec.rb new file mode 100644 index 000000000..b024b7f82 --- /dev/null +++ b/spec/lib/auth/backend/developer_spec.rb @@ -0,0 +1,94 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' +require 'lib/auth/backend/backend_examples' + +RSpec.describe Auth::Backend::Developer do + + let(:user) { create(:user) } + let(:password) { 'not_used' } + let(:auth) { Auth.new(user.login, password) } + let(:instance) { described_class.new({ adapter: described_class.name }, auth) } + + describe '#valid?' do + it_behaves_like 'Auth backend' + + context 'when Setting developer_mode is true' do + + before do + Setting.set('developer_mode', true) + end + + context 'when password is "test"' do + + let(:password) { 'test' } + + it 'authenticates' do + expect(instance.valid?).to be true + end + end + + context 'when password matches actual User password' do + + let(:user) { create(:user, password: 'secure') } + let(:password) { user.password_plain } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + end + + context 'when Rails.env is "test"' do + + before do + allow(Rails).to receive(:env) { 'test'.inquiry } # rubocop:disable Rails/Inquiry + end + + context 'when password is blank' do + + let(:password) { '' } + + it 'authenticates' do + expect(instance.valid?).to be true + end + end + + context 'when password matches actual User password' do + + let(:user) { create(:user, password: 'secure') } + let(:password) { user.password_plain } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + end + + context 'when Rails.env is "production"' do + + before do + allow(Rails).to receive(:env) { 'production'.inquiry } # rubocop:disable Rails/Inquiry + end + + context 'when password is blank' do + + let(:password) { '' } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + + context 'when password matches actual User password' do + + let(:user) { create(:user, password: 'secure') } + let(:password) { user.password_plain } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + end + end +end diff --git a/spec/lib/auth/backend/internal_spec.rb b/spec/lib/auth/backend/internal_spec.rb new file mode 100644 index 000000000..7704b329e --- /dev/null +++ b/spec/lib/auth/backend/internal_spec.rb @@ -0,0 +1,89 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' +require 'lib/auth/backend/backend_examples' + +RSpec.describe Auth::Backend::Internal do + + let(:user) { create(:user) } + let(:password) { 'secure' } + let(:auth) { Auth.new(user.login, password) } + let(:instance) { described_class.new({ adapter: described_class.name }, auth) } + + describe '#valid?' do + it_behaves_like 'Auth backend' + + context 'when password is given' do + let(:user) { create(:user, password: password) } + + it 'authenticates' do + expect(instance.valid?).to be true + end + end + + context 'when given password matches stored hash' do + + let(:password) { user.password } + let(:user) { create(:user, password: 'secure') } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + + context 'when given password is blank' do + let(:password) { '' } + let(:user) { create(:user, password: 'secure') } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + + context 'with legacy SHA2 passwords' do + let(:user) { create(:user, password: PasswordHash.sha2(password)) } + + it 'is password hash crypted' do + expect(PasswordHash.crypted?(user.password)).to be true + end + + it 'is password hash legacy' do + expect(PasswordHash.legacy?(user.password, password)).to be true + end + + it 'valid authentication' do + expect(instance.valid?).to be true + end + + it 'is password hash not legacy after validation' do + instance.valid? + expect(PasswordHash.legacy?(user.reload.password, password)).to be false + end + + it 'is password hash crypted after validation' do + instance.valid? + expect(PasswordHash.crypted?(user.password)).to be true + end + end + + context 'when affecting Auth#increase_login_failed_attempts' do + + context 'when authentication fails' do + let(:password) { 'wrong' } + let(:user) { create(:user, password: 'secure') } + + it 'sets Auth#increase_login_failed_attempts flag to true' do + expect { instance.valid? }.to change(auth, :increase_login_failed_attempts).from(false).to(true) + end + end + + context 'when authentication succeeds' do + let(:user) { create(:user, password: password) } + + it "doesn't change Auth#increase_login_failed_attempts flag" do + expect { instance.valid? }.not_to change(auth, :increase_login_failed_attempts) + end + end + end + end +end diff --git a/spec/lib/auth/backend/ldap_spec.rb b/spec/lib/auth/backend/ldap_spec.rb new file mode 100644 index 000000000..b3727b569 --- /dev/null +++ b/spec/lib/auth/backend/ldap_spec.rb @@ -0,0 +1,91 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' +require 'lib/auth/backend/backend_examples' + +RSpec.describe ::Auth::Backend::Ldap do + + let(:user) { create(:user, source: 'Ldap') } + let(:password) { 'secure' } + let(:auth) { Auth.new(user.login, password) } + let(:config) do + { + adapter: described_class.name + } + end + let(:instance) { described_class.new(config, auth) } + let(:ldap_integration) { true } + + before do + Setting.set('ldap_integration', ldap_integration) + end + + describe '#valid?' do + let(:ldap_user) { instance_double(Ldap::User) } + + before do + allow(Ldap::User).to receive(:new).with(any_args).and_return(ldap_user) + allow(ldap_user).to receive(:valid?).with(any_args).and_return(true) + end + + it_behaves_like 'Auth backend' + + it 'authenticates users' do + expect(instance.valid?).to be true + end + + context 'when custom login attribute is configured' do + + let(:config) do + super().merge( + login_attributes: %w[firstname] + ) + end + + it 'authenticates' do + allow(ldap_user).to receive(:valid?).with(user.firstname, password).and_return(true) + + expect(instance.valid?).to be true + end + end + + context 'when Setting ldap_integration is false' do + + let(:ldap_integration) { false } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + + context 'when LDAP authentication fails' do + + it "doesn't authenticate" do + allow(ldap_user).to receive(:valid?).with(any_args).and_return(false) + + expect(instance.valid?).to be false + end + end + + context 'when User#source does not match Ldap' do + + context 'when blank' do + + let(:user) { create(:user) } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + + context 'when some other value' do + + let(:user) { create(:user, source: 'some other value') } + + it "doesn't authenticate" do + expect(instance.valid?).to be false + end + end + end + end +end diff --git a/spec/lib/auth/backend_spec.rb b/spec/lib/auth/backend_spec.rb new file mode 100644 index 000000000..29cb36e9b --- /dev/null +++ b/spec/lib/auth/backend_spec.rb @@ -0,0 +1,108 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe Auth::Backend do + + let(:user) { create(:user) } + let(:password) { 'secure' } + let(:auth) { Auth.new(user.login, password) } + let(:instance) { described_class.new(auth) } + + describe '#valid?' do + + context 'when invalid Setting is present in DB' do + + context 'when value is blank' do + + before do + create(:setting, + area: 'Security::Authentication', + state: {},) + end + + it "doesn't raise an exception" do + expect { instance.valid? }.not_to raise_exception + end + end + + context "when adapter can't be constantized" do + before do + create(:setting, + area: 'Security::Authentication', + state: { + adapter: 'This::Will::Never::Work' + },) + end + + it "doesn't raise an exception" do + expect { instance.valid? }.not_to raise_exception + end + end + end + + context 'when backend prioritization is relevant' do + + let(:previous_class_namespace) { 'Auth::Backend::TopPrio' } + let(:later_class_namespace) { 'Auth::Backend::LeastPrio' } + + let(:previous_backend_class) { Class.new(Auth::Backend::Base) } + let(:later_backend_class) { Class.new(Auth::Backend::Base) } + + let(:previous_backend_instance) { instance_double(previous_class_namespace) } + let(:later_backend_instance) { instance_double(later_class_namespace) } + + before do + stub_const previous_class_namespace, previous_backend_class + stub_const later_class_namespace, later_backend_class + + Setting.where(area: 'Security::Authentication').destroy_all + + create(:setting, + area: 'Security::Authentication', + state: { + adapter: previous_class_namespace, + priority: 1 + },) + create(:setting, + area: 'Security::Authentication', + state: { + adapter: later_class_namespace, + priority: 2 + },) + + allow(previous_class_namespace.constantize).to receive(:new).and_return(previous_backend_instance) + allow(later_class_namespace.constantize).to receive(:new).and_return(later_backend_instance) + + allow(previous_backend_instance).to receive(:valid?) + allow(later_backend_instance).to receive(:valid?) + end + + context 'when previous backend was valid' do + + before do + allow(previous_backend_instance).to receive(:valid?).and_return(true) + end + + it "doesn't call valid on later backend" do + instance.valid? + + expect(later_backend_instance).not_to have_received(:valid?) + end + end + + context 'when previous backend was not valid' do + + before do + allow(previous_backend_instance).to receive(:valid?).and_return(false) + end + + it 'calls valid on later backend' do + instance.valid? + + expect(later_backend_instance).to have_received(:valid?) + end + end + end + end +end diff --git a/spec/lib/auth/developer_spec.rb b/spec/lib/auth/developer_spec.rb deleted file mode 100644 index 80f7227cb..000000000 --- a/spec/lib/auth/developer_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ - -require 'rails_helper' -require 'lib/auth/backend_examples' - -RSpec.describe Auth::Developer do - - let(:user) { create(:user) } - let(:instance) { described_class.new({ adapter: described_class.name }) } - - describe '#valid?' do - it_behaves_like 'Auth backend' - - it "authenticates users with password 'test'" do - - allow(Setting).to receive(:get) - allow(Setting).to receive(:get).with('developer_mode').and_return(true) - - result = instance.valid?(user, 'test') - - expect(result).to be true - end - - context 'invalid' do - - let(:password) { 'zammad' } - - it "doesn't authenticate if developer mode is off" do - - allow(Setting).to receive(:get) - allow(Setting).to receive(:get).with('developer_mode').and_return(false) - - result = instance.valid?(user, password) - - expect(result).to be false - end - - it "doesn't authenticate with correct password" do - - allow(Setting).to receive(:get) - allow(Setting).to receive(:get).with('developer_mode').and_return(true) - - result = instance.valid?(user, password) - - expect(result).to be false - end - end - end -end diff --git a/spec/lib/auth/internal_spec.rb b/spec/lib/auth/internal_spec.rb deleted file mode 100644 index b899a53e9..000000000 --- a/spec/lib/auth/internal_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ - -require 'rails_helper' -require 'lib/auth/backend_examples' - -RSpec.describe Auth::Internal do - - let(:password) { 'zammad' } - let(:user) { create(:user, password: password) } - let(:instance) { described_class.new({ adapter: described_class.name }) } - - describe '#valid?' do - it_behaves_like 'Auth backend' - - it 'authenticates via password' do - result = instance.valid?(user, password) - expect(result).to be true - end - - it "doesn't authenticate via plain password" do - result = instance.valid?(user, user.password) - expect(result).to be_falsy - end - - it 'converts legacy sha2 passwords' do - - sha2 = PasswordHash.sha2(password) - user = create(:user, password: sha2) - - expect(PasswordHash.crypted?(user.password)).to be true - expect(PasswordHash.legacy?(user.password, password)).to be true - - result = instance.valid?(user, password) - expect(result).to be true - - expect(PasswordHash.legacy?(user.password, password)).to be false - expect(PasswordHash.crypted?(user.password)).to be true - end - end -end diff --git a/spec/lib/auth/ldap_spec.rb b/spec/lib/auth/ldap_spec.rb deleted file mode 100644 index ebbd50809..000000000 --- a/spec/lib/auth/ldap_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ - -require 'rails_helper' -require 'lib/auth/backend_examples' -require 'auth/ldap' - -RSpec.describe ::Auth::Ldap do - - let(:user) { create(:user) } - let(:password) { 'somepassword' } - let(:instance) { described_class.new({ adapter: described_class.name }) } - - describe '#valid?' do - it_behaves_like 'Auth backend' - - it 'authenticates users' do - - allow(Setting).to receive(:get) - allow(Setting).to receive(:get).with('ldap_integration').and_return(true) - - ldap_user = double(valid?: true) - allow(::Ldap::User).to receive(:new).and_return(ldap_user) - - result = instance.valid?(user, password) - expect(result).to be true - end - - it 'authenticates via configurable user attributes' do - - allow(Setting).to receive(:get) - allow(Setting).to receive(:get).with('ldap_integration').and_return(true) - - instance = described_class.new( - adapter: described_class.name, - login_attributes: %w[firstname], - ) - - ldap_user = double - allow(ldap_user).to receive(:valid?).with(user.firstname, password).and_return(true) - - allow(::Ldap::User).to receive(:new).and_return(ldap_user) - - result = instance.valid?(user, password) - expect(result).to be true - end - - context 'invalid' do - - it "doesn't authenticate if 'ldap_integration' Setting is disabled" do - - allow(Setting).to receive(:get) - allow(Setting).to receive(:get).with('ldap_integration').and_return(false) - - result = instance.valid?(user, password) - expect(result).to be false - end - - it "doesn't authenticate if ldap says 'nope'" do - - allow(Setting).to receive(:get) - allow(Setting).to receive(:get).with('ldap_integration').and_return(true) - - ldap_user = double(valid?: false) - allow(::Ldap::User).to receive(:new).and_return(ldap_user) - - result = instance.valid?(user, password) - expect(result).to be false - end - end - end -end diff --git a/spec/lib/auth/user_spec.rb b/spec/lib/auth/user_spec.rb new file mode 100644 index 000000000..39d552ad7 --- /dev/null +++ b/spec/lib/auth/user_spec.rb @@ -0,0 +1,97 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe Auth::User do + + let(:user) { create(:user) } + let(:instance) { described_class.new(user.login) } + + describe '.can_login?' do + it 'responds to can_login?' do + expect(instance).to respond_to(:can_login?) + end + + shared_examples 'check loginable' do + it 'checks if users can login' do + expect(instance.can_login?).to be true + end + end + + shared_examples 'check not loginable' do + it 'check that user can not login' do + expect(instance.can_login?).to be false + end + end + + context 'with valid user login' do + include_examples 'check loginable' + end + + context 'with to many failed logins' do + let(:user) { create(:user, login_failed: 999) } + + include_examples 'check not loginable' + end + + context 'with not active user' do + let(:user) { create(:user, active: false) } + + include_examples 'check not loginable' + end + + context 'with invalid instance username parameter' do + let(:instance) { described_class.new('not_existing') } + + include_examples 'check not loginable' + end + + context 'with empty instance username parameter' do + let(:instance) { described_class.new('') } + + include_examples 'check not loginable' + end + + context 'with given default password_max_login_failed' do + context 'with 5 attempts' do + let(:user) { create(:user, login_failed: 5) } + + include_examples 'check loginable' + end + + context 'with 6 attempts' do + let(:user) { create(:user, login_failed: 6) } + + include_examples 'check not loginable' + end + end + + context 'when "password_max_login_failed" Setting is changed' do + + context 'when changed to lower value' do + before do + Setting.set('password_max_login_failed', 5) + user.update(login_failed: 6) + end + + include_examples 'check not loginable' + end + + context 'when changed to nil' do + before do + Setting.set('password_max_login_failed', nil) + end + + include_examples 'check loginable' + + context 'when User login failed once' do + before do + user.update(login_failed: 1) + end + + include_examples 'check not loginable' + end + end + end + end +end diff --git a/spec/lib/auth_spec.rb b/spec/lib/auth_spec.rb index 489a46b1d..7bedbcefb 100644 --- a/spec/lib/auth_spec.rb +++ b/spec/lib/auth_spec.rb @@ -3,75 +3,242 @@ require 'rails_helper' RSpec.describe Auth do - - describe '.can_login?' do - it 'responds to can_login?' do - expect(described_class).to respond_to(:can_login?) - end - - it 'checks if users can login' do - user = create(:user) - result = described_class.can_login?(user) - expect(result).to be true - end - - context 'not loginable' do - - it 'fails if user has too many failed logins' do - user = create(:user, login_failed: 999) - result = described_class.can_login?(user) - expect(result).to be false - end - - it "fails if user isn't active" do - user = create(:user, active: false) - result = described_class.can_login?(user) - expect(result).to be false - end - - it 'fails if parameter is no User instance' do - result = described_class.can_login?('user') - expect(result).to be false - end - end - - context 'given default password_max_login_failed' do - it 'passes with 5 attempts' do - user = create(:user, login_failed: 5) - result = described_class.can_login?(user) - expect(result).to be true - end - - it 'fails with 6 attempts' do - user = create(:user, login_failed: 6) - result = described_class.can_login?(user) - expect(result).to be false - end - end - end + let(:password) { 'zammad' } + let(:user) { create(:user, password: password) } + let(:instance) { described_class.new(user.login, password) } describe '.valid?' do it 'responds to valid?' do - expect(described_class).to respond_to(:valid?) + expect(instance).to respond_to(:valid?) end - it 'authenticates users' do - password = 'zammad' - user = create(:user, password: password) - result = described_class.valid?(user, password) - expect(result).to be true - end - end + context 'with an internal user' do + context 'with valid credentials' do + it 'check for valid credentials' do + expect(instance.valid?).to be true + end - describe '.backends' do - it 'responds to backends' do - expect(described_class).to respond_to(:backends) + it 'check for not increased failed login count' do + expect { instance.valid? }.not_to change { user.reload.login_failed } + end + + context 'when not case-sensitive' do + let(:instance) { described_class.new(user.login.upcase, password) } + + it 'returns true' do + expect(instance.valid?).to be true + end + end + + context 'when email is used' do + let(:instance) { described_class.new(user.email, password) } + + it 'check for valid credentials' do + expect(instance.valid?).to be true + end + end + + context 'when previous login was' do + context 'when never logged in' do + it 'updates #last_login and #updated_at' do + expect { instance.valid? }.to change { user.reload.last_login }.and change { user.reload.updated_at } + end + end + + context 'when less than 10 minutes ago' do + before do + instance.valid? + travel 9.minutes + end + + it 'does not update #last_login and #updated_at' do + expect { instance.valid? }.to not_change { user.reload.last_login }.and not_change { user.reload.updated_at } + end + end + + context 'when more than 10 minutes ago' do + before do + instance.valid? + travel 11.minutes + end + + it 'updates #last_login and #updated_at' do + expect { instance.valid? }.to change { user.reload.last_login }.and change { user.reload.updated_at } + end + end + end + end + + context 'with valid user and invalid password' do + let(:instance) { described_class.new(user.login, 'wrong') } + + it 'check for invalid credentials' do + expect(instance.valid?).to be false + end + + it 'check for increased failed login count' do + expect { instance.valid? }.to change { user.reload.login_failed }.from(0).to(1) + end + + it 'failed login avoids brute force attack' do + allow(instance).to receive(:sleep) + instance.valid? + expect(instance).to have_received(:sleep).with(1) + end + end + + context 'with inactive user login' do + let(:user) { create(:user, active: false) } + + it 'returns false' do + expect(instance.valid?).to be false + end + end + + context 'with non-existent user login' do + let(:instance) { described_class.new('not_existing', password) } + + it 'returns false' do + expect(instance.valid?).to be false + end + end + + context 'with empty user login' do + let(:instance) { described_class.new('', password) } + + it 'returns false' do + expect(instance.valid?).to be false + end + end + + context 'when password is empty' do + before do + # Remove adapter from auth developer setting, to avoid execution for this test case, because of special empty + # password handling in adapter. + Setting.set('auth_developer', {}) + end + + context 'with empty password string' do + let(:password) { '' } + + it 'returns false' do + expect(instance.valid?).to be false + end + end + + shared_examples 'check empty password' do + context 'when password is an empty string' do + let(:password) { '' } + + it 'returns false' do + expect(instance.valid?).to be false + end + end + + context 'when password is nil' do + let(:password) { nil } + + it 'returns false' do + expect(instance.valid?).to be false + end + end + end + + context 'with empty password string when the stored password is an empty string' do + before { user.update_column(:password, '') } + + include_examples 'check empty password' + end + + context 'with empty password string when the stored hash represents an empty string' do + before { user.update(password: PasswordHash.crypt('')) } + + include_examples 'check empty password' + end + end end - it 'returns a list of Hashes' do - result = described_class.backends - expect(result).to be_an(Array) - expect(result.first).to be_a(Hash) + context 'with a ldap user' do + let(:password_ldap) { 'zammad_ldap' } + let(:ldap_user) { instance_double(Ldap::User) } + + before do + Setting.set('ldap_integration', true) + + allow(Ldap::User).to receive(:new).with(any_args).and_return(ldap_user) + end + + context 'with a ldap user without internal password' do + let(:user) { create(:user, source: 'Ldap') } + let(:password) { password_ldap } + + context 'with valid credentials' do + before do + allow(ldap_user).to receive(:valid?).with(any_args).and_return(true) + end + + it 'returns true' do + expect(instance.valid?).to be true + end + end + + context 'with invalid credentials' do + let(:password) { 'wrong' } + + before do + allow(ldap_user).to receive(:valid?).with(any_args).and_return(false) + end + + it 'returns false' do + expect(instance.valid?).to be false + end + + it 'check for not increased failed login count' do + expect { instance.valid? }.not_to change { user.reload.login_failed } + end + end + end + + context 'with a ldap user which also has a internal password' do + let(:user) { create(:user, source: 'Ldap', password: password) } + let(:password) { password_ldap } + + context 'with valid ldap credentials' do + before do + allow(ldap_user).to receive(:valid?).with(any_args).and_return(true) + end + + it 'returns true' do + expect(instance.valid?).to be true + end + end + + context 'with invalid ldap credentials' do + let(:instance) { described_class.new(user.login, 'wrong') } + + before do + allow(ldap_user).to receive(:valid?).with(any_args).and_return(false) + end + + it 'returns false' do + expect(instance.valid?).to be false + end + + it 'check for not increased failed login count' do + expect { instance.valid? }.to change { user.reload.login_failed }.from(0).to(1) + end + end + + context 'with valid internal credentials' do + before do + allow(ldap_user).to receive(:valid?).with(any_args).and_return(false) + end + + it 'returns true' do + expect(instance.valid?).to be true + end + end + end end end end diff --git a/spec/lib/auto_wizard_spec.rb b/spec/lib/auto_wizard_spec.rb index 131140eb6..02b0c13d0 100644 --- a/spec/lib/auto_wizard_spec.rb +++ b/spec/lib/auto_wizard_spec.rb @@ -63,7 +63,7 @@ RSpec.describe AutoWizard do .and change { User.last.firstname }.to('Test Master') .and change { User.last.lastname }.to('Agent') .and change { User.last.email }.to('master_unit_test01@example.com') - .and change { User.authenticate(User.last.email, 'test') }.from(nil) + .and change { Auth.new(User.last.email, 'test').valid? }.from(false) end end @@ -90,7 +90,7 @@ RSpec.describe AutoWizard do .and change { User.last.firstname }.to('Test Master') .and change { User.last.lastname }.to('Agent') .and change { User.last.email }.to('master_unit_test01@example.com') - .and change { User.authenticate(User.last.email, 'test') }.from(nil) + .and change { Auth.new(User.last.email, 'test').valid? }.from(false) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7cd984aeb..77666cd5f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -35,154 +35,6 @@ RSpec.describe User, type: :model do it_behaves_like 'UserPerformsGeoLookup' describe 'Class methods:' do - describe '.authenticate' do - subject(:user) { create(:user, password: password) } - - let(:password) { Faker::Internet.password } - - context 'with valid credentials' do - context 'using #login' do - it 'returns the matching user' do - expect(described_class.authenticate(user.login, password)) - .to eq(user) - end - - it 'is not case-sensitive' do - expect(described_class.authenticate(user.login.upcase, password)) - .to eq(user) - end - end - - context 'using #email' do - it 'returns the matching user' do - expect(described_class.authenticate(user.email, password)) - .to eq(user) - end - - it 'is not case-sensitive' do - expect(described_class.authenticate(user.email.upcase, password)) - .to eq(user) - end - end - - context 'but exceeding failed login limit' do - before { user.update(login_failed: 999) } - - it 'returns nil' do - expect(described_class.authenticate(user.login, password)) - .to be(nil) - end - end - - context 'when previous login was' do - context 'never' do - it 'updates #last_login and #updated_at' do - expect { described_class.authenticate(user.login, password) } - .to change { user.reload.last_login } - .and change { user.reload.updated_at } - end - end - - context 'less than 10 minutes ago' do - before do - described_class.authenticate(user.login, password) - travel 9.minutes - end - - it 'does not update #last_login and #updated_at' do - expect { described_class.authenticate(user.login, password) } - .to not_change { user.reload.last_login } - .and not_change { user.reload.updated_at } - end - end - - context 'more than 10 minutes ago' do - before do - described_class.authenticate(user.login, password) - travel 11.minutes - end - - it 'updates #last_login and #updated_at' do - expect { described_class.authenticate(user.login, password) } - .to change { user.reload.last_login } - .and change { user.reload.updated_at } - end - end - end - end - - context 'with valid user and invalid password' do - it 'increments failed login count' do - expect(described_class).to receive(:sleep).with(1) - expect { described_class.authenticate(user.login, password.next) } - .to change { user.reload.login_failed }.by(1) - end - - it 'returns nil' do - expect(described_class).to receive(:sleep).with(1) - expect(described_class.authenticate(user.login, password.next)).to be(nil) - end - end - - context 'with inactive user’s login' do - before { user.update(active: false) } - - it 'returns nil' do - expect(described_class.authenticate(user.login, password)).to be(nil) - end - end - - context 'with non-existent user login' do - it 'returns nil' do - expect(described_class.authenticate('john.doe', password)).to be(nil) - end - end - - context 'with empty login string' do - it 'returns nil' do - expect(described_class.authenticate('', password)).to be(nil) - end - end - - context 'with empty password string' do - it 'returns nil' do - expect(described_class.authenticate(user.login, '')).to be(nil) - end - end - - context 'with empty password string when the stored password is an empty string' do - before { user.update_column(:password, '') } - - context 'when password is an empty string' do - it 'returns nil' do - expect(described_class.authenticate(user.login, '')).to be(nil) - end - end - - context 'when password is nil' do - it 'returns nil' do - expect(described_class.authenticate(user.login, nil)).to be(nil) - end - end - end - - context 'with empty password string when the stored hash represents an empty string' do - before { user.update(password: PasswordHash.crypt('')) } - - context 'when password is an empty string' do - it 'returns nil' do - expect(described_class.authenticate(user.login, '')).to be(nil) - end - end - - context 'when password is nil' do - it 'returns nil' do - expect(described_class.authenticate(user.login, nil)).to be(nil) - end - end - end - end - describe '.identify' do it 'returns users by given login' do expect(described_class.identify(user.login)).to eq(user) @@ -191,37 +43,14 @@ RSpec.describe User, type: :model do it 'returns users by given email' do expect(described_class.identify(user.email)).to eq(user) end + + it 'returns nil for empty username' do + expect(described_class.identify('')).to eq(nil) + end end end describe 'Instance methods:' do - describe '#max_login_failed?' do - it { is_expected.to respond_to(:max_login_failed?) } - - context 'with "password_max_login_failed" setting' do - before do - Setting.set('password_max_login_failed', 5) - user.update(login_failed: 5) - end - - it 'returns true once user’s #login_failed count exceeds the setting' do - expect { user.update(login_failed: 6) } - .to change(user, :max_login_failed?).to(true) - end - end - - context 'without password_max_login_failed setting' do - before do - Setting.set('password_max_login_failed', nil) - user.update(login_failed: 0) - end - - it 'defaults to 0' do - expect { user.update(login_failed: 1) } - .to change(user, :max_login_failed?).to(true) - end - end - end describe '#out_of_office?' do context 'without any out_of_office_* attributes set' do diff --git a/spec/support/request.rb b/spec/support/request.rb index 9783a9fcc..7b04c4459 100644 --- a/spec/support/request.rb +++ b/spec/support/request.rb @@ -70,12 +70,6 @@ module ZammadSpecSupportRequest password = options[:password] || user.password.to_s login = options[:login] || user.login - # mock authentication otherwise login won't - # if user has no password (which is expensive to create) - if password.blank? - allow(User).to receive(:authenticate).with(login, '') { user.update_last_login }.and_return(user) - end - case via when :api_client # ensure that always the correct header value is set