synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-09 11:45:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: make dynamic parameters respect owner in form (#18013)

Browse Source 
Closes https://github.com/coder/coder/issues/18012

---------

Co-authored-by: Jaayden Halko <jaayden.halko@gmail.com>
This commit is contained in:
Steven Masley
2025-05-27 15:43:00 -05:00
committed by GitHub
parent 5b9c40481f
commit b4531c4218
13 changed files with 264 additions and 189 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
coderd/apidoc/docs.go generated
View File

@ -5880,6 +5880,43 @@ const docTemplate = `{
}
}
},
"/templateversions/{templateversion}/dynamic-parameters": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"tags": [
"Templates"
],
"summary": "Open dynamic parameters WebSocket by template version",
"operationId": "open-dynamic-parameters-websocket-by-template-version",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "user",
"in": "path",
"required": true
},
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "templateversion",
"in": "path",
"required": true
}
],
"responses": {
"101": {
"description": "Switching Protocols"
}
}
}
},
"/templateversions/{templateversion}/external-auth": {
"get": {
"security": [
@ -7735,43 +7772,6 @@ const docTemplate = `{
}
}
},
"/users/{user}/templateversions/{templateversion}/parameters": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"tags": [
"Templates"
],
"summary": "Open dynamic parameters WebSocket by template version",
"operationId": "open-dynamic-parameters-websocket-by-template-version",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "user",
"in": "path",
"required": true
},
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "templateversion",
"in": "path",
"required": true
}
],
"responses": {
"101": {
"description": "Switching Protocols"
}
}
}
},
"/users/{user}/webpush/subscription": {
"post": {
"security": [

70
coderd/apidoc/swagger.json generated
View File

@ -5197,6 +5197,41 @@
}
}
},
"/templateversions/{templateversion}/dynamic-parameters": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"tags": ["Templates"],
"summary": "Open dynamic parameters WebSocket by template version",
"operationId": "open-dynamic-parameters-websocket-by-template-version",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "user",
"in": "path",
"required": true
},
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "templateversion",
"in": "path",
"required": true
}
],
"responses": {
"101": {
"description": "Switching Protocols"
}
}
}
},
"/templateversions/{templateversion}/external-auth": {
"get": {
"security": [
@ -6834,41 +6869,6 @@
}
}
},
"/users/{user}/templateversions/{templateversion}/parameters": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"tags": ["Templates"],
"summary": "Open dynamic parameters WebSocket by template version",
"operationId": "open-dynamic-parameters-websocket-by-template-version",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "user",
"in": "path",
"required": true
},
{
"type": "string",
"format": "uuid",
"description": "Template version ID",
"name": "templateversion",
"in": "path",
"required": true
}
],
"responses": {
"101": {
"description": "Switching Protocols"
}
}
}
},
"/users/{user}/webpush/subscription": {
"post": {
"security": [

21
coderd/coderd.go
View File

@ -1122,6 +1122,7 @@ func New(options *Options) *API {
})
})
})
r.Route("/templateversions/{templateversion}", func(r chi.Router) {
r.Use(
apiKeyMiddleware,
@ -1150,6 +1151,13 @@ func New(options *Options) *API {
r.Get("/{jobID}/matched-provisioners", api.templateVersionDryRunMatchedProvisioners)
r.Patch("/{jobID}/cancel", api.patchTemplateVersionDryRunCancel)
})
r.Group(func(r chi.Router) {
r.Use(
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters),
)
r.Get("/dynamic-parameters", api.templateVersionDynamicParameters)
})
})
r.Route("/users", func(r chi.Router) {
r.Get("/first", api.firstUser)
@ -1210,19 +1218,6 @@ func New(options *Options) *API {
r.Group(func(r chi.Router) {
r.Use(httpmw.ExtractUserParam(options.Database))
// Similarly to creating a workspace, evaluating parameters for a
// new workspace should also match the authz story of
// postWorkspacesByOrganization
// TODO: Do not require site wide read user permission. Make this work
// with org member permissions.
r.Route("/templateversions/{templateversion}", func(r chi.Router) {
r.Use(
httpmw.ExtractTemplateVersionParam(options.Database),
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters),
)
r.Get("/parameters", api.templateVersionDynamicParameters)
})
r.Post("/convert-login", api.postConvertLoginType)
r.Delete("/", api.deleteUser)
r.Get("/", api.userByName)

75
coderd/parameters.go
View File

@ -36,7 +36,7 @@ import (
// @Param user path string true "Template version ID" format(uuid)
// @Param templateversion path string true "Template version ID" format(uuid)
// @Success 101
// @Router /users/{user}/templateversions/{templateversion}/parameters [get]
// @Router /templateversions/{templateversion}/dynamic-parameters [get]
func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
templateVersion := httpmw.TemplateVersionParam(r)
@ -77,12 +77,12 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
}
}
type previewFunction func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics)
type previewFunction func(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics)
func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request, tf database.TemplateVersionTerraformValue, templateVersion database.TemplateVersion) {
var (
ctx = r.Context()
user = httpmw.UserParam(r)
ctx = r.Context()
apikey = httpmw.APIKey(r)
)
// nolint:gocritic // We need to fetch the templates files for the Terraform
@ -130,7 +130,7 @@ func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request,
templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}})
}
owner, err := getWorkspaceOwnerData(ctx, api.Database, user, templateVersion.OrganizationID)
owner, err := getWorkspaceOwnerData(ctx, api.Database, apikey.UserID, templateVersion.OrganizationID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching workspace owner.",
@ -145,10 +145,46 @@ func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request,
Owner: owner,
}
api.handleParameterWebsocket(rw, r, func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) {
// failedOwners keeps track of which owners failed to fetch from the database.
// This prevents db spam on repeated requests for the same failed owner.
failedOwners := make(map[uuid.UUID]error)
failedOwnerDiag := hcl.Diagnostics{
{
Severity: hcl.DiagError,
Summary: "Failed to fetch workspace owner",
Detail: "Please check your permissions or the user may not exist.",
Extra: previewtypes.DiagnosticExtra{
Code: "owner_not_found",
},
},
}
api.handleParameterWebsocket(rw, r, apikey.UserID, func(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) {
if ownerID == uuid.Nil {
// Default to the authenticated user
// Nice for testing
ownerID = apikey.UserID
}
if _, ok := failedOwners[ownerID]; ok {
// If it has failed once, assume it will fail always.
// Re-open the websocket to try again.
return nil, failedOwnerDiag
}
// Update the input values with the new values.
// The rest of the input is unchanged.
input.ParameterValues = values
// Update the owner if there is a change
if input.Owner.ID != ownerID.String() {
owner, err = getWorkspaceOwnerData(ctx, api.Database, ownerID, templateVersion.OrganizationID)
if err != nil {
failedOwners[ownerID] = err
return nil, failedOwnerDiag
}
input.Owner = owner
}
return preview.Preview(ctx, input, templateFS)
})
}
@ -239,7 +275,7 @@ func (api *API) handleStaticParameters(rw http.ResponseWriter, r *http.Request,
params = append(params, param)
}
api.handleParameterWebsocket(rw, r, func(_ context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) {
api.handleParameterWebsocket(rw, r, uuid.Nil, func(_ context.Context, _ uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) {
for i := range params {
param := &params[i]
paramValue, ok := values[param.Name]
@ -264,7 +300,7 @@ func (api *API) handleStaticParameters(rw http.ResponseWriter, r *http.Request,
})
}
func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request, render previewFunction) {
func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request, ownerID uuid.UUID, render previewFunction) {
ctx, cancel := context.WithTimeout(r.Context(), 30*time.Minute)
defer cancel()
@ -284,7 +320,7 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
)
// Send an initial form state, computed without any user input.
result, diagnostics := render(ctx, map[string]string{})
result, diagnostics := render(ctx, ownerID, map[string]string{})
response := codersdk.DynamicParametersResponse{
ID: -1, // Always start with -1.
Diagnostics: db2sdk.HCLDiagnostics(diagnostics),
@ -312,7 +348,7 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
return
}
result, diagnostics := render(ctx, update.Inputs)
result, diagnostics := render(ctx, update.OwnerID, update.Inputs)
response := codersdk.DynamicParametersResponse{
ID: update.ID,
Diagnostics: db2sdk.HCLDiagnostics(diagnostics),
@ -332,17 +368,24 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
func getWorkspaceOwnerData(
ctx context.Context,
db database.Store,
user database.User,
ownerID uuid.UUID,
organizationID uuid.UUID,
) (previewtypes.WorkspaceOwner, error) {
var g errgroup.Group
// TODO: @emyrk we should only need read access on the org member, not the
// site wide user object. Figure out a better way to handle this.
user, err := db.GetUserByID(ctx, ownerID)
if err != nil {
return previewtypes.WorkspaceOwner{}, xerrors.Errorf("fetch user: %w", err)
}
var ownerRoles []previewtypes.WorkspaceOwnerRBACRole
g.Go(func() error {
// nolint:gocritic // This is kind of the wrong query to use here, but it
// matches how the provisioner currently works. We should figure out
// something that needs less escalation but has the correct behavior.
row, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID)
row, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), ownerID)
if err != nil {
return err
}
@ -372,7 +415,7 @@ func getWorkspaceOwnerData(
// The correct public key has to be sent. This will not be leaked
// unless the template leaks it.
// nolint:gocritic
key, err := db.GetGitSSHKey(dbauthz.AsSystemRestricted(ctx), user.ID)
key, err := db.GetGitSSHKey(dbauthz.AsSystemRestricted(ctx), ownerID)
if err != nil {
return err
}
@ -388,7 +431,7 @@ func getWorkspaceOwnerData(
// nolint:gocritic
groups, err := db.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{
OrganizationID: organizationID,
HasMemberID: user.ID,
HasMemberID: ownerID,
})
if err != nil {
return err
@ -400,7 +443,7 @@ func getWorkspaceOwnerData(
return nil
})
err := g.Wait()
err = g.Wait()
if err != nil {
return previewtypes.WorkspaceOwner{}, err
}

49
coderd/parameters_test.go
View File

@ -32,7 +32,7 @@ func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) {
cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)}
ownerClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg})
owner := coderdtest.CreateFirstUser(t, ownerClient)
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
templateAdmin, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/public_key/main.tf")
require.NoError(t, err)
@ -57,7 +57,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, templateAdminUser.ID, version.ID)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
require.NoError(t, err)
defer stream.Close(websocket.StatusGoingAway)
@ -210,6 +210,47 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
// test to make it obvious what this test is doing.
require.Zero(t, setup.api.FileCache.Count())
})
t.Run("BadOwner", func(t *testing.T) {
t.Parallel()
dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf")
require.NoError(t, err)
modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules"))
require.NoError(t, err)
setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{
provisionerDaemonVersion: provProto.CurrentVersion.String(),
mainTF: dynamicParametersTerraformSource,
modulesArchive: modulesArchive,
plan: nil,
static: nil,
})
ctx := testutil.Context(t, testutil.WaitShort)
stream := setup.stream
previews := stream.Chan()
// Should see the output of the module represented
preview := testutil.RequireReceive(ctx, t, previews)
require.Equal(t, -1, preview.ID)
require.Empty(t, preview.Diagnostics)
err = stream.Send(codersdk.DynamicParametersRequest{
ID: 1,
Inputs: map[string]string{
"jetbrains_ide": "GO",
},
OwnerID: uuid.New(),
})
require.NoError(t, err)
preview = testutil.RequireReceive(ctx, t, previews)
require.Equal(t, 1, preview.ID)
require.Len(t, preview.Diagnostics, 1)
require.Equal(t, preview.Diagnostics[0].Extra.Code, "owner_not_found")
})
}
type setupDynamicParamsTestParams struct {
@ -242,7 +283,7 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn
})
owner := coderdtest.CreateFirstUser(t, ownerClient)
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
templateAdmin, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
files := echo.WithExtraFiles(map[string][]byte{
"main.tf": args.mainTF,
@ -262,7 +303,7 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn
_ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)
ctx := testutil.Context(t, testutil.WaitShort)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID)
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
if args.expectWebsocketError {
require.Errorf(t, err, "expected error forming websocket")
} else {