Skip to content

Commit 027d4f3

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

File tree

9 files changed

+83
-76
lines changed

9 files changed

+83
-76
lines changed

‎coderd/database/dbauthz/dbauthz.go

+1-3
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/modelmethods.go

+4
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/rbac/object.go

+6
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

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/roles.go

+6-4
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

+7-22
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

+32-23
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

@@ -1775,20 +1782,22 @@ func TestUserAutofillParameters(t *testing.T) {
17751782
Required: true,
17761783
}).Do()
17771784

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-
})
1785+
dbfake.WorkspaceBuild(t, db, database.Workspace{}).Params(
1786+
database.WorkspaceBuildParameter{
1787+
Name: "param",
1788+
Value: "foo",
1789+
},
1790+
).Do()
17871791

1788-
params, err := client.UserAutofillParameters(
1792+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1793+
defer cancel()
1794+
1795+
// Use client2 since client1 is site admin, so
1796+
// we don't get good coverage on RBAC working.
1797+
params, err := client2.UserAutofillParameters(
17891798
ctx,
1790-
u1.UserID.String(),
1791-
version.Template.ID.String(),
1799+
u2.ID.String(),
1800+
version.Template.ID,
17921801
)
17931802
require.NoError(t, err)
17941803

‎codersdk/rbacresources.go

+25-23
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,

‎codersdk/users.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ type UserParameter struct {
227227
}
228228

229229
// UserAutofillParameters returns all recently used parameters for the given user.
230-
func (c *Client) UserAutofillParameters(ctx context.Context, user string, templateID string) ([]UserParameter, error) {
230+
func (c *Client) UserAutofillParameters(ctx context.Context, user string, templateID uuid.UUID) ([]UserParameter, error) {
231231
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/autofill-parameters?template_id=%s", user, templateID), nil)
232232
if err != nil {
233233
return nil, err

0 commit comments

Comments
 (0)