Skip to content

fix: add workspace owner id as query param to websocket #18363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion coderd/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,25 @@ func (api *API) templateVersionDynamicParametersEvaluate(rw http.ResponseWriter,
// @Router /templateversions/{templateversion}/dynamic-parameters [get]
func (api *API) templateVersionDynamicParametersWebsocket(rw http.ResponseWriter, r *http.Request) {
apikey := httpmw.APIKey(r)
userID := apikey.UserID

qUserID := r.URL.Query().Get("user_id")
if qUserID != "" && qUserID != codersdk.Me {
uid, err := uuid.Parse(qUserID)
if err != nil {
httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid user_id query parameter",
Detail: err.Error(),
})
return
}
userID = uid
}

api.templateVersionDynamicParameters(true, codersdk.DynamicParametersRequest{
ID: -1,
Inputs: map[string]string{},
OwnerID: apikey.UserID,
OwnerID: userID,
})(rw, r)
}

Expand Down
4 changes: 2 additions & 2 deletions coderd/parameters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) {
_ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)

ctx := testutil.Context(t, testutil.WaitShort)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, codersdk.Me, version.ID)
require.NoError(t, err)
defer stream.Close(websocket.StatusGoingAway)

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

ctx := testutil.Context(t, testutil.WaitShort)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, codersdk.Me, version.ID)
if args.expectWebsocketError {
require.Errorf(t, err, "expected error forming websocket")
} else {
Expand Down
14 changes: 12 additions & 2 deletions codersdk/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

"github.com/google/uuid"
"golang.org/x/xerrors"

"github.com/coder/coder/v2/codersdk/wsjson"
"github.com/coder/websocket"
Expand Down Expand Up @@ -125,8 +126,17 @@ type DynamicParametersResponse struct {
// TODO: Workspace tags
}

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

conn, err := c.Dial(ctx, endpoint, nil)
if err != nil {
return nil, err
}
Expand Down
20 changes: 19 additions & 1 deletion enterprise/coderd/parameters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
},
)
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
_, noGroupUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)

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

ctx := testutil.Context(t, testutil.WaitShort)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)

// First check with a no group admin user, that they do not see the extra group
// Use the admin client, as the user might not have access to the template.
// Also checking that the admin can see the form for the other user.
noGroupStream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, noGroupUser.ID.String(), version.ID)
require.NoError(t, err)
defer noGroupStream.Close(websocket.StatusGoingAway)
noGroupPreviews := noGroupStream.Chan()
noGroupPreview := testutil.RequireReceive(ctx, t, noGroupPreviews)
require.Equal(t, -1, noGroupPreview.ID)
require.Empty(t, noGroupPreview.Diagnostics)
require.Equal(t, "group", noGroupPreview.Parameters[0].Name)
require.Equal(t, database.EveryoneGroup, noGroupPreview.Parameters[0].Value.Value)
require.Equal(t, 1, len(noGroupPreview.Parameters[0].Options)) // Only 1 group
noGroupStream.Close(websocket.StatusGoingAway)

// Now try with a user with more than 1 group
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, codersdk.Me, version.ID)
require.NoError(t, err)
defer stream.Close(websocket.StatusGoingAway)

Expand Down
2 changes: 2 additions & 0 deletions site/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,7 @@ class ApiMethods {

templateVersionDynamicParameters = (
versionId: string,
userId: string,
{
onMessage,
onError,
Expand All @@ -1029,6 +1030,7 @@ class ApiMethods {
): WebSocket => {
const socket = createWebSocket(
`/api/v2/templateversions/${versionId}/dynamic-parameters`,
new URLSearchParams({ user_id: userId }),
Comment on lines 1031 to +1033
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this

);

socket.addEventListener("message", (event) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,31 +148,35 @@ const CreateWorkspacePageExperimental: FC = () => {
useEffect(() => {
if (!realizedVersionId) return;

const socket = API.templateVersionDynamicParameters(realizedVersionId, {
onMessage,
onError: (error) => {
if (ws.current === socket) {
setWsError(error);
}
},
onClose: () => {
if (ws.current === socket) {
setWsError(
new DetailedError(
"Websocket connection for dynamic parameters unexpectedly closed.",
"Refresh the page to reset the form.",
),
);
}
const socket = API.templateVersionDynamicParameters(
realizedVersionId,
defaultOwner.id,
{
onMessage,
onError: (error) => {
if (ws.current === socket) {
setWsError(error);
}
},
onClose: () => {
if (ws.current === socket) {
setWsError(
new DetailedError(
"Websocket connection for dynamic parameters unexpectedly closed.",
"Refresh the page to reset the form.",
),
);
}
},
},
});
);

ws.current = socket;

return () => {
socket.close();
};
}, [realizedVersionId, onMessage]);
}, [realizedVersionId, onMessage, defaultOwner.id]);

const organizationId = templateQuery.data?.organization_id;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ const WorkspaceParametersPageExperimental: FC = () => {

const socket = API.templateVersionDynamicParameters(
templateVersionId ?? workspace.latest_build.template_version_id,
workspace.owner_id,
{
onMessage,
onError: (error) => {
Expand Down Expand Up @@ -140,6 +141,7 @@ const WorkspaceParametersPageExperimental: FC = () => {
templateVersionId,
workspace.latest_build.template_version_id,
onMessage,
workspace.owner_id,
]);

const updateParameters = useMutation({
Expand Down
Loading