Merge branch 'feature/user-audit-2' into 'master'

Add additional user actions in Audit events

Closes #1370

See merge request gitlab-org/gitlab-ee!2103

app/controllers/admin/applications_controller.rb

 class Admin::ApplicationsController < Admin::ApplicationController
   include OauthApplications
+  prepend EE::Admin::ApplicationsController
 
   before_action :set_application, only: [:show, :edit, :update, :destroy]
   before_action :load_scopes, only: [:new, :create, :edit, :update]
@@ -22,8 +23,7 @@ class Admin::ApplicationsController < Admin::ApplicationController
     @application = Doorkeeper::Application.new(application_params)
 
     if @application.save
-      flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
-      redirect_to admin_application_url(@application)
+      redirect_to_admin_page
     else
       render :new
     end
@@ -42,6 +42,13 @@ class Admin::ApplicationsController < Admin::ApplicationController
     redirect_to admin_applications_url, status: 302, notice: 'Application was successfully destroyed.'
   end
 
+  protected
+
+  def redirect_to_admin_page
+    flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
+    redirect_to admin_application_url(@application)
+  end
+
   private
 
   def set_application

app/controllers/admin/users_controller.rb

@@ -128,7 +128,7 @@ class Admin::UsersController < Admin::ApplicationController
     end
 
     respond_to do |format|
-      result = Users::UpdateService.new(user, user_params_with_pass).execute do |user|
+      result = Users::UpdateService.new(current_user, user_params_with_pass.merge(user: user)).execute do |user|
         user.skip_reconfirmation!
       end
 
@@ -155,7 +155,7 @@ class Admin::UsersController < Admin::ApplicationController
 
   def remove_email
     email = user.emails.find(params[:email_id])
-    success = Emails::DestroyService.new(user, email: email.email).execute
+    success = Emails::DestroyService.new(current_user, user: user, email: email.email).execute
 
     respond_to do |format|
       if success
@@ -226,7 +226,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def update_user(&block)
-    result = Users::UpdateService.new(user).execute(&block)
+    result = Users::UpdateService.new(current_user, user: user).execute(&block)
 
     result[:status] == :success
   end

app/controllers/concerns/oauth_applications.rb

 module OauthApplications
   extend ActiveSupport::Concern
+  prepend ::EE::Concerns::OauthApplications
 
   included do
     before_action :prepare_scopes, only: [:create, :update]

app/controllers/confirmations_controller.rb

 class ConfirmationsController < Devise::ConfirmationsController
+  prepend ::EE::ConfirmationsController
+
   def almost_there
     flash[:notice] = nil
     render layout: "devise_empty"
@@ -12,10 +14,14 @@ class ConfirmationsController < Devise::ConfirmationsController
 
   def after_confirmation_path_for(resource_name, resource)
     if signed_in?(resource_name)
-      after_sign_in_path_for(resource)
+      after_sign_in(resource)
     else
       flash[:notice] += " Please sign in."
       new_session_path(resource_name)
     end
   end
+
+  def after_sign_in(resource)
+    after_sign_in_path_for(resource)
+  end
 end

app/controllers/oauth/applications_controller.rb

@@ -3,6 +3,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   include Gitlab::GonHelper
   include PageLayoutHelper
   include OauthApplications
+  prepend ::EE::Oauth::ApplicationsController
 
   before_action :verify_user_oauth_applications_enabled
   before_action :authenticate_user!
@@ -21,14 +22,20 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
     @application.owner = current_user
 
     if @application.save
-      flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
-      redirect_to oauth_application_url(@application)
+      redirect_to_oauth_application_page
     else
       set_index_vars
       render :index
     end
   end
 
+  protected
+
+  def redirect_to_oauth_application_page
+    flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
+    redirect_to oauth_application_url(@application)
+  end
+
   private
 
   def verify_user_oauth_applications_enabled

app/controllers/passwords_controller.rb

@@ -3,6 +3,8 @@ class PasswordsController < Devise::PasswordsController
   before_action :prevent_ldap_reset, only: [:create]
   before_action :throttle_reset,      only: [:create]
 
+  prepend EE::PasswordsController
+
   def edit
     super
     reset_password_token = Devise.token_generator.digest(

app/controllers/profiles/avatars_controller.rb

@@ -2,7 +2,7 @@ class Profiles::AvatarsController < Profiles::ApplicationController
   def destroy
     @user = current_user
 
-    Users::UpdateService.new(@user).execute { |user| user.remove_avatar! }
+    Users::UpdateService.new(current_user, user: @user).execute { |user| user.remove_avatar! }
 
     redirect_to profile_path, status: 302
   end

app/controllers/profiles/emails_controller.rb

@@ -5,7 +5,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
   end
 
   def create
-    @email = Emails::CreateService.new(current_user, email_params).execute
+    @email = Emails::CreateService.new(current_user, email_params.merge(user: current_user)).execute
 
     if @email.errors.blank?
       NotificationService.new.new_email(@email)
@@ -19,7 +19,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
   def destroy
     @email = current_user.emails.find(params[:id])
 
-    Emails::DestroyService.new(current_user, email: @email.email).execute
+    Emails::DestroyService.new(current_user, user: current_user, email: @email.email).execute
 
     respond_to do |format|
       format.html { redirect_to profile_emails_url, status: 302 }

app/controllers/profiles/keys_controller.rb

 class Profiles::KeysController < Profiles::ApplicationController
+  prepend ::EE::Profiles::KeysController
+
   skip_before_action :authenticate_user!, only: [:get_keys]
 
   def index
@@ -14,7 +16,7 @@ class Profiles::KeysController < Profiles::ApplicationController
     @key = Keys::CreateService.new(current_user, key_params).execute
 
     if @key.persisted?
-      redirect_to profile_key_path(@key)
+      redirect_to_profile_key_path
     else
       @keys = current_user.keys.select(&:persisted?)
       render :index
@@ -50,6 +52,12 @@ class Profiles::KeysController < Profiles::ApplicationController
     end
   end
 
+  protected
+
+  def redirect_to_profile_key_path
+    redirect_to profile_key_path(@key)
+  end
+
   private
 
   def key_params

app/controllers/profiles/notifications_controller.rb

@@ -7,7 +7,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController
   end
 
   def update
-    result = Users::UpdateService.new(current_user, user_params).execute
+    result = Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute
 
     if result[:status] == :success
       flash[:notice] = "Notification settings saved"

app/controllers/profiles/passwords_controller.rb

@@ -21,10 +21,10 @@ class Profiles::PasswordsController < Profiles::ApplicationController
       password_automatically_set: false
     }
 
-    result = Users::UpdateService.new(@user, password_attributes).execute
+    result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute
 
     if result[:status] == :success
-      Users::UpdateService.new(@user, password_expires_at: nil).execute
+      Users::UpdateService.new(current_user, user: @user, password_expires_at: nil).execute
 
       redirect_to root_path, notice: 'Password successfully changed'
     else
@@ -46,7 +46,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
       return
     end
 
-    result = Users::UpdateService.new(@user, password_attributes).execute
+    result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute
 
     if result[:status] == :success
       flash[:notice] = "Password was successfully updated. Please login with it"

app/controllers/profiles/preferences_controller.rb

@@ -6,7 +6,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController
 
   def update
     begin
-      result = Users::UpdateService.new(user, preferences_params).execute
+      result = Users::UpdateService.new(current_user, preferences_params.merge(user: user)).execute
 
       if result[:status] == :success
         flash[:notice] = 'Preferences saved.'

app/controllers/profiles/two_factor_auths_controller.rb

@@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
       current_user.otp_grace_period_started_at = Time.current
     end
 
-    Users::UpdateService.new(current_user).execute!
+    Users::UpdateService.new(current_user, user: current_user).execute!
 
     if two_factor_authentication_required? && !current_user.two_factor_enabled?
       two_factor_authentication_reason(
@@ -41,7 +41,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
   def create
     if current_user.validate_and_consume_otp!(params[:pin_code])
-      Users::UpdateService.new(current_user, otp_required_for_login: true).execute! do |user|
+      Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute! do |user|
         @codes = user.generate_otp_backup_codes!
       end
 
@@ -70,7 +70,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
   end
 
   def codes
-    Users::UpdateService.new(current_user).execute! do |user|
+    Users::UpdateService.new(current_user, user: current_user).execute! do |user|
       @codes = user.generate_otp_backup_codes!
     end
   end

app/controllers/profiles_controller.rb

@@ -10,7 +10,7 @@ class ProfilesController < Profiles::ApplicationController
 
   def update
     respond_to do |format|
-      result = Users::UpdateService.new(@user, user_params).execute
+      result = Users::UpdateService.new(current_user, user_params.merge(user: @user)).execute
 
       if result[:status] == :success
         message = "Profile was successfully updated"
@@ -25,7 +25,7 @@ class ProfilesController < Profiles::ApplicationController
   end
 
   def reset_private_token
-    Users::UpdateService.new(@user).execute! do |user|
+    Users::UpdateService.new(current_user, user: @user).execute! do |user|
       user.reset_authentication_token!
     end
 
@@ -35,7 +35,7 @@ class ProfilesController < Profiles::ApplicationController
   end
 
   def reset_incoming_email_token
-    Users::UpdateService.new(@user).execute! do |user|
+    Users::UpdateService.new(current_user, user: @user).execute! do |user|
       user.reset_incoming_email_token!
     end
 
@@ -45,7 +45,7 @@ class ProfilesController < Profiles::ApplicationController
   end
 
   def reset_rss_token
-    Users::UpdateService.new(@user).execute! do |user|
+    Users::UpdateService.new(current_user, user: @user).execute! do |user|
       user.reset_rss_token!
     end
 
@@ -61,7 +61,7 @@ class ProfilesController < Profiles::ApplicationController
   end
 
   def update_username
-    result = Users::UpdateService.new(@user, username: user_params[:username]).execute
+    result = Users::UpdateService.new(current_user, user: @user, username: user_params[:username]).execute
 
     options = if result[:status] == :success
                 { notice: "Username successfully changed" }

app/controllers/sessions_controller.rb

@@ -57,7 +57,7 @@ class SessionsController < Devise::SessionsController
 
     return unless user && user.require_password_creation?
 
-    Users::UpdateService.new(user).execute do |user|
+    Users::UpdateService.new(current_user, user: user).execute do |user|
       @token = user.generate_reset_token
     end
 

app/models/license.rb

@@ -38,6 +38,7 @@ class License < ActiveRecord::Base
     cross_project_pipelines
     db_load_balancing
     deploy_board
+    extended_audit_events
     file_locks
     geo
     group_issue_boards
@@ -107,6 +108,7 @@ class License < ActiveRecord::Base
     auditor_user
     db_load_balancing
     elastic_search
+    extended_audit_events
     geo
     ldap_extras
     object_storage

app/models/user.rb

@@ -63,7 +63,7 @@ class User < ActiveRecord::Base
     lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i)
     return unless lease.try_obtain
 
-    Users::UpdateService.new(self).execute(validate: false)
+    Users::UpdateService.new(self, user: self).execute(validate: false)
   end
 
   attr_accessor :force_random_password
@@ -545,8 +545,8 @@ class User < ActiveRecord::Base
   def update_emails_with_primary_email
     primary_email_record = emails.find_by(email: email)
     if primary_email_record
-      Emails::DestroyService.new(self, email: email).execute
-      Emails::CreateService.new(self, email: email_was).execute
+      Emails::DestroyService.new(self, user: self, email: email).execute
+      Emails::CreateService.new(self, user: self, email: email_was).execute
     end
   end
 
@@ -1023,7 +1023,7 @@ class User < ActiveRecord::Base
     if attempts_exceeded?
       lock_access! unless access_locked?
     else
-      Users::UpdateService.new(self).execute(validate: false)
+      Users::UpdateService.new(self, user: self).execute(validate: false)
     end
   end
 
@@ -1209,7 +1209,7 @@ class User < ActiveRecord::Base
       &creation_block
     )
 
-    Users::UpdateService.new(user).execute(validate: false)
+    Users::UpdateService.new(user, user: user).execute(validate: false)
     user
   ensure
     Gitlab::ExclusiveLease.cancel(lease_key, uuid)

app/services/emails/base_service.rb

 module Emails
   class BaseService
-    def initialize(user, opts)
-      @user = user
+    def initialize(current_user, opts)
+      @current_user = current_user
+      @user = opts.delete(:user)
       @email = opts[:email]
     end
   end

app/services/emails/create_service.rb

 module Emails
   class CreateService < ::Emails::BaseService
+    prepend ::EE::Emails::CreateService
+
     def execute
       @user.emails.create(email: @email)
     end

app/services/emails/destroy_service.rb

 module Emails
   class DestroyService < ::Emails::BaseService
+    prepend ::EE::Emails::DestroyService
+
     def execute
-      Email.find_by_email!(@email).destroy && update_secondary_emails!
+      update_secondary_emails! if Email.find_by_email!(@email).destroy
     end
 
     private
 
     def update_secondary_emails!
-      result = ::Users::UpdateService.new(@user).execute do |user|
+      result = ::Users::UpdateService.new(@current_user, user: @user).execute do |user|
         user.update_secondary_emails!
       end
 

app/services/users/update_service.rb

 module Users
   class UpdateService < BaseService
     include NewUserNotifier
+    prepend EE::Users::UpdateService
 
-    def initialize(user, params = {})
-      @user = user
+    def initialize(current_user, params = {})
+      @current_user = current_user
+      @user = params.delete(:user)
       @params = params.dup
     end
 
@@ -15,9 +17,7 @@ module Users
       user_exists = @user.persisted?
 
       if @user.save(validate: validate)
-        notify_new_user(@user, nil) unless user_exists
-
-        success
+        notify_success(user_exists)
       else
         error(@user.errors.full_messages.uniq.join('. '))
       end
@@ -31,6 +31,14 @@ module Users
       true
     end
 
+    protected
+
+    def notify_success(user_exists)
+      notify_new_user(@user, nil) unless user_exists
+
+      success
+    end
+
     private
 
     def assign_attributes(&block)

ee/app/controllers/ee/admin/applications_controller.rb

+module EE
+  module Admin
+    module ApplicationsController
+      protected
+
+      def redirect_to_admin_page
+        raise NotImplementedError unless defined?(super)
+
+        log_audit_event
+
+        super
+      end
+    end
+  end
+end

ee/app/controllers/ee/concerns/oauth_applications.rb

+module EE
+  module Concerns
+    module OauthApplications
+      extend ActiveSupport::Concern
+
+      def log_audit_event
+        ::AuditEventService.new(current_user,
+                                current_user,
+                                action: :custom,
+                                custom_message: 'OAuth access granted',
+                                ip_address: request.remote_ip)
+            .for_user(@application.name).security_event
+      end
+    end
+  end
+end

ee/app/controllers/ee/confirmations_controller.rb

+module EE
+  module ConfirmationsController
+    include EE::Audit::Changes
+
+    protected
+
+    def after_sign_in(resource)
+      raise NotImplementedError unless defined?(super)
+
+      audit_changes(:email, as: 'email address', model: resource)
+
+      super(resource)
+    end
+  end
+end

ee/app/controllers/ee/oauth/applications_controller.rb

+module EE
+  module Oauth
+    module ApplicationsController
+      protected
+
+      def redirect_to_oauth_application_page
+        raise NotImplementedError unless defined?(super)
+
+        log_audit_event
+
+        super
+      end
+    end
+  end
+end

ee/app/controllers/ee/passwords_controller.rb

+module EE
+  module PasswordsController
+    extend ActiveSupport::Concern
+
+    prepended do
+      before_action :log_audit_event, only: [:create]
+    end
+
+    private
+
+    def log_audit_event
+      ::AuditEventService.new(current_user,
+                            resource,
+                            action: :custom,
+                            custom_message: 'Ask for password reset',
+                            ip_address: request.remote_ip)
+          .for_user(resource_params[:email]).unauth_security_event
+    end
+  end
+end

ee/app/controllers/ee/profiles/keys_controller.rb

+module EE
+  module Profiles
+    module KeysController
+      protected
+
+      def redirect_to_profile_key_path
+        raise NotImplementedError unless defined?(super)
+
+        log_audit_event
+
+        super
+      end
+
+      private
+
+      def log_audit_event
+        ::AuditEventService.new(current_user,
+                              current_user,
+                              action: :custom,
+                              custom_message: 'Added SSH key',
+                              ip_address: request.remote_ip)
+            .for_user(@key.title).security_event
+      end
+    end
+  end
+end

ee/app/services/ee/audit_event_service.rb

@@ -41,33 +41,6 @@ module EE
       self
     end
 
-    def for_deploy_key(key_title)
-      action = @details[:action]
-      author_name = @author.name
-
-      @details =
-        case action
-        when :destroy
-          {
-            remove: "deploy_key",
-            author_name: author_name,
-            target_id: key_title,
-            target_type: "DeployKey",
-            target_details: key_title
-          }
-        when :create
-          {
-            add: "deploy_key",
-            author_name: author_name,
-            target_id: key_title,
-            target_type: "DeployKey",
-            target_details: key_title
-          }
-        end
-
-      self
-    end
-
     def for_failed_login
       ip = @details[:ip_address]
       auth = @details[:with] || 'STANDARD'
@@ -82,6 +55,20 @@ module EE
       self
     end
 
+    def for_changes
+      @details =
+        {
+            change: @details[:as] || @details[:column],
+            from: @details[:from],
+            to: @details[:to],
+            author_name: @author.name,
+            target_id: @entity.id,
+            target_type: @entity.class,
+            target_details: @entity.name
+        }
+      self
+    end
+
     def security_event
       if admin_audit_log_enabled?
         add_security_event_admin_details!
@@ -89,35 +76,92 @@ module EE
         return super
       end
 
-      super if audit_events_enabled?
+      super if audit_events_enabled? || entity_audit_events_enabled?
     end
 
-    def add_security_event_admin_details!
-      @details.merge!(ip_address: @author.current_sign_in_ip,
-                      entity_path: @entity.full_path)
+    def unauth_security_event
+      return unless audit_events_enabled?
+
+      @details.delete(:ip_address) unless admin_audit_log_enabled?
+      @details[:entity_path] = @entity&.full_path if admin_audit_log_enabled?
+
+      SecurityEvent.create(
+        author_id: @author.respond_to?(:id) ? @author.id : -1,
+        entity_id: @entity.respond_to?(:id) ? @entity.id : -1,
+        entity_type: 'User',
+        details: @details
+      )
     end
 
-    def audit_events_enabled?
-      return true unless @entity.respond_to?(:feature_available?)
+    def entity_audit_events_enabled?
+      @entity.respond_to?(:feature_available?) && @entity.feature_available?(:audit_events)
+    end
 
-      @entity.feature_available?(:audit_events)
+    def audit_events_enabled?
+      # Always log auth events. Log all other events if `extended_audit_events` is enabled
+      @details[:with] || License.feature_available?(:extended_audit_events)
     end
 
     def admin_audit_log_enabled?
       License.feature_available?(:admin_audit_log)
     end
 
-    def unauth_security_event
-      return unless audit_events_enabled?
+    def method_missing(method_sym, *arguments, &block)
+      super(method_sym, *arguments, &block) unless respond_to?(method_sym)
 
-      @details.delete(:ip_address) unless admin_audit_log_enabled?
+      for_custom_model(method_sym.to_s.split('for_').last, *arguments)
+    end
 
-      SecurityEvent.create(
-        author_id: -1,
-        entity_id: -1,
-        entity_type: 'User',
-        details: @details
-      )
+    def respond_to?(method, include_private = false)
+      method.to_s.start_with?('for_') || super
+    end
+
+    private
+
+    def for_custom_model(model, key_title)
+      action = @details[:action]
+      model_class = model.camelize
+      custom_message = @details[:custom_message]
+
+      @details =
+        case action
+        when :destroy
+          {
+              remove: model,
+              author_name: @author.name,
+              target_id: key_title,
+              target_type: model_class,
+              target_details: key_title
+          }
+        when :create
+          {
+              add: model,
+              author_name: @author.name,
+              target_id: key_title,
+              target_type: model_class,
+              target_details: key_title
+          }
+        when :custom
+          {
+              custom_message: custom_message,
+              author_name: @author&.name,
+              target_id: key_title,
+              target_type: model_class,
+              target_details: key_title,
+              ip_address: @details[:ip_address]
+          }
+        end
+
+      self
+    end
+
+    def ip_address
+      @author&.current_sign_in_ip || @details[:ip_address]
+    end
+
+    def add_security_event_admin_details!
+      @details.merge!(ip_address: ip_address,
+                      entity_path: @entity.full_path)
     end
   end
 end

ee/app/services/ee/emails/base_service.rb

+module EE
+  module Emails
+    module BaseService
+      private
+
+      def log_audit_event(options = {})
+        ::AuditEventService.new(@current_user, @user, options)
+            .for_email(@email).security_event
+      end
+    end
+  end
+end

ee/app/services/ee/emails/create_service.rb

+module EE
+  module Emails
+    module CreateService
+      include ::EE::Emails::BaseService
+
+      def execute
+        super.tap do |email|
+          log_audit_event(action: :create) if email.persisted?
+        end
+      end
+    end
+  end
+end

ee/app/services/ee/emails/destroy_service.rb

+module EE
+  module Emails
+    module DestroyService
+      include ::EE::Emails::BaseService
+
+      def execute
+        super.tap do
+          log_audit_event(action: :destroy)
+        end
+      end
+    end
+  end
+end

ee/app/services/ee/users/update_service.rb

+module EE
+  module Users
+    module UpdateService
+      include EE::Audit::Changes
+
+      private
+
+      def notify_success(user_exists)
+        notify_new_user(@user, nil) unless user_exists
+
+        audit_changes(:email, as: 'email address')
+        audit_changes(:encrypted_password, as: 'password', skip_changes: true)
+
+        success
+      end
+
+      def model
+        @user
+      end
+    end
+  end
+end

lib/api/internal.rb

@@ -149,7 +149,7 @@ module API
 
         codes = nil
 
-        ::Users::UpdateService.new(user).execute! do |user|
+        ::Users::UpdateService.new(current_user, user: user).execute! do |user|
           codes = user.generate_otp_backup_codes!
         end
 

lib/api/notification_settings.rb

@@ -35,7 +35,7 @@ module API
             new_notification_email = params.delete(:notification_email)
 
             if new_notification_email
-              ::Users::UpdateService.new(current_user, notification_email: new_notification_email).execute
+              ::Users::UpdateService.new(current_user, user: current_user, notification_email: new_notification_email).execute
             end
 
             notification_setting.update(declared_params(include_missing: false))

lib/api/users.rb

@@ -172,7 +172,7 @@ module API
 
         user_params[:password_expires_at] = Time.now if user_params[:password].present?
 
-        result = ::Users::UpdateService.new(user, user_params.except(:extern_uid, :provider)).execute
+        result = ::Users::UpdateService.new(current_user, user_params.except(:extern_uid, :provider).merge(user: user)).execute
 
         if result[:status] == :success
           present user, with: Entities::UserPublic
@@ -332,7 +332,7 @@ module API
         user = User.find_by(id: params.delete(:id))
         not_found!('User') unless user
 
-        email = Emails::CreateService.new(user, declared_params(include_missing: false)).execute
+        email = Emails::CreateService.new(current_user,  declared_params(include_missing: false).merge(user: user)).execute
 
         if email.errors.blank?
           NotificationService.new.new_email(email)
@@ -373,7 +373,7 @@ module API
         not_found!('Email') unless email
 
         destroy_conditionally!(email) do |email|
-          Emails::DestroyService.new(current_user, email: email.email).execute
+          Emails::DestroyService.new(current_user, user: user, email: email.email).execute
         end
 
         user.update_secondary_emails!
@@ -678,7 +678,7 @@ module API
         requires :email, type: String, desc: 'The new email'
       end
       post "emails" do
-        email = Emails::CreateService.new(current_user, declared_params).execute
+        email = Emails::CreateService.new(current_user, declared_params.merge(user: current_user)).execute
 
         if email.errors.blank?
           NotificationService.new.new_email(email)
@@ -697,7 +697,7 @@ module API
         not_found!('Email') unless email
 
         destroy_conditionally!(email) do |email|
-          Emails::DestroyService.new(current_user, email: email.email).execute
+          Emails::DestroyService.new(current_user, user: current_user, email: email.email).execute
         end
 
         current_user.update_secondary_emails!

lib/audit/details.rb

 module Audit
   class Details
-    ACTIONS = %i[add remove failed_login change].freeze
+    ACTIONS = %i[add remove failed_login change custom_message].freeze
 
     def self.humanize(*args)
       new(*args).humanize
@@ -31,8 +31,17 @@ module Audit
         "Removed #{value}"
       when :failed_login
         "Failed to login with #{Gitlab::OAuth::Provider.label_for(value).upcase} authentication"
+      when :custom_message
+        value
       else
-        "Changed #{value} from #{@details[:from]} to #{@details[:to]}"
+        text_for_change(value)
+      end
+    end
+
+    def text_for_change(value)
+      "Changed #{value}".tap do |changed_string|
+        changed_string << " from #{@details[:from]}" if @details[:from]
+        changed_string << " to #{@details[:to]}" if @details[:to]
       end
     end
   end

lib/ee/audit/changes.rb

+module EE
+  module Audit
+    module Changes
+      def audit_changes(column, options = {})
+        column = options[:column] || column
+        @model = options[:model]
+
+        return unless changed?(column)
+
+        audit_event(parse_options(column, options))
+      end
+
+      protected
+
+      def model
+        @model
+      end
+
+      private
+
+      def changed?(column)
+        model.previous_changes.has_key?(column)
+      end
+
+      def changes(column)
+        model.previous_changes[column]
+      end
+
+      def parse_options(column, options)
+        options.tap do |options_hash|
+          options_hash[:column] = column
+          options_hash[:action] = :update
+
+          unless options[:skip_changes]
+            options_hash[:from] = changes(column).first
+            options_hash[:to] = changes(column).last
+          end
+        end
+      end
+
+      def audit_event(options)
+        ::AuditEventService.new(@current_user, model, options)
+          .for_changes.security_event
+      end
+    end
+  end
+end

lib/gitlab/ldap/access.rb

@@ -20,7 +20,7 @@ module Gitlab
           # permissions to keep things clean
           if access.allowed?
             access.update_user
-            Users::UpdateService.new(user, last_credential_check_at: Time.now).execute
+            Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute
 
             true
           else
@@ -172,8 +172,9 @@ module Gitlab
 
         return false if user.email == ldap_email
 
-        user.skip_reconfirmation!
-        user.update(email: ldap_email)
+        Users::UpdateService.new(user, user: user, email: ldap_email).execute do |user|
+          user.skip_reconfirmation!
+        end
       end
 
       def update_identity

lib/gitlab/o_auth/user.rb

@@ -34,7 +34,7 @@ module Gitlab
 
         block_after_save = needs_blocking?
 
-        Users::UpdateService.new(gl_user).execute!
+        Users::UpdateService.new(gl_user, user: gl_user).execute!
 
         gl_user.block if block_after_save
 

spec/controllers/admin/applications_controller_spec.rb

@@ -28,6 +28,8 @@ describe Admin::ApplicationsController do
 
   describe 'POST #create' do
     it 'creates the application' do
+      stub_licensed_features(extended_audit_events: true)
+
       create_params = attributes_for(:application, trusted: true)
 
       expect do
@@ -38,6 +40,7 @@ describe Admin::ApplicationsController do
 
       expect(response).to redirect_to(admin_application_path(application))
       expect(application).to have_attributes(create_params.except(:uid, :owner_type))
+      expect(SecurityEvent.count).to eq(1)
     end
 
     it 'renders the application form on errors' do

spec/controllers/oauth/applications_controller_spec.rb

@@ -25,5 +25,18 @@ describe Oauth::ApplicationsController do
         expect(response).to redirect_to(profile_path)
       end
     end
+
+    describe 'POST #create' do
+      it 'logs the audit event' do
+        stub_licensed_features(extended_audit_events: true)
+
+        sign_in(user)
+
+        application = build(:oauth_application)
+        application_attributes = application.attributes.merge(scopes: [])
+
+        expect { post :create, doorkeeper_application:  application_attributes }.to change { SecurityEvent.count }.by(1)
+      end
+    end
   end
 end

spec/controllers/profiles/keys_controller_spec.rb

@@ -66,4 +66,16 @@ describe Profiles::KeysController do
       end
     end
   end
+
+  describe '#create' do
+    it 'logs the audit event' do
+      stub_licensed_features(extended_audit_events: true)
+
+      sign_in(user)
+
+      key = build(:key)
+
+      expect { post :create, key: key.attributes }.to change { SecurityEvent.count }.by(1)
+    end
+  end
 end

spec/ee/spec/services/ee/audit_event_service_spec.rb

@@ -7,6 +7,10 @@ describe AuditEventService do
     let(:service) { described_class.new(author_name, nil, ip_address: ip_address) }
     let(:event) { service.for_failed_login.unauth_security_event }
 
+    before do
+      stub_licensed_features(extended_audit_events: true)
+    end
+
     it 'has the right type' do
       expect(event.entity_type).to eq('User')
     end

spec/features/login_spec.rb

@@ -155,6 +155,8 @@ feature 'Login' do
 
       it 'creates a security event after failed OAuth login' do
         stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config])
+        stub_licensed_features(extended_audit_events: true)
+
         user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')
         gitlab_sign_in_via('saml', user, 'wrong-uid')
 
@@ -177,6 +179,8 @@ feature 'Login' do
     end
 
     it 'blocks invalid login' do
+      stub_licensed_features(extended_audit_events: true)
+
       user = create(:user, password: 'not-the-default')
 
       gitlab_sign_in(user)

spec/lib/audit/details_spec.rb

@@ -75,5 +75,23 @@ describe Audit::Details do
         expect(described_class.humanize(removal_action)).to eq('Removed deploy key')
       end
     end
+
+    context 'change email' do
+      let(:action) do
+        {
+            change: 'email',
+            from: 'a@b.com',
+            to: 'c@b.com',
+            author_name: 'author',
+            target_id: '',
+            target_type: 'Email',
+            target_details: 'Email'
+        }
+      end
+
+      it 'humanizes the removal action' do
+        expect(described_class.humanize(action)).to eq('Changed email from a@b.com to c@b.com')
+      end
+    end
   end
 end

spec/lib/ee/audit/changes_spec.rb

+require 'spec_helper'
+
+describe EE::Audit::Changes do
+  describe '.audit_changes' do
+    let(:user) { create(:user) }
+    let(:foo_instance) { Class.new { include EE::Audit::Changes }.new }
+
+    before do
+      stub_licensed_features(extended_audit_events: true)
+
+      foo_instance.instance_variable_set(:@current_user, user)
+      foo_instance.instance_variable_set(:@user, user)
+
+      allow(foo_instance).to receive(:model).and_return(user)
+    end
+
+    describe 'non audit changes' do
+      it 'does not call the audit event service' do
+        user.update!(name: 'new name')
+
+        expect { foo_instance.audit_changes(:email) }.not_to change { SecurityEvent.count }
+      end
+    end
+
+    describe 'audit changes' do
+      it 'calls the audit event service' do
+        user.update!(name: 'new name')
+
+        expect { foo_instance.audit_changes(:name) }.to change { SecurityEvent.count }.by(1)
+      end
+    end
+  end
+end

spec/requests/api/users_spec.rb

@@ -474,10 +474,13 @@ describe API::Users do
     end
 
     it "updates user with new password and forces reset on next login" do
+      stub_licensed_features(extended_audit_events: true)
+
       put api("/users/#{user.id}", admin), password: '12345678'
 
       expect(response).to have_http_status(200)
       expect(user.reload.password_expires_at).to be <= Time.now
+      expect(AuditEvent.count).to eq(1)
     end
 
     it "updates user with organization" do

spec/services/audit_event_service_spec.rb

@@ -56,20 +56,20 @@ describe AuditEventService do
     end
   end
 
-  describe '#audit_events_enabled?' do
+  describe '#entity_audit_events_enabled??' do
     context 'entity is a project' do
       let(:service) { described_class.new(user, project, { action: :destroy }) }
 
       it 'returns false when project is unlicensed' do
         stub_licensed_features(audit_events: false)
 
-        expect(service.audit_events_enabled?).to be_falsy
+        expect(service.entity_audit_events_enabled?).to be_falsy
       end
 
       it 'returns true when project is licensed' do
         stub_licensed_features(audit_events: true)
 
-        expect(service.audit_events_enabled?).to be_truthy
+        expect(service.entity_audit_events_enabled?).to be_truthy
       end
     end
 
@@ -80,19 +80,35 @@ describe AuditEventService do
       it 'returns false when group is unlicensed' do
         stub_licensed_features(audit_events: false)
 
-        expect(service.audit_events_enabled?).to be_falsy
+        expect(service.entity_audit_events_enabled?).to be_falsy
       end
 
       it 'returns true when group is licensed' do
         stub_licensed_features(audit_events: true)
 
-        expect(service.audit_events_enabled?).to be_truthy
+        expect(service.entity_audit_events_enabled?).to be_truthy
       end
     end
 
     context 'entity is a user' do
       let(:service) { described_class.new(user, user, { action: :destroy }) }
 
+      it 'returns false when unlicensed' do
+        stub_licensed_features(audit_events: false, admin_audit_log: false)
+
+        expect(service.audit_events_enabled?).to be_falsey
+      end
+
+      it 'returns true when licensed with extended events' do
+        stub_licensed_features(extended_audit_events: true)
+
+        expect(service.audit_events_enabled?).to be_truthy
+      end
+    end
+
+    context 'auth event' do
+      let(:service) { described_class.new(user, user, { with: 'auth' }) }
+
       it 'returns true when unlicensed' do
         stub_licensed_features(audit_events: false, admin_audit_log: false)
 

spec/services/emails/create_service_spec.rb

@@ -2,11 +2,15 @@ require 'spec_helper'
 
 describe Emails::CreateService do
   let(:user) { create(:user) }
-  let(:opts) { { email: 'new@email.com' } }
+  let(:opts) { { email: 'new@email.com', user: user } }
 
   subject(:service) { described_class.new(user, opts) }
 
   describe '#execute' do
+    before do
+      stub_licensed_features(extended_audit_events: true)
+    end
+
     it 'creates an email with valid attributes' do
       expect { service.execute }.to change { Email.count }.by(1)
       expect(Email.where(opts)).not_to be_empty
@@ -17,5 +21,9 @@ describe Emails::CreateService do
 
       expect(user.emails).to eq(Email.where(opts))
     end
+
+    it 'registers a security event' do
+      expect { service.execute }.to change { SecurityEvent.count }.by(1)
+    end
   end
 end

spec/services/emails/destroy_service_spec.rb

@@ -4,11 +4,19 @@ describe Emails::DestroyService do
   let!(:user) { create(:user) }
   let!(:email) { create(:email, user: user) }
 
-  subject(:service) { described_class.new(user, email: email.email) }
+  subject(:service) { described_class.new(user, user: user, email: email.email) }
+
+  before do
+    stub_licensed_features(extended_audit_events: true)
+  end
 
   describe '#execute' do
     it 'removes an email' do
       expect { service.execute }.to change { user.emails.count }.by(-1)
     end
+
+    it 'registers a security event' do
+      expect { service.execute }.to change { SecurityEvent.count }.by(1)
+    end
   end
 end

spec/services/users/update_service_spec.rb

@@ -31,13 +31,13 @@ describe Users::UpdateService do
     end
 
     def update_user(user, opts)
-      described_class.new(user, opts).execute
+      described_class.new(user, opts.merge(user: user)).execute
     end
   end
 
   describe '#execute!' do
     it 'updates the name' do
-      service = described_class.new(user, name: 'New Name')
+      service = described_class.new(user, user: user, name: 'New Name')
       expect(service).not_to receive(:notify_new_user)
 
       result = service.execute!
@@ -55,7 +55,7 @@ describe Users::UpdateService do
     it 'fires system hooks when a new user is saved' do
       system_hook_service = spy(:system_hook_service)
       user = build(:user)
-      service = described_class.new(user, name: 'John Doe')
+      service = described_class.new(user, user: user, name: 'John Doe')
       expect(service).to receive(:notify_new_user).and_call_original
       expect(service).to receive(:system_hook_service).and_return(system_hook_service)
 
@@ -65,7 +65,7 @@ describe Users::UpdateService do
     end
 
     def update_user(user, opts)
-      described_class.new(user, opts).execute!
+      described_class.new(user, opts.merge(user: user)).execute!
     end
   end
 end