Skip to content

Commit 32af1e6

Browse files
EmyrkEmyrk
committed
feat: Move all authorize calls into each handler
1 parent 307f6c0 commit 32af1e6

File tree

6 files changed

+140
-65
lines changed

6 files changed

+140
-65
lines changed

‎coderd/coderd_test.go

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,13 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
4242
organization, err := client.Organization(context.Background(), admin.OrganizationID)
4343
require.NoError(t, err, "fetch org")
4444

45+
// Setup some data in the database.
46+
coderdtest.NewProvisionerDaemon(t, client)
47+
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
48+
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
49+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
50+
coderdtest.CreateWorkspace(t, client, admin.OrganizationID, template.ID)
51+
4552
// Always fail auth from this point forward
4653
authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
4754

@@ -128,9 +135,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
128135
"GET:/api/v2/files/{hash}": {NoAuthorize: true},
129136

130137
// These endpoints have more assertions. This is good, add more endpoints to assert if you can!
131-
"GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.InOrg(admin.OrganizationID)},
132-
"GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization},
133-
"GET:/api/v2/users/{user}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
138+
"GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.InOrg(admin.OrganizationID)},
139+
"GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization},
140+
"GET:/api/v2/users/{user}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
141+
"GET:/api/v2/organizations/{organization}/workspaces/{user}": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
134142
}
135143

136144
c, _ := srv.Config.Handler.(*chi.Mux)
@@ -159,17 +167,19 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
159167
if !routeAssertions.NoAuthorize {
160168
assert.NotNil(t, authorizer.Called, "authorizer expected")
161169
assert.Equal(t, routeAssertions.StatusCode, resp.StatusCode, "expect unauthorized")
162-
if routeAssertions.AssertObject.Type != "" {
163-
assert.Equal(t, routeAssertions.AssertObject.Type, authorizer.Called.Object.Type, "resource type")
164-
}
165-
if routeAssertions.AssertObject.Owner != "" {
166-
assert.Equal(t, routeAssertions.AssertObject.Owner, authorizer.Called.Object.Owner, "resource owner")
167-
}
168-
if routeAssertions.AssertObject.OrgID != "" {
169-
assert.Equal(t, routeAssertions.AssertObject.OrgID, authorizer.Called.Object.OrgID, "resource org")
170-
}
171-
if routeAssertions.AssertObject.ResourceID != "" {
172-
assert.Equal(t, routeAssertions.AssertObject.ResourceID, authorizer.Called.Object.ResourceID, "resource ID")
170+
if authorizer.Called != nil {
171+
if routeAssertions.AssertObject.Type != "" {
172+
assert.Equal(t, routeAssertions.AssertObject.Type, authorizer.Called.Object.Type, "resource type")
173+
}
174+
if routeAssertions.AssertObject.Owner != "" {
175+
assert.Equal(t, routeAssertions.AssertObject.Owner, authorizer.Called.Object.Owner, "resource owner")
176+
}
177+
if routeAssertions.AssertObject.OrgID != "" {
178+
assert.Equal(t, routeAssertions.AssertObject.OrgID, authorizer.Called.Object.OrgID, "resource org")
179+
}
180+
if routeAssertions.AssertObject.ResourceID != "" {
181+
assert.Equal(t, routeAssertions.AssertObject.ResourceID, authorizer.Called.Object.ResourceID, "resource ID")
182+
}
173183
}
174184
} else {
175185
assert.Nil(t, authorizer.Called, "authorize not expected")

‎coderd/gitsshkey.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55
"net/http"
66

7+
"github.com/coder/coder/coderd/rbac"
8+
79
"github.com/coder/coder/coderd/database"
810
"github.com/coder/coder/coderd/gitsshkey"
911
"github.com/coder/coder/coderd/httpapi"
@@ -13,6 +15,11 @@ import (
1315

1416
func (api *api) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
1517
user := httpmw.UserParam(r)
18+
19+
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUserData.WithOwner(user.ID.String())) {
20+
return
21+
}
22+
1623
privateKey, publicKey, err := gitsshkey.Generate(api.SSHKeygenAlgorithm)
1724
if err != nil {
1825
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
@@ -53,6 +60,11 @@ func (api *api) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
5360

5461
func (api *api) gitSSHKey(rw http.ResponseWriter, r *http.Request) {
5562
user := httpmw.UserParam(r)
63+
64+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
65+
return
66+
}
67+
5668
gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
5769
if err != nil {
5870
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{

‎coderd/organizations.go

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"fmt"
88
"net/http"
99

10+
"github.com/coder/coder/coderd/rbac"
11+
1012
"github.com/go-chi/chi/v5"
1113
"github.com/google/uuid"
1214
"github.com/moby/moby/pkg/namesgenerator"
@@ -18,8 +20,15 @@ import (
1820
"github.com/coder/coder/codersdk"
1921
)
2022

21-
func (*api) organization(rw http.ResponseWriter, r *http.Request) {
23+
func (api *api) organization(rw http.ResponseWriter, r *http.Request) {
2224
organization := httpmw.OrganizationParam(r)
25+
26+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceOrganization.
27+
InOrg(organization.ID).
28+
WithID(organization.ID.String())) {
29+
return
30+
}
31+
2332
httpapi.Write(rw, http.StatusOK, convertOrganization(organization))
2433
}
2534

@@ -327,6 +336,11 @@ func (api *api) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
327336

328337
func (api *api) workspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
329338
organization := httpmw.OrganizationParam(r)
339+
340+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.InOrg(organization.ID)) {
341+
return
342+
}
343+
330344
workspaces, err := api.Database.GetWorkspacesByOrganizationID(r.Context(), database.GetWorkspacesByOrganizationIDParams{
331345
OrganizationID: organization.ID,
332346
Deleted: false,
@@ -352,6 +366,8 @@ func (api *api) workspacesByOrganization(rw http.ResponseWriter, r *http.Request
352366

353367
func (api *api) workspacesByOwner(rw http.ResponseWriter, r *http.Request) {
354368
owner := httpmw.UserParam(r)
369+
roles := httpmw.UserRoles(r)
370+
355371
workspaces, err := api.Database.GetWorkspacesByOwnerID(r.Context(), database.GetWorkspacesByOwnerIDParams{
356372
OwnerID: owner.ID,
357373
})
@@ -364,7 +380,19 @@ func (api *api) workspacesByOwner(rw http.ResponseWriter, r *http.Request) {
364380
})
365381
return
366382
}
367-
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, workspaces)
383+
384+
allowed := make([]database.Workspace, 0)
385+
for i := range workspaces {
386+
w := workspaces[i]
387+
err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead,
388+
rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()).WithID(w.ID.String()))
389+
390+
if err == nil {
391+
allowed = append(allowed, w)
392+
}
393+
}
394+
395+
apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, allowed)
368396
if err != nil {
369397
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
370398
Message: fmt.Sprintf("convert workspaces: %s", err),
@@ -379,6 +407,10 @@ func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
379407
organization := httpmw.OrganizationParam(r)
380408
workspaceName := chi.URLParam(r, "workspace")
381409

410+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceWorkspace.InOrg(organization.ID).WithOwner(owner.ID.String())) {
411+
return
412+
}
413+
382414
workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{
383415
OwnerID: owner.ID,
384416
Name: workspaceName,
@@ -431,11 +463,18 @@ func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
431463

432464
// Create a new workspace for the currently authenticated user.
433465
func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
466+
organization := httpmw.OrganizationParam(r)
467+
apiKey := httpmw.APIKey(r)
468+
434469
var createWorkspace codersdk.CreateWorkspaceRequest
435470
if !httpapi.Read(rw, r, &createWorkspace) {
436471
return
437472
}
438-
apiKey := httpmw.APIKey(r)
473+
474+
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceWorkspace.InOrg(organization.ID).WithOwner(apiKey.UserID.String())) {
475+
return
476+
}
477+
439478
template, err := api.Database.GetTemplateByID(r.Context(), createWorkspace.TemplateID)
440479
if errors.Is(err, sql.ErrNoRows) {
441480
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
@@ -453,7 +492,7 @@ func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
453492
})
454493
return
455494
}
456-
organization := httpmw.OrganizationParam(r)
495+
457496
if organization.ID != template.OrganizationID {
458497
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
459498
Message: fmt.Sprintf("template is not in organization %q", organization.Name),

‎coderd/rbac/object.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,20 @@ var (
5454
}
5555

5656
// ResourceUser is the user in the 'users' table.
57+
// ResourceUser never has any owners or in an org, as it's site wide.
5758
// create/delete = make or delete a new user.
5859
// read = view all user's settings
5960
// update = update all user field & settings
6061
ResourceUser = Object{
6162
Type: "user",
6263
}
6364

65+
// ResourceUserData is any data associated with a user. A user has control
66+
// over their data (profile, password, etc). So this resource has an owner.
67+
ResourceUserData = Object{
68+
Type: "user_data",
69+
}
70+
6471
// ResourceOrganizationMember is a user's membership in an organization.
6572
// Has ONLY an organization owner.
6673
// create/delete = Create/delete member from org.

‎coderd/roles.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,37 @@ import (
1111
)
1212

1313
// assignableSiteRoles returns all site wide roles that can be assigned.
14-
func (*api) assignableSiteRoles(rw http.ResponseWriter, _ *http.Request) {
14+
func (api *api) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) {
1515
// TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the
1616
// role of the user.
1717

18+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserRole) {
19+
return
20+
}
21+
1822
roles := rbac.SiteRoles()
1923
httpapi.Write(rw, http.StatusOK, convertRoles(roles))
2024
}
2125

2226
// assignableSiteRoles returns all site wide roles that can be assigned.
23-
func (*api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
27+
func (api *api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
2428
// TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the
2529
// role of the user.
2630
organization := httpmw.OrganizationParam(r)
31+
32+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserRole.InOrg(organization.ID)) {
33+
return
34+
}
35+
2736
roles := rbac.OrganizationRoles(organization.ID)
2837
httpapi.Write(rw, http.StatusOK, convertRoles(roles))
2938
}
3039

3140
func (api *api) checkPermissions(rw http.ResponseWriter, r *http.Request) {
3241
roles := httpmw.UserRoles(r)
3342
user := httpmw.UserParam(r)
34-
if user.ID != roles.ID {
35-
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
36-
// TODO: @Emyrk in the future we could have an rbac check here.
37-
// If the user can masquerade/impersonate as the user passed in,
38-
// we could allow this or something like that.
39-
Message: "only allowed to check permissions on yourself",
40-
})
43+
44+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
4145
return
4246
}
4347

0 commit comments

Comments
 (0)