Skip to content

Commit

Permalink
[3.2.0 backport] CBG-4113: Differentiate between nil set and empty …
Browse files Browse the repository at this point in the history
…set of enabled db audit events
  • Loading branch information
bbrks authored and torcolvin committed Jul 29, 2024
1 parent 8482e62 commit 957f4a4
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 34 deletions.
47 changes: 28 additions & 19 deletions rest/admin_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,10 @@ func (h *handler) handleGetDbAuditConfig() error {
// grab runtime version of config, so that we can see what events would be enabled
if runtimeConfig.Logging != nil && runtimeConfig.Logging.Audit != nil {
dbAuditEnabled = base.BoolDefault(runtimeConfig.Logging.Audit.Enabled, false)
for _, event := range runtimeConfig.Logging.Audit.EnabledEvents {
enabledEvents[base.AuditID(event)] = struct{}{}
if runtimeConfig.Logging.Audit.EnabledEvents != nil {
for _, event := range *runtimeConfig.Logging.Audit.EnabledEvents {
enabledEvents[base.AuditID(event)] = struct{}{}
}
}
dbAuditDisabledUsers = runtimeConfig.Logging.Audit.DisabledUsers
dbAuditDisabledRoles = runtimeConfig.Logging.Audit.DisabledRoles
Expand Down Expand Up @@ -877,14 +879,14 @@ func mutateConfigFromDbAuditConfigBody(isReplace bool, existingAuditConfig *DbAu
existingAuditConfig.DisabledRoles = requestAuditConfig.DisabledRoles

// we don't need to do anything to "disable" events, other than not enable them
existingAuditConfig.EnabledEvents = func() []uint {
existingAuditConfig.EnabledEvents = func() *[]uint {
enabledEvents := make([]uint, 0)
for event, shouldEnable := range eventsToChange {
if shouldEnable {
enabledEvents = append(enabledEvents, uint(event))
}
}
return enabledEvents
return &enabledEvents
}()
} else {
if requestAuditConfig.Enabled != nil {
Expand All @@ -896,22 +898,29 @@ func mutateConfigFromDbAuditConfigBody(isReplace bool, existingAuditConfig *DbAu
if requestAuditConfig.DisabledRoles != nil {
existingAuditConfig.DisabledRoles = requestAuditConfig.DisabledRoles
}

for i, event := range existingAuditConfig.EnabledEvents {
if shouldEnable, ok := eventsToChange[base.AuditID(event)]; ok {
if shouldEnable {
// already enabled
} else {
// disable by removing
existingAuditConfig.EnabledEvents = append(existingAuditConfig.EnabledEvents[:i], existingAuditConfig.EnabledEvents[i+1:]...)
}
// drop from toChange so we don't duplicate IDs
delete(eventsToChange, base.AuditID(event))
if len(eventsToChange) > 0 {
if existingAuditConfig.EnabledEvents == nil {
// initialize to non-nil set of defaults before modifying from request
existingAuditConfig.EnabledEvents = &base.DefaultDbAuditEventIDs
}
}
for id, enabled := range eventsToChange {
if enabled {
existingAuditConfig.EnabledEvents = append(existingAuditConfig.EnabledEvents, uint(id))
if existingAuditConfig.EnabledEvents != nil {
for i, event := range *existingAuditConfig.EnabledEvents {
if shouldEnable, ok := eventsToChange[base.AuditID(event)]; ok {
if shouldEnable {
// already enabled
} else {
// disable by removing
*existingAuditConfig.EnabledEvents = append((*existingAuditConfig.EnabledEvents)[:i], (*existingAuditConfig.EnabledEvents)[i+1:]...)
}
// drop from toChange so we don't duplicate IDs
delete(eventsToChange, base.AuditID(event))
}
}
for id, enabled := range eventsToChange {
if enabled {
*existingAuditConfig.EnabledEvents = append(*existingAuditConfig.EnabledEvents, uint(id))
}
}
}
}
}
Expand Down
19 changes: 16 additions & 3 deletions rest/adminapitest/admin_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4297,6 +4297,17 @@ func TestDatabaseConfigAuditAPI(t *testing.T) {
assert.False(t, responseBody["events"].(map[string]interface{})[base.AuditIDISGRStatus.String()].(bool), "audit enabled event should be disabled by default")
assert.True(t, responseBody["events"].(map[string]interface{})[base.AuditIDPublicUserAuthenticated.String()].(bool), "public user authenticated event should be enabled by default")

// CBG-4111: Try to disable events on top of the default (nil) set... either PUT or POST where *all* of the given IDs are set to false. Bug results in a no-op.
resp = rt.SendAdminRequest(http.MethodPost, "/db/_config/audit", fmt.Sprintf(`{"enabled":true,"events":{"%s":false}}`, base.AuditIDPublicUserAuthenticated))
rest.RequireStatus(t, resp, http.StatusOK)
// check event we just tried to disable
resp = rt.SendAdminRequest(http.MethodGet, "/db/_config/audit", "")
rest.RequireStatus(t, resp, http.StatusOK)
resp.DumpBody()
responseBody = nil
require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &responseBody))
assert.False(t, responseBody["events"].(map[string]interface{})[base.AuditIDPublicUserAuthenticated.String()].(bool), "public user authenticated event should be disabled")

// do a PUT to completely replace the full config (events not declared here will be disabled)
// enable AuditEnabled event, but implicitly others
resp = rt.SendAdminRequest(http.MethodPut, "/db/_config/audit", fmt.Sprintf(`{"enabled":true,"events":{"%s":true}}`, base.AuditIDISGRStatus))
Expand Down Expand Up @@ -4396,9 +4407,11 @@ func RequireEventCount(t *testing.T, runtimeConfig *rest.RuntimeDatabaseConfig,
require.Zero(t, expectedCount)
}
actualCount := 0
for _, configID := range loggingConfig.Audit.EnabledEvents {
if configID == uint(auditID) {
actualCount++
if loggingConfig.Audit.EnabledEvents != nil {
for _, configID := range *loggingConfig.Audit.EnabledEvents {
if configID == uint(auditID) {
actualCount++
}
}
}
require.Equal(t, expectedCount, actualCount)
Expand Down
2 changes: 1 addition & 1 deletion rest/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestAuditLoggingFields(t *testing.T) {
dbConfig.Logging = &DbLoggingConfig{
Audit: &DbAuditLoggingConfig{
Enabled: base.BoolPtr(true),
EnabledEvents: base.AllDbAuditeventIDs, // enable everything for testing
EnabledEvents: &base.AllDbAuditeventIDs, // enable everything for testing
DisabledUsers: []base.AuditLoggingPrincipal{
{Name: filteredPublicUsername, Domain: string(base.UserDomainSyncGateway)},
{Name: filteredAdminUsername, Domain: string(base.UserDomainCBServer)},
Expand Down
22 changes: 12 additions & 10 deletions rest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ type DbConsoleLoggingConfig struct {
// DbAuditLoggingConfig are per-db options configurable for audit logging
type DbAuditLoggingConfig struct {
Enabled *bool `json:"enabled,omitempty"` // Whether audit logging is enabled for this database
EnabledEvents []uint `json:"enabled_events,omitempty"` // List of audit event IDs that are enabled
EnabledEvents *[]uint `json:"enabled_events,omitempty"` // List of audit event IDs that are enabled - pointer to differentiate between empty slice and nil
DisabledUsers []base.AuditLoggingPrincipal `json:"disabled_users,omitempty"` // List of users to disable audit logging for
DisabledRoles []base.AuditLoggingPrincipal `json:"disabled_roles,omitempty"` // List of roles to disable audit logging for
}
Expand Down Expand Up @@ -1068,12 +1068,14 @@ func (dbConfig *DbConfig) validateVersion(ctx context.Context, isEnterpriseEditi
base.WarnfCtx(ctx, eeOnlyWarningMsg, "logging.audit.enabled", *dbConfig.Logging.Audit.Enabled, false)
dbConfig.Logging.Audit.Enabled = nil
}
for _, id := range dbConfig.Logging.Audit.EnabledEvents {
id := base.AuditID(id)
if e, ok := base.AuditEvents[id]; !ok {
multiError = multiError.Append(fmt.Errorf("unknown audit event ID %q", id))
} else if e.IsGlobalEvent {
multiError = multiError.Append(fmt.Errorf("event %q is not configurable at the database level", id))
if dbConfig.Logging.Audit.EnabledEvents != nil {
for _, id := range *dbConfig.Logging.Audit.EnabledEvents {
id := base.AuditID(id)
if e, ok := base.AuditEvents[id]; !ok {
multiError = multiError.Append(fmt.Errorf("unknown audit event ID %q", id))
} else if e.IsGlobalEvent {
multiError = multiError.Append(fmt.Errorf("event %q is not configurable at the database level", id))
}
}
}
}
Expand Down Expand Up @@ -2272,12 +2274,12 @@ func (c *DbConfig) toDbLogConfig(ctx context.Context) *base.DbLogConfig {
var aud *base.DbAuditLogConfig
if l.Audit != nil {
// per-event configuration
enabledEvents := make(map[base.AuditID]struct{}, len(l.Audit.EnabledEvents))
events := l.Audit.EnabledEvents
if events == nil {
events = base.DefaultDbAuditEventIDs
events = &base.DefaultDbAuditEventIDs
}
for _, event := range events {
enabledEvents := make(map[base.AuditID]struct{}, len(*events))
for _, event := range *events {
enabledEvents[base.AuditID(event)] = struct{}{}
}

Expand Down
2 changes: 1 addition & 1 deletion rest/config_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func DefaultPerDBLogging(bootstrapLoggingCnf base.LoggingConfig) *DbLoggingConfi
}
dblc.Audit = &DbAuditLoggingConfig{
Enabled: base.BoolPtr(base.DefaultDbAuditEnabled),
EnabledEvents: base.DefaultDbAuditEventIDs,
EnabledEvents: &base.DefaultDbAuditEventIDs,
}
return dblc
}
Expand Down

0 comments on commit 957f4a4

Please sign in to comment.