CSS-5179 Audit log enhacement (#1026)

* Improve audit logging

- Fixed inability to filter by method
- Added ability to sort by time

* Added test for API params conversion

* Fixed godoc

* Tweak json for sortTime

api/params/params.go

@@ -204,6 +204,10 @@ type FindAuditEventsRequest struct {
 
 	// Limit is the maximum number of audit events to return.
 	Limit int `json:"limit,omitempty"`
+
+	// SortTime will sort by most recent (time descending) when true.
+	// When false no explicit ordering will be applied.
+	SortTime bool `json:"sortTime,omitempty"`
 }
 
 // A ListControllersResponse is the response that is sent in a

cmd/jimmctl/cmd/listauditevents.go

@@ -73,6 +73,8 @@ func (c *listAuditEventsCommand) SetFlags(f *gnuflag.FlagSet) {
 	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.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")
+
 }
 
 // Init implements the cmd.Command interface.

internal/db/audit.go

@@ -55,6 +55,10 @@ type AuditLogFilter struct {
 	// Limit is the maximum number of audit events to return.
 	// A value of zero will ignore the limit.
 	Limit int `json:"limit,omitempty"`
+
+	// SortTime will sort by most recent first (time descending) when true.
+	// When false no explicit ordering will be applied.
+	SortTime bool `json:"sortTime,omitempty"`
 }
 
 // ForEachAuditLogEntry iterates through all audit log entries that match
@@ -82,6 +86,9 @@ func (d *Database) ForEachAuditLogEntry(ctx context.Context, filter AuditLogFilt
 	if filter.Method != "" {
 		db = db.Where("facade_method = ?", filter.Method)
 	}
+	if filter.SortTime {
+		db = db.Order("time DESC")
+	}
 	db = db.Limit(filter.Limit)
 	db = db.Offset(filter.Offset)
 

internal/jimm/jimm_test.go

@@ -114,6 +114,14 @@ func TestFindAuditEvents(t *testing.T) {
 			Model: "TestModel",
 		},
 		expectedEvents: []dbmodel.AuditLogEntry{events[2], events[3]},
+	}, {
+		about: "admin/privileged user is allowed to find audit events by model and sort by time",
+		users: []*openfga.User{admin, privileged},
+		filter: db.AuditLogFilter{
+			Model:    "TestModel",
+			SortTime: true,
+		},
+		expectedEvents: []dbmodel.AuditLogEntry{events[3], events[2]},
 	}, {
 		about: "admin/privileged user is allowed to find audit events with limit/offset",
 		users: []*openfga.User{admin, privileged},

internal/jujuapi/export_test.go

@@ -16,6 +16,9 @@ var (
 	ParseTag              = parseTag
 	ResolveTag            = resolveTag
 	ModelInfoFromPath     = modelInfoFromPath
+	AuditParamsToFilter   = auditParamsToFilter
+	AuditLogDefaultLimit  = limitDefault
+	AuditLogUpperLimit    = maxLimit
 )
 
 func NewModelSummaryWatcher() *modelSummaryWatcher {

internal/jujuapi/jimm.go

@@ -324,32 +324,32 @@ func (r *controllerRoot) SetControllerDeprecated(ctx context.Context, req apipar
 const maxLimit = 1000
 const limitDefault = 50
 
-// FindAuditEvents finds the audit-log entries that match the given filter.
-func (r *controllerRoot) FindAuditEvents(ctx context.Context, req apiparams.FindAuditEventsRequest) (apiparams.AuditEvents, error) {
-	const op = errors.Op("jujuapi.FindAuditEvents")
-
+func auditParamsToFilter(req apiparams.FindAuditEventsRequest) (db.AuditLogFilter, error) {
 	var filter db.AuditLogFilter
 	var err error
+	filter.Method = req.Method
+	filter.Model = req.Model
+	filter.SortTime = req.SortTime
+
 	if req.After != "" {
 		filter.Start, err = time.Parse(time.RFC3339, req.After)
 		if err != nil {
-			return apiparams.AuditEvents{}, errors.E(op, err, errors.CodeBadRequest, `invalid "after" filter`)
+			return filter, errors.E(err, errors.CodeBadRequest, `invalid "after" filter`)
 		}
 	}
 	if req.Before != "" {
 		filter.End, err = time.Parse(time.RFC3339, req.Before)
 		if err != nil {
-			return apiparams.AuditEvents{}, errors.E(op, err, errors.CodeBadRequest, `invalid "before" filter`)
+			return filter, errors.E(err, errors.CodeBadRequest, `invalid "before" filter`)
 		}
 	}
 	if req.UserTag != "" {
 		tag, err := names.ParseUserTag(req.UserTag)
 		if err != nil {
-			return apiparams.AuditEvents{}, errors.E(op, err, errors.CodeBadRequest, `invalid "user-tag" filter`)
+			return filter, errors.E(err, errors.CodeBadRequest, `invalid "user-tag" filter`)
 		}
 		filter.UserTag = tag.String()
 	}
-	filter.Model = req.Model
 
 	limit := int(req.Limit)
 	if limit < 1 {
@@ -364,7 +364,16 @@ func (r *controllerRoot) FindAuditEvents(ctx context.Context, req apiparams.Find
 		offset = 0
 	}
 	filter.Offset = offset
+	return filter, nil
+}
 
+// FindAuditEvents finds the audit-log entries that match the given filter.
+func (r *controllerRoot) FindAuditEvents(ctx context.Context, req apiparams.FindAuditEventsRequest) (apiparams.AuditEvents, error) {
+	const op = errors.Op("jujuapi.FindAuditEvents")
+	filter, err := auditParamsToFilter(req)
+	if err != nil {
+		return apiparams.AuditEvents{}, errors.E(op, err)
+	}
 	entries, err := r.jimm.FindAuditEvents(ctx, r.user, filter)
 	if err != nil {
 		return apiparams.AuditEvents{}, errors.E(op, err)

internal/jujuapi/jimm_test.go

@@ -4,8 +4,10 @@ package jujuapi_test
 
 import (
 	"context"
+	"testing"
 	"time"
 
+	qt "github.com/frankban/quicktest"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/juju/juju/api/client/modelmanager"
 	"github.com/juju/juju/apiserver/common"
@@ -18,6 +20,7 @@ import (
 
 	"github.com/canonical/jimm/api"
 	apiparams "github.com/canonical/jimm/api/params"
+	"github.com/canonical/jimm/internal/db"
 	"github.com/canonical/jimm/internal/dbmodel"
 	"github.com/canonical/jimm/internal/jimmtest"
 	"github.com/canonical/jimm/internal/jujuapi"
@@ -400,6 +403,77 @@ func (s *jimmSuite) TestAuditLog(c *gc.C) {
 	c.Check(evs, jc.DeepEquals, expectedEvents)
 }
 
+func (s *jimmSuite) TestAuditLogFilterByMethod(c *gc.C) {
+	conn := s.open(c, nil, "alice")
+	defer conn.Close()
+	client := api.NewClient(conn)
+	evs, err := client.FindAuditEvents(&apiparams.FindAuditEventsRequest{Method: "Deploy"})
+	c.Assert(err, gc.Equals, nil)
+	c.Assert(len(evs.Events), gc.Equals, 0)
+}
+
+// TestAuditLogAPIParamsConversion tests the conversion of API params to a AuditLogFilter struct.
+// Note that this test doesn't require a running Juju/JIMM controller so it doesn't use gc + the jimmSuite.
+func TestAuditLogAPIParamsConversion(t *testing.T) {
+	c := qt.New(t)
+	testCases := []struct {
+		about   string
+		request apiparams.FindAuditEventsRequest
+		result  db.AuditLogFilter
+		err     error
+	}{
+		{
+			about: "Test basic conversion",
+			request: apiparams.FindAuditEventsRequest{
+				After:    "2023-08-14T00:00:00Z",
+				Before:   "2023-08-14T00:00:00Z",
+				UserTag:  "user-alice",
+				Model:    "123",
+				Method:   "Deploy",
+				Offset:   10,
+				Limit:    10,
+				SortTime: false,
+			},
+			result: db.AuditLogFilter{
+				Start:    time.Date(2023, 8, 14, 0, 0, 0, 0, time.UTC),
+				End:      time.Date(2023, 8, 14, 0, 0, 0, 0, time.UTC),
+				UserTag:  "user-alice",
+				Model:    "123",
+				Method:   "Deploy",
+				Offset:   10,
+				Limit:    10,
+				SortTime: false,
+			},
+		}, {
+			about: "Test limit lower bound",
+			request: apiparams.FindAuditEventsRequest{
+				Limit: 0,
+			},
+			result: db.AuditLogFilter{
+				Limit: jujuapi.AuditLogDefaultLimit,
+			},
+		}, {
+			about: "Test limit upper bound",
+			request: apiparams.FindAuditEventsRequest{
+				Limit: jujuapi.AuditLogUpperLimit + 1,
+			},
+			result: db.AuditLogFilter{
+				Limit: jujuapi.AuditLogUpperLimit,
+			},
+		},
+	}
+	for _, test := range testCases {
+		c.Log(test.about)
+		res, err := jujuapi.AuditParamsToFilter(test.request)
+		if test.err == nil {
+			c.Assert(err, qt.IsNil)
+			c.Assert(res, qt.DeepEquals, test.result)
+		} else {
+			c.Assert(err, qt.ErrorMatches, test.err)
+		}
+	}
+}
+
 func (s *jimmSuite) TestFullModelStatus(c *gc.C) {
 	s.AddController(c, "controller-2", s.APIInfo(c))
 	mt := s.AddModel(c, names.NewUserTag("charlie@external"), "model-1", names.NewCloudTag(jimmtest.TestCloudName), jimmtest.TestCloudRegionName, s.Model2.CloudCredential.ResourceTag())