fix: fill out missing user properties to /audit

code-asher · code-asher · commit bf85f1d81181 · 2024-06-18T16:48:39.000-08:00
All except the organization IDs.  We can add this as well if needed, but
the complaint was specifically about last_seen_at.
diff --git 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"
 )
@@ -182,31 +181,10 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
 	diff := codersdk.AuditDiff{}
 	_ = 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))
-		}
-	}
-
 	var (
+		// For now leaving the organization IDs blank; not sure they are useful for
+		// the audit query anyway?
+		user                  = db2sdk.User(dblog.User, []uuid.UUID{})
 		additionalFieldsBytes = []byte(dblog.AdditionalFields)
 		additionalFields      audit.AdditionalFields
 		err                   = json.Unmarshal(additionalFieldsBytes, &additionalFields)
@@ -249,7 +227,7 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
 		Diff:             diff,
 		StatusCode:       dblog.StatusCode,
 		AdditionalFields: dblog.AdditionalFields,
-		User:             user,
+		User:             &user,
 		Description:      auditLogDescription(dblog),
 		ResourceLink:     resourceLink,
 		IsDeleted:        isDeleted,
diff --git 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,53 @@ 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.
+		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
@@ -1947,8 +1947,12 @@ func (q *FakeQuerier) GetAuditLogsOffset(_ context.Context, arg database.GetAudi
 			}
 		}
 
+		// The query is a left join, so allow no user here, although this probably
+		// does not happen in practice since users are soft deleted?
 		user, err := q.getUserByIDNoLock(alog.UserID)
-		userValid := err == nil
+		if err != nil && !errors.Is(err, sql.ErrNoRows) {
+			return logs, err
+		}
 
 		logs = append(logs, database.GetAuditLogsOffsetRow{
 			ID:               alog.ID,
@@ -1965,11 +1969,7 @@ func (q *FakeQuerier) GetAuditLogsOffset(_ context.Context, arg database.GetAudi
 			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,
+			User:             user,
 			Count:            0,
 		})
 
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/auditlogs.sql b/coderd/database/queries/auditlogs.sql
@@ -3,12 +3,7 @@
 -- name: GetAuditLogsOffset :many
 SELECT
     audit_logs.*,
-    users.username AS user_username,
-    users.email AS user_email,
-    users.created_at AS user_created_at,
-    users.status AS user_status,
-    users.rbac_roles AS user_roles,
-    users.avatar_url AS user_avatar_url,
+    sqlc.embed(users),
     COUNT(audit_logs.*) OVER () AS count
 FROM
     audit_logs
