From b7073691f187fd8225e6efb79993dea8cea6907f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= <15726+Senen@users.noreply.github.com> Date: Thu, 20 Apr 2023 13:24:15 +0200 Subject: [PATCH 1/2] Log successful and failed login attempts in a separate log file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We log the login parameter and the request IP address. Quoting the ENS: > [op.acc.5.r5.1] Se registrarán los accesos con éxito y los fallidos. --- config/application.rb | 3 ++ config/initializers/warden.rb | 24 ++++++++++ config/secrets.yml.example | 4 ++ lib/authentication_logger.rb | 24 ++++++++++ .../users/sessions_controller_spec.rb | 47 +++++++++++++++++++ spec/lib/authentication_logger_spec.rb | 41 ++++++++++++++++ 6 files changed, 143 insertions(+) create mode 100644 config/initializers/warden.rb create mode 100644 lib/authentication_logger.rb create mode 100644 spec/lib/authentication_logger_spec.rb diff --git a/config/application.rb b/config/application.rb index 636d4552b..b041b7a0b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -138,6 +138,9 @@ module Consul config.paths["app/views"].unshift(Rails.root.join("app", "views", "custom")) + # Set to true to enable user authentication log + config.authentication_logs = Rails.application.secrets.dig(:security, :authentication_logs) || false + # Set to true to enable devise user lockable feature config.devise_lockable = Rails.application.secrets.devise_lockable diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb new file mode 100644 index 000000000..fd3f7a850 --- /dev/null +++ b/config/initializers/warden.rb @@ -0,0 +1,24 @@ +Warden::Manager.after_authentication do |user, auth, opts| + if Rails.application.config.authentication_logs + request = auth.request + login = request.params.dig(opts[:scope].to_s, "login") + message = "The user #{login} with IP address: #{request.ip} successfully signed in." + AuthenticationLogger.log(message) + end +end + +Warden::Manager.before_failure do |env, opts| + if Rails.application.config.authentication_logs + request = Rack::Request.new(env) + login = request.params.dig(opts[:scope].to_s, "login") + message = "The user #{login} with IP address: #{request.ip} failed to sign in." + AuthenticationLogger.log(message) + + user = User.find_by(username: login) || User.find_by(email: login) + if user&.failed_attempts == User.maximum_attempts.to_i + message = "The user #{login} with IP address: #{request.ip} reached maximum attempts " \ + "and it's temporarily locked." + AuthenticationLogger.log(message) + end + end +end diff --git a/config/secrets.yml.example b/config/secrets.yml.example index 7b61e7a15..8b132be01 100644 --- a/config/secrets.yml.example +++ b/config/secrets.yml.example @@ -21,6 +21,7 @@ development: devise_lockable: false multitenancy: false security: + authentication_logs: false last_sign_in: false password_complexity: false # lockable: @@ -59,6 +60,7 @@ staging: managers_application_key: "" multitenancy: false security: + authentication_logs: false last_sign_in: false password_complexity: false # lockable: @@ -102,6 +104,7 @@ preproduction: managers_application_key: "" multitenancy: false security: + authentication_logs: false last_sign_in: false password_complexity: false # lockable: @@ -150,6 +153,7 @@ production: managers_application_key: "" multitenancy: false security: + authentication_logs: false last_sign_in: false password_complexity: false # lockable: diff --git a/lib/authentication_logger.rb b/lib/authentication_logger.rb new file mode 100644 index 000000000..8aa940bd7 --- /dev/null +++ b/lib/authentication_logger.rb @@ -0,0 +1,24 @@ +class AuthenticationLogger + @loggers = {} + + class << self + def log(message) + logger.info(message) + end + + def path + Rails.root.join("log", Tenant.subfolder_path, "authentication.log") + end + + private + + def logger + @loggers[Apartment::Tenant.current] ||= build_logger + end + + def build_logger + FileUtils.mkdir_p(File.dirname(path)) + ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(path, level: :info)) + end + end +end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 29f0ace2d..5f13ddb10 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -22,4 +22,51 @@ describe Users::SessionsController do end end end + + describe "access logs" do + context "when feature is enabled" do + before { allow(Rails.application.config).to receive(:authentication_logs).and_return(true) } + + it "when a sign in process succeeds it calls the authentication logger" do + message = "The user citizen@consul.org with IP address: 0.0.0.0 successfully signed in." + expect(AuthenticationLogger).to receive(:log).with(message) + + post :create, params: { user: { login: "citizen@consul.org", password: "12345678" }} + end + + it "when a sign in process fails it calls the authentication logger" do + message = "The user citizen@consul.org with IP address: 0.0.0.0 failed to sign in." + expect(AuthenticationLogger).to receive(:log).with(message) + + post :create, params: { user: { login: "citizen@consul.org", password: "wrong" }} + end + + it "when maximum attempts is reached it tracks the user account lock" do + allow(User).to receive(:maximum_attempts).and_return(1) + message_1 = "The user citizen@consul.org with IP address: 0.0.0.0 failed to sign in." + message_2 = "The user citizen@consul.org with IP address: 0.0.0.0 reached maximum attempts " \ + "and it's temporarily locked." + expect(AuthenticationLogger).to receive(:log).once.with(message_1) + expect(AuthenticationLogger).to receive(:log).once.with(message_2) + + post :create, params: { user: { login: "citizen@consul.org", password: "wrong" }} + end + end + + context "when feature is disabled" do + before { allow(Rails.application.config).to receive(:authentication_logs).and_return(false) } + + it "when a sign in process succeeds it does not call the authentication logger" do + expect(AuthenticationLogger).not_to receive(:log) + + post :create, params: { user: { login: "citizen@consul.org", password: "12345678" }} + end + + it "when a sign in process fails it does not call the authentication logger" do + expect(AuthenticationLogger).not_to receive(:log) + + post :create, params: { user: { login: "citizen@consul.org", password: "wrong" }} + end + end + end end diff --git a/spec/lib/authentication_logger_spec.rb b/spec/lib/authentication_logger_spec.rb new file mode 100644 index 000000000..bcf40c11b --- /dev/null +++ b/spec/lib/authentication_logger_spec.rb @@ -0,0 +1,41 @@ +require "rails_helper" + +describe AuthenticationLogger do + describe ".path" do + context "when multitenancy is disabled" do + before { allow(Rails.application.config).to receive(:multitenancy).and_return(false) } + + it "uses default file" do + expect(AuthenticationLogger.path).to eq(Rails.root.join("log", "authentication.log")) + end + end + + context "when multitenancy is enabled" do + before { allow(Rails.application.config).to receive(:multitenancy).and_return(true) } + + it "uses the default file for the public schema" do + Tenant.switch("public") do + path = Rails.root.join("log", "authentication.log") + + expect(AuthenticationLogger.path).to eq(path) + end + end + + it "uses a separate file for any other tenant" do + create(:tenant, schema: "tenant1") + Tenant.switch("tenant1") do + path = Rails.root.join("log", "tenants", "tenant1", "authentication.log") + + expect(AuthenticationLogger.path).to eq(path) + end + + create(:tenant, schema: "tenant2") + Tenant.switch("tenant2") do + path = Rails.root.join("log", "tenants", "tenant2", "authentication.log") + + expect(AuthenticationLogger.path).to eq(path) + end + end + end + end +end From 9112d2d73bb12c2f4beb56fce335dd777186a128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= <15726+Senen@users.noreply.github.com> Date: Thu, 17 Aug 2023 17:58:57 +0200 Subject: [PATCH 2/2] Include a timestamp in every authentication logger message --- lib/authentication_logger.rb | 4 +++- spec/lib/authentication_logger_spec.rb | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/authentication_logger.rb b/lib/authentication_logger.rb index 8aa940bd7..510d68115 100644 --- a/lib/authentication_logger.rb +++ b/lib/authentication_logger.rb @@ -3,7 +3,9 @@ class AuthenticationLogger class << self def log(message) - logger.info(message) + logger.tagged(Time.current) do + logger.info(message) + end end def path diff --git a/spec/lib/authentication_logger_spec.rb b/spec/lib/authentication_logger_spec.rb index bcf40c11b..2e773072c 100644 --- a/spec/lib/authentication_logger_spec.rb +++ b/spec/lib/authentication_logger_spec.rb @@ -38,4 +38,12 @@ describe AuthenticationLogger do end end end + + describe "log" do + it "includes current time in each log entry", :with_frozen_time do + expect_any_instance_of(ActiveSupport::TaggedLogging).to receive(:tagged).with(Time.current) + + AuthenticationLogger.log("Just logging something!") + end + end end