Skip to content

Commit 9a432b8

Browse files
jaaydenhEmyrk
jaaydenh
and
Emyrk
authored
fix: add workspace owner id as query param to websocket (#18363)
Co-authored-by: Steven Masley <stevenmasley@gmail.com>
1 parent c1341cc commit 9a432b8

7 files changed

Lines changed: 74 additions & 24 deletions

File tree

‎coderd/parameters.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,25 @@ func (api *API) templateVersionDynamicParametersEvaluate(rw http.ResponseWriter,
5858
// @Router /templateversions/{templateversion}/dynamic-parameters [get]
5959
func (api *API) templateVersionDynamicParametersWebsocket(rw http.ResponseWriter, r *http.Request) {
6060
apikey := httpmw.APIKey(r)
61+
userID := apikey.UserID
62+
63+
qUserID := r.URL.Query().Get("user_id")
64+
if qUserID != "" && qUserID != codersdk.Me {
65+
uid, err := uuid.Parse(qUserID)
66+
if err != nil {
67+
httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{
68+
Message: "Invalid user_id query parameter",
69+
Detail: err.Error(),
70+
})
71+
return
72+
}
73+
userID = uid
74+
}
6175

6276
api.templateVersionDynamicParameters(true, codersdk.DynamicParametersRequest{
6377
ID: -1,
6478
Inputs: map[string]string{},
65-
OwnerID: apikey.UserID,
79+
OwnerID: userID,
6680
})(rw, r)
6781
}
6882

‎coderd/parameters_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) {
5656
_ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)
5757

5858
ctx := testutil.Context(t, testutil.WaitShort)
59-
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
59+
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, codersdk.Me, version.ID)
6060
require.NoError(t, err)
6161
defer stream.Close(websocket.StatusGoingAway)
6262

@@ -387,7 +387,7 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn
387387
require.NoError(t, err)
388388

389389
ctx := testutil.Context(t, testutil.WaitShort)
390-
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
390+
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, codersdk.Me, version.ID)
391391
if args.expectWebsocketError {
392392
require.Errorf(t, err, "expected error forming websocket")
393393
} else {

‎codersdk/parameters.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66

77
"github.com/google/uuid"
8+
"golang.org/x/xerrors"
89

910
"github.com/coder/coder/v2/codersdk/wsjson"
1011
"github.com/coder/websocket"
@@ -125,8 +126,17 @@ type DynamicParametersResponse struct {
125126
// TODO: Workspace tags
126127
}
127128

128-
func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) {
129-
conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/templateversions/%s/dynamic-parameters", version), nil)
129+
func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, userID string, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) {
130+
endpoint := fmt.Sprintf("/api/v2/templateversions/%s/dynamic-parameters", version)
131+
if userID != Me {
132+
uid, err := uuid.Parse(userID)
133+
if err != nil {
134+
return nil, xerrors.Errorf("invalid user ID: %w", err)
135+
}
136+
endpoint += fmt.Sprintf("?user_id=%s", uid.String())
137+
}
138+
139+
conn, err := c.Dial(ctx, endpoint, nil)
130140
if err != nil {
131141
return nil, err
132142
}

‎enterprise/coderd/parameters_test.go‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
3232
},
3333
)
3434
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
35+
_, noGroupUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
3536

3637
// Create the group to be asserted
3738
group := coderdtest.CreateGroup(t, ownerClient, owner.OrganizationID, "bloob", templateAdminUser)
@@ -57,7 +58,24 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
5758
_ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)
5859

5960
ctx := testutil.Context(t, testutil.WaitShort)
60-
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
61+
62+
// First check with a no group admin user, that they do not see the extra group
63+
// Use the admin client, as the user might not have access to the template.
64+
// Also checking that the admin can see the form for the other user.
65+
noGroupStream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, noGroupUser.ID.String(), version.ID)
66+
require.NoError(t, err)
67+
defer noGroupStream.Close(websocket.StatusGoingAway)
68+
noGroupPreviews := noGroupStream.Chan()
69+
noGroupPreview := testutil.RequireReceive(ctx, t, noGroupPreviews)
70+
require.Equal(t, -1, noGroupPreview.ID)
71+
require.Empty(t, noGroupPreview.Diagnostics)
72+
require.Equal(t, "group", noGroupPreview.Parameters[0].Name)
73+
require.Equal(t, database.EveryoneGroup, noGroupPreview.Parameters[0].Value.Value)
74+
require.Equal(t, 1, len(noGroupPreview.Parameters[0].Options)) // Only 1 group
75+
noGroupStream.Close(websocket.StatusGoingAway)
76+
77+
// Now try with a user with more than 1 group
78+
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, codersdk.Me, version.ID)
6179
require.NoError(t, err)
6280
defer stream.Close(websocket.StatusGoingAway)
6381

‎site/src/api/api.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,7 @@ class ApiMethods {
10171017

10181018
templateVersionDynamicParameters = (
10191019
versionId: string,
1020+
userId: string,
10201021
{
10211022
onMessage,
10221023
onError,
@@ -1029,6 +1030,7 @@ class ApiMethods {
10291030
): WebSocket => {
10301031
const socket = createWebSocket(
10311032
`/api/v2/templateversions/${versionId}/dynamic-parameters`,
1033+
new URLSearchParams({ user_id: userId }),
10321034
);
10331035

10341036
socket.addEventListener("message", (event) =>

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -148,31 +148,35 @@ const CreateWorkspacePageExperimental: FC = () => {
148148
useEffect(() => {
149149
if (!realizedVersionId) return;
150150

151-
const socket = API.templateVersionDynamicParameters(realizedVersionId, {
152-
onMessage,
153-
onError: (error) => {
154-
if (ws.current === socket) {
155-
setWsError(error);
156-
}
157-
},
158-
onClose: () => {
159-
if (ws.current === socket) {
160-
setWsError(
161-
new DetailedError(
162-
"Websocket connection for dynamic parameters unexpectedly closed.",
163-
"Refresh the page to reset the form.",
164-
),
165-
);
166-
}
151+
const socket = API.templateVersionDynamicParameters(
152+
realizedVersionId,
153+
defaultOwner.id,
154+
{
155+
onMessage,
156+
onError: (error) => {
157+
if (ws.current === socket) {
158+
setWsError(error);
159+
}
160+
},
161+
onClose: () => {
162+
if (ws.current === socket) {
163+
setWsError(
164+
new DetailedError(
165+
"Websocket connection for dynamic parameters unexpectedly closed.",
166+
"Refresh the page to reset the form.",
167+
),
168+
);
169+
}
170+
},
167171
},
168-
});
172+
);
169173

170174
ws.current = socket;
171175

172176
return () => {
173177
socket.close();
174178
};
175-
}, [realizedVersionId, onMessage]);
179+
}, [realizedVersionId, onMessage, defaultOwner.id]);
176180

177181
const organizationId = templateQuery.data?.organization_id;
178182

‎site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ const WorkspaceParametersPageExperimental: FC = () => {
111111

112112
const socket = API.templateVersionDynamicParameters(
113113
templateVersionId ?? workspace.latest_build.template_version_id,
114+
workspace.owner_id,
114115
{
115116
onMessage,
116117
onError: (error) => {
@@ -140,6 +141,7 @@ const WorkspaceParametersPageExperimental: FC = () => {
140141
templateVersionId,
141142
workspace.latest_build.template_version_id,
142143
onMessage,
144+
workspace.owner_id,
143145
]);
144146

145147
const updateParameters = useMutation({

0 commit comments

Comments
 (0)