From 4cd06777c05069e5139ba1d496800470a612fc88 Mon Sep 17 00:00:00 2001
From: "stonezdj(Daojun Zhang)" <stonezdj@gmail.com>
Date: Tue, 25 Feb 2025 11:42:48 +0800
Subject: [PATCH] Fix issue with user create/delete/update event (#21651)
Signed-off-by: stonezdj <stone.zhang@broadcom.com>
---
src/pkg/auditext/dao/dao_test.go | 2 +-
src/pkg/auditext/event/basic.go | 30 ++++++++++++++--------------
src/pkg/auditext/event/basic_test.go | 10 +++++-----
src/pkg/auditext/event/user/user.go | 8 ++++----
src/pkg/auditext/model/model.go | 4 ++--
5 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/pkg/auditext/dao/dao_test.go b/src/pkg/auditext/dao/dao_test.go
index 2296cc64b..76ada0284 100644
--- a/src/pkg/auditext/dao/dao_test.go
+++ b/src/pkg/auditext/dao/dao_test.go
@@ -186,7 +186,7 @@ func TestPermitEventTypes(t *testing.T) {
}
// test other event types
- otherEventTypes := permitEventTypes([]string{"create_artifact", "delete_artifact", "pull_artifact", "other_events"})
+ otherEventTypes := permitEventTypes([]string{"create_artifact", "delete_artifact", "pull_artifact", "other"})
if len(otherEventTypes) != len(model.EventTypes) {
t.Errorf("permitOtherEventTypes failed, it should include all event types")
}
diff --git a/src/pkg/auditext/event/basic.go b/src/pkg/auditext/event/basic.go
index b2418e851..5210fce99 100644
--- a/src/pkg/auditext/event/basic.go
+++ b/src/pkg/auditext/event/basic.go
@@ -31,25 +31,25 @@ import (
)
const (
- createOp = "create"
- updateOp = "update"
- deleteOp = "delete"
- resourceIDPattern = `^%v/(\d+)$`
+ createOp = "create"
+ updateOp = "update"
+ deleteOp = "delete"
)
// ResolveIDToNameFunc is the function to resolve the resource name from resource id
type ResolveIDToNameFunc func(string) string
type Resolver struct {
- BaseURLPattern string
- ResourceType string
- SucceedCodes []int
+ ResourceType string
+ SucceedCodes []int
// SensitiveAttributes is the attributes that need to be redacted
SensitiveAttributes []string
// ShouldResolveName indicates if the resource name should be resolved before delete, if true, need to resolve the resource name before delete
ShouldResolveName bool
// IDToNameFunc is used to resolve the resource name from resource id
IDToNameFunc ResolveIDToNameFunc
+ // ResourceIDPattern the URL pattern to match the resourceID
+ ResourceIDPattern string
}
// PreCheck check if the event should be captured and resolve the resource name if needed, if need to resolve the resource name, return the resource name
@@ -61,9 +61,9 @@ func (e *Resolver) PreCheck(ctx context.Context, url string, method string) (cap
// for delete operation on a resource has name, need to resolve the resource id to resource name before delete
resName := ""
if capture && method == http.MethodDelete && e.ShouldResolveName {
- re := regexp.MustCompile(fmt.Sprintf(resourceIDPattern, e.BaseURLPattern))
+ re := regexp.MustCompile(e.ResourceIDPattern)
m := re.FindStringSubmatch(url)
- if len(m) == 2 && e.IDToNameFunc != nil {
+ if len(m) >= 2 && e.IDToNameFunc != nil {
resName = e.IDToNameFunc(m[1])
}
}
@@ -92,9 +92,9 @@ func (e *Resolver) Resolve(ce *commonevent.Metadata, event *event.Event) error {
case createOp:
if len(ce.ResponseLocation) > 0 {
// extract resource id from response location
- re := regexp.MustCompile(fmt.Sprintf(resourceIDPattern, e.BaseURLPattern))
+ re := regexp.MustCompile(e.ResourceIDPattern)
m := re.FindStringSubmatch(ce.ResponseLocation)
- if len(m) != 2 {
+ if len(m) < 2 {
return nil
}
evt.ResourceName = m[1]
@@ -107,9 +107,9 @@ func (e *Resolver) Resolve(ce *commonevent.Metadata, event *event.Event) error {
}
case deleteOp:
- re := regexp.MustCompile(fmt.Sprintf(resourceIDPattern, e.BaseURLPattern))
+ re := regexp.MustCompile(e.ResourceIDPattern)
m := re.FindStringSubmatch(ce.RequestURL)
- if len(m) != 2 {
+ if len(m) < 2 {
return nil
}
evt.ResourceName = m[1]
@@ -118,9 +118,9 @@ func (e *Resolver) Resolve(ce *commonevent.Metadata, event *event.Event) error {
}
case updateOp:
- re := regexp.MustCompile(fmt.Sprintf(resourceIDPattern, e.BaseURLPattern))
+ re := regexp.MustCompile(e.ResourceIDPattern)
m := re.FindStringSubmatch(ce.RequestURL)
- if len(m) != 2 {
+ if len(m) < 2 {
return nil
}
evt.ResourceName = m[1]
diff --git a/src/pkg/auditext/event/basic_test.go b/src/pkg/auditext/event/basic_test.go
index 700a44153..3fbc4fc8e 100644
--- a/src/pkg/auditext/event/basic_test.go
+++ b/src/pkg/auditext/event/basic_test.go
@@ -21,12 +21,12 @@ import (
func TestEventResolver_PreCheck(t *testing.T) {
type fields struct {
- BaseURLPattern string
ResourceType string
SucceedCodes []int
SensitiveAttributes []string
ShouldResolveName bool
IDToNameFunc ResolveIDToNameFunc
+ ResourceIDPattern string
}
type args struct {
ctx context.Context
@@ -40,14 +40,14 @@ func TestEventResolver_PreCheck(t *testing.T) {
wantCapture bool
wantResourceName string
}{
- {"test normal", fields{BaseURLPattern: `/api/v2.0/tests`, ResourceType: "test", SucceedCodes: []int{200}, ShouldResolveName: true, IDToNameFunc: func(string) string { return "test" }}, args{context.Background(), "/api/v2.0/tests/123", "DELETE"}, true, "test"},
- {"test resource name", fields{BaseURLPattern: `/api/v2.0/tests`, ResourceType: "test", SucceedCodes: []int{200}, ShouldResolveName: true, IDToNameFunc: func(string) string { return "test_resource_name" }}, args{context.Background(), "/api/v2.0/tests/234", "DELETE"}, true, "test_resource_name"},
- {"test no resource name", fields{BaseURLPattern: `/api/v2.0/tests`, ResourceType: "test", SucceedCodes: []int{200}, ShouldResolveName: true}, args{context.Background(), "/api/v2.0/tests/234", "GET"}, true, ""},
+ {"test normal", fields{ResourceIDPattern: `/api/v2.0/tests/(\d+)`, ResourceType: "test", SucceedCodes: []int{200}, ShouldResolveName: true, IDToNameFunc: func(string) string { return "test" }}, args{context.Background(), "/api/v2.0/tests/123", "DELETE"}, true, "test"},
+ {"test resource name", fields{ResourceIDPattern: `/api/v2.0/tests/(\d+)`, ResourceType: "test", SucceedCodes: []int{200}, ShouldResolveName: true, IDToNameFunc: func(string) string { return "test_resource_name" }}, args{context.Background(), "/api/v2.0/tests/234", "DELETE"}, true, "test_resource_name"},
+ {"test no resource name", fields{ResourceIDPattern: `/api/v2.0/tests/(\d+)`, ResourceType: "test", SucceedCodes: []int{200}, ShouldResolveName: true}, args{context.Background(), "/api/v2.0/tests/234", "GET"}, true, ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := &Resolver{
- BaseURLPattern: tt.fields.BaseURLPattern,
+ ResourceIDPattern: tt.fields.ResourceIDPattern,
ResourceType: tt.fields.ResourceType,
SucceedCodes: tt.fields.SucceedCodes,
SensitiveAttributes: tt.fields.SensitiveAttributes,
diff --git a/src/pkg/auditext/event/user/user.go b/src/pkg/auditext/event/user/user.go
index 5d96bd605..96d19722b 100644
--- a/src/pkg/auditext/event/user/user.go
+++ b/src/pkg/auditext/event/user/user.go
@@ -26,21 +26,21 @@ import (
pkgUser "github.com/goharbor/harbor/src/pkg/user"
)
+const urlPattern = `^/api/v2.0/users/(\d+)(/password|/sysadmin)?$`
+
func init() {
var userResolver = &userEventResolver{
Resolver: event.Resolver{
- BaseURLPattern: "/api/v2.0/users",
ResourceType: rbac.ResourceUser.String(),
SucceedCodes: []int{http.StatusCreated, http.StatusOK},
SensitiveAttributes: []string{"password"},
ShouldResolveName: true,
IDToNameFunc: userIDToName,
+ ResourceIDPattern: urlPattern,
},
}
commonevent.RegisterResolver(`/api/v2.0/users$`, userResolver)
- commonevent.RegisterResolver(`^/api/v2.0/users/\d+/password$`, userResolver)
- commonevent.RegisterResolver(`^/api/v2.0/users/\d+/sysadmin$`, userResolver)
- commonevent.RegisterResolver(`^/api/v2.0/users/\d+$`, userResolver)
+ commonevent.RegisterResolver(urlPattern, userResolver)
}
type userEventResolver struct {
diff --git a/src/pkg/auditext/model/model.go b/src/pkg/auditext/model/model.go
index 5338cac5a..3dc820d39 100644
--- a/src/pkg/auditext/model/model.go
+++ b/src/pkg/auditext/model/model.go
@@ -20,7 +20,7 @@ import (
beego_orm "github.com/beego/beego/v2/client/orm"
)
-const OtherEvents = "other_events"
+const OtherEvents = "other"
func init() {
beego_orm.RegisterModel(&AuditLogExt{})
@@ -60,7 +60,7 @@ var EventTypes = []string{
"update_user",
"create_robot",
"delete_robot",
- "update_configure",
+ "update_configuration",
}
// OtherEventTypes defines the types of other audit log event types excludes previous EventTypes: create_artifact, delete_artifact, pull_artifact