CSS-4838 Audit Log improvements (#1042)

* Audit log fixes

* Add omitempty on relevant audit log fields

* Fix test

* Moved NewConversationID to utils package

- Created new utils package

api/params/params.go

@@ -100,32 +100,32 @@ type AuditEvent struct {
 	MessageId uint64 `json:"message-id" yaml:"message-id"`
 
 	// FacadeName contains the request facade name.
-	FacadeName string `json:"facade-name" yaml:"facade-name"`
+	FacadeName string `json:"facade-name,omitempty" yaml:"facade-name,omitempty"`
 
 	// FacadeMethod contains the specific method to be executed on the facade.
-	FacadeMethod string `json:"facade-method" yaml:"facade-method"`
+	FacadeMethod string `json:"facade-method,omitempty" yaml:"facade-method,omitempty"`
 
 	// FacadeVersion contains the requested version for the facade method.
-	FacadeVersion int `json:"facade-version" yaml:"facade-version"`
+	FacadeVersion int `json:"facade-version,omitempty" yaml:"facade-version,omitempty"`
 
 	// ObjectId contains the object id to act on, only used by certain facades.
-	ObjectId string `json:"object-id" yaml:"object-id"`
+	ObjectId string `json:"object-id,omitempty" yaml:"object-id,omitempty"`
 
 	// UserTag contains the user tag of authenticated user that performed
 	// the action.
-	UserTag string `json:"user-tag" yaml:"user-tag"`
+	UserTag string `json:"user-tag,omitempty" yaml:"user-tag,omitempty"`
 
 	// Model contains the name of the model the event was performed against.
-	Model string `json:"model" yaml:"model"`
+	Model string `json:"model,omitempty" yaml:"model,omitempty"`
 
 	// IsResponse indicates whether the message is a request/response.
 	IsResponse bool `json:"is-response" yaml:"is-response"`
 
 	// Params contains client request parameters.
-	Params map[string]any `json:"params" yaml:"params"`
+	Params map[string]any `json:"params,omitempty" yaml:"params,omitempty"`
 
 	// Errors contains error info received from the controller.
-	Errors map[string]any `json:"error" yaml:"errors"`
+	Errors map[string]any `json:"error,omitempty" yaml:"errors,omitempty"`
 }
 
 // An AuditEvents contains events from the audit log.

cmd/jimmctl/cmd/listauditevents.go

@@ -71,7 +71,7 @@ func (c *listAuditEventsCommand) SetFlags(f *gnuflag.FlagSet) {
 	f.StringVar(&c.args.UserTag, "user-tag", "", "display events performed by authenticated user")
 	f.StringVar(&c.args.Method, "method", "", "display events for a specific method call")
 	f.StringVar(&c.args.Model, "model", "", "display events for a specific model (model name is controller/model)")
-	f.IntVar(&c.args.Limit, "offset", 0, "offset the set of returned audit events")
+	f.IntVar(&c.args.Offset, "offset", 0, "offset the set of returned audit events")
 	f.IntVar(&c.args.Limit, "limit", 0, "limit the maximum number of returned audit events")
 	f.BoolVar(&c.args.SortTime, "reverse", false, "reverse the order of logs, showing the most recent first")
 

cmd/jimmctl/cmd/listauditevents_test.go

@@ -37,24 +37,18 @@ func (s *listAuditEventsSuite) TestListAuditEventsSuperuser(c *gc.C) {
   facade-name: Admin
   facade-method: Login
   facade-version: \d
-  object-id: ""
   user-tag: user-
-  model: ""
   is-response: false
   params:
     params: redacted
-  errors: .*
 - time: .*
   conversation-id: .*
   message-id: 1
   facade-name: Admin
   facade-method: Login
   facade-version: \d
-  object-id: ""
   user-tag: user-
-  model: ""
   is-response: true
-  params: {}
   errors:
     results:
     - error:

cmd/jimmctl/cmd/purge_logs.go

@@ -25,7 +25,9 @@ const purgeLogsDoc = `
 
 // NewPurgeLogsCommand returns a command to purge logs.
 func NewPurgeLogsCommand() cmd.Command {
-	cmd := &purgeLogsCommand{}
+	cmd := &purgeLogsCommand{
+		store: jujuclient.NewFileClientStore(),
+	}
 	return modelcmd.WrapBase(cmd)
 }
 

internal/jimm/audit_log.go

@@ -4,8 +4,6 @@ package jimm
 
 import (
 	"context"
-	"crypto/rand"
-	"encoding/hex"
 	"encoding/json"
 	"time"
 
@@ -18,6 +16,7 @@ import (
 	"github.com/canonical/jimm/internal/db"
 	"github.com/canonical/jimm/internal/dbmodel"
 	"github.com/canonical/jimm/internal/servermon"
+	"github.com/canonical/jimm/internal/utils"
 )
 
 type DbAuditLogger struct {
@@ -26,19 +25,11 @@ type DbAuditLogger struct {
 	getUser        func() names.UserTag
 }
 
-// newConversationID generates a unique ID that is used for the
-// lifetime of a websocket connection.
-func newConversationID() string {
-	buf := make([]byte, 8)
-	rand.Read(buf) // Can't fail
-	return hex.EncodeToString(buf)
-}
-
 // NewDbAuditLogger returns a new audit logger that logs to the database.
 func NewDbAuditLogger(j *JIMM, getUserFunc func() names.UserTag) DbAuditLogger {
 	logger := DbAuditLogger{
 		jimm:           j,
-		conversationId: newConversationID(),
+		conversationId: utils.NewConversationID(),
 		getUser:        getUserFunc,
 	}
 	return logger
@@ -112,7 +103,7 @@ type recorder struct {
 func NewRecorder(logger DbAuditLogger) recorder {
 	return recorder{
 		start:          time.Now(),
-		conversationId: newConversationID(),
+		conversationId: utils.NewConversationID(),
 		logger:         logger,
 	}
 }
@@ -166,11 +157,11 @@ func (a *auditLogCleanupService) Start(ctx context.Context) {
 // from the service's context. It calculates the poll duration at 9am each day
 // UTC.
 func (a *auditLogCleanupService) poll(ctx context.Context) {
-	retentionDate := time.Now().AddDate(0, 0, -(a.auditLogRetentionPeriodInDays))
 
 	for {
 		select {
 		case <-time.After(calculateNextPollDuration(time.Now().UTC())):
+			retentionDate := time.Now().AddDate(0, 0, -(a.auditLogRetentionPeriodInDays))
 			deleted, err := a.db.DeleteAuditLogsBefore(ctx, retentionDate)
 			if err != nil {
 				zapctx.Error(ctx, "failed to cleanup audit logs", zap.Error(err))

internal/rpc/proxy.go

@@ -17,6 +17,7 @@ import (
 	"github.com/canonical/jimm/internal/auth"
 	"github.com/canonical/jimm/internal/dbmodel"
 	"github.com/canonical/jimm/internal/errors"
+	"github.com/canonical/jimm/internal/utils"
 )
 
 // TokenGenerator authenticates a user and generates a JWT token.
@@ -459,10 +460,11 @@ func ProxySockets(ctx context.Context, helpers ProxyHelpers) error {
 	// after the first message has been received so that any errors can be properly sent back to the client.
 	clProxy := clientProxy{
 		modelProxy: modelProxy{
-			src:      &client,
-			msgs:     &msgInFlight,
-			tokenGen: helpers.TokenGen,
-			auditLog: helpers.AuditLog,
+			src:            &client,
+			msgs:           &msgInFlight,
+			tokenGen:       helpers.TokenGen,
+			auditLog:       helpers.AuditLog,
+			conversationId: utils.NewConversationID(),
 		},
 		errChan:              errChan,
 		createControllerConn: helpers.ConnectController,

internal/utils/utils.go

@@ -0,0 +1,14 @@
+package utils
+
+import (
+	"crypto/rand"
+	"encoding/hex"
+)
+
+// NewConversationID generates a unique ID that is used for the
+// lifetime of a websocket connection.
+func NewConversationID() string {
+	buf := make([]byte, 8)
+	rand.Read(buf) // Can't fail
+	return hex.EncodeToString(buf)
+}

internal/utils/utils_test.go

@@ -0,0 +1,15 @@
+package utils_test
+
+import (
+	"testing"
+
+	qt "github.com/frankban/quicktest"
+
+	"github.com/canonical/jimm/internal/utils"
+)
+
+func TestNewConversationID(t *testing.T) {
+	c := qt.New(t)
+	res := utils.NewConversationID()
+	c.Assert(res, qt.HasLen, 16)
+}