Skip to content

Commit e37022a

Browse files
ammarioammario
committed
Address spike + mathias + cian comments
1 parent 4b00f45 commit e37022a

File tree

11 files changed

+131
-77
lines changed

11 files changed

+131
-77
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1759,9 +1759,7 @@ func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params da
17591759
if err != nil {
17601760
return nil, err
17611761
}
1762-
// The ability to update the user implies either the user themselves or someone
1763-
// with complete admin access to the user account.
1764-
if err := q.authorizeContext(ctx, rbac.ActionUpdate, u.UserDataRBACObject()); err != nil {
1762+
if err := q.authorizeContext(ctx, rbac.ActionRead, u.UserWorkspaceBuildParametersObject()); err != nil {
17651763
return nil, err
17661764
}
17671765
return q.db.GetUserWorkspaceBuildParameters(ctx, params)

‎coderd/database/dbmem/dbmem.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3773,8 +3773,42 @@ func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, params
37733773
userWorkspaceIDs[ws.ID] = struct{}{}
37743774
}
37753775

3776+
userWorkspaceBuilds := make(map[uuid.UUID]struct{})
3777+
for _, wb := range q.workspaceBuilds {
3778+
if _, ok := userWorkspaceIDs[wb.WorkspaceID]; !ok {
3779+
continue
3780+
}
3781+
userWorkspaceBuilds[wb.ID] = struct{}{}
3782+
}
3783+
3784+
templateVersions := make(map[uuid.UUID]struct{})
3785+
for _, tv := range q.templateVersions {
3786+
if tv.TemplateID.UUID != params.TemplateID {
3787+
continue
3788+
}
3789+
templateVersions[tv.ID] = struct{}{}
3790+
}
3791+
3792+
tvps := make(map[string]struct{})
3793+
for _, tvp := range q.templateVersionParameters {
3794+
if _, ok := templateVersions[tvp.TemplateVersionID]; !ok {
3795+
continue
3796+
}
3797+
3798+
if _, ok := tvps[tvp.Name]; !ok && !tvp.Ephemeral {
3799+
tvps[tvp.Name] = struct{}{}
3800+
}
3801+
}
3802+
37763803
userWorkspaceBuildParameters := make([]database.GetUserWorkspaceBuildParametersRow, 0)
37773804
for _, wbp := range q.workspaceBuildParameters {
3805+
fmt.Printf("wbp: %+v\nbuilds: %+v\n", wbp, userWorkspaceBuilds)
3806+
if _, ok := userWorkspaceBuilds[wbp.WorkspaceBuildID]; !ok {
3807+
continue
3808+
}
3809+
if _, ok := tvps[wbp.Name]; !ok {
3810+
continue
3811+
}
37783812
userWorkspaceBuildParameters = append(userWorkspaceBuildParameters, database.GetUserWorkspaceBuildParametersRow{
37793813
Name: wbp.Name,
37803814
Value: wbp.Value,

‎coderd/database/modelmethods.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,10 @@ func (u User) UserDataRBACObject() rbac.Object {
255255
return rbac.ResourceUserData.WithID(u.ID).WithOwner(u.ID.String())
256256
}
257257

258+
func (u User) UserWorkspaceBuildParametersObject() rbac.Object {
259+
return rbac.ResourceUserWorkspaceBuildParameters.WithID(u.ID).WithOwner(u.ID.String())
260+
}
261+
258262
func (u GetUsersRow) RBACObject() rbac.Object {
259263
return rbac.ResourceUserObject(u.ID)
260264
}

‎coderd/database/queries/workspacebuildparameters.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ FROM (
3838
AND wb.transition = 'start'
3939
AND w.template_id = $2
4040
AND tvp.ephemeral = false
41+
AND tvp.name = wbp.name
4142
) sub
4243
WHERE
4344
sub.rn = 1

‎coderd/rbac/object.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ var (
148148
Type: "user_data",
149149
}
150150

151+
// ResourceUserWorkspaceBuildParameters is the user's workspace build
152+
// parameter history.
153+
ResourceUserWorkspaceBuildParameters = Object{
154+
Type: "user_workspace_build_parameters",
155+
}
156+
151157
// ResourceOrganizationMember is a user's membership in an organization.
152158
// Has ONLY an organization owner.
153159
// create/delete = Create/delete member from org.

‎coderd/rbac/object_gen.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/roles.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
154154
Permissions(map[string][]Action{
155155
// Users cannot do create/update/delete on themselves, but they
156156
// can read their own details.
157-
ResourceUser.Type: {ActionRead},
157+
ResourceUser.Type: {ActionRead},
158+
ResourceUserWorkspaceBuildParameters.Type: {ActionRead},
158159
// Users can create provisioner daemons scoped to themselves.
159160
ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate},
160161
})...,
@@ -209,9 +210,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
209210
Name: userAdmin,
210211
DisplayName: "User Admin",
211212
Site: Permissions(map[string][]Action{
212-
ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
213-
ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
214-
ResourceUserData.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
213+
ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
214+
ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
215+
ResourceUserData.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
216+
ResourceUserWorkspaceBuildParameters.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
215217
// Full perms to manage org members
216218
ResourceOrganizationMember.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
217219
ResourceGroup.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},

‎coderd/users.go

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -581,30 +581,15 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) {
581581
// @Success 200 {array} codersdk.UserParameter
582582
// @Router /users/{user}/autofill-parameters [get]
583583
func (api *API) userAutofillParameters(rw http.ResponseWriter, r *http.Request) {
584-
var (
585-
apiKey = httpmw.APIKey(r)
586-
user = httpmw.UserParam(r)
587-
)
588-
589-
if apiKey.UserID != user.ID {
590-
httpapi.Write(r.Context(), rw, http.StatusForbidden, codersdk.Response{
591-
Message: "You are not authorized to view this user's parameters.",
592-
})
593-
return
594-
}
595-
596-
templateIDStr := r.URL.Query().Get("template_id")
597-
if templateIDStr == "" {
598-
httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{
599-
Message: "template_id is required.",
600-
})
601-
return
602-
}
584+
user := httpmw.UserParam(r)
603585

604-
templateID, err := uuid.Parse(templateIDStr)
605-
if err != nil {
586+
p := httpapi.NewQueryParamParser().Required("template_id")
587+
templateID := p.UUID(r.URL.Query(), uuid.UUID{}, "template_id")
588+
p.ErrorExcessParams(r.URL.Query())
589+
if len(p.Errors) > 0 {
606590
httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{
607-
Message: "template_id must be a valid UUID.",
591+
Message: "Invalid query parameters.",
592+
Validations: p.Errors,
608593
})
609594
return
610595
}

‎coderd/users_test.go

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,11 +1726,11 @@ func TestUserAutofillParameters(t *testing.T) {
17261726
t.Parallel()
17271727
t.Run("NotSelf", func(t *testing.T) {
17281728
t.Parallel()
1729-
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
1729+
client1, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{})
17301730

1731-
u1 := coderdtest.CreateFirstUser(t, client)
1731+
u1 := coderdtest.CreateFirstUser(t, client1)
17321732

1733-
_, u2 := coderdtest.CreateAnotherUser(t, client, u1.OrganizationID)
1733+
client2, u2 := coderdtest.CreateAnotherUser(t, client1, u1.OrganizationID)
17341734

17351735
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
17361736
defer cancel()
@@ -1745,25 +1745,32 @@ func TestUserAutofillParameters(t *testing.T) {
17451745
Required: true,
17461746
}).Do()
17471747

1748-
_, err := client.UserAutofillParameters(
1748+
_, err := client2.UserAutofillParameters(
17491749
ctx,
1750-
u2.ID.String(),
1751-
version.Template.ID.String(),
1750+
u1.UserID.String(),
1751+
version.Template.ID,
17521752
)
17531753

17541754
var apiErr *codersdk.Error
17551755
require.ErrorAs(t, err, &apiErr)
1756-
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
1756+
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
1757+
1758+
// u1 should be able to read u2's parameters as u1 is site admin.
1759+
_, err = client1.UserAutofillParameters(
1760+
ctx,
1761+
u2.ID.String(),
1762+
version.Template.ID,
1763+
)
1764+
require.NoError(t, err)
17571765
})
17581766

17591767
t.Run("FindsParameters", func(t *testing.T) {
17601768
t.Parallel()
1761-
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
1769+
client1, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{})
17621770

1763-
u1 := coderdtest.CreateFirstUser(t, client)
1771+
u1 := coderdtest.CreateFirstUser(t, client1)
17641772

1765-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1766-
defer cancel()
1773+
client2, u2 := coderdtest.CreateAnotherUser(t, client1, u1.OrganizationID)
17671774

17681775
db := api.Database
17691776

@@ -1773,22 +1780,36 @@ func TestUserAutofillParameters(t *testing.T) {
17731780
}).Params(database.TemplateVersionParameter{
17741781
Name: "param",
17751782
Required: true,
1776-
}).Do()
1783+
},
1784+
database.TemplateVersionParameter{
1785+
Name: "param2",
1786+
Ephemeral: true,
1787+
},
1788+
).Do()
1789+
1790+
dbfake.WorkspaceBuild(t, db, database.Workspace{
1791+
OwnerID: u2.ID,
1792+
TemplateID: version.Template.ID,
1793+
}).Params(
1794+
database.WorkspaceBuildParameter{
1795+
Name: "param",
1796+
Value: "foo",
1797+
},
1798+
database.WorkspaceBuildParameter{
1799+
Name: "param2",
1800+
Value: "bar",
1801+
},
1802+
).Do()
17771803

1778-
coderdtest.CreateWorkspace(t, client, u1.OrganizationID, version.Template.ID,
1779-
func(cwr *codersdk.CreateWorkspaceRequest) {
1780-
cwr.RichParameterValues = []codersdk.WorkspaceBuildParameter{
1781-
{
1782-
Name: "param",
1783-
Value: "foo",
1784-
},
1785-
}
1786-
})
1804+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1805+
defer cancel()
17871806

1788-
params, err := client.UserAutofillParameters(
1807+
// Use client2 since client1 is site admin, so
1808+
// we don't get good coverage on RBAC working.
1809+
params, err := client2.UserAutofillParameters(
17891810
ctx,
1790-
u1.UserID.String(),
1791-
version.Template.ID.String(),
1811+
u2.ID.String(),
1812+
version.Template.ID,
17921813
)
17931814
require.NoError(t, err)
17941815

‎codersdk/rbacresources.go

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,30 @@ package codersdk
33
type RBACResource string
44

55
const (
6-
ResourceWorkspace RBACResource = "workspace"
7-
ResourceWorkspaceProxy RBACResource = "workspace_proxy"
8-
ResourceWorkspaceExecution RBACResource = "workspace_execution"
9-
ResourceWorkspaceApplicationConnect RBACResource = "application_connect"
10-
ResourceAuditLog RBACResource = "audit_log"
11-
ResourceTemplate RBACResource = "template"
12-
ResourceGroup RBACResource = "group"
13-
ResourceFile RBACResource = "file"
14-
ResourceProvisionerDaemon RBACResource = "provisioner_daemon"
15-
ResourceOrganization RBACResource = "organization"
16-
ResourceRoleAssignment RBACResource = "assign_role"
17-
ResourceOrgRoleAssignment RBACResource = "assign_org_role"
18-
ResourceAPIKey RBACResource = "api_key"
19-
ResourceUser RBACResource = "user"
20-
ResourceUserData RBACResource = "user_data"
21-
ResourceOrganizationMember RBACResource = "organization_member"
22-
ResourceLicense RBACResource = "license"
23-
ResourceDeploymentValues RBACResource = "deployment_config"
24-
ResourceDeploymentStats RBACResource = "deployment_stats"
25-
ResourceReplicas RBACResource = "replicas"
26-
ResourceDebugInfo RBACResource = "debug_info"
27-
ResourceSystem RBACResource = "system"
28-
ResourceTemplateInsights RBACResource = "template_insights"
6+
ResourceWorkspace RBACResource = "workspace"
7+
ResourceWorkspaceProxy RBACResource = "workspace_proxy"
8+
ResourceWorkspaceExecution RBACResource = "workspace_execution"
9+
ResourceWorkspaceApplicationConnect RBACResource = "application_connect"
10+
ResourceAuditLog RBACResource = "audit_log"
11+
ResourceTemplate RBACResource = "template"
12+
ResourceGroup RBACResource = "group"
13+
ResourceFile RBACResource = "file"
14+
ResourceProvisionerDaemon RBACResource = "provisioner_daemon"
15+
ResourceOrganization RBACResource = "organization"
16+
ResourceRoleAssignment RBACResource = "assign_role"
17+
ResourceOrgRoleAssignment RBACResource = "assign_org_role"
18+
ResourceAPIKey RBACResource = "api_key"
19+
ResourceUser RBACResource = "user"
20+
ResourceUserData RBACResource = "user_data"
21+
ResourceUserWorkspaceBuildParameters RBACResource = "user_workspace_build_parameters"
22+
ResourceOrganizationMember RBACResource = "organization_member"
23+
ResourceLicense RBACResource = "license"
24+
ResourceDeploymentValues RBACResource = "deployment_config"
25+
ResourceDeploymentStats RBACResource = "deployment_stats"
26+
ResourceReplicas RBACResource = "replicas"
27+
ResourceDebugInfo RBACResource = "debug_info"
28+
ResourceSystem RBACResource = "system"
29+
ResourceTemplateInsights RBACResource = "template_insights"
2930
)
3031

3132
const (
@@ -52,6 +53,7 @@ var (
5253
ResourceAPIKey,
5354
ResourceUser,
5455
ResourceUserData,
56+
ResourceUserWorkspaceBuildParameters,
5557
ResourceOrganizationMember,
5658
ResourceLicense,
5759
ResourceDeploymentValues,

0 commit comments

Comments
 (0)