Skip to content

Commit

Permalink
Audit log for user authentication
Browse files Browse the repository at this point in the history
  • Loading branch information
vsizov committed Jul 6, 2015
1 parent 8ba83cb commit 411829f
Show file tree
Hide file tree
Showing 21 changed files with 144 additions and 33 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG
Expand Up @@ -28,7 +28,8 @@ v 7.13.0 (unreleased)
- Users with guest access level can not set assignee, labels or milestones for issue and merge request
- Reporter role can manage issue tracker now: edit any issue, set assignee or milestone and manage labels
- Better performance for pages with events list, issues list and commits list
- Faster automerge check and merge itself when source and target branches are in same repository
- Faster automerge check and merge itself when source and target branches are in same repository
- Audit log for user authentication

v 7.12.1
- Fix error when deleting a user who has projects (Stan Hu)
Expand Down
8 changes: 8 additions & 0 deletions app/controllers/omniauth_callbacks_controller.rb
Expand Up @@ -28,6 +28,7 @@ def ldap

# Do additional LDAP checks for the user filter and EE features
if @user.allowed?
log_audit_event(gl_user, with: :ldap)
sign_in_and_redirect(gl_user)
else
flash[:alert] = "Access denied for your LDAP account."
Expand All @@ -47,13 +48,15 @@ def handle_omniauth
if current_user
# Add new authentication method
current_user.identities.find_or_create_by(extern_uid: oauth['uid'], provider: oauth['provider'])
log_audit_event(current_user, with: oauth['provider'])
redirect_to profile_account_path, notice: 'Authentication method updated'
else
@user = Gitlab::OAuth::User.new(oauth)
@user.save

# Only allow properly saved users to login.
if @user.persisted? && @user.valid?
log_audit_event(@user.gl_user, with: oauth['provider'])
sign_in_and_redirect(@user.gl_user)
else
error_message =
Expand Down Expand Up @@ -83,4 +86,9 @@ def handle_omniauth
def oauth
@oauth ||= request.env['omniauth.auth']
end

def log_audit_event(user, options = {})
AuditEventService.new(user, user, options).
for_authentication.security_event
end
end
7 changes: 5 additions & 2 deletions app/controllers/profiles_controller.rb
Expand Up @@ -37,8 +37,11 @@ def reset_private_token
redirect_to profile_account_path
end

def history
@events = current_user.recent_events.page(params[:page]).per(PER_PAGE)
def audit_log
@events = AuditEvent.where(entity_type: "User", entity_id: current_user.id).
order("created_at DESC").
page(params[:page]).
per(PER_PAGE)
end

def update_username
Expand Down
7 changes: 7 additions & 0 deletions app/controllers/sessions_controller.rb
Expand Up @@ -37,6 +37,8 @@ def create
resource.update_attributes(reset_password_token: nil,
reset_password_sent_at: nil)
end
authenticated_with = user_params[:otp_attempt] ? "two-factor" : "standard"
log_audit_event(current_user, with: authenticated_with)
end
end

Expand Down Expand Up @@ -95,4 +97,9 @@ def valid_otp_attempt?(user)
user.valid_otp?(user_params[:otp_attempt]) ||
user.invalidate_otp_backup_code!(user_params[:otp_attempt])
end

def log_audit_event(user, options = {})
AuditEventService.new(user, user, options).
for_authentication.security_event
end
end
19 changes: 19 additions & 0 deletions app/models/audit_event.rb
@@ -0,0 +1,19 @@
class AuditEvent < ActiveRecord::Base
serialize :details, Hash

belongs_to :user, foreign_key: :author_id

validates :author_id, presence: true
validates :entity_id, presence: true
validates :entity_type, presence: true

after_initialize :initialize_details

def initialize_details
self.details = {} if details.nil?
end

def author_name
self.user.name
end
end
2 changes: 2 additions & 0 deletions app/models/security_event.rb
@@ -0,0 +1,2 @@
class SecurityEvent < AuditEvent
end
25 changes: 25 additions & 0 deletions app/services/audit_event_service.rb
@@ -0,0 +1,25 @@
class AuditEventService
def initialize(author, entity, details = {})
@author, @entity, @details = author, entity, details
end

def for_authentication
@details = {
with: @details[:with],
target_id: @author.id,
target_type: "User",
target_details: @author.name,
}

self
end

def security_event
SecurityEvent.create(
author_id: @author.id,
entity_id: @entity.id,
entity_type: @entity.class.name,
details: @details
)
end
end
6 changes: 3 additions & 3 deletions app/views/layouts/nav/_profile.html.haml
Expand Up @@ -44,8 +44,8 @@
= icon('image fw')
%span
Preferences
= nav_link(path: 'profiles#history') do
= link_to history_profile_path, title: 'History', data: {placement: 'right'} do
= nav_link(path: 'profiles#audit_log') do
= link_to audit_log_profile_path, title: 'Audit Log', data: {placement: 'right'} do
= icon('history fw')
%span
History
Audit Log
16 changes: 16 additions & 0 deletions app/views/profiles/_event_table.html.haml
@@ -0,0 +1,16 @@
%table.table#audits
%thead
%tr
%th Action
%th When

%tbody
- events.each do |event|
%tr
%td
%span
Signed in with
%b= event.details[:with]
authentication
%td #{time_ago_in_words event.created_at} ago
= paginate events, theme: "gitlab"
5 changes: 5 additions & 0 deletions app/views/profiles/audit_log.html.haml
@@ -0,0 +1,5 @@
- page_title "Audit Log"
%h3.page-title Audit Log
%p.light History of authentications

= render 'event_table', events: @events
11 changes: 0 additions & 11 deletions app/views/profiles/history.html.haml

This file was deleted.

2 changes: 1 addition & 1 deletion config/routes.rb
Expand Up @@ -207,7 +207,7 @@
#
resource :profile, only: [:show, :update] do
member do
get :history
get :audit_log
get :applications

put :reset_private_token
Expand Down
22 changes: 22 additions & 0 deletions db/migrate/20141118150935_add_audit_event.rb
@@ -0,0 +1,22 @@
class AddAuditEvent < ActiveRecord::Migration
def change
create_table :audit_events do |t|
t.integer :author_id, null: false
t.string :type, null: false

# "Namespace" where the change occurs
# eg. On a project, group or user
t.integer :entity_id, null: false
t.string :entity_type, null: false

# Details for the event
t.text :details

t.timestamps
end

add_index :audit_events, :author_id
add_index :audit_events, :type
add_index :audit_events, [:entity_id, :entity_type]
end
end
18 changes: 16 additions & 2 deletions db/schema.rb
Expand Up @@ -28,16 +28,30 @@
t.integer "default_branch_protection", default: 2
t.boolean "twitter_sharing_enabled", default: true
t.text "restricted_visibility_levels"
t.boolean "version_check_enabled", default: true
t.integer "max_attachment_size", default: 10, null: false
t.integer "default_project_visibility"
t.integer "default_snippet_visibility"
t.text "restricted_signup_domains"
t.boolean "version_check_enabled", default: true
t.boolean "user_oauth_applications", default: true
t.string "after_sign_out_path"
t.integer "session_expire_delay", default: 10080, null: false
end

create_table "audit_events", force: true do |t|
t.integer "author_id", null: false
t.string "type", null: false
t.integer "entity_id", null: false
t.string "entity_type", null: false
t.text "details"
t.datetime "created_at"
t.datetime "updated_at"
end

add_index "audit_events", ["author_id"], name: "index_audit_events_on_author_id", using: :btree
add_index "audit_events", ["entity_id", "entity_type"], name: "index_audit_events_on_entity_id_and_entity_type", using: :btree
add_index "audit_events", ["type"], name: "index_audit_events_on_type", using: :btree

create_table "broadcast_messages", force: true do |t|
t.text "message", null: false
t.datetime "starts_at"
Expand Down Expand Up @@ -496,12 +510,12 @@
t.string "bitbucket_access_token"
t.string "bitbucket_access_token_secret"
t.string "location"
t.string "public_email", default: "", null: false
t.string "encrypted_otp_secret"
t.string "encrypted_otp_secret_iv"
t.string "encrypted_otp_secret_salt"
t.boolean "otp_required_for_login", default: false, null: false
t.text "otp_backup_codes"
t.string "public_email", default: "", null: false
t.integer "dashboard", default: 0
end

Expand Down
6 changes: 3 additions & 3 deletions features/profile/active_tab.feature
Expand Up @@ -23,7 +23,7 @@ Feature: Profile Active Tab
Then the active main tab should be Preferences
And no other main tabs should be active

Scenario: On Profile History
Given I visit profile history page
Then the active main tab should be History
Scenario: On Profile Audit Log
Given I visit Audit Log page
Then the active main tab should be Audit Log
And no other main tabs should be active
2 changes: 1 addition & 1 deletion features/profile/profile.feature
Expand Up @@ -63,7 +63,7 @@ Feature: Profile

Scenario: I visit history tab
Given I have activity
When I visit profile history page
When I visit Audit Log page
Then I should see my activity

Scenario: I visit my user page
Expand Down
4 changes: 2 additions & 2 deletions features/steps/profile/active_tab.rb
Expand Up @@ -19,7 +19,7 @@ class Spinach::Features::ProfileActiveTab < Spinach::FeatureSteps
ensure_active_main_tab('Preferences')
end

step 'the active main tab should be History' do
ensure_active_main_tab('History')
step 'the active main tab should be Audit Log' do
ensure_active_main_tab('Audit Log')
end
end
2 changes: 1 addition & 1 deletion features/steps/profile/profile.rb
Expand Up @@ -115,7 +115,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
end

step 'I should see my activity' do
expect(page).to have_content "#{current_user.name} closed issue"
expect(page).to have_content "Signed in with standard authentication"
end

step 'my password is expired' do
Expand Down
4 changes: 2 additions & 2 deletions features/steps/shared/paths.rb
Expand Up @@ -127,8 +127,8 @@ module SharedPaths
visit profile_preferences_path
end

step 'I visit profile history page' do
visit history_profile_path
step 'I visit Audit Log page' do
visit audit_log_profile_path
end

# ----------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions spec/features/security/profile_access_spec.rb
Expand Up @@ -45,8 +45,8 @@
it { is_expected.to be_denied_for :visitor }
end

describe "GET /profile/history" do
subject { history_profile_path }
describe "GET /profile/audit_log" do
subject { audit_log_profile_path }

it { is_expected.to be_allowed_for @u1 }
it { is_expected.to be_allowed_for :admin }
Expand Down
4 changes: 2 additions & 2 deletions spec/routing/routing_spec.rb
Expand Up @@ -108,8 +108,8 @@
expect(get("/profile/account")).to route_to('profiles/accounts#show')
end

it "to #history" do
expect(get("/profile/history")).to route_to('profiles#history')
it "to #audit_log" do
expect(get("/profile/audit_log")).to route_to('profiles#audit_log')
end

it "to #reset_private_token" do
Expand Down

0 comments on commit 411829f

Please sign in to comment.