summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2018-01-14 21:10:48 -0800
committerStan Hu <stanhu@gmail.com>2018-01-14 22:22:06 -0800
commit0d187a9a65c5a8eae4bcb09228270cb974abd466 (patch)
tree7a958c51641edb5c8606380d673587f35deeeb8f
parent74f2f9b30fb1972a26481072486b358eb943309f (diff)
downloadgitlab-ce-sh-log-when-user-blocked.tar.gz
Log and send a system hook if a blocked user fails to loginsh-log-when-user-blocked
Closes #41633
-rw-r--r--app/models/user.rb8
-rw-r--r--app/services/system_hooks_service.rb5
-rw-r--r--changelogs/unreleased/sh-log-when-user-blocked.yml5
-rw-r--r--config/initializers/warden.rb4
-rw-r--r--doc/system_hooks/system_hooks.md20
-rw-r--r--lib/gitlab/auth/blocked_user_tracker.rb36
-rw-r--r--spec/lib/gitlab/auth/blocked_user_tracker_spec.rb53
-rw-r--r--spec/services/system_hooks_service_spec.rb13
8 files changed, 140 insertions, 4 deletions
diff --git a/app/models/user.rb b/app/models/user.rb
index 4484ee9ff4c..09aa5a7b318 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -53,7 +53,10 @@ class User < ActiveRecord::Base
serialize :otp_backup_codes, JSON # rubocop:disable Cop/ActiveRecordSerialize
devise :lockable, :recoverable, :rememberable, :trackable,
- :validatable, :omniauthable, :confirmable, :registerable
+ :validatable, :omniauthable, :confirmable, :registerable
+
+ BLOCKED_MESSAGE = "Your account has been blocked. Please contact your GitLab " \
+ "administrator if you think this is an error.".freeze
# Override Devise::Models::Trackable#update_tracked_fields!
# to limit database writes to at most once every hour
@@ -217,8 +220,7 @@ class User < ActiveRecord::Base
end
def inactive_message
- "Your account has been blocked. Please contact your GitLab " \
- "administrator if you think this is an error."
+ BLOCKED_MESSAGE
end
end
end
diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb
index 690918b4a00..af6d77ef5e8 100644
--- a/app/services/system_hooks_service.rb
+++ b/app/services/system_hooks_service.rb
@@ -41,8 +41,11 @@ class SystemHooksService
when User
data.merge!(user_data(model))
- if event == :rename
+ case event
+ when :rename
data[:old_username] = model.username_was
+ when :failed_login
+ data[:state] = model.state
end
when ProjectMember
data.merge!(project_member_data(model))
diff --git a/changelogs/unreleased/sh-log-when-user-blocked.yml b/changelogs/unreleased/sh-log-when-user-blocked.yml
new file mode 100644
index 00000000000..9abf2017514
--- /dev/null
+++ b/changelogs/unreleased/sh-log-when-user-blocked.yml
@@ -0,0 +1,5 @@
+---
+title: Log and send a system hook if a blocked user attempts to login
+merge_request:
+author:
+type: added
diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb
index 3d83fb92d56..ee034d21eae 100644
--- a/config/initializers/warden.rb
+++ b/config/initializers/warden.rb
@@ -2,4 +2,8 @@ Rails.application.configure do |config|
Warden::Manager.after_set_user do |user, auth, opts|
Gitlab::Auth::UniqueIpsLimiter.limit_user!(user)
end
+
+ Warden::Manager.before_failure do |env, opts|
+ Gitlab::Auth::BlockedUserTracker.log_if_user_blocked(env)
+ end
end
diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md
index f2a9b1d769b..8c8501bcc23 100644
--- a/doc/system_hooks/system_hooks.md
+++ b/doc/system_hooks/system_hooks.md
@@ -11,6 +11,7 @@ Your GitLab instance can perform HTTP POST requests on the following events:
- `user_remove_from_team`
- `user_create`
- `user_destroy`
+- `user_failed_login`
- `user_rename`
- `key_create`
- `key_destroy`
@@ -22,6 +23,8 @@ Your GitLab instance can perform HTTP POST requests on the following events:
The triggers for most of these are self-explanatory, but `project_update` and `project_rename` deserve some clarification: `project_update` is fired any time an attribute of a project is changed (name, description, tags, etc.) *unless* the `path` attribute is also changed. In that case, a `project_rename` is triggered instead (so that, for instance, if all you care about is the repo URL, you can just listen for `project_rename`).
+`user_failed_login` is sent whenever a **blocked** user attempts to login and denied access.
+
System hooks can be used, e.g. for logging or changing information in a LDAP server.
> **Note:**
@@ -196,6 +199,23 @@ Please refer to `group_rename` and `user_rename` for that case.
}
```
+**User failed login:**
+
+```json
+{
+ "event_name": "user_failed_login",
+ "created_at": "2017-10-03T06:08:48Z",
+ "updated_at": "2018-01-15T04:52:06Z",
+ "name": "John Smith",
+ "email": "user4@example.com",
+ "user_id": 26,
+ "username": "user4",
+ "state": "blocked"
+}
+```
+
+If the user is blocked via LDAP, `state` will be `ldap_blocked`.
+
**User renamed:**
```json
diff --git a/lib/gitlab/auth/blocked_user_tracker.rb b/lib/gitlab/auth/blocked_user_tracker.rb
new file mode 100644
index 00000000000..dae03a179e4
--- /dev/null
+++ b/lib/gitlab/auth/blocked_user_tracker.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+module Gitlab
+ module Auth
+ class BlockedUserTracker
+ ACTIVE_RECORD_REQUEST_PARAMS = 'action_dispatch.request.request_parameters'
+
+ def self.log_if_user_blocked(env)
+ message = env.dig('warden.options', :message)
+
+ # Devise calls User#active_for_authentication? on the User model and then
+ # throws an exception to Warden with User#inactive_message:
+ # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8
+ #
+ # Since Warden doesn't pass the user record to the failure handler, we
+ # need to do a database lookup with the username. We can limit the
+ # lookups to happen when the user was blocked by checking the inactive
+ # message passed along by Warden.
+ return unless message == User::BLOCKED_MESSAGE
+
+ login = env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login')
+
+ return unless login.present?
+
+ user = User.by_login(login)
+
+ return unless user&.blocked?
+
+ Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{env['REMOTE_ADDR']}")
+ SystemHooksService.new.execute_hooks_for(user, :failed_login)
+
+ true
+ rescue TypeError
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb
new file mode 100644
index 00000000000..726a3c1c83a
--- /dev/null
+++ b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe Gitlab::Auth::BlockedUserTracker do
+ set(:user) { create(:user) }
+
+ describe '.log_if_user_blocked' do
+ it 'does not log if user failed to login due to undefined reason' do
+ expect_any_instance_of(SystemHooksService).not_to receive(:execute_hooks_for)
+
+ expect(described_class.log_if_user_blocked({})).to be_nil
+ end
+
+ it 'gracefully handles malformed environment variables' do
+ env = { 'warden.options' => 'test' }
+
+ expect(described_class.log_if_user_blocked(env)).to be_nil
+ end
+
+ context 'failed login due to blocked user' do
+ let(:env) do
+ {
+ 'warden.options' => { message: User::BLOCKED_MESSAGE },
+ described_class::ACTIVE_RECORD_REQUEST_PARAMS => { 'user' => { 'login' => user.username } }
+ }
+ end
+
+ subject { described_class.log_if_user_blocked(env) }
+
+ before do
+ expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login)
+ end
+
+ it 'logs a blocked user' do
+ user.block!
+
+ expect(subject).to be_truthy
+ end
+
+ it 'logs a blocked user by e-mail' do
+ user.block!
+ env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email
+
+ expect(subject).to be_truthy
+ end
+
+ it 'logs a LDAP blocked user' do
+ user.ldap_block!
+
+ expect(subject).to be_truthy
+ end
+ end
+ end
+end
diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb
index 46cd10cdc12..c40cd5b7548 100644
--- a/spec/services/system_hooks_service_spec.rb
+++ b/spec/services/system_hooks_service_spec.rb
@@ -105,12 +105,25 @@ describe SystemHooksService do
expect(data[:old_username]).to eq(user.username_was)
end
end
+
+ context 'user_failed_login' do
+ it 'contains state of user' do
+ user.ldap_block!
+
+ data = event_data(user, :failed_login)
+
+ expect(data).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :state)
+ expect(data[:username]).to eq(user.username)
+ expect(data[:state]).to eq('ldap_blocked')
+ end
+ end
end
context 'event names' do
it { expect(event_name(user, :create)).to eq "user_create" }
it { expect(event_name(user, :destroy)).to eq "user_destroy" }
it { expect(event_name(user, :rename)).to eq 'user_rename' }
+ it { expect(event_name(user, :failed_login)).to eq 'user_failed_login' }
it { expect(event_name(project, :create)).to eq "project_create" }
it { expect(event_name(project, :destroy)).to eq "project_destroy" }
it { expect(event_name(project, :rename)).to eq "project_rename" }