From 40830046e8f2b3b3d64ef1091448949e2c2356de Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 28 Jun 2023 22:42:06 +0530 Subject: [PATCH] feat: Audit Logs for Account User Changes (#7405) - Audit log for user invitations: https://linear.app/chatwoot/issue/CW-1768/invited-a-user-to-the-account - Audit log for change role: https://linear.app/chatwoot/issue/CW-1767/name-or-email-changed-the-role-of-the-user-email-to-agent-or-admin - Audit log for status change: https://linear.app/chatwoot/issue/CW-1766/availability-status-as-events-for-audit-logs Co-authored-by: Shivam Mishra --- .../dashboard/helper/auditlogHelper.js | 147 ++++++++++++++++++ .../helper/specs/auditlogHelper.spec.js | 146 +++++++++++++++++ .../dashboard/i18n/locale/ar/auditLogs.json | 1 - .../dashboard/i18n/locale/en/auditLogs.json | 7 + .../dashboard/settings/auditlogs/Index.vue | 50 ++---- app/models/account_user.rb | 11 ++ .../models/enterprise/audit/account_user.rb | 13 ++ .../app/models/enterprise/audit/user.rb | 5 +- .../v1/accounts/audit_logs/show.json.jbuilder | 1 + .../v1/accounts/audit_logs_controller_spec.rb | 5 +- spec/enterprise/models/account_user_spec.rb | 27 ++++ spec/enterprise/models/user_spec.rb | 15 ++ spec/models/account_user_spec.rb | 2 +- 13 files changed, 384 insertions(+), 46 deletions(-) create mode 100644 app/javascript/dashboard/helper/auditlogHelper.js create mode 100644 app/javascript/dashboard/helper/specs/auditlogHelper.spec.js create mode 100644 enterprise/app/models/enterprise/audit/account_user.rb create mode 100644 spec/enterprise/models/account_user_spec.rb create mode 100644 spec/enterprise/models/user_spec.rb diff --git a/app/javascript/dashboard/helper/auditlogHelper.js b/app/javascript/dashboard/helper/auditlogHelper.js new file mode 100644 index 000000000..7f912fb95 --- /dev/null +++ b/app/javascript/dashboard/helper/auditlogHelper.js @@ -0,0 +1,147 @@ +const roleMapping = { + 0: 'agent', + 1: 'administrator', +}; + +const availabilityMapping = { + 0: 'online', + 1: 'offline', + 2: 'busy', +}; + +const translationKeys = { + 'automationrule:create': `AUDIT_LOGS.AUTOMATION_RULE.ADD`, + 'automationrule:update': `AUDIT_LOGS.AUTOMATION_RULE.EDIT`, + 'automationrule:destroy': `AUDIT_LOGS.AUTOMATION_RULE.DELETE`, + 'webhook:create': `AUDIT_LOGS.WEBHOOK.ADD`, + 'webhook:update': `AUDIT_LOGS.WEBHOOK.EDIT`, + 'webhook:destroy': `AUDIT_LOGS.WEBHOOK.DELETE`, + 'inbox:create': `AUDIT_LOGS.INBOX.ADD`, + 'inbox:update': `AUDIT_LOGS.INBOX.EDIT`, + 'inbox:destroy': `AUDIT_LOGS.INBOX.DELETE`, + 'user:sign_in': `AUDIT_LOGS.USER_ACTION.SIGN_IN`, + 'user:sign_out': `AUDIT_LOGS.USER_ACTION.SIGN_OUT`, + 'team:create': `AUDIT_LOGS.TEAM.ADD`, + 'team:update': `AUDIT_LOGS.TEAM.EDIT`, + 'team:destroy': `AUDIT_LOGS.TEAM.DELETE`, + 'macro:create': `AUDIT_LOGS.MACRO.ADD`, + 'macro:update': `AUDIT_LOGS.MACRO.EDIT`, + 'macro:destroy': `AUDIT_LOGS.MACRO.DELETE`, + 'accountuser:create': `AUDIT_LOGS.ACCOUNT_USER.ADD`, + 'accountuser:update:self': `AUDIT_LOGS.ACCOUNT_USER.EDIT.SELF`, + 'accountuser:update:other': `AUDIT_LOGS.ACCOUNT_USER.EDIT.OTHER`, +}; + +function extractAttrChange(attrChange) { + if (Array.isArray(attrChange)) { + return attrChange[attrChange.length - 1]; + } + return attrChange; +} + +export function extractChangedAccountUserValues(auditedChanges) { + let changes = []; + let values = []; + + // Check roles + if (auditedChanges.role && auditedChanges.role.length) { + changes.push('role'); + values.push(roleMapping[extractAttrChange(auditedChanges.role)]); + } + + // Check availability + if (auditedChanges.availability && auditedChanges.availability.length) { + changes.push('availability'); + values.push( + availabilityMapping[extractAttrChange(auditedChanges.availability)] + ); + } + + return { changes, values }; +} + +function getAgentName(userId, agentList) { + if (userId === null) { + return 'System'; + } + + const agentName = agentList.find(agent => agent.id === userId)?.name; + + // If agent does not exist(removed/deleted), return userId + return agentName || userId; +} + +function handleAccountUserCreate(auditLogItem, translationPayload, agentList) { + translationPayload.invitee = getAgentName( + auditLogItem.audited_changes.user_id, + agentList + ); + + const roleKey = auditLogItem.audited_changes.role; + translationPayload.role = roleMapping[roleKey] || 'unknown'; // 'unknown' as a fallback in case an unrecognized key is provided + + return translationPayload; +} + +function handleAccountUserUpdate(auditLogItem, translationPayload, agentList) { + if (auditLogItem.user_id !== auditLogItem.auditable.user_id) { + translationPayload.user = getAgentName( + auditLogItem.auditable.user_id, + agentList + ); + } + + const accountUserChanges = extractChangedAccountUserValues( + auditLogItem.audited_changes + ); + if (accountUserChanges) { + translationPayload.attributes = accountUserChanges.changes; + translationPayload.values = accountUserChanges.values; + } + return translationPayload; +} + +export function generateTranslationPayload(auditLogItem, agentList) { + let translationPayload = { + agentName: getAgentName(auditLogItem.user_id, agentList), + id: auditLogItem.auditable_id, + }; + + const auditableType = auditLogItem.auditable_type.toLowerCase(); + const action = auditLogItem.action.toLowerCase(); + + if (auditableType === 'accountuser') { + if (action === 'create') { + translationPayload = handleAccountUserCreate( + auditLogItem, + translationPayload, + agentList + ); + } + + if (action === 'update') { + translationPayload = handleAccountUserUpdate( + auditLogItem, + translationPayload, + agentList + ); + } + } + + return translationPayload; +} + +export const generateLogActionKey = auditLogItem => { + const auditableType = auditLogItem.auditable_type.toLowerCase(); + const action = auditLogItem.action.toLowerCase(); + let logActionKey = `${auditableType}:${action}`; + + if (auditableType === 'accountuser' && action === 'update') { + logActionKey += + auditLogItem.user_id === auditLogItem.auditable.user_id + ? ':self' + : ':other'; + } + + return translationKeys[logActionKey] || ''; +}; diff --git a/app/javascript/dashboard/helper/specs/auditlogHelper.spec.js b/app/javascript/dashboard/helper/specs/auditlogHelper.spec.js new file mode 100644 index 000000000..2545668b4 --- /dev/null +++ b/app/javascript/dashboard/helper/specs/auditlogHelper.spec.js @@ -0,0 +1,146 @@ +import { + extractChangedAccountUserValues, + generateTranslationPayload, + generateLogActionKey, +} from '../auditlogHelper'; // import the functions + +describe('Helper functions', () => { + const agentList = [ + { id: 1, name: 'Agent 1' }, + { id: 2, name: 'Agent 2' }, + { id: 3, name: 'Agent 3' }, + ]; + + describe('extractChangedAccountUserValues', () => { + it('should correctly extract values when role is changed', () => { + const changes = { + role: [0, 1], + }; + const { + changes: extractedChanges, + values, + } = extractChangedAccountUserValues(changes); + expect(extractedChanges).toEqual(['role']); + expect(values).toEqual(['administrator']); + }); + + it('should correctly extract values when availability is changed', () => { + const changes = { + availability: [0, 2], + }; + const { + changes: extractedChanges, + values, + } = extractChangedAccountUserValues(changes); + expect(extractedChanges).toEqual(['availability']); + expect(values).toEqual(['busy']); + }); + + it('should correctly extract values when both are changed', () => { + const changes = { + role: [1, 0], + availability: [1, 2], + }; + const { + changes: extractedChanges, + values, + } = extractChangedAccountUserValues(changes); + expect(extractedChanges).toEqual(['role', 'availability']); + expect(values).toEqual(['agent', 'busy']); + }); + }); + + describe('generateTranslationPayload', () => { + it('should handle AccountUser create', () => { + const auditLogItem = { + auditable_type: 'AccountUser', + action: 'create', + user_id: 1, + auditable_id: 123, + audited_changes: { + user_id: 2, + role: 1, + }, + }; + + const payload = generateTranslationPayload(auditLogItem, agentList); + expect(payload).toEqual({ + agentName: 'Agent 1', + id: 123, + invitee: 'Agent 2', + role: 'administrator', + }); + }); + + it('should handle AccountUser update', () => { + const auditLogItem = { + auditable_type: 'AccountUser', + action: 'update', + user_id: 1, + auditable_id: 123, + audited_changes: { + user_id: 2, + role: [1, 0], + availability: [0, 2], + }, + auditable: { + user_id: 3, + }, + }; + + const payload = generateTranslationPayload(auditLogItem, agentList); + expect(payload).toEqual({ + agentName: 'Agent 1', + id: 123, + user: 'Agent 3', + attributes: ['role', 'availability'], + values: ['agent', 'busy'], + }); + }); + + it('should handle generic case like Team create', () => { + const auditLogItem = { + auditable_type: 'Team', + action: 'create', + user_id: 1, + auditable_id: 456, + }; + + const payload = generateTranslationPayload(auditLogItem, agentList); + expect(payload).toEqual({ + agentName: 'Agent 1', + id: 456, + }); + }); + }); + + describe('generateLogActionKey', () => { + it('should generate correct action key when user updates self', () => { + const auditLogItem = { + auditable_type: 'AccountUser', + action: 'update', + user_id: 1, + auditable: { + user_id: 1, + }, + }; + + const logActionKey = generateLogActionKey(auditLogItem); + expect(logActionKey).toEqual('AUDIT_LOGS.ACCOUNT_USER.EDIT.SELF'); + }); + + it('should generate correct action key when user updates other agent', () => { + const auditLogItem = { + auditable_type: 'AccountUser', + action: 'update', + user_id: 1, + auditable: { + user_id: 2, + }, + }; + + const logActionKey = generateLogActionKey(auditLogItem); + expect(logActionKey).toEqual('AUDIT_LOGS.ACCOUNT_USER.EDIT.OTHER'); + }); + }); +}); diff --git a/app/javascript/dashboard/i18n/locale/ar/auditLogs.json b/app/javascript/dashboard/i18n/locale/ar/auditLogs.json index cea35e07e..16aea346a 100644 --- a/app/javascript/dashboard/i18n/locale/ar/auditLogs.json +++ b/app/javascript/dashboard/i18n/locale/ar/auditLogs.json @@ -19,7 +19,6 @@ "SUCCESS_MESSAGE": "AuditLogs retrieved successfully", "ERROR_MESSAGE": "تعذر الاتصال بالخادم، الرجاء المحاولة مرة أخرى لاحقاً" }, - "DEFAULT_USER": "System", "AUTOMATION_RULE": { "ADD": "%{agentName} created a new automation rule (#%{id})", "EDIT": "%{agentName} updated an automation rule (#%{id})", diff --git a/app/javascript/dashboard/i18n/locale/en/auditLogs.json b/app/javascript/dashboard/i18n/locale/en/auditLogs.json index 2bad188fa..a1c64b51e 100644 --- a/app/javascript/dashboard/i18n/locale/en/auditLogs.json +++ b/app/javascript/dashboard/i18n/locale/en/auditLogs.json @@ -25,6 +25,13 @@ "EDIT": "%{agentName} updated an automation rule (#%{id})", "DELETE": "%{agentName} deleted an automation rule (#%{id})" }, + "ACCOUNT_USER": { + "ADD": "%{agentName} invited %{invitee} to the account as an %{role}", + "EDIT": { + "SELF": "%{agentName} changed their %{attributes} to %{values}", + "OTHER": "%{agentName} changed %{attributes} of %{user} to %{values}" + } + }, "INBOX": { "ADD": "%{agentName} created a new inbox (#%{id})", "EDIT": "%{agentName} updated an inbox (#%{id})", diff --git a/app/javascript/dashboard/routes/dashboard/settings/auditlogs/Index.vue b/app/javascript/dashboard/routes/dashboard/settings/auditlogs/Index.vue index 5b988b760..997317515 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/auditlogs/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/auditlogs/Index.vue @@ -66,6 +66,10 @@ import { mapGetters } from 'vuex'; import TableFooter from 'dashboard/components/widgets/TableFooter'; import timeMixin from 'dashboard/mixins/time'; import alertMixin from 'shared/mixins/alertMixin'; +import { + generateTranslationPayload, + generateLogActionKey, +} from 'dashboard/helper/auditlogHelper'; export default { components: { @@ -107,48 +111,14 @@ export default { this.showAlert(errorMessage); }); }, - getAgentName(email) { - if (email === null) { - return this.$t('AUDIT_LOGS.DEFAULT_USER'); - } - const agentName = this.agentList.find(agent => agent.email === email) - ?.name; - // If agent does not exist(removed/deleted), return email from audit log - return agentName || email; - }, generateLogText(auditLogItem) { - const agentName = this.getAgentName(auditLogItem.username); - const auditableType = auditLogItem.auditable_type.toLowerCase(); - const action = auditLogItem.action.toLowerCase(); - const auditId = auditLogItem.auditable_id; - const logActionKey = `${auditableType}:${action}`; + const translationPayload = generateTranslationPayload( + auditLogItem, + this.agentList + ); + const translationKey = generateLogActionKey(auditLogItem); - const translationPayload = { - agentName, - id: auditId, - }; - - const translationKeys = { - 'automationrule:create': `AUDIT_LOGS.AUTOMATION_RULE.ADD`, - 'automationrule:update': `AUDIT_LOGS.AUTOMATION_RULE.EDIT`, - 'automationrule:destroy': `AUDIT_LOGS.AUTOMATION_RULE.DELETE`, - 'webhook:create': `AUDIT_LOGS.WEBHOOK.ADD`, - 'webhook:update': `AUDIT_LOGS.WEBHOOK.EDIT`, - 'webhook:destroy': `AUDIT_LOGS.WEBHOOK.DELETE`, - 'inbox:create': `AUDIT_LOGS.INBOX.ADD`, - 'inbox:update': `AUDIT_LOGS.INBOX.EDIT`, - 'inbox:destroy': `AUDIT_LOGS.INBOX.DELETE`, - 'user:sign_in': `AUDIT_LOGS.USER_ACTION.SIGN_IN`, - 'user:sign_out': `AUDIT_LOGS.USER_ACTION.SIGN_OUT`, - 'team:create': `AUDIT_LOGS.TEAM.ADD`, - 'team:update': `AUDIT_LOGS.TEAM.EDIT`, - 'team:destroy': `AUDIT_LOGS.TEAM.DELETE`, - 'macro:create': `AUDIT_LOGS.MACRO.ADD`, - 'macro:update': `AUDIT_LOGS.MACRO.EDIT`, - 'macro:destroy': `AUDIT_LOGS.MACRO.DELETE`, - }; - - return this.$t(translationKeys[logActionKey] || '', translationPayload); + return this.$t(translationKey, translationPayload); }, onPageChange(page) { window.history.pushState({}, null, `${this.$route.path}?page=${page}`); diff --git a/app/models/account_user.rb b/app/models/account_user.rb index 2e913c60f..78b874334 100644 --- a/app/models/account_user.rb +++ b/app/models/account_user.rb @@ -49,6 +49,15 @@ class AccountUser < ApplicationRecord ::Agents::DestroyJob.perform_later(account, user) end + def push_event_data + { + id: id, + availability: availability, + role: role, + user_id: user_id + } + end + private def notify_creation @@ -63,3 +72,5 @@ class AccountUser < ApplicationRecord OnlineStatusTracker.set_status(account.id, user.id, availability) end end + +AccountUser.include_mod_with('Audit::AccountUser') diff --git a/enterprise/app/models/enterprise/audit/account_user.rb b/enterprise/app/models/enterprise/audit/account_user.rb new file mode 100644 index 000000000..303871932 --- /dev/null +++ b/enterprise/app/models/enterprise/audit/account_user.rb @@ -0,0 +1,13 @@ +module Enterprise::Audit::AccountUser + extend ActiveSupport::Concern + + included do + audited only: [ + :availability, + :role, + :account_id, + :inviter_id, + :user_id + ], on: [:create, :update], associated_with: :account + end +end diff --git a/enterprise/app/models/enterprise/audit/user.rb b/enterprise/app/models/enterprise/audit/user.rb index 5d0276607..fc2b49818 100644 --- a/enterprise/app/models/enterprise/audit/user.rb +++ b/enterprise/app/models/enterprise/audit/user.rb @@ -2,12 +2,13 @@ module Enterprise::Audit::User extend ActiveSupport::Concern included do + # required only for sign_in and sign_out events, which we are logging manually + # hence the proc that always returns false audited only: [ :availability, :display_name, :email, :name - ] - audited associated_with: :account + ], unless: proc { |_u| true } end end diff --git a/enterprise/app/views/api/v1/accounts/audit_logs/show.json.jbuilder b/enterprise/app/views/api/v1/accounts/audit_logs/show.json.jbuilder index 0df659304..67c392459 100644 --- a/enterprise/app/views/api/v1/accounts/audit_logs/show.json.jbuilder +++ b/enterprise/app/views/api/v1/accounts/audit_logs/show.json.jbuilder @@ -7,6 +7,7 @@ json.audit_logs do json.id audit_log.id json.auditable_id audit_log.auditable_id json.auditable_type audit_log.auditable_type + json.auditable audit_log.auditable.try(:push_event_data) json.associated_id audit_log.associated_id json.associated_type audit_log.associated_type json.user_id audit_log.user_id diff --git a/spec/enterprise/controllers/api/v1/accounts/audit_logs_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/audit_logs_controller_spec.rb index 53721c5c5..ab2e3f3f4 100644 --- a/spec/enterprise/controllers/api/v1/accounts/audit_logs_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/audit_logs_controller_spec.rb @@ -2,8 +2,8 @@ require 'rails_helper' RSpec.describe 'Enterprise Audit API', type: :request do let!(:account) { create(:account) } - let!(:inbox) { create(:inbox, account: account) } let!(:admin) { create(:user, account: account, role: :administrator) } + let!(:inbox) { create(:inbox, account: account) } describe 'GET /api/v1/accounts/{account.id}/audit_logs' do context 'when it is an un-authenticated user' do @@ -52,7 +52,8 @@ RSpec.describe 'Enterprise Audit API', type: :request do expect(json_response['audit_logs'][0]['audited_changes']['name']).to eql(inbox.name) expect(json_response['audit_logs'][0]['associated_id']).to eql(account.id) expect(json_response['current_page']).to be(1) - expect(json_response['total_entries']).to be(1) + # contains audit log for account user as well + expect(json_response['total_entries']).to be(2) end end end diff --git a/spec/enterprise/models/account_user_spec.rb b/spec/enterprise/models/account_user_spec.rb new file mode 100644 index 000000000..3850cfa9e --- /dev/null +++ b/spec/enterprise/models/account_user_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe AccountUser do + describe 'audit log' do + context 'when account user is created' do + it 'has associated audit log created' do + account_user = create(:account_user) + account_user_audit_log = Audited::Audit.where(auditable_type: 'AccountUser', action: 'create').first + expect(account_user_audit_log).to be_present + expect(account_user_audit_log.associated).to eq(account_user.account) + end + end + + context 'when account user is updated' do + it 'has associated audit log created' do + account_user = create(:account_user) + account_user.update!(availability: 'offline') + account_user_audit_log = Audited::Audit.where(auditable_type: 'AccountUser', action: 'update').first + expect(account_user_audit_log).to be_present + expect(account_user_audit_log.associated).to eq(account_user.account) + expect(account_user_audit_log.audited_changes).to eq('availability' => [0, 1]) + end + end + end +end diff --git a/spec/enterprise/models/user_spec.rb b/spec/enterprise/models/user_spec.rb new file mode 100644 index 000000000..ba1890a78 --- /dev/null +++ b/spec/enterprise/models/user_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe User do + let(:user) { create(:user) } + + describe 'audit log' do + context 'when user is created' do + it 'has no associated audit log created' do + expect(Audited::Audit.where(auditable_type: 'User', action: 'create').count).to eq 0 + end + end + end +end diff --git a/spec/models/account_user_spec.rb b/spec/models/account_user_spec.rb index be6471050..2b8ef790a 100644 --- a/spec/models/account_user_spec.rb +++ b/spec/models/account_user_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe User do +RSpec.describe AccountUser do include ActiveJob::TestHelper let!(:account_user) { create(:account_user) }