Skip to content

Commit

Permalink
MM-51321 Refactor audit log application. (#22481) (#22541)
Browse files Browse the repository at this point in the history 
* Refactor audit log application.

* Fix incorrect API Usage for Auditing

* Fixups.

* Rename Object audit to Auditable. Some small fixes.

---------

Co-authored-by: Daniel Schalla <daniel@schalla.me>
  • Loading branch information
@crspeller @DSchalla
crspeller and DSchalla committed Mar 16, 2023
1 parent cb653d3 commit 975848b
Show file tree
Hide file tree
Showing 50 changed files with 563 additions and 307 deletions.
20 changes: 10 additions & 10 deletions api4/bot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func createBot(c *Context, w http.ResponseWriter, r *http.Request) {

auditRec := c.MakeAuditRecord("createBot", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("bot", bot)
audit.AddEventParameterAuditable(auditRec, "bot", bot)

if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionCreateBot) {
c.SetPermissionError(model.PermissionCreateBot)
Expand Down Expand Up @@ -90,8 +90,8 @@ func patchBot(c *Context, w http.ResponseWriter, r *http.Request) {

auditRec := c.MakeAuditRecord("patchBot", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("id", botUserId)
auditRec.AddEventParameter("bot", botPatch)
audit.AddEventParameter(auditRec, "id", botUserId)
audit.AddEventParameterAuditable(auditRec, "bot", botPatch)

if err := c.App.SessionHasPermissionToManageBot(*c.AppContext.Session(), botUserId); err != nil {
c.Err = err
Expand Down Expand Up @@ -208,8 +208,8 @@ func updateBotActive(c *Context, w http.ResponseWriter, active bool) {

auditRec := c.MakeAuditRecord("updateBotActive", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("id", botUserId)
auditRec.AddEventParameter("enable", active)
audit.AddEventParameter(auditRec, "id", botUserId)
audit.AddEventParameter(auditRec, "enable", active)

if err := c.App.SessionHasPermissionToManageBot(*c.AppContext.Session(), botUserId); err != nil {
c.Err = err
Expand Down Expand Up @@ -242,8 +242,8 @@ func assignBot(c *Context, w http.ResponseWriter, _ *http.Request) {

auditRec := c.MakeAuditRecord("assignBot", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("id", botUserId)
auditRec.AddEventParameter("user_id", userId)
audit.AddEventParameter(auditRec, "id", botUserId)
audit.AddEventParameter(auditRec, "user_id", userId)

if err := c.App.SessionHasPermissionToManageBot(*c.AppContext.Session(), botUserId); err != nil {
c.Err = err
Expand Down Expand Up @@ -295,9 +295,9 @@ func convertBotToUser(c *Context, w http.ResponseWriter, r *http.Request) {

auditRec := c.MakeAuditRecord("convertBotToUser", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("bot", bot)
auditRec.AddEventParameter("userPatch", userPatch)
auditRec.AddEventParameter("set_system_admin", systemAdmin)
audit.AddEventParameterAuditable(auditRec, "bot", bot)
audit.AddEventParameterAuditable(auditRec, "user_patch", &userPatch)
audit.AddEventParameter(auditRec, "set_system_admin", systemAdmin)

if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
c.SetPermissionError(model.PermissionManageSystem)
Expand Down
72 changes: 39 additions & 33 deletions api4/channel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func createChannel(c *Context, w http.ResponseWriter, r *http.Request) {

auditRec := c.MakeAuditRecord("createChannel", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("channel", channel)
audit.AddEventParameterAuditable(auditRec, "channel", channel)

if channel.Type == model.ChannelTypeOpen && !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionCreatePublicChannel) {
c.SetPermissionError(model.PermissionCreatePublicChannel)
Expand Down Expand Up @@ -137,7 +137,7 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) {
}

auditRec := c.MakeAuditRecord("updateChannel", audit.Fail)
auditRec.AddEventParameter("channel", channel)
audit.AddEventParameterAuditable(auditRec, "channel", channel)
defer c.LogAuditRec(auditRec)

originalOldChannel, appErr := c.App.GetChannel(c.AppContext, channel.Id)
Expand Down Expand Up @@ -202,7 +202,7 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) {

if channel.Name != "" {
oldChannel.Name = channel.Name
auditRec.AddMeta("new_channel_name", oldChannel.Name)
audit.AddEventParameter(auditRec, "new_channel_name", oldChannel.Name)
}

if channel.GroupConstrained != nil {
Expand All @@ -214,7 +214,6 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) {
c.Err = appErr
return
}
auditRec.AddMeta("update", updatedChannel)

if oldChannelDisplayName != channel.DisplayName {
if err := c.App.PostUpdateChannelDisplayNameMessage(c.AppContext, c.AppContext.Session().UserId, channel, oldChannelDisplayName, channel.DisplayName); err != nil {
Expand All @@ -238,22 +237,25 @@ func updateChannelPrivacy(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

auditRec := c.MakeAuditRecord("updateChannelPrivacy", audit.Fail)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
defer c.LogAuditRec(auditRec)

props := model.StringInterfaceFromJSON(r.Body)
privacy, ok := props["privacy"].(string)
if !ok || (model.ChannelType(privacy) != model.ChannelTypeOpen && model.ChannelType(privacy) != model.ChannelTypePrivate) {
c.SetInvalidParam("privacy")
return
}

audit.AddEventParameter(auditRec, "privacy", privacy)

channel, err := c.App.GetChannel(c.AppContext, c.Params.ChannelId)
if err != nil {
c.Err = err
return
}

auditRec := c.MakeAuditRecord("updateChannelPrivacy", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("props", props)
auditRec.AddEventPriorState(channel)

if model.ChannelType(privacy) == model.ChannelTypeOpen && !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionConvertPrivateChannelToPublic) {
Expand Down Expand Up @@ -316,7 +318,7 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) {

auditRec := c.MakeAuditRecord("patchChannel", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("channel", patch)
audit.AddEventParameterAuditable(auditRec, "channel", patch)
auditRec.AddEventPriorState(oldChannel)

switch oldChannel.Type {
Expand Down Expand Up @@ -431,7 +433,7 @@ func createDirectChannel(c *Context, w http.ResponseWriter, r *http.Request) {
}

auditRec := c.MakeAuditRecord("createDirectChannel", audit.Fail)
auditRec.AddEventParameter("user_ids", userIds)
audit.AddEventParameter(auditRec, "user_ids", userIds)
defer c.LogAuditRec(auditRec)

if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionCreateDirectChannel) {
Expand All @@ -449,7 +451,7 @@ func createDirectChannel(c *Context, w http.ResponseWriter, r *http.Request) {
otherUserId = userIds[1]
}

auditRec.AddEventParameter("user_id", otherUserId)
audit.AddEventParameter(auditRec, "user_id", otherUserId)

canSee, err := c.App.UserCanSeeOtherUser(c.AppContext.Session().UserId, otherUserId)
if err != nil {
Expand Down Expand Up @@ -521,7 +523,7 @@ func createGroupChannel(c *Context, w http.ResponseWriter, r *http.Request) {
}

auditRec := c.MakeAuditRecord("createGroupChannel", audit.Fail)
auditRec.AddEventParameter("user_ids", userIds)
audit.AddEventParameter(auditRec, "user_ids", userIds)
defer c.LogAuditRec(auditRec)

if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionCreateGroupChannel) {
Expand Down Expand Up @@ -1224,7 +1226,7 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
}

auditRec := c.MakeAuditRecord("deleteChannel", audit.Fail)
auditRec.AddEventParameter("id", c.Params.ChannelId)
audit.AddEventParameter(auditRec, "id", c.Params.ChannelId)
auditRec.AddEventPriorState(channel)
defer c.LogAuditRec(auditRec)

Expand Down Expand Up @@ -1521,8 +1523,8 @@ func updateChannelMemberRoles(c *Context, w http.ResponseWriter, r *http.Request

auditRec := c.MakeAuditRecord("updateChannelMemberRoles", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("props", props)
auditRec.AddEventParameter("channel_id", c.Params.ChannelId)
audit.AddEventParameter(auditRec, "props", props)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)

if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionManageChannelRoles) {
c.SetPermissionError(model.PermissionManageChannelRoles)
Expand Down Expand Up @@ -1553,8 +1555,8 @@ func updateChannelMemberSchemeRoles(c *Context, w http.ResponseWriter, r *http.R

auditRec := c.MakeAuditRecord("updateChannelMemberSchemeRoles", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("channel_id", c.Params.ChannelId)
auditRec.AddEventParameter("roles", schemeRoles)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
audit.AddEventParameterAuditable(auditRec, "roles", &schemeRoles)

if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), c.Params.ChannelId, model.PermissionManageChannelRoles) {
c.SetPermissionError(model.PermissionManageChannelRoles)
Expand Down Expand Up @@ -1585,8 +1587,8 @@ func updateChannelMemberNotifyProps(c *Context, w http.ResponseWriter, r *http.R

auditRec := c.MakeAuditRecord("updateChannelMemberNotifyProps", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("channel_id", c.Params.ChannelId)
auditRec.AddEventParameter("props", props)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
audit.AddEventParameter(auditRec, "props", props)

if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), c.Params.UserId) {
c.SetPermissionError(model.PermissionEditOtherUsers)
Expand Down Expand Up @@ -1619,7 +1621,8 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {

auditRec := c.MakeAuditRecord("addChannelMember", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("props", props)
audit.AddEventParameter(auditRec, "user_id", userId)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)

member := &model.ChannelMember{
ChannelId: c.Params.ChannelId,
Expand All @@ -1632,6 +1635,8 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

audit.AddEventParameter(auditRec, "post_root_id", postRootId)

if ok && len(postRootId) == 26 {
rootPost, err := c.App.GetSinglePost(postRootId, false)
if err != nil {
Expand All @@ -1650,8 +1655,6 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

auditRec.AddEventParameter("channel_id", member.ChannelId)

if channel.Type == model.ChannelTypeDirect || channel.Type == model.ChannelTypeGroup {
c.Err = model.NewAppError("addUserToChannel", "api.channel.add_user_to_channel.type.app_error", nil, "", http.StatusBadRequest)
return
Expand Down Expand Up @@ -1752,6 +1755,11 @@ func removeChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

auditRec := c.MakeAuditRecord("removeChannelMember", audit.Fail)
defer c.LogAuditRec(auditRec)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
audit.AddEventParameter(auditRec, "user_id", c.Params.UserId)

channel, err := c.App.GetChannel(c.AppContext, c.Params.ChannelId)
if err != nil {
c.Err = err
Expand All @@ -1764,11 +1772,6 @@ func removeChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

auditRec := c.MakeAuditRecord("removeChannelMember", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("channel_id", channel.Id)
auditRec.AddEventParameter("user_id", user.Id)

if !(channel.Type == model.ChannelTypeOpen || channel.Type == model.ChannelTypePrivate) {
c.Err = model.NewAppError("removeChannelMember", "api.channel.remove_channel_member.type.app_error", nil, "", http.StatusBadRequest)
return
Expand Down Expand Up @@ -1808,16 +1811,18 @@ func updateChannelScheme(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

auditRec := c.MakeAuditRecord("updateChannelScheme", audit.Fail)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
defer c.LogAuditRec(auditRec)

var p model.SchemeIDPatch
if jsonErr := json.NewDecoder(r.Body).Decode(&p); jsonErr != nil || p.SchemeID == nil || !model.IsValidId(*p.SchemeID) {
c.SetInvalidParamWithErr("scheme_id", jsonErr)
return
}
schemeID := p.SchemeID

auditRec := c.MakeAuditRecord("updateChannelScheme", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("scheme_id", *schemeID)
audit.AddEventParameter(auditRec, "scheme_id", *schemeID)

if c.App.Channels().License() == nil {
c.Err = model.NewAppError("Api4.UpdateChannelScheme", "api.channel.update_channel_scheme.license.error", nil, "", http.StatusForbidden)
Expand Down Expand Up @@ -2008,7 +2013,7 @@ func patchChannelModerations(c *Context, w http.ResponseWriter, r *http.Request)
c.Err = appErr
return
}
auditRec.AddMeta("channel", channel)
audit.AddEventParameterAuditable(auditRec, "channel", channel)

var channelModerationsPatch []*model.ChannelModerationPatch
err := json.NewDecoder(r.Body).Decode(&channelModerationsPatch)
Expand All @@ -2022,7 +2027,7 @@ func patchChannelModerations(c *Context, w http.ResponseWriter, r *http.Request)
c.Err = appErr
return
}
auditRec.AddEventParameter("patch", channelModerationsPatch)
audit.AddEventParameterAuditableArray(auditRec, "channel_moderations_patch", channelModerationsPatch)

b, err := json.Marshal(channelModerations)
if err != nil {
Expand Down Expand Up @@ -2067,8 +2072,9 @@ func moveChannel(c *Context, w http.ResponseWriter, r *http.Request) {

auditRec := c.MakeAuditRecord("moveChannel", audit.Fail)
defer c.LogAuditRec(auditRec)
auditRec.AddEventParameter("channel_id", c.Params.ChannelId)
auditRec.AddEventParameter("props", props)
audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
audit.AddEventParameter(auditRec, "team_id", teamId)
audit.AddEventParameter(auditRec, "force", force)
auditRec.AddEventPriorState(channel)

// TODO check and verify if the below three things are parameters or prior state if any
Expand Down

0 comments on commit 975848b

Please sign in to comment.