Skip to content

Commit 049799e

Browse files
authored
fix(audit): improve delete audit events for rollout and rule (#4346)
1 parent 3f44d37 commit 049799e

File tree

5 files changed

+54
-1
lines changed

5 files changed

+54
-1
lines changed

internal/server/audit/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,7 @@ func NewRollout(r *flipt.Rollout) *Rollout {
216216

217217
return rollout
218218
}
219+
220+
type contextKey string
221+
222+
const DeletedRecordCtxKey contextKey = "x-deleted-record"

internal/server/middleware/grpc/middleware.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"net/http"
9+
"slices"
910
"time"
1011

1112
"github.com/blang/semver/v4"
@@ -264,6 +265,17 @@ func AuditEventUnaryInterceptor(logger *zap.Logger, eventPairChecker audit.Event
264265
}
265266
}()
266267

268+
// When delete occurs there is no object returned. Audit log for rollout and rule
269+
// includes a limited set of fields, so we need to store the deleted record in the context
270+
// to improve audit log information. See https://github.com/orgs/flipt-io/discussions/4311
271+
if slices.ContainsFunc(requests, func(r flipt.Request) bool {
272+
return r.Action == flipt.ActionDelete && slices.Contains([]flipt.Subject{
273+
flipt.SubjectRollout, flipt.SubjectRule,
274+
}, r.Subject)
275+
}) {
276+
ctx = context.WithValue(ctx, audit.DeletedRecordCtxKey, map[any]any{})
277+
}
278+
267279
resp, err := handler(ctx, req)
268280
for _, request := range requests {
269281
if err != nil {
@@ -279,7 +291,18 @@ func AuditEventUnaryInterceptor(logger *zap.Logger, eventPairChecker audit.Event
279291
// Delete and Order request(s) have to be handled separately because they do not
280292
// return the concrete type but rather an *empty.Empty response.
281293
if request.Action == flipt.ActionDelete {
282-
events = append(events, audit.NewEvent(request, actor, r))
294+
payload := any(r)
295+
// Try to load deleted record from context and add extra info if possible.
296+
data, ok := ctx.Value(audit.DeletedRecordCtxKey).(map[any]any)
297+
if ok && data[req] != nil {
298+
switch r := data[req].(type) {
299+
case *flipt.Rollout:
300+
payload = audit.NewRollout(r)
301+
case *flipt.Rule:
302+
payload = audit.NewRule(r)
303+
}
304+
}
305+
events = append(events, audit.NewEvent(request, actor, payload))
283306
continue
284307
}
285308

internal/server/middleware/grpc/middleware_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,6 +1167,7 @@ func TestAuditUnaryInterceptor_DeleteRollout(t *testing.T) {
11671167
)
11681168

11691169
store.On("DeleteRollout", mock.Anything, req).Return(nil)
1170+
store.On("GetRollout", mock.Anything, mock.Anything, req.Id).Return(&flipt.Rollout{}, nil)
11701171

11711172
unaryInterceptor := AuditEventUnaryInterceptor(logger, &checkerDummy{})
11721173

@@ -1329,6 +1330,7 @@ func TestAuditUnaryInterceptor_DeleteRule(t *testing.T) {
13291330
)
13301331

13311332
store.On("DeleteRule", mock.Anything, req).Return(nil)
1333+
store.On("GetRule", mock.Anything, mock.Anything, req.Id).Return(&flipt.Rule{}, nil)
13321334

13331335
unaryInterceptor := AuditEventUnaryInterceptor(logger, &checkerDummy{})
13341336

internal/server/rollout.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package server
33
import (
44
"context"
55

6+
"go.flipt.io/flipt/internal/server/audit"
67
"go.flipt.io/flipt/internal/storage"
78
flipt "go.flipt.io/flipt/rpc/flipt"
89
"go.uber.org/zap"
@@ -59,6 +60,17 @@ func (s *Server) UpdateRollout(ctx context.Context, r *flipt.UpdateRolloutReques
5960

6061
func (s *Server) DeleteRollout(ctx context.Context, r *flipt.DeleteRolloutRequest) (*empty.Empty, error) {
6162
s.logger.Debug("delete rollout rule", zap.Stringer("request", r))
63+
// prefetch and store the rollout in the audit context if possible
64+
data, ok := ctx.Value(audit.DeletedRecordCtxKey).(map[any]any)
65+
if ok && data != nil {
66+
rollout, err := s.store.GetRollout(ctx,
67+
storage.NewNamespace(r.NamespaceKey), r.Id,
68+
)
69+
if err == nil {
70+
data[r] = rollout
71+
}
72+
}
73+
// delete the rollout
6274
err := s.store.DeleteRollout(ctx, r)
6375
s.logger.Debug("delete rollout rule", zap.Error(err))
6476
return &empty.Empty{}, err

internal/server/rule.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package server
33
import (
44
"context"
55

6+
"go.flipt.io/flipt/internal/server/audit"
67
"go.flipt.io/flipt/internal/storage"
78
flipt "go.flipt.io/flipt/rpc/flipt"
89
"go.uber.org/zap"
@@ -64,6 +65,17 @@ func (s *Server) UpdateRule(ctx context.Context, r *flipt.UpdateRuleRequest) (*f
6465
// DeleteRule deletes a rule
6566
func (s *Server) DeleteRule(ctx context.Context, r *flipt.DeleteRuleRequest) (*empty.Empty, error) {
6667
s.logger.Debug("delete rule", zap.Stringer("request", r))
68+
// prefetch and store the rule in the audit context if possible
69+
data, ok := ctx.Value(audit.DeletedRecordCtxKey).(map[any]any)
70+
if ok && data != nil {
71+
rule, err := s.store.GetRule(ctx,
72+
storage.NewNamespace(r.NamespaceKey), r.Id,
73+
)
74+
if err == nil {
75+
data[r] = rule
76+
}
77+
}
78+
// delete the rule
6779
if err := s.store.DeleteRule(ctx, r); err != nil {
6880
return nil, err
6981
}

0 commit comments

Comments
 (0)