From 1eb189a9128ecf8a12a1bcb39f3d55b760d961e0 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 18 Jun 2024 16:28:59 -0800 Subject: [PATCH] fix: fill out missing user properties to /audit All except the organization IDs. We can add this as well if needed, but the complaint was specifically about last_seen_at. --- coderd/audit.go | 40 ++++++++-------- coderd/audit_test.go | 51 ++++++++++++++++++++ coderd/database/dbmem/dbmem.go | 47 +++++++++++-------- coderd/database/queries.sql.go | 67 ++++++++++++++++++--------- coderd/database/queries/auditlogs.sql | 9 ++++ 5 files changed, 151 insertions(+), 63 deletions(-) diff --git a/coderd/audit.go b/coderd/audit.go index ab82e91698d8c..a818f681038ed 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -20,7 +20,6 @@ import ( "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" - "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/searchquery" "github.com/coder/coder/v2/codersdk" ) @@ -183,27 +182,26 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs _ = json.Unmarshal(dblog.Diff, &diff) var user *codersdk.User - if dblog.UserUsername.Valid { - user = &codersdk.User{ - ReducedUser: codersdk.ReducedUser{ - MinimalUser: codersdk.MinimalUser{ - ID: dblog.UserID, - Username: dblog.UserUsername.String, - AvatarURL: dblog.UserAvatarUrl.String, - }, - Email: dblog.UserEmail.String, - CreatedAt: dblog.UserCreatedAt.Time, - Status: codersdk.UserStatus(dblog.UserStatus.UserStatus), - }, - Roles: []codersdk.SlimRole{}, - } - - for _, input := range dblog.UserRoles { - roleName, _ := rbac.RoleNameFromString(input) - rbacRole, _ := rbac.RoleByName(roleName) - user.Roles = append(user.Roles, db2sdk.SlimRole(rbacRole)) - } + // Leaving the organization IDs blank for now; not sure they are useful for + // the audit query anyway? + sdkUser := db2sdk.User(database.User{ + ID: dblog.UserID, + Email: dblog.UserEmail.String, + Username: dblog.UserUsername.String, + CreatedAt: dblog.UserCreatedAt.Time, + UpdatedAt: dblog.UserUpdatedAt.Time, + Status: dblog.UserStatus.UserStatus, + RBACRoles: dblog.UserRoles, + LoginType: dblog.UserLoginType.LoginType, + AvatarURL: dblog.UserAvatarUrl.String, + Deleted: dblog.UserDeleted.Bool, + LastSeenAt: dblog.UserLastSeenAt.Time, + QuietHoursSchedule: dblog.UserQuietHoursSchedule.String, + ThemePreference: dblog.UserThemePreference.String, + Name: dblog.UserName.String, + }, []uuid.UUID{}) + user = &sdkUser } var ( diff --git a/coderd/audit_test.go b/coderd/audit_test.go index b8b62cf27ecf0..9de6f65071a02 100644 --- a/coderd/audit_test.go +++ b/coderd/audit_test.go @@ -8,11 +8,13 @@ import ( "testing" "time" + "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" ) @@ -42,6 +44,55 @@ func TestAuditLogs(t *testing.T) { require.Len(t, alogs.AuditLogs, 1) }) + t.Run("User", func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + client2, user2 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner()) + + err := client2.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{ + ResourceID: user2.ID, + }) + require.NoError(t, err) + + alogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{ + Pagination: codersdk.Pagination{ + Limit: 1, + }, + }) + require.NoError(t, err) + require.Equal(t, int64(1), alogs.Count) + require.Len(t, alogs.AuditLogs, 1) + + // Make sure the returned user is fully populated. + foundUser, err := client.User(ctx, user2.ID.String()) + foundUser.OrganizationIDs = []uuid.UUID{} // Not included. + require.NoError(t, err) + require.Equal(t, foundUser, *alogs.AuditLogs[0].User) + + // Delete the user and try again. This is a soft delete so nothing should + // change. If users are hard deleted we should get nil, but there is no way + // to test this at the moment. + err = client.DeleteUser(ctx, user2.ID) + require.NoError(t, err) + + alogs, err = client.AuditLogs(ctx, codersdk.AuditLogsRequest{ + Pagination: codersdk.Pagination{ + Limit: 1, + }, + }) + require.NoError(t, err) + require.Equal(t, int64(1), alogs.Count) + require.Len(t, alogs.AuditLogs, 1) + + foundUser, err = client.User(ctx, user2.ID.String()) + foundUser.OrganizationIDs = []uuid.UUID{} // Not included. + require.NoError(t, err) + require.Equal(t, foundUser, *alogs.AuditLogs[0].User) + }) + t.Run("WorkspaceBuildAuditLink", func(t *testing.T) { t.Parallel() diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 47d75e831469e..f83838dd4be94 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1969,26 +1969,33 @@ func (q *FakeQuerier) GetAuditLogsOffset(_ context.Context, arg database.GetAudi userValid := err == nil logs = append(logs, database.GetAuditLogsOffsetRow{ - ID: alog.ID, - RequestID: alog.RequestID, - OrganizationID: alog.OrganizationID, - Ip: alog.Ip, - UserAgent: alog.UserAgent, - ResourceType: alog.ResourceType, - ResourceID: alog.ResourceID, - ResourceTarget: alog.ResourceTarget, - ResourceIcon: alog.ResourceIcon, - Action: alog.Action, - Diff: alog.Diff, - StatusCode: alog.StatusCode, - AdditionalFields: alog.AdditionalFields, - UserID: alog.UserID, - UserUsername: sql.NullString{String: user.Username, Valid: userValid}, - UserEmail: sql.NullString{String: user.Email, Valid: userValid}, - UserCreatedAt: sql.NullTime{Time: user.CreatedAt, Valid: userValid}, - UserStatus: database.NullUserStatus{UserStatus: user.Status, Valid: userValid}, - UserRoles: user.RBACRoles, - Count: 0, + ID: alog.ID, + RequestID: alog.RequestID, + OrganizationID: alog.OrganizationID, + Ip: alog.Ip, + UserAgent: alog.UserAgent, + ResourceType: alog.ResourceType, + ResourceID: alog.ResourceID, + ResourceTarget: alog.ResourceTarget, + ResourceIcon: alog.ResourceIcon, + Action: alog.Action, + Diff: alog.Diff, + StatusCode: alog.StatusCode, + AdditionalFields: alog.AdditionalFields, + UserID: alog.UserID, + UserUsername: sql.NullString{String: user.Username, Valid: userValid}, + UserName: sql.NullString{String: user.Name, Valid: userValid}, + UserEmail: sql.NullString{String: user.Email, Valid: userValid}, + UserCreatedAt: sql.NullTime{Time: user.CreatedAt, Valid: userValid}, + UserUpdatedAt: sql.NullTime{Time: user.UpdatedAt, Valid: userValid}, + UserLastSeenAt: sql.NullTime{Time: user.LastSeenAt, Valid: userValid}, + UserLoginType: database.NullLoginType{LoginType: user.LoginType, Valid: userValid}, + UserDeleted: sql.NullBool{Bool: user.Deleted, Valid: userValid}, + UserThemePreference: sql.NullString{String: user.ThemePreference, Valid: userValid}, + UserQuietHoursSchedule: sql.NullString{String: user.QuietHoursSchedule, Valid: userValid}, + UserStatus: database.NullUserStatus{UserStatus: user.Status, Valid: userValid}, + UserRoles: user.RBACRoles, + Count: 0, }) if len(logs) >= int(arg.Limit) { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2c0b21824ad28..4f113323a024f 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -444,12 +444,21 @@ func (q *sqlQuerier) UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDP const getAuditLogsOffset = `-- name: GetAuditLogsOffset :many SELECT audit_logs.id, audit_logs.time, audit_logs.user_id, audit_logs.organization_id, audit_logs.ip, audit_logs.user_agent, audit_logs.resource_type, audit_logs.resource_id, audit_logs.resource_target, audit_logs.action, audit_logs.diff, audit_logs.status_code, audit_logs.additional_fields, audit_logs.request_id, audit_logs.resource_icon, + -- sqlc.embed(users) would be nice but it does not seem to play well with + -- left joins. users.username AS user_username, + users.name AS user_name, users.email AS user_email, users.created_at AS user_created_at, + users.updated_at AS user_updated_at, + users.last_seen_at AS user_last_seen_at, users.status AS user_status, + users.login_type AS user_login_type, users.rbac_roles AS user_roles, users.avatar_url AS user_avatar_url, + users.deleted AS user_deleted, + users.theme_preference AS user_theme_preference, + users.quiet_hours_schedule AS user_quiet_hours_schedule, COUNT(audit_logs.*) OVER () AS count FROM audit_logs @@ -563,28 +572,35 @@ type GetAuditLogsOffsetParams struct { } type GetAuditLogsOffsetRow struct { - ID uuid.UUID `db:"id" json:"id"` - Time time.Time `db:"time" json:"time"` - UserID uuid.UUID `db:"user_id" json:"user_id"` - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - Ip pqtype.Inet `db:"ip" json:"ip"` - UserAgent sql.NullString `db:"user_agent" json:"user_agent"` - ResourceType ResourceType `db:"resource_type" json:"resource_type"` - ResourceID uuid.UUID `db:"resource_id" json:"resource_id"` - ResourceTarget string `db:"resource_target" json:"resource_target"` - Action AuditAction `db:"action" json:"action"` - Diff json.RawMessage `db:"diff" json:"diff"` - StatusCode int32 `db:"status_code" json:"status_code"` - AdditionalFields json.RawMessage `db:"additional_fields" json:"additional_fields"` - RequestID uuid.UUID `db:"request_id" json:"request_id"` - ResourceIcon string `db:"resource_icon" json:"resource_icon"` - UserUsername sql.NullString `db:"user_username" json:"user_username"` - UserEmail sql.NullString `db:"user_email" json:"user_email"` - UserCreatedAt sql.NullTime `db:"user_created_at" json:"user_created_at"` - UserStatus NullUserStatus `db:"user_status" json:"user_status"` - UserRoles pq.StringArray `db:"user_roles" json:"user_roles"` - UserAvatarUrl sql.NullString `db:"user_avatar_url" json:"user_avatar_url"` - Count int64 `db:"count" json:"count"` + ID uuid.UUID `db:"id" json:"id"` + Time time.Time `db:"time" json:"time"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + Ip pqtype.Inet `db:"ip" json:"ip"` + UserAgent sql.NullString `db:"user_agent" json:"user_agent"` + ResourceType ResourceType `db:"resource_type" json:"resource_type"` + ResourceID uuid.UUID `db:"resource_id" json:"resource_id"` + ResourceTarget string `db:"resource_target" json:"resource_target"` + Action AuditAction `db:"action" json:"action"` + Diff json.RawMessage `db:"diff" json:"diff"` + StatusCode int32 `db:"status_code" json:"status_code"` + AdditionalFields json.RawMessage `db:"additional_fields" json:"additional_fields"` + RequestID uuid.UUID `db:"request_id" json:"request_id"` + ResourceIcon string `db:"resource_icon" json:"resource_icon"` + UserUsername sql.NullString `db:"user_username" json:"user_username"` + UserName sql.NullString `db:"user_name" json:"user_name"` + UserEmail sql.NullString `db:"user_email" json:"user_email"` + UserCreatedAt sql.NullTime `db:"user_created_at" json:"user_created_at"` + UserUpdatedAt sql.NullTime `db:"user_updated_at" json:"user_updated_at"` + UserLastSeenAt sql.NullTime `db:"user_last_seen_at" json:"user_last_seen_at"` + UserStatus NullUserStatus `db:"user_status" json:"user_status"` + UserLoginType NullLoginType `db:"user_login_type" json:"user_login_type"` + UserRoles pq.StringArray `db:"user_roles" json:"user_roles"` + UserAvatarUrl sql.NullString `db:"user_avatar_url" json:"user_avatar_url"` + UserDeleted sql.NullBool `db:"user_deleted" json:"user_deleted"` + UserThemePreference sql.NullString `db:"user_theme_preference" json:"user_theme_preference"` + UserQuietHoursSchedule sql.NullString `db:"user_quiet_hours_schedule" json:"user_quiet_hours_schedule"` + Count int64 `db:"count" json:"count"` } // GetAuditLogsBefore retrieves `row_limit` number of audit logs before the provided @@ -628,11 +644,18 @@ func (q *sqlQuerier) GetAuditLogsOffset(ctx context.Context, arg GetAuditLogsOff &i.RequestID, &i.ResourceIcon, &i.UserUsername, + &i.UserName, &i.UserEmail, &i.UserCreatedAt, + &i.UserUpdatedAt, + &i.UserLastSeenAt, &i.UserStatus, + &i.UserLoginType, &i.UserRoles, &i.UserAvatarUrl, + &i.UserDeleted, + &i.UserThemePreference, + &i.UserQuietHoursSchedule, &i.Count, ); err != nil { return nil, err diff --git a/coderd/database/queries/auditlogs.sql b/coderd/database/queries/auditlogs.sql index fc48489ca2104..d05b5bbe371e0 100644 --- a/coderd/database/queries/auditlogs.sql +++ b/coderd/database/queries/auditlogs.sql @@ -3,12 +3,21 @@ -- name: GetAuditLogsOffset :many SELECT audit_logs.*, + -- sqlc.embed(users) would be nice but it does not seem to play well with + -- left joins. users.username AS user_username, + users.name AS user_name, users.email AS user_email, users.created_at AS user_created_at, + users.updated_at AS user_updated_at, + users.last_seen_at AS user_last_seen_at, users.status AS user_status, + users.login_type AS user_login_type, users.rbac_roles AS user_roles, users.avatar_url AS user_avatar_url, + users.deleted AS user_deleted, + users.theme_preference AS user_theme_preference, + users.quiet_hours_schedule AS user_quiet_hours_schedule, COUNT(audit_logs.*) OVER () AS count FROM audit_logs