Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: allow dynamic parameters without requiring org membership#18531

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 4 commits intomainfromstevenmasley/fix_owner_fetch
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes fromall 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
46 changes: 26 additions & 20 deletionscoderd/dynamicparameters/render.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
}
Comment on lines +246 to +269
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

If the Renderer had the authz object, we could fetch the user with elevated perms, then check the user's perms. Removing 1 sql query round trip 😢

}

// nolint:gocritic // This is kind of the wrong query to use here, but it
Expand DownExpand Up@@ -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,
Expand Down
66 changes: 62 additions & 4 deletionsenterprise/coderd/workspaces_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -287,11 +287,9 @@ 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.
ctx = testutil.Context(t, testutil.WaitLong)

wrk, err := creator.CreateUserWorkspace(ctx, adminID.ID.String(), codersdk.CreateWorkspaceRequest{
TemplateID: template.ID,
Expand All@@ -306,6 +304,66 @@ func TestCreateUserWorkspace(t *testing.T) {
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.
t.Run("AuthzStory", func(t *testing.T) {
t.Parallel()
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp