Skip to content

Commit 3120c94

Browse files
sreyasreya
authored
feat: add template RBAC/groups (coder#4235)
1 parent 2687e3d commit 3120c94

122 files changed

Lines changed: 8088 additions & 1062 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎coderd/audit.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ func auditSearchQuery(query string) (database.GetAuditLogsOffsetParams, []coders
265265
Username: parser.String(searchParams, "", "username"),
266266
Email: parser.String(searchParams, "", "email"),
267267
}
268+
268269
return filter, parser.Errors
269270
}
270271

@@ -296,6 +297,7 @@ func actionFromString(actionString string) string {
296297
return actionString
297298
case codersdk.AuditActionDelete:
298299
return actionString
300+
default:
299301
}
300302
return ""
301303
}

‎coderd/authorize.go‎

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

7+
"github.com/google/uuid"
8+
79
"golang.org/x/xerrors"
810

911
"cdr.dev/slog"
@@ -18,7 +20,7 @@ import (
1820
// This is faster than calling Authorize() on each object.
1921
func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
2022
roles := httpmw.UserAuthorization(r)
21-
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), action, objects)
23+
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, objects)
2224
if err != nil {
2325
// Log the error as Filter should not be erroring.
2426
h.Logger.Error(r.Context(), "filter failed",
@@ -63,7 +65,7 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec
6365
// }
6466
func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
6567
roles := httpmw.UserAuthorization(r)
66-
err := h.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), action, object.RBACObject())
68+
err := h.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, object.RBACObject())
6769
if err != nil {
6870
// Log the errors for debugging
6971
internalError := new(rbac.UnauthorizedError)
@@ -95,7 +97,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
9597
// Note the authorization is only for the given action and object type.
9698
func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action rbac.Action, objectType string) (rbac.AuthorizeFilter, error) {
9799
roles := httpmw.UserAuthorization(r)
98-
prepared, err := h.Authorizer.PrepareByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), action, objectType)
100+
prepared, err := h.Authorizer.PrepareByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, objectType)
99101
if err != nil {
100102
return nil, xerrors.Errorf("prepare filter: %w", err)
101103
}
@@ -127,6 +129,28 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
127129
)
128130

129131
response := make(codersdk.AuthorizationResponse)
132+
// Prevent using too many resources by ID. This prevents database abuse
133+
// from this endpoint. This also prevents misuse of this endpoint, as
134+
// resource_id should be used for single objects, not for a list of them.
135+
var (
136+
idFetch int
137+
maxFetch = 10
138+
)
139+
for _, v := range params.Checks {
140+
if v.Object.ResourceID != "" {
141+
idFetch++
142+
}
143+
}
144+
if idFetch > maxFetch {
145+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
146+
Message: fmt.Sprintf(
147+
"Endpoint only supports using \"resource_id\" field %d times, found %d usages. Remove %d objects with this field set.",
148+
maxFetch, idFetch, idFetch-maxFetch,
149+
),
150+
})
151+
return
152+
}
153+
130154
for k, v := range params.Checks {
131155
if v.Object.ResourceType == "" {
132156
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
@@ -135,15 +159,60 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
135159
return
136160
}
137161

138-
if v.Object.OwnerID == "me" {
139-
v.Object.OwnerID = auth.ID.String()
162+
obj := rbac.Object{
163+
Owner: v.Object.OwnerID,
164+
OrgID: v.Object.OrganizationID,
165+
Type: v.Object.ResourceType,
140166
}
141-
err := api.Authorizer.ByRoleName(r.Context(), auth.ID.String(), auth.Roles, auth.Scope.ToRBAC(), rbac.Action(v.Action),
142-
rbac.Object{
143-
Owner: v.Object.OwnerID,
144-
OrgID: v.Object.OrganizationID,
145-
Type: v.Object.ResourceType,
146-
})
167+
if obj.Owner == "me" {
168+
obj.Owner = auth.ID.String()
169+
}
170+
171+
// If a resource ID is specified, fetch that specific resource.
172+
if v.Object.ResourceID != "" {
173+
id, err := uuid.Parse(v.Object.ResourceID)
174+
if err != nil {
175+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
176+
Message: fmt.Sprintf("Object %q id is not a valid uuid.", v.Object.ResourceID),
177+
Validations: []codersdk.ValidationError{{Field: "resource_id", Detail: err.Error()}},
178+
})
179+
return
180+
}
181+
182+
var dbObj rbac.Objecter
183+
var dbErr error
184+
// Only support referencing some resources by ID.
185+
switch v.Object.ResourceType {
186+
case rbac.ResourceWorkspaceExecution.Type:
187+
wrkSpace, err := api.Database.GetWorkspaceByID(ctx, id)
188+
if err == nil {
189+
dbObj = wrkSpace.ExecutionRBAC()
190+
}
191+
dbErr = err
192+
case rbac.ResourceWorkspace.Type:
193+
dbObj, dbErr = api.Database.GetWorkspaceByID(ctx, id)
194+
case rbac.ResourceTemplate.Type:
195+
dbObj, dbErr = api.Database.GetTemplateByID(ctx, id)
196+
case rbac.ResourceUser.Type:
197+
dbObj, dbErr = api.Database.GetUserByID(ctx, id)
198+
case rbac.ResourceGroup.Type:
199+
dbObj, dbErr = api.Database.GetGroupByID(ctx, id)
200+
default:
201+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
202+
Message: fmt.Sprintf("Object type %q does not support \"resource_id\" field.", v.Object.ResourceType),
203+
Validations: []codersdk.ValidationError{{Field: "resource_type", Detail: err.Error()}},
204+
})
205+
return
206+
}
207+
if dbErr != nil {
208+
// 404 or unauthorized is false
209+
response[k] = false
210+
continue
211+
}
212+
obj = dbObj.RBACObject()
213+
}
214+
215+
err := api.Authorizer.ByRoleName(r.Context(), auth.ID.String(), auth.Roles, auth.Scope.ToRBAC(), auth.Groups, rbac.Action(v.Action), obj)
147216
response[k] = err == nil
148217
}
149218

‎coderd/authorize_test.go‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ func TestCheckPermissions(t *testing.T) {
1919
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
2020
t.Cleanup(cancel)
2121

22-
adminClient := coderdtest.New(t, nil)
22+
adminClient := coderdtest.New(t, &coderdtest.Options{
23+
IncludeProvisionerDaemon: true,
24+
})
2325
// Create adminClient, member, and org adminClient
2426
adminUser := coderdtest.CreateFirstUser(t, adminClient)
2527
memberClient := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID)
@@ -29,12 +31,17 @@ func TestCheckPermissions(t *testing.T) {
2931
orgAdminUser, err := orgAdminClient.User(ctx, codersdk.Me)
3032
require.NoError(t, err)
3133

34+
version := coderdtest.CreateTemplateVersion(t, adminClient, adminUser.OrganizationID, nil)
35+
coderdtest.AwaitTemplateVersionJob(t, adminClient, version.ID)
36+
template := coderdtest.CreateTemplate(t, adminClient, adminUser.OrganizationID, version.ID)
37+
3238
// With admin, member, and org admin
3339
const (
34-
readAllUsers = "read-all-users"
35-
readOrgWorkspaces = "read-org-workspaces"
36-
readMyself = "read-myself"
37-
readOwnWorkspaces = "read-own-workspaces"
40+
readAllUsers = "read-all-users"
41+
readOrgWorkspaces = "read-org-workspaces"
42+
readMyself = "read-myself"
43+
readOwnWorkspaces = "read-own-workspaces"
44+
updateSpecificTemplate = "update-specific-template"
3845
)
3946
params := map[string]codersdk.AuthorizationCheck{
4047
readAllUsers: {
@@ -64,6 +71,13 @@ func TestCheckPermissions(t *testing.T) {
6471
},
6572
Action: "read",
6673
},
74+
updateSpecificTemplate: {
75+
Object: codersdk.AuthorizationObject{
76+
ResourceType: rbac.ResourceTemplate.Type,
77+
ResourceID: template.ID.String(),
78+
},
79+
Action: "update",
80+
},
6781
}
6882

6983
testCases := []struct {
@@ -77,32 +91,35 @@ func TestCheckPermissions(t *testing.T) {
7791
Client: adminClient,
7892
UserID: adminUser.UserID,
7993
Check: map[string]bool{
80-
readAllUsers: true,
81-
readMyself: true,
82-
readOwnWorkspaces: true,
83-
readOrgWorkspaces: true,
94+
readAllUsers: true,
95+
readMyself: true,
96+
readOwnWorkspaces: true,
97+
readOrgWorkspaces: true,
98+
updateSpecificTemplate: true,
8499
},
85100
},
86101
{
87102
Name: "OrgAdmin",
88103
Client: orgAdminClient,
89104
UserID: orgAdminUser.ID,
90105
Check: map[string]bool{
91-
readAllUsers: false,
92-
readMyself: true,
93-
readOwnWorkspaces: true,
94-
readOrgWorkspaces: true,
106+
readAllUsers: false,
107+
readMyself: true,
108+
readOwnWorkspaces: true,
109+
readOrgWorkspaces: true,
110+
updateSpecificTemplate: true,
95111
},
96112
},
97113
{
98114
Name: "Member",
99115
Client: memberClient,
100116
UserID: memberUser.ID,
101117
Check: map[string]bool{
102-
readAllUsers: false,
103-
readMyself: true,
104-
readOwnWorkspaces: true,
105-
readOrgWorkspaces: false,
118+
readAllUsers: false,
119+
readMyself: true,
120+
readOwnWorkspaces: true,
121+
readOrgWorkspaces: false,
122+
updateSpecificTemplate: false,
106123
},
107124
},
108125
}

‎coderd/coderd.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ func New(options *Options) *API {
283283
r.Get("/{hash}", api.fileByHash)
284284
r.Post("/", api.postFile)
285285
})
286+
286287
r.Route("/provisionerdaemons", func(r chi.Router) {
287288
r.Use(
288289
apiKeyMiddleware,

‎coderd/coderdtest/authorize.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
499499
type authCall struct {
500500
SubjectID string
501501
Roles []string
502+
Groups []string
502503
Scope rbac.Scope
503504
Action rbac.Action
504505
Object rbac.Object
@@ -513,29 +514,31 @@ var _ rbac.Authorizer = (*RecordingAuthorizer)(nil)
513514

514515
// ByRoleNameSQL does not record the call. This matches the postgres behavior
515516
// of not calling Authorize()
516-
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.Scope, _ rbac.Action, _ rbac.Object) error {
517+
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.Scope, _ []string, _ rbac.Action, _ rbac.Object) error {
517518
return r.AlwaysReturn
518519
}
519520

520-
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.Scope, action rbac.Action, object rbac.Object) error {
521+
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.Scope, groups []string, action rbac.Action, object rbac.Object) error {
521522
r.Called = &authCall{
522523
SubjectID: subjectID,
523524
Roles: roleNames,
525+
Groups: groups,
524526
Scope: scope,
525527
Action: action,
526528
Object: object,
527529
}
528530
return r.AlwaysReturn
529531
}
530532

531-
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.Scope, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
533+
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.Scope, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
532534
return &fakePreparedAuthorizer{
533535
Original: r,
534536
SubjectID: subjectID,
535537
Roles: roles,
536538
Scope: scope,
537539
Action: action,
538540
HardCodedSQLString: "true",
541+
Groups: groups,
539542
}, nil
540543
}
541544

@@ -549,12 +552,13 @@ type fakePreparedAuthorizer struct {
549552
Roles []string
550553
Scope rbac.Scope
551554
Action rbac.Action
555+
Groups []string
552556
HardCodedSQLString string
553557
HardCodedRegoString string
554558
}
555559

556560
func (f *fakePreparedAuthorizer) Authorize(ctx context.Context, object rbac.Object) error {
557-
return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Scope, f.Action, object)
561+
return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Scope, f.Groups, f.Action, object)
558562
}
559563

560564
// Compile returns a compiled version of the authorizer that will work for
@@ -564,7 +568,7 @@ func (f *fakePreparedAuthorizer) Compile() (rbac.AuthorizeFilter, error) {
564568
}
565569

566570
func (f *fakePreparedAuthorizer) Eval(object rbac.Object) bool {
567-
return f.Original.ByRoleNameSQL(context.Background(), f.SubjectID, f.Roles, f.Scope, f.Action, object) == nil
571+
return f.Original.ByRoleNameSQL(context.Background(), f.SubjectID, f.Roles, f.Scope, f.Groups, f.Action, object) == nil
568572
}
569573

570574
func (f fakePreparedAuthorizer) RegoString() string {

‎coderd/coderdtest/coderdtest.go‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"crypto/sha256"
1010
"crypto/x509"
1111
"crypto/x509/pkix"
12-
"database/sql"
1312
"encoding/base64"
1413
"encoding/json"
1514
"encoding/pem"
@@ -21,7 +20,6 @@ import (
2120
"net/http"
2221
"net/http/httptest"
2322
"net/url"
24-
"os"
2523
"strconv"
2624
"strings"
2725
"testing"
@@ -49,8 +47,7 @@ import (
4947
"github.com/coder/coder/coderd/autobuild/executor"
5048
"github.com/coder/coder/coderd/awsidentity"
5149
"github.com/coder/coder/coderd/database"
52-
"github.com/coder/coder/coderd/database/databasefake"
53-
"github.com/coder/coder/coderd/database/postgres"
50+
"github.com/coder/coder/coderd/database/dbtestutil"
5451
"github.com/coder/coder/coderd/gitsshkey"
5552
"github.com/coder/coder/coderd/rbac"
5653
"github.com/coder/coder/coderd/telemetry"
@@ -139,26 +136,7 @@ func NewOptions(t *testing.T, options *Options) (*httptest.Server, context.Cance
139136
})
140137
}
141138

142-
// This can be hotswapped for a live database instance.
143-
db := databasefake.New()
144-
pubsub := database.NewPubsubInMemory()
145-
if os.Getenv("DB") != "" {
146-
connectionURL, closePg, err := postgres.Open()
147-
require.NoError(t, err)
148-
t.Cleanup(closePg)
149-
sqlDB, err := sql.Open("postgres", connectionURL)
150-
require.NoError(t, err)
151-
t.Cleanup(func() {
152-
_ = sqlDB.Close()
153-
})
154-
db = database.New(sqlDB)
155-
156-
pubsub, err = database.NewPubsub(context.Background(), sqlDB, connectionURL)
157-
require.NoError(t, err)
158-
t.Cleanup(func() {
159-
_ = pubsub.Close()
160-
})
161-
}
139+
db, pubsub := dbtestutil.NewDB(t)
162140

163141
ctx, cancelFunc := context.WithCancel(context.Background())
164142
lifecycleExecutor := executor.New(
@@ -399,6 +377,7 @@ func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationI
399377
// with the responses provided. It uses the "echo" provisioner for compatibility
400378
// with testing.
401379
func CreateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, res *echo.Responses) codersdk.TemplateVersion {
380+
t.Helper()
402381
data, err := echo.Tar(res)
403382
require.NoError(t, err)
404383
file, err := client.Upload(context.Background(), codersdk.ContentTypeTar, data)

0 commit comments

Comments
 (0)