Content-Length: 12891 | pFad | http://github.com/coder/coder/pull/18531.patch

thub.com From a28a6014aff73e9a52e406fc92555e6299fe7b61 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 24 Jun 2025 09:28:45 -0500 Subject: [PATCH 1/4] fix: dynamic parameters to not require org membership Prebuilds user was failing to fetch this way --- coderd/dynamicparameters/render.go | 46 +++++++++++++++++------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/coderd/dynamicparameters/render.go b/coderd/dynamicparameters/render.go index fd050b4062e5f..3392f9c3abdc3 100644 --- a/coderd/dynamicparameters/render.go +++ b/coderd/dynamicparameters/render.go @@ -243,24 +243,30 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui return nil // already fetched } - // You only need to be able to read the organization member to get the owner - // data. Only the terraform files can therefore leak more information than the - // caller should have access to. All this info should be public assuming you can - // read the user though. - mem, err := database.ExpectOne(r.db.OrganizationMembers(ctx, database.OrganizationMembersParams{ - OrganizationID: r.data.templateVersion.OrganizationID, - UserID: ownerID, - IncludeSystem: true, - })) + user, err := r.db.GetUserByID(ctx, ownerID) if err != nil { - return err - } + // If the user failed to read, we also try to read the user from their + // organization member. You only need to be able to read the organization member + // to get the owner data. + // + // Only the terraform files can therefore leak more information than the + // caller should have access to. All this info should be public assuming you can + // read the user though. + mem, err := database.ExpectOne(r.db.OrganizationMembers(ctx, database.OrganizationMembersParams{ + OrganizationID: r.data.templateVersion.OrganizationID, + UserID: ownerID, + IncludeSystem: true, + })) + if err != nil { + return xerrors.Errorf("fetch user: %w", err) + } - // User data is required for the form. Org member is checked above - // nolint:gocritic - user, err := r.db.GetUserByID(dbauthz.AsProvisionerd(ctx), mem.OrganizationMember.UserID) - if err != nil { - return xerrors.Errorf("fetch user: %w", err) + // Org member fetched, so use the provisioner context to fetch the user. + //nolint:gocritic // Has the correct permissions, and matches the provisioning flow. + user, err = r.db.GetUserByID(dbauthz.AsProvisionerd(ctx), mem.OrganizationMember.UserID) + if err != nil { + return xerrors.Errorf("fetch user: %w", err) + } } // nolint:gocritic // This is kind of the wrong query to use here, but it @@ -314,10 +320,10 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui } r.currentOwner = &previewtypes.WorkspaceOwner{ - ID: mem.OrganizationMember.UserID.String(), - Name: mem.Username, - FullName: mem.Name, - Email: mem.Email, + ID: user.ID.String(), + Name: user.Username, + FullName: user.Name, + Email: user.Email, LoginType: string(user.LoginType), RBACRoles: ownerRoles, SSHPublicKey: key.PublicKey, From 4a621489cb2ae989a59dee5589fba06cb2046ad6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 24 Jun 2025 09:59:01 -0500 Subject: [PATCH 2/4] test --- enterprise/coderd/workspaces_test.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index ef9d1b977ea00..8808195b8c159 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -260,8 +260,9 @@ func TestCreateUserWorkspace(t *testing.T) { }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ - codersdk.FeatureCustomRoles: 1, - codersdk.FeatureTemplateRBAC: 1, + codersdk.FeatureCustomRoles: 1, + codersdk.FeatureTemplateRBAC: 1, + codersdk.FeatureMultipleOrganizations: 1, }, }, }) @@ -278,8 +279,15 @@ func TestCreateUserWorkspace(t *testing.T) { }) require.NoError(t, err) - // use admin for setting up test - admin, adminID := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleTemplateAdmin()) + secondOrg := coderdenttest.CreateOrganization(t, owner, coderdenttest.CreateOrganizationOptions{}) + + // user to make the workspace for, **note** the user is not a member of the first org. + // This is strange, but technically valid. The creator can create a workspace for + // this user in this org, even though the user cannot access the workspace. + _, forUser := coderdtest.CreateAnotherUser(t, owner, secondOrg.ID) + + // Need an admin to make the template + admin, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(first.OrganizationID)) // try the test action with this user & custom role creator, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleMember(), rbac.RoleIdentifier{ @@ -287,20 +295,18 @@ func TestCreateUserWorkspace(t *testing.T) { OrganizationID: first.OrganizationID, }) - version := coderdtest.CreateTemplateVersion(t, admin, first.OrganizationID, nil) - coderdtest.AwaitTemplateVersionJobCompleted(t, admin, version.ID) - template := coderdtest.CreateTemplate(t, admin, first.OrganizationID, version.ID) + template, _ := coderdtest.DynamicParameterTemplate(t, admin, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{}) ctx = testutil.Context(t, testutil.WaitLong*1000) // Reset the context to avoid timeouts. - wrk, err := creator.CreateUserWorkspace(ctx, adminID.ID.String(), codersdk.CreateWorkspaceRequest{ + wrk, err := creator.CreateUserWorkspace(ctx, forUser.ID.String(), codersdk.CreateWorkspaceRequest{ TemplateID: template.ID, Name: "workspace", }) require.NoError(t, err) - coderdtest.AwaitWorkspaceBuildJobCompleted(t, admin, wrk.LatestBuild.ID) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, creator, wrk.LatestBuild.ID) - _, err = creator.WorkspaceByOwnerAndName(ctx, adminID.Username, wrk.Name, codersdk.WorkspaceOptions{ + _, err = creator.WorkspaceByOwnerAndName(ctx, forUser.Username, wrk.Name, codersdk.WorkspaceOptions{ IncludeDeleted: false, }) require.NoError(t, err) From da656d14682ec815fe754924fef9b16ad421f094 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 24 Jun 2025 10:12:23 -0500 Subject: [PATCH 3/4] test: fixup tests --- enterprise/coderd/workspaces_test.go | 74 ++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 10 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 8808195b8c159..fe35e833157cd 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -254,6 +254,59 @@ func TestCreateUserWorkspace(t *testing.T) { t.Run("ForAnotherUser", func(t *testing.T) { t.Parallel() + owner, first := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + ctx := testutil.Context(t, testutil.WaitShort) + //nolint:gocritic // using owner to setup roles + r, err := owner.CreateOrganizationRole(ctx, codersdk.Role{ + Name: "creator", + OrganizationID: first.OrganizationID.String(), + DisplayName: "Creator", + OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionCreate, codersdk.ActionWorkspaceStart, codersdk.ActionUpdate, codersdk.ActionRead}, + codersdk.ResourceOrganizationMember: {codersdk.ActionRead}, + }), + }) + require.NoError(t, err) + + // use admin for setting up test + admin, adminID := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleTemplateAdmin()) + + // try the test action with this user & custom role + creator, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleMember(), rbac.RoleIdentifier{ + Name: r.Name, + OrganizationID: first.OrganizationID, + }) + + template, _ := coderdtest.DynamicParameterTemplate(t, admin, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{}) + + ctx = testutil.Context(t, testutil.WaitLong*1000) // Reset the context to avoid timeouts. + + wrk, err := creator.CreateUserWorkspace(ctx, adminID.ID.String(), codersdk.CreateWorkspaceRequest{ + TemplateID: template.ID, + Name: "workspace", + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, admin, wrk.LatestBuild.ID) + + _, err = creator.WorkspaceByOwnerAndName(ctx, adminID.Username, wrk.Name, codersdk.WorkspaceOptions{ + IncludeDeleted: false, + }) + require.NoError(t, err) + }) + + t.Run("ForANonOrgMember", func(t *testing.T) { + t.Parallel() + owner, first := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, @@ -279,23 +332,24 @@ func TestCreateUserWorkspace(t *testing.T) { }) require.NoError(t, err) - secondOrg := coderdenttest.CreateOrganization(t, owner, coderdenttest.CreateOrganizationOptions{}) - // user to make the workspace for, **note** the user is not a member of the first org. // This is strange, but technically valid. The creator can create a workspace for // this user in this org, even though the user cannot access the workspace. + secondOrg := coderdenttest.CreateOrganization(t, owner, coderdenttest.CreateOrganizationOptions{}) _, forUser := coderdtest.CreateAnotherUser(t, owner, secondOrg.ID) - // Need an admin to make the template - admin, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(first.OrganizationID)) - // try the test action with this user & custom role - creator, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleMember(), rbac.RoleIdentifier{ - Name: r.Name, - OrganizationID: first.OrganizationID, - }) + creator, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleMember(), + rbac.RoleTemplateAdmin(), // Need site wide access to make workspace for non-org + rbac.RoleIdentifier{ + Name: r.Name, + OrganizationID: first.OrganizationID, + }, + ) - template, _ := coderdtest.DynamicParameterTemplate(t, admin, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{}) + version := coderdtest.CreateTemplateVersion(t, creator, first.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, creator, version.ID) + template := coderdtest.CreateTemplate(t, creator, first.OrganizationID, version.ID) ctx = testutil.Context(t, testutil.WaitLong*1000) // Reset the context to avoid timeouts. From fb9664e79274cc7f595edb84cd5d1611ed14c410 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 24 Jun 2025 10:15:09 -0500 Subject: [PATCH 4/4] make template dynamic --- enterprise/coderd/workspaces_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index fe35e833157cd..228b11f485a96 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -289,7 +289,7 @@ func TestCreateUserWorkspace(t *testing.T) { template, _ := coderdtest.DynamicParameterTemplate(t, admin, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{}) - ctx = testutil.Context(t, testutil.WaitLong*1000) // Reset the context to avoid timeouts. + ctx = testutil.Context(t, testutil.WaitLong) wrk, err := creator.CreateUserWorkspace(ctx, adminID.ID.String(), codersdk.CreateWorkspaceRequest{ TemplateID: template.ID, @@ -347,11 +347,9 @@ func TestCreateUserWorkspace(t *testing.T) { }, ) - version := coderdtest.CreateTemplateVersion(t, creator, first.OrganizationID, nil) - coderdtest.AwaitTemplateVersionJobCompleted(t, creator, version.ID) - template := coderdtest.CreateTemplate(t, creator, first.OrganizationID, version.ID) + template, _ := coderdtest.DynamicParameterTemplate(t, creator, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{}) - ctx = testutil.Context(t, testutil.WaitLong*1000) // Reset the context to avoid timeouts. + ctx = testutil.Context(t, testutil.WaitLong) wrk, err := creator.CreateUserWorkspace(ctx, forUser.ID.String(), codersdk.CreateWorkspaceRequest{ TemplateID: template.ID,

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!