fix: allow dynamic parameters without requiring org membership (#18531)

This commit is contained in:
Steven Masley
2025-06-24 10:33:10 -05:00
committed by GitHub
parent 5816455207
commit 341b54e604
2 changed files with 88 additions and 24 deletions

View File

@ -243,8 +243,13 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui
return nil // already fetched return nil // already fetched
} }
// You only need to be able to read the organization member to get the owner user, err := r.db.GetUserByID(ctx, ownerID)
// data. Only the terraform files can therefore leak more information than the if err != nil {
// 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 // caller should have access to. All this info should be public assuming you can
// read the user though. // read the user though.
mem, err := database.ExpectOne(r.db.OrganizationMembers(ctx, database.OrganizationMembersParams{ mem, err := database.ExpectOne(r.db.OrganizationMembers(ctx, database.OrganizationMembersParams{
@ -253,15 +258,16 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui
IncludeSystem: true, IncludeSystem: true,
})) }))
if err != nil { if err != nil {
return err return xerrors.Errorf("fetch user: %w", err)
} }
// User data is required for the form. Org member is checked above // Org member fetched, so use the provisioner context to fetch the user.
// nolint:gocritic //nolint:gocritic // Has the correct permissions, and matches the provisioning flow.
user, err := r.db.GetUserByID(dbauthz.AsProvisionerd(ctx), mem.OrganizationMember.UserID) user, err = r.db.GetUserByID(dbauthz.AsProvisionerd(ctx), mem.OrganizationMember.UserID)
if err != nil { if err != nil {
return xerrors.Errorf("fetch user: %w", err) return xerrors.Errorf("fetch user: %w", err)
} }
}
// nolint:gocritic // This is kind of the wrong query to use here, but it // nolint:gocritic // This is kind of the wrong query to use here, but it
// matches how the provisioner currently works. We should figure out // matches how the provisioner currently works. We should figure out
@ -314,10 +320,10 @@ func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uui
} }
r.currentOwner = &previewtypes.WorkspaceOwner{ r.currentOwner = &previewtypes.WorkspaceOwner{
ID: mem.OrganizationMember.UserID.String(), ID: user.ID.String(),
Name: mem.Username, Name: user.Username,
FullName: mem.Name, FullName: user.Name,
Email: mem.Email, Email: user.Email,
LoginType: string(user.LoginType), LoginType: string(user.LoginType),
RBACRoles: ownerRoles, RBACRoles: ownerRoles,
SSHPublicKey: key.PublicKey, SSHPublicKey: key.PublicKey,

View File

@ -287,11 +287,9 @@ func TestCreateUserWorkspace(t *testing.T) {
OrganizationID: first.OrganizationID, OrganizationID: first.OrganizationID,
}) })
version := coderdtest.CreateTemplateVersion(t, admin, first.OrganizationID, nil) template, _ := coderdtest.DynamicParameterTemplate(t, admin, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{})
coderdtest.AwaitTemplateVersionJobCompleted(t, admin, version.ID)
template := coderdtest.CreateTemplate(t, admin, first.OrganizationID, version.ID)
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{ wrk, err := creator.CreateUserWorkspace(ctx, adminID.ID.String(), codersdk.CreateWorkspaceRequest{
TemplateID: template.ID, TemplateID: template.ID,
@ -306,6 +304,66 @@ func TestCreateUserWorkspace(t *testing.T) {
require.NoError(t, err) 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,
},
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureCustomRoles: 1,
codersdk.FeatureTemplateRBAC: 1,
codersdk.FeatureMultipleOrganizations: 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)
// 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)
// try the test action with this user & custom role
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, creator, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{})
ctx = testutil.Context(t, testutil.WaitLong)
wrk, err := creator.CreateUserWorkspace(ctx, forUser.ID.String(), codersdk.CreateWorkspaceRequest{
TemplateID: template.ID,
Name: "workspace",
})
require.NoError(t, err)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, creator, wrk.LatestBuild.ID)
_, err = creator.WorkspaceByOwnerAndName(ctx, forUser.Username, wrk.Name, codersdk.WorkspaceOptions{
IncludeDeleted: false,
})
require.NoError(t, err)
})
// Asserting some authz calls when creating a workspace. // Asserting some authz calls when creating a workspace.
t.Run("AuthzStory", func(t *testing.T) { t.Run("AuthzStory", func(t *testing.T) {
t.Parallel() t.Parallel()