From 41db2952bfd8411e4c4ae275096005e5bb8bb1f3 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Wed, 26 Nov 2025 08:35:24 -0800 Subject: [PATCH 01/15] codersdk changes --- coderd/workspaces.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index a769095e8018a..ddb9a8ded8343 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2652,6 +2652,7 @@ func convertWorkspace( } } } + sharedWith := []codersdk.SharedWorkspaceActor{} ttlMillis := convertWorkspaceTTLMillis(workspace.Ttl) // If the template doesn't allow a workspace-configured value, then report the From 52ffe84af427b46645778893d20e6d764b639749 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Tue, 25 Nov 2025 23:33:20 -0800 Subject: [PATCH 02/15] hide `shared_with` in workspace response behind experiment --- coderd/workspaces.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index ddb9a8ded8343..ac0d9f295915a 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2652,7 +2652,13 @@ func convertWorkspace( } } } - sharedWith := []codersdk.SharedWorkspaceActor{} + // TODO(geokat): using a pointer that's serialized with + // "omitempty" so that we can hide it behind an experiment. This + // won't be necessary once workspace sharing is in beta. + var sharedWith *[]codersdk.SharedWorkspaceActor + if experiments.Enabled(codersdk.ExperimentWorkspaceSharing) { + sharedWith = &[]codersdk.SharedWorkspaceActor{} + } ttlMillis := convertWorkspaceTTLMillis(workspace.Ttl) // If the template doesn't allow a workspace-configured value, then report the From 7fbe5bc4ee2c024ef27741e53c91e846e559477e Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Fri, 28 Nov 2025 18:53:46 -0800 Subject: [PATCH 03/15] Fetch user and group data from DB --- coderd/aitasks.go | 2 ++ coderd/workspaces.go | 85 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/coderd/aitasks.go b/coderd/aitasks.go index e919b70d3754d..3ead9e747b404 100644 --- a/coderd/aitasks.go +++ b/coderd/aitasks.go @@ -572,6 +572,8 @@ func (api *API) taskGet(rw http.ResponseWriter, r *http.Request) { data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, + nil, + nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/coderd/workspaces.go b/coderd/workspaces.go index ac0d9f295915a..797d31a480892 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -123,6 +123,8 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) { data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, + nil, + nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -345,6 +347,8 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, + nil, + nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -876,6 +880,8 @@ func createWorkspace( template, api.Options.AllowWorkspaceRenames, codersdk.WorkspaceAppStatus{}, + nil, + nil, ) if err != nil { return codersdk.Workspace{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{ @@ -1522,6 +1528,8 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, + nil, + nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -2102,6 +2110,8 @@ func (api *API) watchWorkspace( data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, + nil, + nil, ) if err != nil { _ = sendEvent(codersdk.ServerSentEvent{ @@ -2552,6 +2562,8 @@ func convertWorkspaces( requesterID uuid.UUID, workspaces []database.Workspace, data workspaceData, + userData map[string]database.User, + groupData map[string]database.Group, ) ([]codersdk.Workspace, error) { buildByWorkspaceID := map[uuid.UUID]codersdk.WorkspaceBuild{} for _, workspaceBuild := range data.builds { @@ -2593,6 +2605,8 @@ func convertWorkspaces( template, data.allowRenames, appStatus, + userData, + groupData, ) if err != nil { return nil, xerrors.Errorf("convert workspace: %w", err) @@ -2613,6 +2627,8 @@ func convertWorkspace( template database.Template, allowRenames bool, latestAppStatus codersdk.WorkspaceAppStatus, + userData map[string]database.User, + groupData map[string]database.Group, ) (codersdk.Workspace, error) { if requesterID == uuid.Nil { return codersdk.Workspace{}, xerrors.Errorf("developer error: requesterID cannot be uuid.Nil!") @@ -2911,3 +2927,72 @@ func convertToWorkspaceRole(actions []policy.Action) codersdk.WorkspaceRole { return codersdk.WorkspaceRoleDeleted } + +// findWorkspaceUsersAndGroups fetches all users and groups present in +// workspaces' ACLs. +func findWorkspaceUsersAndGroups( + ctx context.Context, + api *API, + workspaces []database.Workspace, +) ( + userData map[string]database.User, + groupData map[string]database.Group, + err error, +) { + // Get all the user IDs and group IDs that we need to fetch + var ( + uids []uuid.UUID + gids []uuid.UUID + ) + for _, ws := range workspaces { + // ws.UserACL is a map[id]... + for id := range ws.UserACL { + uid, err := uuid.Parse(id) + if err != nil { + api.Logger.Warn(ctx, "found invalid user uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) + continue + } + uids = append(uids, uid) + } + for id := range ws.GroupACL { + gid, err := uuid.Parse(id) + if err != nil { + api.Logger.Warn(ctx, "found invalid group uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) + continue + } + gids = append(gids, gid) + } + } + + // Fetch the users + if uids != nil { + uids = slice.Unique(uids) + + users, err := api.Database.GetUsersByIDs(ctx, uids) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, nil, xerrors.Errorf("get users by IDs: %w", err) + } + + userData = make(map[string]database.User, len(uids)) + for _, user := range users { + userData[user.ID.String()] = user + } + } + + // Fetch the groups + if gids != nil { + gids = slice.Unique(gids) + + groupRows, err := api.Database.GetGroups(ctx, database.GetGroupsParams{GroupIds: gids}) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, nil, xerrors.Errorf("get groups: %w", err) + } + + groupData = make(map[string]database.Group, len(gids)) + for _, groupRow := range groupRows { + groupData[groupRow.Group.ID.String()] = groupRow.Group + } + } + + return userData, groupData, nil +} From 00013b5fa7bd6714eb3455c33314b3dd1bdb171f Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Fri, 28 Nov 2025 20:52:20 -0800 Subject: [PATCH 04/15] Add user and group data to endpoint response --- coderd/workspaces.go | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 797d31a480892..d9eb68531c9b5 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2670,10 +2670,36 @@ func convertWorkspace( } // TODO(geokat): using a pointer that's serialized with // "omitempty" so that we can hide it behind an experiment. This - // won't be necessary once workspace sharing is in beta. + // won't be necessary once workspace sharing is in GA. var sharedWith *[]codersdk.SharedWorkspaceActor if experiments.Enabled(codersdk.ExperimentWorkspaceSharing) { - sharedWith = &[]codersdk.SharedWorkspaceActor{} + out := make([]codersdk.SharedWorkspaceActor, 0, len(workspace.UserACL)+len(workspace.GroupACL)) + + // Users + for uid, aclEntry := range workspace.UserACL { + user := userData[uid] + out = append(out, codersdk.SharedWorkspaceActor{ + ID: user.ID, + ActorType: codersdk.SharedWorkspaceActorTypeUser, + Name: user.Name, + AvatarURL: user.AvatarURL, + Roles: []codersdk.WorkspaceRole{convertToWorkspaceRole(aclEntry.Permissions)}, + }) + } + + // Groups + for gid, aclEntry := range workspace.GroupACL { + group := groupData[gid] + out = append(out, codersdk.SharedWorkspaceActor{ + ID: group.ID, + ActorType: codersdk.SharedWorkspaceActorTypeGroup, + Name: group.Name, + AvatarURL: group.AvatarURL, + Roles: []codersdk.WorkspaceRole{convertToWorkspaceRole(aclEntry.Permissions)}, + }) + } + + sharedWith = &out } ttlMillis := convertWorkspaceTTLMillis(workspace.Ttl) From b07e6b3eb981671dfe666bff9352183287a17feb Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Sun, 30 Nov 2025 19:43:58 -0800 Subject: [PATCH 05/15] Only display member and group data if user has required authorization --- coderd/workspaces.go | 138 +++++++++++++++++++++++++------------------ 1 file changed, 82 insertions(+), 56 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index d9eb68531c9b5..40221f8c6bb37 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2562,8 +2562,8 @@ func convertWorkspaces( requesterID uuid.UUID, workspaces []database.Workspace, data workspaceData, - userData map[string]database.User, - groupData map[string]database.Group, + memberData map[uuid.UUID]database.OrganizationMembersRow, + groupData map[uuid.UUID]database.Group, ) ([]codersdk.Workspace, error) { buildByWorkspaceID := map[uuid.UUID]codersdk.WorkspaceBuild{} for _, workspaceBuild := range data.builds { @@ -2605,7 +2605,7 @@ func convertWorkspaces( template, data.allowRenames, appStatus, - userData, + memberData, groupData, ) if err != nil { @@ -2627,8 +2627,8 @@ func convertWorkspace( template database.Template, allowRenames bool, latestAppStatus codersdk.WorkspaceAppStatus, - userData map[string]database.User, - groupData map[string]database.Group, + memberData map[uuid.UUID]database.OrganizationMembersRow, + groupData map[uuid.UUID]database.Group, ) (codersdk.Workspace, error) { if requesterID == uuid.Nil { return codersdk.Workspace{}, xerrors.Errorf("developer error: requesterID cannot be uuid.Nil!") @@ -2675,28 +2675,50 @@ func convertWorkspace( if experiments.Enabled(codersdk.ExperimentWorkspaceSharing) { out := make([]codersdk.SharedWorkspaceActor, 0, len(workspace.UserACL)+len(workspace.GroupACL)) - // Users - for uid, aclEntry := range workspace.UserACL { - user := userData[uid] - out = append(out, codersdk.SharedWorkspaceActor{ - ID: user.ID, + // Members + for id, aclEntry := range workspace.UserACL { + userID, err := uuid.Parse(id) + if err != nil { + logger.Warn(ctx, "found invalid user uuid in workspace acl", slog.Error(err), slog.F("workspace_id", workspace.ID)) + continue + } + + actor := codersdk.SharedWorkspaceActor{ + ID: userID, ActorType: codersdk.SharedWorkspaceActorTypeUser, - Name: user.Name, - AvatarURL: user.AvatarURL, Roles: []codersdk.WorkspaceRole{convertToWorkspaceRole(aclEntry.Permissions)}, - }) + } + + // Member data is only available if user has access to it + if member, ok := memberData[userID]; ok { + actor.Name = member.Name + actor.AvatarURL = member.AvatarURL + } + + out = append(out, actor) } // Groups - for gid, aclEntry := range workspace.GroupACL { - group := groupData[gid] - out = append(out, codersdk.SharedWorkspaceActor{ - ID: group.ID, + for id, aclEntry := range workspace.GroupACL { + groupID, err := uuid.Parse(id) + if err != nil { + logger.Warn(ctx, "found invalid group uuid in workspace acl", slog.Error(err), slog.F("workspace_id", workspace.ID)) + continue + } + + actor := codersdk.SharedWorkspaceActor{ + ID: groupID, ActorType: codersdk.SharedWorkspaceActorTypeGroup, - Name: group.Name, - AvatarURL: group.AvatarURL, Roles: []codersdk.WorkspaceRole{convertToWorkspaceRole(aclEntry.Permissions)}, - }) + } + + // Group data is only available if user has access to it + if group, ok := groupData[groupID]; ok { + actor.Name = group.Name + actor.AvatarURL = group.AvatarURL + } + + out = append(out, actor) } sharedWith = &out @@ -2954,71 +2976,75 @@ func convertToWorkspaceRole(actions []policy.Action) codersdk.WorkspaceRole { return codersdk.WorkspaceRoleDeleted } -// findWorkspaceUsersAndGroups fetches all users and groups present in -// workspaces' ACLs. -func findWorkspaceUsersAndGroups( +// findWorkspaceMembersAndGroups fetches all organization members and +// groups present in workspaces' ACLs. All workspaces must belong to +// the same organization. +func findWorkspaceMembersAndGroups( ctx context.Context, api *API, workspaces []database.Workspace, ) ( - userData map[string]database.User, - groupData map[string]database.Group, + memberData map[uuid.UUID]database.OrganizationMembersRow, + groupData map[uuid.UUID]database.Group, err error, ) { - // Get all the user IDs and group IDs that we need to fetch + if len(workspaces) == 0 { + return + } + + // Get all the group IDs that we need to fetch. + // TODO(geokat): Implement a way to fetch org members by IDs. For + // now we have to fetch all of them even if we only need one. var ( - uids []uuid.UUID - gids []uuid.UUID + groupIDs []uuid.UUID + fetchMembers bool ) for _, ws := range workspaces { - // ws.UserACL is a map[id]... - for id := range ws.UserACL { - uid, err := uuid.Parse(id) - if err != nil { - api.Logger.Warn(ctx, "found invalid user uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) - continue - } - uids = append(uids, uid) + if ws.OrganizationID != workspaces[0].OrganizationID { + return nil, nil, xerrors.New("all workspaces must belong to the same organization") + } + if len(ws.UserACL) != 0 { + fetchMembers = true } for id := range ws.GroupACL { - gid, err := uuid.Parse(id) + groupID, err := uuid.Parse(id) if err != nil { api.Logger.Warn(ctx, "found invalid group uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) continue } - gids = append(gids, gid) + groupIDs = append(groupIDs, groupID) } } - // Fetch the users - if uids != nil { - uids = slice.Unique(uids) - - users, err := api.Database.GetUsersByIDs(ctx, uids) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - return nil, nil, xerrors.Errorf("get users by IDs: %w", err) + // Fetch org members + if fetchMembers { + params := database.OrganizationMembersParams{ + OrganizationID: workspaces[0].OrganizationID, } - - userData = make(map[string]database.User, len(uids)) - for _, user := range users { - userData[user.ID.String()] = user + members, err := api.Database.OrganizationMembers(ctx, params) + if err != nil && !httpapi.Is404Error(err) { + return nil, nil, xerrors.Errorf("get organization members: %w", err) + } + memberData = make(map[uuid.UUID]database.OrganizationMembersRow, len(members)) + for _, member := range members { + memberData[member.OrganizationMember.UserID] = member } } // Fetch the groups - if gids != nil { - gids = slice.Unique(gids) + if groupIDs != nil { + groupIDs = slice.Unique(groupIDs) - groupRows, err := api.Database.GetGroups(ctx, database.GetGroupsParams{GroupIds: gids}) - if err != nil && !errors.Is(err, sql.ErrNoRows) { + groupRows, err := api.Database.GetGroups(ctx, database.GetGroupsParams{GroupIds: groupIDs}) + if err != nil && !httpapi.Is404Error(err) { return nil, nil, xerrors.Errorf("get groups: %w", err) } - groupData = make(map[string]database.Group, len(gids)) + groupData = make(map[uuid.UUID]database.Group, len(groupIDs)) for _, groupRow := range groupRows { - groupData[groupRow.Group.ID.String()] = groupRow.Group + groupData[groupRow.Group.ID] = groupRow.Group } } - return userData, groupData, nil + return memberData, groupData, nil } From ed9a77ad505a390484c5bedbf9097bcc2751b2af Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Mon, 1 Dec 2025 09:22:17 -0800 Subject: [PATCH 06/15] Clean up, refactor --- coderd/workspaces.go | 57 ++------------------------------------------ 1 file changed, 2 insertions(+), 55 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 40221f8c6bb37..a0c6f3ff79633 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2668,61 +2668,6 @@ func convertWorkspace( } } } - // TODO(geokat): using a pointer that's serialized with - // "omitempty" so that we can hide it behind an experiment. This - // won't be necessary once workspace sharing is in GA. - var sharedWith *[]codersdk.SharedWorkspaceActor - if experiments.Enabled(codersdk.ExperimentWorkspaceSharing) { - out := make([]codersdk.SharedWorkspaceActor, 0, len(workspace.UserACL)+len(workspace.GroupACL)) - - // Members - for id, aclEntry := range workspace.UserACL { - userID, err := uuid.Parse(id) - if err != nil { - logger.Warn(ctx, "found invalid user uuid in workspace acl", slog.Error(err), slog.F("workspace_id", workspace.ID)) - continue - } - - actor := codersdk.SharedWorkspaceActor{ - ID: userID, - ActorType: codersdk.SharedWorkspaceActorTypeUser, - Roles: []codersdk.WorkspaceRole{convertToWorkspaceRole(aclEntry.Permissions)}, - } - - // Member data is only available if user has access to it - if member, ok := memberData[userID]; ok { - actor.Name = member.Name - actor.AvatarURL = member.AvatarURL - } - - out = append(out, actor) - } - - // Groups - for id, aclEntry := range workspace.GroupACL { - groupID, err := uuid.Parse(id) - if err != nil { - logger.Warn(ctx, "found invalid group uuid in workspace acl", slog.Error(err), slog.F("workspace_id", workspace.ID)) - continue - } - - actor := codersdk.SharedWorkspaceActor{ - ID: groupID, - ActorType: codersdk.SharedWorkspaceActorTypeGroup, - Roles: []codersdk.WorkspaceRole{convertToWorkspaceRole(aclEntry.Permissions)}, - } - - // Group data is only available if user has access to it - if group, ok := groupData[groupID]; ok { - actor.Name = group.Name - actor.AvatarURL = group.AvatarURL - } - - out = append(out, actor) - } - - sharedWith = &out - } ttlMillis := convertWorkspaceTTLMillis(workspace.Ttl) // If the template doesn't allow a workspace-configured value, then report the @@ -2739,6 +2684,8 @@ func convertWorkspace( appStatus = nil } + sharedWith := sharedWorkspaceActors(ctx, experiments, logger, workspace, memberData, groupData) + return codersdk.Workspace{ ID: workspace.ID, CreatedAt: workspace.CreatedAt, From e6cca76ad24fbf8068c4014d909b948498461877 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Mon, 1 Dec 2025 15:21:11 -0800 Subject: [PATCH 07/15] Parallelize user/group fetching --- coderd/workspaces.go | 54 ++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index a0c6f3ff79633..ea6e858d0bbf3 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2963,34 +2963,44 @@ func findWorkspaceMembersAndGroups( } } + var eg errgroup.Group + // Fetch org members if fetchMembers { - params := database.OrganizationMembersParams{ - OrganizationID: workspaces[0].OrganizationID, - } - members, err := api.Database.OrganizationMembers(ctx, params) - if err != nil && !httpapi.Is404Error(err) { - return nil, nil, xerrors.Errorf("get organization members: %w", err) - } - memberData = make(map[uuid.UUID]database.OrganizationMembersRow, len(members)) - for _, member := range members { - memberData[member.OrganizationMember.UserID] = member - } + eg.Go(func() (err error) { + params := database.OrganizationMembersParams{ + OrganizationID: workspaces[0].OrganizationID, + } + members, err := api.Database.OrganizationMembers(ctx, params) + if err != nil && !httpapi.Is404Error(err) { + return xerrors.Errorf("get organization members: %w", err) + } + memberData = make(map[uuid.UUID]database.OrganizationMembersRow, len(members)) + for _, member := range members { + memberData[member.OrganizationMember.UserID] = member + } + return nil + }) } - // Fetch the groups - if groupIDs != nil { - groupIDs = slice.Unique(groupIDs) + if len(groupIDs) > 0 { + eg.Go(func() (err error) { + groupIDs = slice.Unique(groupIDs) - groupRows, err := api.Database.GetGroups(ctx, database.GetGroupsParams{GroupIds: groupIDs}) - if err != nil && !httpapi.Is404Error(err) { - return nil, nil, xerrors.Errorf("get groups: %w", err) - } + groupRows, err := api.Database.GetGroups(ctx, database.GetGroupsParams{GroupIds: groupIDs}) + if err != nil && !httpapi.Is404Error(err) { + return xerrors.Errorf("get groups: %w", err) + } - groupData = make(map[uuid.UUID]database.Group, len(groupIDs)) - for _, groupRow := range groupRows { - groupData[groupRow.Group.ID] = groupRow.Group - } + groupData = make(map[uuid.UUID]database.Group, len(groupIDs)) + for _, groupRow := range groupRows { + groupData[groupRow.Group.ID] = groupRow.Group + } + return nil + }) + } + if err := eg.Wait(); err != nil { + return nil, nil, err } return memberData, groupData, nil From c4fa56c740197bc2fb66a8078d82851ba5d8f3f8 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Mon, 1 Dec 2025 16:03:03 -0800 Subject: [PATCH 08/15] Go back to fetching user and group data with system context --- coderd/workspaces.go | 82 +++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index ea6e858d0bbf3..31a8e88b3eced 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2562,7 +2562,7 @@ func convertWorkspaces( requesterID uuid.UUID, workspaces []database.Workspace, data workspaceData, - memberData map[uuid.UUID]database.OrganizationMembersRow, + userData map[uuid.UUID]database.User, groupData map[uuid.UUID]database.Group, ) ([]codersdk.Workspace, error) { buildByWorkspaceID := map[uuid.UUID]codersdk.WorkspaceBuild{} @@ -2605,7 +2605,7 @@ func convertWorkspaces( template, data.allowRenames, appStatus, - memberData, + userData, groupData, ) if err != nil { @@ -2627,7 +2627,7 @@ func convertWorkspace( template database.Template, allowRenames bool, latestAppStatus codersdk.WorkspaceAppStatus, - memberData map[uuid.UUID]database.OrganizationMembersRow, + userData map[uuid.UUID]database.User, groupData map[uuid.UUID]database.Group, ) (codersdk.Workspace, error) { if requesterID == uuid.Nil { @@ -2684,7 +2684,7 @@ func convertWorkspace( appStatus = nil } - sharedWith := sharedWorkspaceActors(ctx, experiments, logger, workspace, memberData, groupData) + sharedWith := sharedWorkspaceActors(ctx, experiments, logger, workspace, userData, groupData) return codersdk.Workspace{ ID: workspace.ID, @@ -2923,76 +2923,80 @@ func convertToWorkspaceRole(actions []policy.Action) codersdk.WorkspaceRole { return codersdk.WorkspaceRoleDeleted } -// findWorkspaceMembersAndGroups fetches all organization members and -// groups present in workspaces' ACLs. All workspaces must belong to -// the same organization. -func findWorkspaceMembersAndGroups( +// findWorkspaceUsersAndGroups fetches all users and groups present in +// workspaces' ACLs. +func findWorkspaceUsersAndGroups( ctx context.Context, api *API, workspaces []database.Workspace, ) ( - memberData map[uuid.UUID]database.OrganizationMembersRow, + userData map[uuid.UUID]database.User, groupData map[uuid.UUID]database.Group, err error, ) { if len(workspaces) == 0 { - return + return nil, nil, nil } - // Get all the group IDs that we need to fetch. - // TODO(geokat): Implement a way to fetch org members by IDs. For - // now we have to fetch all of them even if we only need one. + // Get all the user IDs and group IDs that we need to fetch var ( - groupIDs []uuid.UUID - fetchMembers bool + uids []uuid.UUID + gids []uuid.UUID ) for _, ws := range workspaces { - if ws.OrganizationID != workspaces[0].OrganizationID { - return nil, nil, xerrors.New("all workspaces must belong to the same organization") - } - if len(ws.UserACL) != 0 { - fetchMembers = true + // ws.UserACL is a map[id]... + for id := range ws.UserACL { + uid, err := uuid.Parse(id) + if err != nil { + api.Logger.Warn(ctx, "found invalid user uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) + continue + } + uids = append(uids, uid) } for id := range ws.GroupACL { - groupID, err := uuid.Parse(id) + gid, err := uuid.Parse(id) if err != nil { api.Logger.Warn(ctx, "found invalid group uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) continue } - groupIDs = append(groupIDs, groupID) + gids = append(gids, gid) } } var eg errgroup.Group - // Fetch org members - if fetchMembers { + // Fetch the users + if len(uids) > 0 { eg.Go(func() (err error) { - params := database.OrganizationMembersParams{ - OrganizationID: workspaces[0].OrganizationID, - } - members, err := api.Database.OrganizationMembers(ctx, params) - if err != nil && !httpapi.Is404Error(err) { - return xerrors.Errorf("get organization members: %w", err) + uids = slice.Unique(uids) + + // For context see https://github.com/coder/coder/pull/19375 + // nolint:gocritic + users, err := api.Database.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), uids) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return xerrors.Errorf("get users by IDs: %w", err) } - memberData = make(map[uuid.UUID]database.OrganizationMembersRow, len(members)) - for _, member := range members { - memberData[member.OrganizationMember.UserID] = member + + userData = make(map[uuid.UUID]database.User, len(users)) + for _, user := range users { + userData[user.ID] = user } return nil }) } // Fetch the groups - if len(groupIDs) > 0 { + if len(gids) > 0 { eg.Go(func() (err error) { - groupIDs = slice.Unique(groupIDs) + gids = slice.Unique(gids) - groupRows, err := api.Database.GetGroups(ctx, database.GetGroupsParams{GroupIds: groupIDs}) - if err != nil && !httpapi.Is404Error(err) { + // For context see https://github.com/coder/coder/pull/19375 + // nolint:gocritic + groupRows, err := api.Database.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: gids}) + if err != nil && !errors.Is(err, sql.ErrNoRows) { return xerrors.Errorf("get groups: %w", err) } - groupData = make(map[uuid.UUID]database.Group, len(groupIDs)) + groupData = make(map[uuid.UUID]database.Group, len(groupRows)) for _, groupRow := range groupRows { groupData[groupRow.Group.ID] = groupRow.Group } @@ -3003,5 +3007,5 @@ func findWorkspaceMembersAndGroups( return nil, nil, err } - return memberData, groupData, nil + return userData, groupData, nil } From 30276735e392352178047717770b2173043f4a07 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Wed, 3 Dec 2025 19:04:51 -0800 Subject: [PATCH 09/15] Fetch user/group visible info for workspaces ACL actors through workspaces_expanded view Instead of N+1 in coderd. --- coderd/aitasks.go | 2 - coderd/database/dump.sql | 8 +- ...orkspaces_expanded_acl_actor_info.down.sql | 41 +++++++ ..._workspaces_expanded_acl_actor_info.up.sql | 63 +++++++++++ coderd/database/queries.sql.go | 4 +- coderd/database/types.go | 2 + coderd/workspaces.go | 105 +----------------- enterprise/coderd/workspaces_test.go | 87 +++++++++++++++ 8 files changed, 202 insertions(+), 110 deletions(-) create mode 100644 coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql create mode 100644 coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql diff --git a/coderd/aitasks.go b/coderd/aitasks.go index 3ead9e747b404..e919b70d3754d 100644 --- a/coderd/aitasks.go +++ b/coderd/aitasks.go @@ -572,8 +572,6 @@ func (api *API) taskGet(rw http.ResponseWriter, r *http.Request) { data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, - nil, - nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 790659669f004..6fdab962daa03 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2925,8 +2925,12 @@ CREATE VIEW workspaces_expanded AS workspaces.automatic_updates, workspaces.favorite, workspaces.next_start_at, - workspaces.group_acl, - workspaces.user_acl, + COALESCE(( SELECT jsonb_object_agg(acl.key, (acl.value || jsonb_build_object('name', g.name, 'avatar_url', COALESCE(g.avatar_url, ''::text)))) AS jsonb_object_agg + FROM (jsonb_each(workspaces.group_acl) acl(key, value) + LEFT JOIN groups g ON ((g.id = (acl.key)::uuid)))), '{}'::jsonb) AS group_acl, + COALESCE(( SELECT jsonb_object_agg(acl.key, (acl.value || jsonb_build_object('name', COALESCE(vu.name, ''::text), 'avatar_url', COALESCE(vu.avatar_url, ''::text)))) AS jsonb_object_agg + FROM (jsonb_each(workspaces.user_acl) acl(key, value) + LEFT JOIN visible_users vu ON ((vu.id = (acl.key)::uuid)))), '{}'::jsonb) AS user_acl, visible_users.avatar_url AS owner_avatar_url, visible_users.username AS owner_username, visible_users.name AS owner_name, diff --git a/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql b/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql new file mode 100644 index 0000000000000..097b7dd59955c --- /dev/null +++ b/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql @@ -0,0 +1,41 @@ +DROP VIEW workspaces_expanded; + +-- Revert to passing through raw user_acl and group_acl columns. +CREATE VIEW workspaces_expanded AS + SELECT workspaces.id, + workspaces.created_at, + workspaces.updated_at, + workspaces.owner_id, + workspaces.organization_id, + workspaces.template_id, + workspaces.deleted, + workspaces.name, + workspaces.autostart_schedule, + workspaces.ttl, + workspaces.last_used_at, + workspaces.dormant_at, + workspaces.deleting_at, + workspaces.automatic_updates, + workspaces.favorite, + workspaces.next_start_at, + workspaces.group_acl, + workspaces.user_acl, + visible_users.avatar_url AS owner_avatar_url, + visible_users.username AS owner_username, + visible_users.name AS owner_name, + organizations.name AS organization_name, + organizations.display_name AS organization_display_name, + organizations.icon AS organization_icon, + organizations.description AS organization_description, + templates.name AS template_name, + templates.display_name AS template_display_name, + templates.icon AS template_icon, + templates.description AS template_description, + tasks.id AS task_id + FROM ((((workspaces + JOIN visible_users ON ((workspaces.owner_id = visible_users.id))) + JOIN organizations ON ((workspaces.organization_id = organizations.id))) + JOIN templates ON ((workspaces.template_id = templates.id))) + LEFT JOIN tasks ON ((workspaces.id = tasks.workspace_id))); + +COMMENT ON VIEW workspaces_expanded IS 'Joins in the display name information such as username, avatar, and organization name.'; diff --git a/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql b/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql new file mode 100644 index 0000000000000..3437a6260677b --- /dev/null +++ b/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql @@ -0,0 +1,63 @@ +DROP VIEW workspaces_expanded; + +-- Enrich group_acl and user_acl with the actors' display information. +CREATE VIEW workspaces_expanded AS + SELECT workspaces.id, + workspaces.created_at, + workspaces.updated_at, + workspaces.owner_id, + workspaces.organization_id, + workspaces.template_id, + workspaces.deleted, + workspaces.name, + workspaces.autostart_schedule, + workspaces.ttl, + workspaces.last_used_at, + workspaces.dormant_at, + workspaces.deleting_at, + workspaces.automatic_updates, + workspaces.favorite, + workspaces.next_start_at, + -- Enrich group_acl with group info + COALESCE(( + SELECT jsonb_object_agg( + acl.key, + acl.value || jsonb_build_object( + 'name', g.name, + 'avatar_url', COALESCE(g.avatar_url, '') + ) + ) + FROM jsonb_each(workspaces.group_acl) AS acl + LEFT JOIN groups g ON g.id = acl.key::uuid + ), '{}'::jsonb) AS group_acl, + -- Enrich user_acl with user info + COALESCE(( + SELECT jsonb_object_agg( + acl.key, + acl.value || jsonb_build_object( + 'name', COALESCE(vu.name, ''), + 'avatar_url', COALESCE(vu.avatar_url, '') + ) + ) + FROM jsonb_each(workspaces.user_acl) AS acl + LEFT JOIN visible_users vu ON vu.id = acl.key::uuid + ), '{}'::jsonb) AS user_acl, + visible_users.avatar_url AS owner_avatar_url, + visible_users.username AS owner_username, + visible_users.name AS owner_name, + organizations.name AS organization_name, + organizations.display_name AS organization_display_name, + organizations.icon AS organization_icon, + organizations.description AS organization_description, + templates.name AS template_name, + templates.display_name AS template_display_name, + templates.icon AS template_icon, + templates.description AS template_description, + tasks.id AS task_id + FROM ((((workspaces + JOIN visible_users ON ((workspaces.owner_id = visible_users.id))) + JOIN organizations ON ((workspaces.organization_id = organizations.id))) + JOIN templates ON ((workspaces.template_id = templates.id))) + LEFT JOIN tasks ON ((workspaces.id = tasks.workspace_id))); + +COMMENT ON VIEW workspaces_expanded IS 'Joins in the display name information such as username, avatar, and organization name.'; diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c803d569365ca..50c7900910435 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -23076,8 +23076,8 @@ type GetWorkspacesRow struct { AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` Favorite bool `db:"favorite" json:"favorite"` NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` - GroupACL json.RawMessage `db:"group_acl" json:"group_acl"` - UserACL json.RawMessage `db:"user_acl" json:"user_acl"` + GroupACL interface{} `db:"group_acl" json:"group_acl"` + UserACL interface{} `db:"user_acl" json:"user_acl"` OwnerAvatarUrl string `db:"owner_avatar_url" json:"owner_avatar_url"` OwnerUsername string `db:"owner_username" json:"owner_username"` OwnerName string `db:"owner_name" json:"owner_name"` diff --git a/coderd/database/types.go b/coderd/database/types.go index 6d68a19bdaf52..b5e0020fdbfd7 100644 --- a/coderd/database/types.go +++ b/coderd/database/types.go @@ -112,6 +112,8 @@ func (t WorkspaceACL) Value() (driver.Value, error) { type WorkspaceACLEntry struct { Permissions []policy.Action `json:"permissions"` + Name string `json:"name"` + AvatarURL string `json:"avatar_url"` } // WorkspaceACLDisplayInfo supplements workspace ACLs with the actors' diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 31a8e88b3eced..9ba9c6308bf92 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -123,8 +123,6 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) { data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, - nil, - nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -347,8 +345,6 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, - nil, - nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -880,8 +876,6 @@ func createWorkspace( template, api.Options.AllowWorkspaceRenames, codersdk.WorkspaceAppStatus{}, - nil, - nil, ) if err != nil { return codersdk.Workspace{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{ @@ -1528,8 +1522,6 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, - nil, - nil, ) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -2110,8 +2102,6 @@ func (api *API) watchWorkspace( data.templates[0], api.Options.AllowWorkspaceRenames, appStatus, - nil, - nil, ) if err != nil { _ = sendEvent(codersdk.ServerSentEvent{ @@ -2562,8 +2552,6 @@ func convertWorkspaces( requesterID uuid.UUID, workspaces []database.Workspace, data workspaceData, - userData map[uuid.UUID]database.User, - groupData map[uuid.UUID]database.Group, ) ([]codersdk.Workspace, error) { buildByWorkspaceID := map[uuid.UUID]codersdk.WorkspaceBuild{} for _, workspaceBuild := range data.builds { @@ -2605,8 +2593,6 @@ func convertWorkspaces( template, data.allowRenames, appStatus, - userData, - groupData, ) if err != nil { return nil, xerrors.Errorf("convert workspace: %w", err) @@ -2627,8 +2613,6 @@ func convertWorkspace( template database.Template, allowRenames bool, latestAppStatus codersdk.WorkspaceAppStatus, - userData map[uuid.UUID]database.User, - groupData map[uuid.UUID]database.Group, ) (codersdk.Workspace, error) { if requesterID == uuid.Nil { return codersdk.Workspace{}, xerrors.Errorf("developer error: requesterID cannot be uuid.Nil!") @@ -2684,7 +2668,7 @@ func convertWorkspace( appStatus = nil } - sharedWith := sharedWorkspaceActors(ctx, experiments, logger, workspace, userData, groupData) + sharedWith := sharedWorkspaceActors(ctx, experiments, logger, workspace) return codersdk.Workspace{ ID: workspace.ID, @@ -2922,90 +2906,3 @@ func convertToWorkspaceRole(actions []policy.Action) codersdk.WorkspaceRole { return codersdk.WorkspaceRoleDeleted } - -// findWorkspaceUsersAndGroups fetches all users and groups present in -// workspaces' ACLs. -func findWorkspaceUsersAndGroups( - ctx context.Context, - api *API, - workspaces []database.Workspace, -) ( - userData map[uuid.UUID]database.User, - groupData map[uuid.UUID]database.Group, - err error, -) { - if len(workspaces) == 0 { - return nil, nil, nil - } - - // Get all the user IDs and group IDs that we need to fetch - var ( - uids []uuid.UUID - gids []uuid.UUID - ) - for _, ws := range workspaces { - // ws.UserACL is a map[id]... - for id := range ws.UserACL { - uid, err := uuid.Parse(id) - if err != nil { - api.Logger.Warn(ctx, "found invalid user uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) - continue - } - uids = append(uids, uid) - } - for id := range ws.GroupACL { - gid, err := uuid.Parse(id) - if err != nil { - api.Logger.Warn(ctx, "found invalid group uuid in workspace acl", slog.Error(err), slog.F("workspace_id", ws.ID)) - continue - } - gids = append(gids, gid) - } - } - - var eg errgroup.Group - - // Fetch the users - if len(uids) > 0 { - eg.Go(func() (err error) { - uids = slice.Unique(uids) - - // For context see https://github.com/coder/coder/pull/19375 - // nolint:gocritic - users, err := api.Database.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), uids) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - return xerrors.Errorf("get users by IDs: %w", err) - } - - userData = make(map[uuid.UUID]database.User, len(users)) - for _, user := range users { - userData[user.ID] = user - } - return nil - }) - } - // Fetch the groups - if len(gids) > 0 { - eg.Go(func() (err error) { - gids = slice.Unique(gids) - - // For context see https://github.com/coder/coder/pull/19375 - // nolint:gocritic - groupRows, err := api.Database.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: gids}) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - return xerrors.Errorf("get groups: %w", err) - } - - groupData = make(map[uuid.UUID]database.Group, len(groupRows)) - for _, groupRow := range groupRows { - groupData[groupRow.Group.ID] = groupRow.Group - } - return nil - }) - } - if err := eg.Wait(); err != nil { - return nil, nil, err - } - - return userData, groupData, nil -} diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index a9f48b6dc8e45..4289de940a0bc 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -4654,4 +4654,91 @@ func TestWorkspacesSharedWith(t *testing.T) { assert.Contains(t, groupActor.Roles, codersdk.WorkspaceRoleAdmin) assert.Equal(t, "/emojis/1f60d.png", groupActor.AvatarURL) }) + + // /workspace endpoint should include the data too + t.Run("WorkspaceResponseIncludesSharedWith", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)} + + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + + _, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + + workspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: workspaceOwner.ID, + OrganizationID: user.OrganizationID, + }).Do().Workspace + + _, sharedWithUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + + ctx := testutil.Context(t, testutil.WaitMedium) + + // Update a shared with user to have a name and avatar + _, err := db.UpdateUserProfile(dbauthz.AsSystemRestricted(ctx), database.UpdateUserProfileParams{ + ID: sharedWithUser.ID, + Username: sharedWithUser.Username, + Name: "Shared User Name", + AvatarURL: "/emojis/1fae1.png", + }) + require.NoError(t, err) + + // Create a shared with group with a name and avatar + sharedWithGroup, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "shared-with-group", + AvatarURL: "/emojis/1f60d.png", + }) + require.NoError(t, err) + + // Share workspace with user and group + err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + UserRoles: map[string]codersdk.WorkspaceRole{ + sharedWithUser.ID.String(): codersdk.WorkspaceRoleUse, + }, + GroupRoles: map[string]codersdk.WorkspaceRole{ + sharedWithGroup.ID.String(): codersdk.WorkspaceRoleAdmin, + }, + }) + require.NoError(t, err) + + // Fetch from the /workspace endpoint as client + ws, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err) + require.NotNil(t, ws.SharedWith) + require.Len(t, *ws.SharedWith, 2) + + sharedWith := *ws.SharedWith + + // Find user actor in response + var userActor, groupActor *codersdk.SharedWorkspaceActor + for i := range sharedWith { + if sharedWith[i].ActorType == codersdk.SharedWorkspaceActorTypeUser { + userActor = &sharedWith[i] + } else if sharedWith[i].ActorType == codersdk.SharedWorkspaceActorTypeGroup { + groupActor = &sharedWith[i] + } + } + + require.NotNil(t, userActor, "expected to find user actor") + assert.Equal(t, sharedWithUser.ID, userActor.ID) + assert.Contains(t, userActor.Roles, codersdk.WorkspaceRoleUse) + assert.Equal(t, "Shared User Name", userActor.Name) + assert.Equal(t, "/emojis/1fae1.png", userActor.AvatarURL) + + require.NotNil(t, groupActor, "expected to find group actor") + assert.Equal(t, sharedWithGroup.ID, groupActor.ID) + assert.Equal(t, sharedWithGroup.Name, groupActor.Name) + assert.Contains(t, groupActor.Roles, codersdk.WorkspaceRoleAdmin) + assert.Equal(t, "/emojis/1f60d.png", groupActor.AvatarURL) + }) } From 04bbb78ab9bafa475cd9454e5fb2ca3a9e7bfafd Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Thu, 4 Dec 2025 09:59:39 -0800 Subject: [PATCH 10/15] Fix migration numbering: 402 already exists in main --- ...orkspaces_expanded_acl_actor_info.down.sql | 41 ------------ ..._workspaces_expanded_acl_actor_info.up.sql | 63 ------------------- 2 files changed, 104 deletions(-) delete mode 100644 coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql delete mode 100644 coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql diff --git a/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql b/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql deleted file mode 100644 index 097b7dd59955c..0000000000000 --- a/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.down.sql +++ /dev/null @@ -1,41 +0,0 @@ -DROP VIEW workspaces_expanded; - --- Revert to passing through raw user_acl and group_acl columns. -CREATE VIEW workspaces_expanded AS - SELECT workspaces.id, - workspaces.created_at, - workspaces.updated_at, - workspaces.owner_id, - workspaces.organization_id, - workspaces.template_id, - workspaces.deleted, - workspaces.name, - workspaces.autostart_schedule, - workspaces.ttl, - workspaces.last_used_at, - workspaces.dormant_at, - workspaces.deleting_at, - workspaces.automatic_updates, - workspaces.favorite, - workspaces.next_start_at, - workspaces.group_acl, - workspaces.user_acl, - visible_users.avatar_url AS owner_avatar_url, - visible_users.username AS owner_username, - visible_users.name AS owner_name, - organizations.name AS organization_name, - organizations.display_name AS organization_display_name, - organizations.icon AS organization_icon, - organizations.description AS organization_description, - templates.name AS template_name, - templates.display_name AS template_display_name, - templates.icon AS template_icon, - templates.description AS template_description, - tasks.id AS task_id - FROM ((((workspaces - JOIN visible_users ON ((workspaces.owner_id = visible_users.id))) - JOIN organizations ON ((workspaces.organization_id = organizations.id))) - JOIN templates ON ((workspaces.template_id = templates.id))) - LEFT JOIN tasks ON ((workspaces.id = tasks.workspace_id))); - -COMMENT ON VIEW workspaces_expanded IS 'Joins in the display name information such as username, avatar, and organization name.'; diff --git a/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql b/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql deleted file mode 100644 index 3437a6260677b..0000000000000 --- a/coderd/database/migrations/000402_workspaces_expanded_acl_actor_info.up.sql +++ /dev/null @@ -1,63 +0,0 @@ -DROP VIEW workspaces_expanded; - --- Enrich group_acl and user_acl with the actors' display information. -CREATE VIEW workspaces_expanded AS - SELECT workspaces.id, - workspaces.created_at, - workspaces.updated_at, - workspaces.owner_id, - workspaces.organization_id, - workspaces.template_id, - workspaces.deleted, - workspaces.name, - workspaces.autostart_schedule, - workspaces.ttl, - workspaces.last_used_at, - workspaces.dormant_at, - workspaces.deleting_at, - workspaces.automatic_updates, - workspaces.favorite, - workspaces.next_start_at, - -- Enrich group_acl with group info - COALESCE(( - SELECT jsonb_object_agg( - acl.key, - acl.value || jsonb_build_object( - 'name', g.name, - 'avatar_url', COALESCE(g.avatar_url, '') - ) - ) - FROM jsonb_each(workspaces.group_acl) AS acl - LEFT JOIN groups g ON g.id = acl.key::uuid - ), '{}'::jsonb) AS group_acl, - -- Enrich user_acl with user info - COALESCE(( - SELECT jsonb_object_agg( - acl.key, - acl.value || jsonb_build_object( - 'name', COALESCE(vu.name, ''), - 'avatar_url', COALESCE(vu.avatar_url, '') - ) - ) - FROM jsonb_each(workspaces.user_acl) AS acl - LEFT JOIN visible_users vu ON vu.id = acl.key::uuid - ), '{}'::jsonb) AS user_acl, - visible_users.avatar_url AS owner_avatar_url, - visible_users.username AS owner_username, - visible_users.name AS owner_name, - organizations.name AS organization_name, - organizations.display_name AS organization_display_name, - organizations.icon AS organization_icon, - organizations.description AS organization_description, - templates.name AS template_name, - templates.display_name AS template_display_name, - templates.icon AS template_icon, - templates.description AS template_description, - tasks.id AS task_id - FROM ((((workspaces - JOIN visible_users ON ((workspaces.owner_id = visible_users.id))) - JOIN organizations ON ((workspaces.organization_id = organizations.id))) - JOIN templates ON ((workspaces.template_id = templates.id))) - LEFT JOIN tasks ON ((workspaces.id = tasks.workspace_id))); - -COMMENT ON VIEW workspaces_expanded IS 'Joins in the display name information such as username, avatar, and organization name.'; From 7f4a0fb3d0e2a648b6f560f158f377e68eaf06e1 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Wed, 10 Dec 2025 11:53:50 -0800 Subject: [PATCH 11/15] Get rid of SharedWith pointer usage which enabled support for `shared_with: []` in response if experiment was enabled --- coderd/workspaces.go | 2 -- enterprise/coderd/workspaces_test.go | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 9ba9c6308bf92..a769095e8018a 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2668,8 +2668,6 @@ func convertWorkspace( appStatus = nil } - sharedWith := sharedWorkspaceActors(ctx, experiments, logger, workspace) - return codersdk.Workspace{ ID: workspace.ID, CreatedAt: workspace.CreatedAt, diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 4289de940a0bc..6b62e742e5143 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -4715,9 +4715,9 @@ func TestWorkspacesSharedWith(t *testing.T) { ws, err := client.Workspace(ctx, workspace.ID) require.NoError(t, err) require.NotNil(t, ws.SharedWith) - require.Len(t, *ws.SharedWith, 2) + require.Len(t, ws.SharedWith, 2) - sharedWith := *ws.SharedWith + sharedWith := ws.SharedWith // Find user actor in response var userActor, groupActor *codersdk.SharedWorkspaceActor From 043027261fa5da51404ba633152b6c11b575b895 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 11 Dec 2025 13:08:40 +0000 Subject: [PATCH 12/15] feat: add workspace sharing indicator --- .../WorkspaceSharingIndicator.tsx | 72 +++++++++++++++++++ .../pages/WorkspacesPage/WorkspacesTable.tsx | 14 +++- 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx diff --git a/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx new file mode 100644 index 0000000000000..5d7e72c888271 --- /dev/null +++ b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx @@ -0,0 +1,72 @@ +import type { SharedWorkspaceActor } from "api/typesGenerated"; +import { Badge } from "components/Badge/Badge"; +import { Link } from "components/Link/Link"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "components/Tooltip/Tooltip"; +import { UsersIcon } from "lucide-react"; +import type { FC } from "react"; + +interface WorkspaceSharingIndicatorProps { + sharedWith: readonly SharedWorkspaceActor[]; + settingsPath: string; +} + +export const WorkspaceSharingIndicator: FC = ({ + sharedWith, + settingsPath, +}) => { + // Sort by type (users then groups) and then alphabetically by name. + const sortedActors = [...sharedWith].sort((a, b) => { + if (a.actor_type !== b.actor_type) { + return a.actor_type === "user" ? -1 : 1; + } + return a.name.localeCompare(b.name); + }); + + return ( + + + + + + + +
+

+ Workspace permissions +

+
+
    + {sortedActors.map((actor) => { + const isAdmin = actor.roles.includes("admin"); + return ( +
  • + {actor.name} + {isAdmin && ( + + Admin + + )} +
    • + ); + })} +
+
+ e.stopPropagation()} + > + Change permissions + +
+ + + ); +}; diff --git a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx index 19b719540f938..0d72987751135 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx @@ -84,6 +84,7 @@ import { useMutation, useQuery, useQueryClient } from "react-query"; import { useNavigate } from "react-router"; import { cn } from "utils/cn"; import { getDisplayWorkspaceTemplateName } from "utils/workspace"; +import { WorkspaceSharingIndicator } from "./WorkspaceSharingIndicator"; import { WorkspacesEmpty } from "./WorkspacesEmpty"; interface WorkspacesTableProps { @@ -216,9 +217,18 @@ export const WorkspacesTable: FC = ({ } subtitle={ -
+
Owner: - {workspace.owner_name} +
+ {workspace.owner_name} + {workspace.shared_with && + workspace.shared_with.length > 0 && ( + + )} +
} avatar={ From 8d4ccbf1b833845b26b8e569edb74e75eef94087 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 11 Dec 2025 13:34:17 +0000 Subject: [PATCH 13/15] chore: add storybook stories --- .../WorkspaceSharingIndicator.stories.tsx | 153 ++++++++++++++++++ .../WorkspaceSharingIndicator.tsx | 2 +- 2 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.stories.tsx diff --git a/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.stories.tsx b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.stories.tsx new file mode 100644 index 0000000000000..553f1050d8a84 --- /dev/null +++ b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.stories.tsx @@ -0,0 +1,153 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import type { SharedWorkspaceActor } from "api/typesGenerated"; +import { expect, screen, userEvent, waitFor } from "storybook/test"; +import { WorkspaceSharingIndicator } from "./WorkspaceSharingIndicator"; + +const mockUser = ( + name: string, + roles: SharedWorkspaceActor["roles"] = ["use"], +): SharedWorkspaceActor => ({ + id: crypto.randomUUID(), + actor_type: "user", + name, + roles, +}); + +const mockGroup = ( + name: string, + roles: SharedWorkspaceActor["roles"] = ["use"], +): SharedWorkspaceActor => ({ + id: crypto.randomUUID(), + actor_type: "group", + name, + roles, +}); + +const hoverTrigger = async (canvasElement: HTMLElement) => { + const trigger = canvasElement.querySelector("svg"); + if (!trigger) { + throw new Error("Could not find trigger element"); + } + await userEvent.hover(trigger); +}; + +const meta: Meta = { + title: "pages/WorkspacesPage/WorkspaceSharingIndicator", + component: WorkspaceSharingIndicator, + args: { + settingsPath: "/@owner/my-workspace/settings/sharing", + }, + decorators: [ + (Story) => ( +
+ +
+ ), + ], +}; + +export default meta; +type Story = StoryObj; + +export const SingleUser: Story = { + args: { + sharedWith: [mockUser("alice")], + }, + play: async ({ canvasElement, step }) => { + await step("activate hover trigger", async () => { + await hoverTrigger(canvasElement); + await waitFor(() => + expect(screen.getByRole("tooltip")).toHaveTextContent("alice"), + ); + }); + }, +}; + +export const SingleAdmin: Story = { + args: { + sharedWith: [mockUser("alice", ["admin"])], + }, + play: async ({ canvasElement, step }) => { + await step("activate hover trigger", async () => { + await hoverTrigger(canvasElement); + await waitFor(() => { + const tooltip = screen.getByRole("tooltip"); + expect(tooltip).toHaveTextContent("alice"); + expect(tooltip).toHaveTextContent("Admin"); + }); + }); + }, +}; + +export const SingleGroup: Story = { + args: { + sharedWith: [mockGroup("Engineering")], + }, + play: async ({ canvasElement, step }) => { + await step("activate hover trigger", async () => { + await hoverTrigger(canvasElement); + await waitFor(() => + expect(screen.getByRole("tooltip")).toHaveTextContent("Engineering"), + ); + }); + }, +}; + +export const UsersAndGroups: Story = { + args: { + sharedWith: [ + mockGroup("Engineering"), + mockUser("alice", ["admin"]), + mockGroup("DevOps", ["admin"]), + mockUser("bob"), + ], + }, + play: async ({ canvasElement, step }) => { + await step("activate hover trigger", async () => { + await hoverTrigger(canvasElement); + await waitFor(() => { + const tooltip = screen.getByRole("tooltip"); + expect(tooltip).toHaveTextContent("alice"); + expect(tooltip).toHaveTextContent("bob"); + expect(tooltip).toHaveTextContent("Engineering"); + expect(tooltip).toHaveTextContent("DevOps"); + }); + }); + }, +}; + +export const ManyActors: Story = { + args: { + sharedWith: [ + mockUser("alice", ["admin"]), + mockUser("bob", ["admin"]), + mockUser("charlie"), + mockGroup("QA"), + mockGroup("HR"), + mockGroup("Finance"), + mockGroup("Marketing"), + mockGroup("Sales"), + mockUser("david"), + mockUser("eve"), + mockUser("frank"), + mockGroup("Engineering"), + mockGroup("DevOps"), + mockGroup("Platform", ["admin"]), + mockGroup("Security", ["admin"]), + mockGroup("IT"), + mockGroup("Legal"), + mockGroup("Customer Support"), + mockGroup("Product"), + ], + }, + play: async ({ canvasElement, step }) => { + await step("activate hover trigger", async () => { + await hoverTrigger(canvasElement); + await waitFor(() => + expect(screen.getByRole("tooltip")).toHaveTextContent( + "Workspace permissions", + ), + ); + }); + }, +}; diff --git a/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx index 5d7e72c888271..f80f8b1f3f82e 100644 --- a/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx +++ b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx @@ -39,7 +39,7 @@ export const WorkspaceSharingIndicator: FC = ({ Workspace permissions

-
    +
      {sortedActors.map((actor) => { const isAdmin = actor.roles.includes("admin"); return ( From 377ebf068686a49ad964f0e5c7e6f486277e92c9 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Mon, 15 Dec 2025 16:56:11 +0000 Subject: [PATCH 14/15] fix: merge error --- coderd/database/dump.sql | 8 +-- coderd/database/queries.sql.go | 4 +- coderd/database/types.go | 2 - enterprise/coderd/workspaces_test.go | 87 ---------------------------- 4 files changed, 4 insertions(+), 97 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 6fdab962daa03..790659669f004 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2925,12 +2925,8 @@ CREATE VIEW workspaces_expanded AS workspaces.automatic_updates, workspaces.favorite, workspaces.next_start_at, - COALESCE(( SELECT jsonb_object_agg(acl.key, (acl.value || jsonb_build_object('name', g.name, 'avatar_url', COALESCE(g.avatar_url, ''::text)))) AS jsonb_object_agg - FROM (jsonb_each(workspaces.group_acl) acl(key, value) - LEFT JOIN groups g ON ((g.id = (acl.key)::uuid)))), '{}'::jsonb) AS group_acl, - COALESCE(( SELECT jsonb_object_agg(acl.key, (acl.value || jsonb_build_object('name', COALESCE(vu.name, ''::text), 'avatar_url', COALESCE(vu.avatar_url, ''::text)))) AS jsonb_object_agg - FROM (jsonb_each(workspaces.user_acl) acl(key, value) - LEFT JOIN visible_users vu ON ((vu.id = (acl.key)::uuid)))), '{}'::jsonb) AS user_acl, + workspaces.group_acl, + workspaces.user_acl, visible_users.avatar_url AS owner_avatar_url, visible_users.username AS owner_username, visible_users.name AS owner_name, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 50c7900910435..c803d569365ca 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -23076,8 +23076,8 @@ type GetWorkspacesRow struct { AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` Favorite bool `db:"favorite" json:"favorite"` NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` - GroupACL interface{} `db:"group_acl" json:"group_acl"` - UserACL interface{} `db:"user_acl" json:"user_acl"` + GroupACL json.RawMessage `db:"group_acl" json:"group_acl"` + UserACL json.RawMessage `db:"user_acl" json:"user_acl"` OwnerAvatarUrl string `db:"owner_avatar_url" json:"owner_avatar_url"` OwnerUsername string `db:"owner_username" json:"owner_username"` OwnerName string `db:"owner_name" json:"owner_name"` diff --git a/coderd/database/types.go b/coderd/database/types.go index b5e0020fdbfd7..6d68a19bdaf52 100644 --- a/coderd/database/types.go +++ b/coderd/database/types.go @@ -112,8 +112,6 @@ func (t WorkspaceACL) Value() (driver.Value, error) { type WorkspaceACLEntry struct { Permissions []policy.Action `json:"permissions"` - Name string `json:"name"` - AvatarURL string `json:"avatar_url"` } // WorkspaceACLDisplayInfo supplements workspace ACLs with the actors' diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 6b62e742e5143..a9f48b6dc8e45 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -4654,91 +4654,4 @@ func TestWorkspacesSharedWith(t *testing.T) { assert.Contains(t, groupActor.Roles, codersdk.WorkspaceRoleAdmin) assert.Equal(t, "/emojis/1f60d.png", groupActor.AvatarURL) }) - - // /workspace endpoint should include the data too - t.Run("WorkspaceResponseIncludesSharedWith", func(t *testing.T) { - t.Parallel() - - dv := coderdtest.DeploymentValues(t) - dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)} - - client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - DeploymentValues: dv, - }, - LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureTemplateRBAC: 1, - }, - }, - }) - - _, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) - - workspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OwnerID: workspaceOwner.ID, - OrganizationID: user.OrganizationID, - }).Do().Workspace - - _, sharedWithUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) - - ctx := testutil.Context(t, testutil.WaitMedium) - - // Update a shared with user to have a name and avatar - _, err := db.UpdateUserProfile(dbauthz.AsSystemRestricted(ctx), database.UpdateUserProfileParams{ - ID: sharedWithUser.ID, - Username: sharedWithUser.Username, - Name: "Shared User Name", - AvatarURL: "/emojis/1fae1.png", - }) - require.NoError(t, err) - - // Create a shared with group with a name and avatar - sharedWithGroup, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ - Name: "shared-with-group", - AvatarURL: "/emojis/1f60d.png", - }) - require.NoError(t, err) - - // Share workspace with user and group - err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ - UserRoles: map[string]codersdk.WorkspaceRole{ - sharedWithUser.ID.String(): codersdk.WorkspaceRoleUse, - }, - GroupRoles: map[string]codersdk.WorkspaceRole{ - sharedWithGroup.ID.String(): codersdk.WorkspaceRoleAdmin, - }, - }) - require.NoError(t, err) - - // Fetch from the /workspace endpoint as client - ws, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err) - require.NotNil(t, ws.SharedWith) - require.Len(t, ws.SharedWith, 2) - - sharedWith := ws.SharedWith - - // Find user actor in response - var userActor, groupActor *codersdk.SharedWorkspaceActor - for i := range sharedWith { - if sharedWith[i].ActorType == codersdk.SharedWorkspaceActorTypeUser { - userActor = &sharedWith[i] - } else if sharedWith[i].ActorType == codersdk.SharedWorkspaceActorTypeGroup { - groupActor = &sharedWith[i] - } - } - - require.NotNil(t, userActor, "expected to find user actor") - assert.Equal(t, sharedWithUser.ID, userActor.ID) - assert.Contains(t, userActor.Roles, codersdk.WorkspaceRoleUse) - assert.Equal(t, "Shared User Name", userActor.Name) - assert.Equal(t, "/emojis/1fae1.png", userActor.AvatarURL) - - require.NotNil(t, groupActor, "expected to find group actor") - assert.Equal(t, sharedWithGroup.ID, groupActor.ID) - assert.Equal(t, sharedWithGroup.Name, groupActor.Name) - assert.Contains(t, groupActor.Roles, codersdk.WorkspaceRoleAdmin) - assert.Equal(t, "/emojis/1f60d.png", groupActor.AvatarURL) - }) } From 868b296cea0b813f877b75f3e7574e3e814997c6 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Mon, 15 Dec 2025 19:39:14 +0000 Subject: [PATCH 15/15] chore: make link non-external --- site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx index f80f8b1f3f82e..220b9dd97f067 100644 --- a/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx +++ b/site/src/pages/WorkspacesPage/WorkspaceSharingIndicator.tsx @@ -62,6 +62,7 @@ export const WorkspaceSharingIndicator: FC = ({ href={settingsPath} className="text-sm text-content-link font-medium" onClick={(e) => e.stopPropagation()} + showExternalIcon={false} > Change permissions