From dfcf4cf5f1e87a29f0d9fcc5ff2bba47258893bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Thu, 18 Jul 2019 10:27:02 +0200 Subject: [PATCH] Add captcha if there are multiple failed login attempts Add method to store session ids by ip Add new specs for storing session ids Add cleaning up records after login Add retrieving anonymous sessions Add login recaptcha setting Add new setting to sessions controller Add conditions for showing captcha Add sessions controller specs Add admin settings specs for login protection Add new settings to api Add stub to devise spec Add new translation key Add cr remarks Rename class call Add cr remarks Change if-clause for consistency Add cr remarks Add code review remarks Refactor AnonymousSession class Add changelog entry Move AnonymousSession class to lib Move store unauthenticated sessions to sessions controller Move link to recaptcha info Regenerate text file Improve copy on the spam page Change action filter for storing anonymous sessions Fix rubocop offences Add code review remarks --- app/controllers/sessions_controller.rb | 44 ++++++- app/helpers/application_settings_helper.rb | 1 + app/models/application_setting.rb | 8 +- .../application_setting_implementation.rb | 1 + .../application_settings/_spam.html.haml | 13 ++- .../application_settings/reporting.html.haml | 4 +- app/views/devise/sessions/_new_base.html.haml | 2 +- ...ity-59549-add-capcha-for-failed-logins.yml | 5 + config/initializers/warden.rb | 1 + ...tection_enabled_to_application_settings.rb | 12 ++ db/schema.rb | 1 + lib/api/settings.rb | 5 + lib/gitlab/anonymous_session.rb | 39 +++++++ lib/gitlab/recaptcha.rb | 6 +- lib/gitlab/redis/shared_state.rb | 1 + locale/gitlab.pot | 7 +- spec/controllers/sessions_controller_spec.rb | 108 +++++++++++++++--- spec/features/admin/admin_settings_spec.rb | 2 + spec/lib/gitlab/anonymous_session_spec.rb | 78 +++++++++++++ .../shared/_signin_box.html.haml_spec.rb | 1 + 20 files changed, 307 insertions(+), 32 deletions(-) create mode 100644 changelogs/unreleased/security-59549-add-capcha-for-failed-logins.yml create mode 100644 db/migrate/20190719122333_add_login_recaptcha_protection_enabled_to_application_settings.rb create mode 100644 lib/gitlab/anonymous_session.rb create mode 100644 spec/lib/gitlab/anonymous_session_spec.rb diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 1880bead3ee..7b682cc0cc5 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -21,10 +21,13 @@ class SessionsController < Devise::SessionsController prepend_before_action :ensure_password_authentication_enabled!, if: -> { action_name == 'create' && password_based_login? } before_action :auto_sign_in_with_provider, only: [:new] + before_action :store_unauthenticated_sessions, only: [:new] + before_action :save_failed_login, if: :action_new_and_failed_login? before_action :load_recaptcha - after_action :log_failed_login, if: -> { action_name == 'new' && failed_login? } - helper_method :captcha_enabled? + after_action :log_failed_login, if: :action_new_and_failed_login? + + helper_method :captcha_enabled?, :captcha_on_login_required? # protect_from_forgery is already prepended in ApplicationController but # authenticate_with_two_factor which signs in the user is prepended before @@ -38,6 +41,7 @@ class SessionsController < Devise::SessionsController protect_from_forgery with: :exception, prepend: true CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze + MAX_FAILED_LOGIN_ATTEMPTS = 5 def new set_minimum_password_length @@ -81,10 +85,14 @@ class SessionsController < Devise::SessionsController request.headers[CAPTCHA_HEADER] && Gitlab::Recaptcha.enabled? end + def captcha_on_login_required? + Gitlab::Recaptcha.enabled_on_login? && unverified_anonymous_user? + end + # From https://github.com/plataformatec/devise/wiki/How-To:-Use-Recaptcha-with-Devise#devisepasswordscontroller def check_captcha return unless user_params[:password].present? - return unless captcha_enabled? + return unless captcha_enabled? || captcha_on_login_required? return unless Gitlab::Recaptcha.load_configurations! if verify_recaptcha @@ -126,10 +134,28 @@ class SessionsController < Devise::SessionsController Gitlab::AppLogger.info("Failed Login: username=#{user_params[:login]} ip=#{request.remote_ip}") end + def action_new_and_failed_login? + action_name == 'new' && failed_login? + end + + def save_failed_login + session[:failed_login_attempts] ||= 0 + session[:failed_login_attempts] += 1 + end + def failed_login? (options = request.env["warden.options"]) && options[:action] == "unauthenticated" end + # storing sessions per IP lets us check if there are associated multiple + # anonymous sessions with one IP and prevent situations when there are + # multiple attempts of logging in + def store_unauthenticated_sessions + return if current_user + + Gitlab::AnonymousSession.new(request.remote_ip, session_id: request.session.id).store_session_id_per_ip + end + # Handle an "initial setup" state, where there's only one user, it's an admin, # and they require a password change. # rubocop: disable CodeReuse/ActiveRecord @@ -240,6 +266,18 @@ class SessionsController < Devise::SessionsController @ldap_servers ||= Gitlab::Auth::LDAP::Config.available_servers end + def unverified_anonymous_user? + exceeded_failed_login_attempts? || exceeded_anonymous_sessions? + end + + def exceeded_failed_login_attempts? + session.fetch(:failed_login_attempts, 0) > MAX_FAILED_LOGIN_ATTEMPTS + end + + def exceeded_anonymous_sessions? + Gitlab::AnonymousSession.new(request.remote_ip).stored_sessions >= MAX_FAILED_LOGIN_ATTEMPTS + end + def authentication_method if user_params[:otp_attempt] "two-factor" diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 3847a35fbab..f13d69d1c37 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -229,6 +229,7 @@ module ApplicationSettingsHelper :recaptcha_enabled, :recaptcha_private_key, :recaptcha_site_key, + :login_recaptcha_protection_enabled, :receive_max_input_size, :repository_checks_enabled, :repository_storages, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index a769a8f07fd..16e35c1be67 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -73,11 +73,11 @@ class ApplicationSetting < ApplicationRecord validates :recaptcha_site_key, presence: true, - if: :recaptcha_enabled + if: :recaptcha_or_login_protection_enabled validates :recaptcha_private_key, presence: true, - if: :recaptcha_enabled + if: :recaptcha_or_login_protection_enabled validates :akismet_api_key, presence: true, @@ -285,4 +285,8 @@ class ApplicationSetting < ApplicationRecord def self.cache_backend Gitlab::ThreadMemoryCache.cache_backend end + + def recaptcha_or_login_protection_enabled + recaptcha_enabled || login_recaptcha_protection_enabled + end end diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 1e612bd0e78..b8e6156d231 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -63,6 +63,7 @@ module ApplicationSettingImplementation polling_interval_multiplier: 1, project_export_enabled: true, recaptcha_enabled: false, + login_recaptcha_protection_enabled: false, repository_checks_enabled: true, repository_storages: ['default'], require_two_factor_authentication: false, diff --git a/app/views/admin/application_settings/_spam.html.haml b/app/views/admin/application_settings/_spam.html.haml index d24e46b2815..f0a19075115 100644 --- a/app/views/admin/application_settings/_spam.html.haml +++ b/app/views/admin/application_settings/_spam.html.haml @@ -7,11 +7,15 @@ = f.check_box :recaptcha_enabled, class: 'form-check-input' = f.label :recaptcha_enabled, class: 'form-check-label' do Enable reCAPTCHA - - recaptcha_v2_link_url = 'https://developers.google.com/recaptcha/docs/versions' - - recaptcha_v2_link_start = ''.html_safe % { url: recaptcha_v2_link_url } %span.form-text.text-muted#recaptcha_help_block - = _('Helps prevent bots from creating accounts. We currently only support %{recaptcha_v2_link_start}reCAPTCHA v2%{recaptcha_v2_link_end}').html_safe % { recaptcha_v2_link_start: recaptcha_v2_link_start, recaptcha_v2_link_end: ''.html_safe } - + = _('Helps prevent bots from creating accounts.') + .form-group + .form-check + = f.check_box :login_recaptcha_protection_enabled, class: 'form-check-input' + = f.label :login_recaptcha_protection_enabled, class: 'form-check-label' do + Enable reCAPTCHA for login + %span.form-text.text-muted#recaptcha_help_block + = _('Helps prevent bots from brute-force attacks.') .form-group = f.label :recaptcha_site_key, 'reCAPTCHA Site Key', class: 'label-bold' = f.text_field :recaptcha_site_key, class: 'form-control' @@ -21,6 +25,7 @@ .form-group = f.label :recaptcha_private_key, 'reCAPTCHA Private Key', class: 'label-bold' + .form-group = f.text_field :recaptcha_private_key, class: 'form-control' .form-group diff --git a/app/views/admin/application_settings/reporting.html.haml b/app/views/admin/application_settings/reporting.html.haml index 46e3d1c4570..c60e44b3864 100644 --- a/app/views/admin/application_settings/reporting.html.haml +++ b/app/views/admin/application_settings/reporting.html.haml @@ -9,7 +9,9 @@ %button.btn.btn-default.js-settings-toggle{ type: 'button' } = expanded_by_default? ? _('Collapse') : _('Expand') %p - = _('Enable reCAPTCHA or Akismet and set IP limits.') + - recaptcha_v2_link_url = 'https://developers.google.com/recaptcha/docs/versions' + - recaptcha_v2_link_start = ''.html_safe % { url: recaptcha_v2_link_url } + = _('Enable reCAPTCHA or Akismet and set IP limits. For reCAPTCHA, we currently only support %{recaptcha_v2_link_start}v2%{recaptcha_v2_link_end}').html_safe % { recaptcha_v2_link_start: recaptcha_v2_link_start, recaptcha_v2_link_end: ''.html_safe } .settings-content = render 'spam' diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index 2f10f08c839..6382849de9c 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -13,7 +13,7 @@ .float-right.forgot-password = link_to "Forgot your password?", new_password_path(:user) %div - - if captcha_enabled? + - if captcha_enabled? || captcha_on_login_required? = recaptcha_tags .submit-container.move-submit-down diff --git a/changelogs/unreleased/security-59549-add-capcha-for-failed-logins.yml b/changelogs/unreleased/security-59549-add-capcha-for-failed-logins.yml new file mode 100644 index 00000000000..55f9e36c39c --- /dev/null +++ b/changelogs/unreleased/security-59549-add-capcha-for-failed-logins.yml @@ -0,0 +1,5 @@ +--- +title: Add :login_recaptcha_protection_enabled setting to prevent bots from brute-force attacks. +merge_request: +author: +type: security diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index 1d2bb2bce0a..d8a4da8cdf9 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -19,6 +19,7 @@ Rails.application.configure do |config| Warden::Manager.after_authentication(scope: :user) do |user, auth, opts| ActiveSession.cleanup(user) + Gitlab::AnonymousSession.new(auth.request.remote_ip, session_id: auth.request.session.id).cleanup_session_per_ip_entries end Warden::Manager.after_set_user(scope: :user, only: :fetch) do |user, auth, opts| diff --git a/db/migrate/20190719122333_add_login_recaptcha_protection_enabled_to_application_settings.rb b/db/migrate/20190719122333_add_login_recaptcha_protection_enabled_to_application_settings.rb new file mode 100644 index 00000000000..4561e1e8aa9 --- /dev/null +++ b/db/migrate/20190719122333_add_login_recaptcha_protection_enabled_to_application_settings.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddLoginRecaptchaProtectionEnabledToApplicationSettings < ActiveRecord::Migration[5.1] + DOWNTIME = false + + def change + add_column :application_settings, :login_recaptcha_protection_enabled, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 6f5fc6c65eb..8acc81c26ab 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -228,6 +228,7 @@ ActiveRecord::Schema.define(version: 2019_07_29_090456) do t.boolean "lock_memberships_to_ldap", default: false, null: false t.boolean "time_tracking_limit_to_hours", default: false, null: false t.string "grafana_url", default: "/-/grafana", null: false + t.boolean "login_recaptcha_protection_enabled", default: false, null: false t.string "outbound_local_requests_whitelist", limit: 255, default: [], null: false, array: true t.integer "raw_blob_request_limit", default: 300, null: false t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id" diff --git a/lib/api/settings.rb b/lib/api/settings.rb index aa9e879160d..5ac6f5bf61a 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -104,6 +104,11 @@ module API requires :recaptcha_site_key, type: String, desc: 'Generate site key at http://www.google.com/recaptcha' requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' end + optional :login_recaptcha_protection_enabled, type: Boolean, desc: 'Helps prevent brute-force attacks' + given login_recaptcha_protection_enabled: ->(val) { val } do + requires :recaptcha_site_key, type: String, desc: 'Generate site key at http://www.google.com/recaptcha' + requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' + end optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects' optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to set up Two-factor authentication' diff --git a/lib/gitlab/anonymous_session.rb b/lib/gitlab/anonymous_session.rb new file mode 100644 index 00000000000..148b6d3310d --- /dev/null +++ b/lib/gitlab/anonymous_session.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + class AnonymousSession + def initialize(remote_ip, session_id: nil) + @remote_ip = remote_ip + @session_id = session_id + end + + def store_session_id_per_ip + Gitlab::Redis::SharedState.with do |redis| + redis.pipelined do + redis.sadd(session_lookup_name, session_id) + redis.expire(session_lookup_name, 24.hours) + end + end + end + + def stored_sessions + Gitlab::Redis::SharedState.with do |redis| + redis.scard(session_lookup_name) + end + end + + def cleanup_session_per_ip_entries + Gitlab::Redis::SharedState.with do |redis| + redis.srem(session_lookup_name, session_id) + end + end + + private + + attr_reader :remote_ip, :session_id + + def session_lookup_name + @session_lookup_name ||= "#{Gitlab::Redis::SharedState::IP_SESSIONS_LOOKUP_NAMESPACE}:#{remote_ip}" + end + end +end diff --git a/lib/gitlab/recaptcha.rb b/lib/gitlab/recaptcha.rb index 772d743c9b0..f3cbe1db901 100644 --- a/lib/gitlab/recaptcha.rb +++ b/lib/gitlab/recaptcha.rb @@ -3,7 +3,7 @@ module Gitlab module Recaptcha def self.load_configurations! - if Gitlab::CurrentSettings.recaptcha_enabled + if Gitlab::CurrentSettings.recaptcha_enabled || enabled_on_login? ::Recaptcha.configure do |config| config.site_key = Gitlab::CurrentSettings.recaptcha_site_key config.secret_key = Gitlab::CurrentSettings.recaptcha_private_key @@ -16,5 +16,9 @@ module Gitlab def self.enabled? Gitlab::CurrentSettings.recaptcha_enabled end + + def self.enabled_on_login? + Gitlab::CurrentSettings.login_recaptcha_protection_enabled + end end end diff --git a/lib/gitlab/redis/shared_state.rb b/lib/gitlab/redis/shared_state.rb index 9066606ca21..270a19e780c 100644 --- a/lib/gitlab/redis/shared_state.rb +++ b/lib/gitlab/redis/shared_state.rb @@ -9,6 +9,7 @@ module Gitlab SESSION_NAMESPACE = 'session:gitlab'.freeze USER_SESSIONS_NAMESPACE = 'session:user:gitlab'.freeze USER_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:user:gitlab'.freeze + IP_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:ip:gitlab'.freeze DEFAULT_REDIS_SHARED_STATE_URL = 'redis://localhost:6382'.freeze REDIS_SHARED_STATE_CONFIG_ENV_VAR_NAME = 'GITLAB_REDIS_SHARED_STATE_CONFIG_FILE'.freeze diff --git a/locale/gitlab.pot b/locale/gitlab.pot index feae99fb95a..e6fc76bd28d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4175,7 +4175,7 @@ msgstr "" msgid "Enable or disable version check and usage ping." msgstr "" -msgid "Enable reCAPTCHA or Akismet and set IP limits." +msgid "Enable reCAPTCHA or Akismet and set IP limits. For reCAPTCHA, we currently only support %{recaptcha_v2_link_start}v2%{recaptcha_v2_link_end}" msgstr "" msgid "Enable shared Runners" @@ -5416,7 +5416,10 @@ msgstr "" msgid "Help page text and support page url." msgstr "" -msgid "Helps prevent bots from creating accounts. We currently only support %{recaptcha_v2_link_start}reCAPTCHA v2%{recaptcha_v2_link_end}" +msgid "Helps prevent bots from brute-force attacks." +msgstr "" + +msgid "Helps prevent bots from creating accounts." msgstr "" msgid "Hide archived projects" diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 9c4ddce5409..68b7bf61231 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -100,16 +100,8 @@ describe SessionsController do end end - context 'when reCAPTCHA is enabled' do - let(:user) { create(:user) } - let(:user_params) { { login: user.username, password: user.password } } - - before do - stub_application_setting(recaptcha_enabled: true) - request.headers[described_class::CAPTCHA_HEADER] = 1 - end - - it 'displays an error when the reCAPTCHA is not solved' do + context 'with reCAPTCHA' do + def unsuccesful_login(user_params, sesion_params: {}) # Without this, `verify_recaptcha` arbitrarily returns true in test env Recaptcha.configuration.skip_verify_env.delete('test') counter = double(:counter) @@ -119,14 +111,10 @@ describe SessionsController do .with(:failed_login_captcha_total, anything) .and_return(counter) - post(:create, params: { user: user_params }) - - expect(response).to render_template(:new) - expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' - expect(subject.current_user).to be_nil + post(:create, params: { user: user_params }, session: sesion_params) end - it 'successfully logs in a user when reCAPTCHA is solved' do + def succesful_login(user_params, sesion_params: {}) # Avoid test ordering issue and ensure `verify_recaptcha` returns true Recaptcha.configuration.skip_verify_env << 'test' counter = double(:counter) @@ -137,9 +125,80 @@ describe SessionsController do .and_return(counter) expect(Gitlab::Metrics).to receive(:counter).and_call_original - post(:create, params: { user: user_params }) + post(:create, params: { user: user_params }, session: sesion_params) + end - expect(subject.current_user).to eq user + context 'when reCAPTCHA is enabled' do + let(:user) { create(:user) } + let(:user_params) { { login: user.username, password: user.password } } + + before do + stub_application_setting(recaptcha_enabled: true) + request.headers[described_class::CAPTCHA_HEADER] = 1 + end + + it 'displays an error when the reCAPTCHA is not solved' do + # Without this, `verify_recaptcha` arbitrarily returns true in test env + + unsuccesful_login(user_params) + + expect(response).to render_template(:new) + expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + expect(subject.current_user).to be_nil + end + + it 'successfully logs in a user when reCAPTCHA is solved' do + succesful_login(user_params) + + expect(subject.current_user).to eq user + end + end + + context 'when reCAPTCHA login protection is enabled' do + let(:user) { create(:user) } + let(:user_params) { { login: user.username, password: user.password } } + + before do + stub_application_setting(login_recaptcha_protection_enabled: true) + end + + context 'when user tried to login 5 times' do + it 'displays an error when the reCAPTCHA is not solved' do + unsuccesful_login(user_params, sesion_params: { failed_login_attempts: 6 }) + + expect(response).to render_template(:new) + expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + expect(subject.current_user).to be_nil + end + + it 'successfully logs in a user when reCAPTCHA is solved' do + succesful_login(user_params, sesion_params: { failed_login_attempts: 6 }) + + expect(subject.current_user).to eq user + end + end + + context 'when there are more than 5 anonymous session with the same IP' do + before do + allow(Gitlab::AnonymousSession).to receive_message_chain(:new, :stored_sessions).and_return(6) + end + + it 'displays an error when the reCAPTCHA is not solved' do + unsuccesful_login(user_params) + + expect(response).to render_template(:new) + expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + expect(subject.current_user).to be_nil + end + + it 'successfully logs in a user when reCAPTCHA is solved' do + expect(Gitlab::AnonymousSession).to receive_message_chain(:new, :cleanup_session_per_ip_entries) + + succesful_login(user_params) + + expect(subject.current_user).to eq user + end + end end end end @@ -348,4 +407,17 @@ describe SessionsController do expect(controller.stored_location_for(:redirect)).to eq(search_path) end end + + context 'when login fails' do + before do + set_devise_mapping(context: @request) + @request.env["warden.options"] = { action: 'unauthenticated' } + end + + it 'does increment failed login counts for session' do + get(:new, params: { user: { login: 'failed' } }) + + expect(session[:failed_login_attempts]).to eq(1) + end + end end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index c77605f3869..4d3cf052744 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -263,6 +263,7 @@ describe 'Admin updates settings' do page.within('.as-spam') do check 'Enable reCAPTCHA' + check 'Enable reCAPTCHA for login' fill_in 'reCAPTCHA Site Key', with: 'key' fill_in 'reCAPTCHA Private Key', with: 'key' fill_in 'IPs per user', with: 15 @@ -271,6 +272,7 @@ describe 'Admin updates settings' do expect(page).to have_content "Application settings saved successfully" expect(current_settings.recaptcha_enabled).to be true + expect(current_settings.login_recaptcha_protection_enabled).to be true expect(current_settings.unique_ips_limit_per_user).to eq(15) end end diff --git a/spec/lib/gitlab/anonymous_session_spec.rb b/spec/lib/gitlab/anonymous_session_spec.rb new file mode 100644 index 00000000000..628aae81ada --- /dev/null +++ b/spec/lib/gitlab/anonymous_session_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do + let(:default_session_id) { '6919a6f1bb119dd7396fadc38fd18d0d' } + let(:additional_session_id) { '7919a6f1bb119dd7396fadc38fd18d0d' } + + subject { new_anonymous_session } + + def new_anonymous_session(session_id = default_session_id) + described_class.new('127.0.0.1', session_id: session_id) + end + + describe '#store_session_id_per_ip' do + it 'adds session id to proper key' do + subject.store_session_id_per_ip + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to eq [default_session_id] + end + end + + it 'adds expiration time to key' do + Timecop.freeze do + subject.store_session_id_per_ip + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.ttl("session:lookup:ip:gitlab:127.0.0.1")).to eq(24.hours.to_i) + end + end + end + + it 'adds id only once' do + subject.store_session_id_per_ip + subject.store_session_id_per_ip + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to eq [default_session_id] + end + end + + context 'when there is already one session' do + it 'adds session id to proper key' do + subject.store_session_id_per_ip + new_anonymous_session(additional_session_id).store_session_id_per_ip + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to contain_exactly(default_session_id, additional_session_id) + end + end + end + end + + describe '#stored_sessions' do + it 'returns all anonymous sessions per ip' do + Gitlab::Redis::SharedState.with do |redis| + redis.sadd("session:lookup:ip:gitlab:127.0.0.1", default_session_id) + redis.sadd("session:lookup:ip:gitlab:127.0.0.1", additional_session_id) + end + + expect(subject.stored_sessions).to eq(2) + end + end + + it 'removes obsolete lookup through ip entries' do + Gitlab::Redis::SharedState.with do |redis| + redis.sadd("session:lookup:ip:gitlab:127.0.0.1", default_session_id) + redis.sadd("session:lookup:ip:gitlab:127.0.0.1", additional_session_id) + end + + subject.cleanup_session_per_ip_entries + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to eq [additional_session_id] + end + end +end diff --git a/spec/views/devise/shared/_signin_box.html.haml_spec.rb b/spec/views/devise/shared/_signin_box.html.haml_spec.rb index 66c064e3fba..5d521d18c70 100644 --- a/spec/views/devise/shared/_signin_box.html.haml_spec.rb +++ b/spec/views/devise/shared/_signin_box.html.haml_spec.rb @@ -7,6 +7,7 @@ describe 'devise/shared/_signin_box' do assign(:ldap_servers, []) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) allow(view).to receive(:captcha_enabled?).and_return(false) + allow(view).to receive(:captcha_on_login_required?).and_return(false) end it 'is shown when Crowd is enabled' do