mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: audit user AI budget override mutations (#25745)
Relates to https://linear.app/codercom/issue/AIGOV-285/add-user-budget-overrides-table-and-crud-api Adds audit-log support for `user_ai_budget_override` mutations. Without it, an admin could quietly change a user's per-user spend cap (e.g. from `$500` to `$50`), reassign it to a different group, or delete it entirely with no record of who did it. Both write (`create-or-update`) and delete actions now generate audit log entries. Unlike group AI budgets, which only track `spend_limit`, overrides also track `group_name`: an override can be reassigned to a different attributed group, so that change needs to show up in the diff. The raw `spend_limit_micros`, IDs, and timestamps are ignored in favor of the human-readable `spend_limit` and `group_name`. Depends on #25439. ## Screenshot <img width="1343" height="514" alt="image" src="https://github.com/user-attachments/assets/aee30f58-6e81-435e-9bca-5bc98f49d8d3" />
This commit is contained in:
Yevhenii Shcherbina
@@ -19,6 +19,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/audit"
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/db2sdk"
|
||||
"github.com/coder/coder/v2/coderd/database/dbauthz"
|
||||
"github.com/coder/coder/v2/coderd/httpapi"
|
||||
"github.com/coder/coder/v2/coderd/httpmw"
|
||||
"github.com/coder/coder/v2/coderd/searchquery"
|
||||
@@ -872,9 +873,10 @@ func (api *API) upsertUserAIBudgetOverride(rw http.ResponseWriter, r *http.Reque
|
||||
return
|
||||
}
|
||||
|
||||
// Look up the group first so a missing or forbidden group_id returns
|
||||
// 404, distinct from the 400 "not a member" case handled below.
|
||||
if _, err := api.Database.GetGroupByID(ctx, req.GroupID); err != nil {
|
||||
// Look up the new group first so a missing or forbidden group_id
|
||||
// returns 404. We also need the group for the audit log.
|
||||
newGroup, err := api.Database.GetGroupByID(ctx, req.GroupID)
|
||||
if err != nil {
|
||||
if httpapi.Is404Error(err) {
|
||||
httpapi.ResourceNotFound(rw)
|
||||
return
|
||||
@@ -884,6 +886,39 @@ func (api *API) upsertUserAIBudgetOverride(rw http.ResponseWriter, r *http.Reque
|
||||
return
|
||||
}
|
||||
|
||||
auditor := api.AGPL.Auditor.Load()
|
||||
aReq, commitAudit := audit.InitRequest[database.AuditableUserAiBudgetOverride](rw, &audit.RequestParams{
|
||||
Audit: *auditor,
|
||||
Log: api.Logger,
|
||||
Request: r,
|
||||
Action: database.AuditActionWrite,
|
||||
OrganizationID: newGroup.OrganizationID,
|
||||
})
|
||||
defer commitAudit()
|
||||
|
||||
// Capture the existing override (if any) so the audit log records the
|
||||
// before-state. An absent row leaves aReq.Old as the zero value.
|
||||
oldOverride, overrideErr := api.Database.GetUserAIBudgetOverride(ctx, user.ID)
|
||||
if overrideErr != nil && !errors.Is(overrideErr, sql.ErrNoRows) {
|
||||
api.Logger.Error(ctx, "fetch existing user AI budget override for audit", slog.Error(overrideErr))
|
||||
httpapi.InternalServerError(rw, overrideErr)
|
||||
return
|
||||
}
|
||||
var oldGroupName string
|
||||
if overrideErr == nil {
|
||||
// This lookup exists only to record the old group's name in the audit
|
||||
// diff. Use a system context so it does not add a read requirement on
|
||||
// the old group that the upsert itself does not impose.
|
||||
oldGroup, groupErr := api.Database.GetGroupByID(dbauthz.AsSystemRestricted(ctx), oldOverride.GroupID) //nolint:gocritic // see above
|
||||
if groupErr != nil {
|
||||
api.Logger.Error(ctx, "fetch old group for user AI budget override audit", slog.Error(groupErr))
|
||||
httpapi.InternalServerError(rw, groupErr)
|
||||
return
|
||||
}
|
||||
oldGroupName = oldGroup.Name
|
||||
}
|
||||
aReq.Old = oldOverride.Auditable(user.Username, oldGroupName)
|
||||
|
||||
override, err := api.Database.UpsertUserAIBudgetOverride(ctx, database.UpsertUserAIBudgetOverrideParams{
|
||||
UserID: user.ID,
|
||||
GroupID: req.GroupID,
|
||||
@@ -911,6 +946,7 @@ func (api *API) upsertUserAIBudgetOverride(rw http.ResponseWriter, r *http.Reque
|
||||
httpapi.InternalServerError(rw, err)
|
||||
return
|
||||
}
|
||||
aReq.New = override.Auditable(user.Username, newGroup.Name)
|
||||
|
||||
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.UserAIBudgetOverride(override))
|
||||
}
|
||||
@@ -926,7 +962,36 @@ func (api *API) deleteUserAIBudgetOverride(rw http.ResponseWriter, r *http.Reque
|
||||
ctx := r.Context()
|
||||
user := httpmw.UserParam(r)
|
||||
|
||||
_, err := api.Database.DeleteUserAIBudgetOverride(ctx, user.ID)
|
||||
// Fetch the existing override first for audit purposes.
|
||||
userOverride, err := api.Database.GetUserAIBudgetOverride(ctx, user.ID)
|
||||
if httpapi.Is404Error(err) {
|
||||
httpapi.ResourceNotFound(rw)
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
api.Logger.Error(ctx, "fetch user AI budget override for delete", slog.Error(err))
|
||||
httpapi.InternalServerError(rw, err)
|
||||
return
|
||||
}
|
||||
|
||||
group, err := api.Database.GetGroupByID(ctx, userOverride.GroupID)
|
||||
if err != nil {
|
||||
api.Logger.Error(ctx, "get group for user AI budget override delete audit", slog.Error(err))
|
||||
httpapi.InternalServerError(rw, err)
|
||||
return
|
||||
}
|
||||
|
||||
auditor := api.AGPL.Auditor.Load()
|
||||
aReq, commitAudit := audit.InitRequest[database.AuditableUserAiBudgetOverride](rw, &audit.RequestParams{
|
||||
Audit: *auditor,
|
||||
Log: api.Logger,
|
||||
Request: r,
|
||||
Action: database.AuditActionDelete,
|
||||
OrganizationID: group.OrganizationID,
|
||||
})
|
||||
defer commitAudit()
|
||||
|
||||
_, err = api.Database.DeleteUserAIBudgetOverride(ctx, user.ID)
|
||||
if httpapi.Is404Error(err) {
|
||||
httpapi.ResourceNotFound(rw)
|
||||
return
|
||||
@@ -936,6 +1001,10 @@ func (api *API) deleteUserAIBudgetOverride(rw http.ResponseWriter, r *http.Reque
|
||||
httpapi.InternalServerError(rw, err)
|
||||
return
|
||||
}
|
||||
// Populate the audit snapshot only after delete succeeds. Setting
|
||||
// it earlier would record a phantom entry if delete races a
|
||||
// concurrent delete and returns 404.
|
||||
aReq.Old = userOverride.Auditable(user.Username, group.Name)
|
||||
|
||||
rw.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
@@ -3087,6 +3087,231 @@ func TestUserAIBudgetOverride(t *testing.T) {
|
||||
require.ErrorAs(t, err, &sdkErr)
|
||||
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
|
||||
})
|
||||
|
||||
t.Run("Audit/CreatesAndDeletes", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, adminClient, owner, targetUser := setupUserAIBudgetOverrideAuditTest(t)
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
group, err := adminClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{
|
||||
Name: "override-audit",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = adminClient.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
|
||||
AddUsers: []string{targetUser.ID.String()},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Upsert (create-or-update) emits an AuditActionWrite entry.
|
||||
_, err = adminClient.UpsertUserAIBudgetOverride(ctx, targetUser.ID, codersdk.UpsertUserAIBudgetOverrideRequest{
|
||||
GroupID: group.ID,
|
||||
SpendLimitMicros: 500_000_000,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Delete emits an AuditActionDelete entry against the same resource.
|
||||
require.NoError(t, adminClient.DeleteUserAIBudgetOverride(ctx, targetUser.ID))
|
||||
|
||||
rows, err := db.GetAuditLogsOffset(
|
||||
ctx,
|
||||
database.GetAuditLogsOffsetParams{
|
||||
ResourceType: string(database.ResourceTypeUserAiBudgetOverride),
|
||||
LimitOpt: 10,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, rows, 2, "expected one upsert and one delete audit entry")
|
||||
// GetAuditLogsOffset returns entries sorted by time in descending order.
|
||||
upsertLog := rows[1].AuditLog
|
||||
deleteLog := rows[0].AuditLog
|
||||
|
||||
require.Equal(t, database.AuditActionWrite, upsertLog.Action)
|
||||
require.Equal(t, targetUser.ID, upsertLog.ResourceID)
|
||||
require.Equal(t, database.ResourceTypeUserAiBudgetOverride, upsertLog.ResourceType)
|
||||
require.Equal(t, targetUser.Username, upsertLog.ResourceTarget)
|
||||
require.Equal(t, owner.OrganizationID, upsertLog.OrganizationID)
|
||||
|
||||
var upsertDiff audit.Map
|
||||
require.NoError(t, json.Unmarshal(upsertLog.Diff, &upsertDiff))
|
||||
require.Contains(t, upsertDiff, "spend_limit")
|
||||
require.Equal(t, "$0.00", upsertDiff["spend_limit"].Old)
|
||||
require.Equal(t, "$500.00", upsertDiff["spend_limit"].New)
|
||||
require.Contains(t, upsertDiff, "group_name")
|
||||
require.Equal(t, "", upsertDiff["group_name"].Old)
|
||||
require.Equal(t, group.Name, upsertDiff["group_name"].New)
|
||||
require.Contains(t, upsertDiff, "group_id")
|
||||
require.Equal(t, "", upsertDiff["group_id"].Old)
|
||||
require.Equal(t, group.ID.String(), upsertDiff["group_id"].New)
|
||||
// Fields marked ActionIgnore must not appear in the diff.
|
||||
require.NotContains(t, upsertDiff, "user_id")
|
||||
require.NotContains(t, upsertDiff, "username")
|
||||
require.NotContains(t, upsertDiff, "spend_limit_micros")
|
||||
require.NotContains(t, upsertDiff, "created_at")
|
||||
require.NotContains(t, upsertDiff, "updated_at")
|
||||
|
||||
require.Equal(t, database.AuditActionDelete, deleteLog.Action)
|
||||
require.Equal(t, targetUser.ID, deleteLog.ResourceID)
|
||||
require.Equal(t, database.ResourceTypeUserAiBudgetOverride, deleteLog.ResourceType)
|
||||
require.Equal(t, targetUser.Username, deleteLog.ResourceTarget)
|
||||
require.Equal(t, owner.OrganizationID, deleteLog.OrganizationID)
|
||||
|
||||
var deleteDiff audit.Map
|
||||
require.NoError(t, json.Unmarshal(deleteLog.Diff, &deleteDiff))
|
||||
require.Contains(t, deleteDiff, "spend_limit")
|
||||
require.Equal(t, "$500.00", deleteDiff["spend_limit"].Old)
|
||||
require.Equal(t, "", deleteDiff["spend_limit"].New)
|
||||
require.Contains(t, deleteDiff, "group_name")
|
||||
require.Equal(t, group.Name, deleteDiff["group_name"].Old)
|
||||
require.Equal(t, "", deleteDiff["group_name"].New)
|
||||
require.Contains(t, deleteDiff, "group_id")
|
||||
require.Equal(t, group.ID.String(), deleteDiff["group_id"].Old)
|
||||
require.Equal(t, "", deleteDiff["group_id"].New)
|
||||
})
|
||||
|
||||
t.Run("Audit/DeleteAbsentEmitsNoEntry", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Deleting an override that does not exist must not emit an audit log entry.
|
||||
db, adminClient, _, targetUser := setupUserAIBudgetOverrideAuditTest(t)
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
err := adminClient.DeleteUserAIBudgetOverride(ctx, targetUser.ID)
|
||||
var sdkErr *codersdk.Error
|
||||
require.ErrorAs(t, err, &sdkErr)
|
||||
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
|
||||
|
||||
rows, err := db.GetAuditLogsOffset(
|
||||
ctx,
|
||||
database.GetAuditLogsOffsetParams{
|
||||
ResourceType: string(database.ResourceTypeUserAiBudgetOverride),
|
||||
LimitOpt: 10,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, rows, "no audit entry expected when delete returns 404")
|
||||
})
|
||||
|
||||
t.Run("Audit/UpsertEverything", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A second upsert that reassigns the attributed group and changes
|
||||
// the spend limit must record the prior state as the audit
|
||||
// before-state.
|
||||
db, adminClient, owner, targetUser := setupUserAIBudgetOverrideAuditTest(t)
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
groupA, err := adminClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{
|
||||
Name: "reassign-audit-a",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = adminClient.PatchGroup(ctx, groupA.ID, codersdk.PatchGroupRequest{
|
||||
AddUsers: []string{targetUser.ID.String()},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
groupB, err := adminClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{
|
||||
Name: "reassign-audit-b",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = adminClient.PatchGroup(ctx, groupB.ID, codersdk.PatchGroupRequest{
|
||||
AddUsers: []string{targetUser.ID.String()},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// First upsert: create the override attributed to groupA.
|
||||
_, err = adminClient.UpsertUserAIBudgetOverride(ctx, targetUser.ID, codersdk.UpsertUserAIBudgetOverrideRequest{
|
||||
GroupID: groupA.ID,
|
||||
SpendLimitMicros: 500_000_000,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Second upsert: reassign to groupB and raise the spend limit.
|
||||
_, err = adminClient.UpsertUserAIBudgetOverride(ctx, targetUser.ID, codersdk.UpsertUserAIBudgetOverrideRequest{
|
||||
GroupID: groupB.ID,
|
||||
SpendLimitMicros: 1_000_000_000,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
rows, err := db.GetAuditLogsOffset(
|
||||
ctx,
|
||||
database.GetAuditLogsOffsetParams{
|
||||
ResourceType: string(database.ResourceTypeUserAiBudgetOverride),
|
||||
LimitOpt: 10,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, rows, 2, "expected one create and one update audit entry")
|
||||
// GetAuditLogsOffset returns entries sorted by time in descending order.
|
||||
updateLog := rows[0].AuditLog
|
||||
|
||||
var updateDiff audit.Map
|
||||
require.NoError(t, json.Unmarshal(updateLog.Diff, &updateDiff))
|
||||
require.Contains(t, updateDiff, "group_name")
|
||||
require.Equal(t, groupA.Name, updateDiff["group_name"].Old)
|
||||
require.Equal(t, groupB.Name, updateDiff["group_name"].New)
|
||||
require.Contains(t, updateDiff, "group_id")
|
||||
require.Equal(t, groupA.ID.String(), updateDiff["group_id"].Old)
|
||||
require.Equal(t, groupB.ID.String(), updateDiff["group_id"].New)
|
||||
require.Contains(t, updateDiff, "spend_limit")
|
||||
require.Equal(t, "$500.00", updateDiff["spend_limit"].Old)
|
||||
require.Equal(t, "$1000.00", updateDiff["spend_limit"].New)
|
||||
})
|
||||
|
||||
t.Run("Audit/UpsertSpendLimit", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A second upsert that keeps the same group and only changes the
|
||||
// spend limit must produce a diff that contains spend_limit and omits
|
||||
// the unchanged group_name and group_id.
|
||||
db, adminClient, owner, targetUser := setupUserAIBudgetOverrideAuditTest(t)
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
group, err := adminClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{
|
||||
Name: "spend-only-audit",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = adminClient.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
|
||||
AddUsers: []string{targetUser.ID.String()},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// First upsert: create the override attributed to the group.
|
||||
_, err = adminClient.UpsertUserAIBudgetOverride(ctx, targetUser.ID, codersdk.UpsertUserAIBudgetOverrideRequest{
|
||||
GroupID: group.ID,
|
||||
SpendLimitMicros: 500_000_000,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Second upsert: keep the same group, raise only the spend limit.
|
||||
_, err = adminClient.UpsertUserAIBudgetOverride(ctx, targetUser.ID, codersdk.UpsertUserAIBudgetOverrideRequest{
|
||||
GroupID: group.ID,
|
||||
SpendLimitMicros: 1_000_000_000,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
rows, err := db.GetAuditLogsOffset(
|
||||
ctx,
|
||||
database.GetAuditLogsOffsetParams{
|
||||
ResourceType: string(database.ResourceTypeUserAiBudgetOverride),
|
||||
LimitOpt: 10,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, rows, 2, "expected one create and one update audit entry")
|
||||
// GetAuditLogsOffset returns entries sorted by time in descending order.
|
||||
updateLog := rows[0].AuditLog
|
||||
|
||||
var updateDiff audit.Map
|
||||
require.NoError(t, json.Unmarshal(updateLog.Diff, &updateDiff))
|
||||
require.Contains(t, updateDiff, "spend_limit")
|
||||
require.Equal(t, "$500.00", updateDiff["spend_limit"].Old)
|
||||
require.Equal(t, "$1000.00", updateDiff["spend_limit"].New)
|
||||
require.NotContains(t, updateDiff, "group_name")
|
||||
require.NotContains(t, updateDiff, "group_id")
|
||||
require.NotContains(t, updateDiff, "spend_limit_micros")
|
||||
})
|
||||
}
|
||||
|
||||
// TestUserAIBudgetOverrideRoleAccess verifies the authz matrix for the roles
|
||||
@@ -3312,6 +3537,41 @@ func setupUserAIBudgetOverrideTest(t *testing.T) (adminClient *codersdk.Client,
|
||||
return adminClient, targetUser, g
|
||||
}
|
||||
|
||||
// setupUserAIBudgetOverrideAuditTest builds a deployment wired with the
|
||||
// enterprise auditor (the mock auditor does not compute diffs) so audit
|
||||
// entries can be read straight from the audit_logs table.
|
||||
func setupUserAIBudgetOverrideAuditTest(t *testing.T) (database.Store, *codersdk.Client, codersdk.CreateFirstUserResponse, codersdk.User) {
|
||||
t.Helper()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
auditor := entaudit.NewAuditor(
|
||||
db,
|
||||
entaudit.DefaultFilter,
|
||||
backends.NewPostgres(db, true),
|
||||
)
|
||||
dv := coderdtest.DeploymentValues(t)
|
||||
dv.AI.BridgeConfig.Enabled = serpent.Bool(true)
|
||||
ownerClient, owner := coderdenttest.New(t, &coderdenttest.Options{
|
||||
AuditLogging: true,
|
||||
Options: &coderdtest.Options{
|
||||
DeploymentValues: dv,
|
||||
Database: db,
|
||||
Pubsub: ps,
|
||||
Auditor: auditor,
|
||||
},
|
||||
LicenseOptions: &coderdenttest.LicenseOptions{
|
||||
Features: license.Features{
|
||||
codersdk.FeatureTemplateRBAC: 1,
|
||||
codersdk.FeatureAIBridge: 1,
|
||||
codersdk.FeatureAuditLog: 1,
|
||||
},
|
||||
},
|
||||
})
|
||||
adminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleUserAdmin())
|
||||
_, targetUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
|
||||
return db, adminClient, owner, targetUser
|
||||
}
|
||||
|
||||
// setupGroupAIBudgetTest returns an Admin client along with a newly created group inside it.
|
||||
func setupGroupAIBudgetTest(t *testing.T) (adminClient *codersdk.Client, group codersdk.Group) {
|
||||
t.Helper()
|
||||
|
||||
Reference in New Issue
Block a user