mirror of
https://github.com/coder/coder.git
synced 2025-07-06 15:41:45 +00:00
fix: fill out zero-value user properties in /audit (#13604)
This commit is contained in:
@ -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 (
|
||||
|
@ -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()
|
||||
|
||||
|
@ -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) {
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
Reference in New Issue
Block a user