Skip to content

Commit cf25746

Browse files
dannykoppingdannykopping
committed
Update usage docs
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent 26ca907 commit cf25746

File tree

1 file changed

+100
-33
lines changed

1 file changed

+100
-33
lines changed

‎coderd/rbac/USAGE.md

Lines changed: 100 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ implementation._
6868

6969
## Creating a new entity
7070

71-
If you're creating a new resource which has to be owned by users of differing
72-
roles, you need to create a new RBAC resource.
71+
If you're creating a new resource which has to be acted upon by users of
72+
differing roles, you need to create a new RBAC resource.
7373

7474
Let's say we're adding a new table called `frobulators` (we'll use this table
7575
later):
@@ -79,10 +79,12 @@ CREATE TABLE frobulators
7979
(
8080
id uuid NOT NULL,
8181
user_id uuid NOT NULL,
82+
org_id uuid NOT NULL,
8283
model_number TEXT NOT NULL,
8384
PRIMARY KEY (id),
8485
UNIQUE (model_number),
85-
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE
86+
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE,
87+
FOREIGN KEY (org_id) REFERENCES organizations (id) ON DELETE CASCADE
8688
);
8789
```
8890

@@ -92,10 +94,10 @@ Let's now add our frobulator noun to `coderd/rbac/policy/policy.go`:
9294
...
9395
"frobulator": {
9496
Actions: map[Action]ActionDefinition{
95-
ActionCreate: actDef("create a frobulator"),
96-
ActionRead: actDef("read a frobulator"),
97-
ActionUpdate: actDef("update a frobulator"),
98-
ActionDelete: actDef("delete a frobulator"),
97+
ActionCreate: {Description: "create a frobulator"},
98+
ActionRead: {Description: "read a frobulator"},
99+
ActionUpdate: {Description: "update a frobulator"},
100+
ActionDelete: {Description: "delete a frobulator"},
99101
},
100102
},
101103
...
@@ -144,20 +146,34 @@ all organization's frobulators; we need to add it to `coderd/rbac/roles.go`:
144146
```go
145147
func ReloadBuiltinRoles(opts *RoleOptions) {
146148
...
149+
auditorRole := Role{
150+
Identifier: RoleAuditor(),
151+
DisplayName: "Auditor",
152+
Site: Permissions(map[string][]policy.Action{
153+
...
154+
// The site-wide auditor is allowed to read *all* frobulators, regardless of who owns them.
155+
ResourceFrobulator.Type: {policy.ActionRead},
156+
...
157+
158+
//
147159
orgAuditor: func(organizationID uuid.UUID) Role {
148160
...
149161
return Role{
150162
...
151163
Org: map[string][]Permission{
152164
organizationID.String(): Permissions(map[string][]policy.Action{
153165
...
166+
// The org-wide auditor is allowed to read *all* frobulators in their own org, regardless of who owns them.
154167
ResourceFrobulator.Type: {policy.ActionRead},
155168
})
156169
...
157170
...
158171
}
159172
```
160173
174+
Note how we added the permission to both the **site-wide** auditor role and the
175+
**org-level** auditor role.
176+
161177
## Testing
162178
163179
The RBAC system is configured to test all possible actions on all available
@@ -222,16 +238,15 @@ func TestRolePermissions(t *testing.T) {
222238
},
223239
},
224240
{
225-
// Users should be able to read their own frobulators
226241
// Admins from the current organization should be able to read any other members' frobulators
227242
// Auditors should be able to read any other members' frobulators
228243
// Owner should be able to read any other user's frobulators
229-
Name: "FrobulatorsReadOnly",
244+
Name: "FrobulatorsReadAnyUserInOrg",
230245
Actions: []policy.Action{policy.ActionRead},
231-
Resource: rbac.ResourceFrobulator.WithOwner(currentUser.String()).InOrg(orgID),
246+
Resource: rbac.ResourceFrobulator.WithOwner(uuid.New().String()).InOrg(orgID), // read frobulators of any user
232247
AuthorizeMap: map[bool][]hasAuthSubjects{
233-
true: {orgMemberMe, orgAdmin, owner, orgAuditor},
234-
false: {setOtherOrg, memberMe, templateAdmin, userAdmin, orgTemplateAdmin, orgUserAdmin},
248+
true: {owner, orgAdmin, orgAuditor},
249+
false: {memberMe, orgMemberMe, setOtherOrg, templateAdmin, userAdmin, orgTemplateAdmin, orgUserAdmin},
235250
},
236251
},
237252
```
@@ -240,11 +255,14 @@ Note how the `FrobulatorsModify` test case is just validating the
240255
`policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete` actions, and
241256
only the **orgMember**, **orgAdmin**, and **owner** can access it.
242257
243-
Similarly, the `FrobulatorsReadOnly` test case is only validating
244-
`policy.ActionRead`, which is allowed on all of the above plus the
245-
**orgAuditor** role.
258+
The `FrobulatorsReadAnyUserInOrg` test case is validating that owners, org
259+
admins & auditors have the `policy.ActionRead` policy which enables them to read
260+
frobulators belonging to any user in a given organization.
261+
262+
The above tests are illustrative not exhaustive, see
263+
[the reference PR](https://github.com/coder/coder/pull/14055) for the rest.
246264
247-
Now the tests pass, because we have covered all the possible scenarios:
265+
Once we have covered all the possible scenarios, the tests will pass:
248266
249267
```bash
250268
$ go test github.com/coder/coder/v2/coderd/rbac -count=1
@@ -280,18 +298,18 @@ Now that we have the RBAC system fully configured, we need to make use of it.
280298
Let's add a SQL query to `coderd/database/queries/frobulators.sql`:
281299

282300
```sql
283-
-- name: GetUserFrobulators :many
301+
-- name: GetFrobulators :many
284302
SELECT *
285303
FROM frobulators
286-
WHERE user_id = @user_id::uuid;
304+
WHERE user_id = $1 AND org_id = $2;
287305
```
288306

289307
Once we run `make gen`, we'll find some stubbed code in
290308
`coderd/database/dbauthz/dbauthz.go`.
291309
292310
```go
293311
...
294-
func (q *querier) GetUserFrobulators(ctx context.Context) ([]database.Frobulator, error) {
312+
func (q *querier) GetFrobulators(ctx context.Context, arg database.GetFrobulatorsParams) ([]database.Frobulator, error) {
295313
panic("not implemented")
296314
}
297315
...
@@ -301,15 +319,33 @@ Let's modify this function:
301319

302320
```go
303321
...
304-
func (q *querier) GetUserFrobulators(ctx context.Context, userID uuid.UUID) ([]database.Frobulator, error) {
305-
return fetch(q.log, q.auth, q.db.GetUserFrobulators)(ctx, id)
322+
func (q *querier) GetFrobulators(ctx context.Context, arg database.GetFrobulatorsParams) ([]database.Frobulator, error) {
323+
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetFrobulators)(ctx, arg)
306324
}
307325
...
308326
```
309327

310-
This states that the `policy.ActionRead` permission is required in this query on
311-
the `ResourceFrobulator` resources, and `WithOwner(userID.String())` specifies
312-
that this user must own the resource.
328+
This states that the `policy.ActionRead` permission is enforced on all entries
329+
returned from the database, ensuring that each requested frobulator is readable
330+
by the given actor.
331+
332+
In order for this to work, we need to implement the `rbac.Objector` interface.
333+
334+
`coderd/database/modelmethods.go` is where we implement this interface for all
335+
RBAC objects:
336+
337+
```go
338+
func (f Frobulator) RBACObject() rbac.Object {
339+
return rbac.ResourceFrobulator.
340+
WithID(f.ID). // Each frobulator has a unique identity.
341+
WithOwner(f.UserID.String()). // It is owned by one and only one user.
342+
InOrg(f.OrgID) // It belongs to an organization.
343+
}
344+
```
345+
346+
These values obviously have to be set on the `Frobulator` instance before this
347+
function can work, hence why we have to fetch the object from the store first
348+
before we validate (this explains the `fetchWithPostFilter` naming).
313349

314350
All queries are executed through `dbauthz`, and now our little frobulators are
315351
protected!
@@ -326,19 +362,50 @@ possible when the requester is unprivileged.
326362
327363
```go
328364
...
329-
func (api *API) listUserFrobulators(rw http.ResponseWriter, r *http.Request) {
365+
func (api *API) createFrobulator(rw http.ResponseWriter, r *http.Request) {
330366
ctx := r.Context()
331-
key := httpmw.APIKey(r)
332-
if !api.Authorize(r, policy.ActionRead, rbac.ResourceFrobulator.WithOwner(key.UserID.String())) {
333-
httpapi.Forbidden(rw)
367+
member := httpmw.OrganizationMemberParam(r)
368+
org := httpmw.OrganizationParam(r)
369+
370+
var req codersdk.InsertFrobulatorRequest
371+
if !httpapi.Read(ctx, rw, r, &req) {
372+
return
373+
}
374+
375+
frob, err := api.Database.InsertFrobulator(ctx, database.InsertFrobulatorParams{
376+
ID: uuid.New(),
377+
UserID: member.UserID,
378+
OrgID: org.ID,
379+
ModelNumber: req.ModelNumber,
380+
})
381+
382+
// This will catch forbidden errors as well.
383+
if httpapi.Is404Error(err) {
384+
httpapi.ResourceNotFound(rw)
334385
return
335386
}
336387
...
388+
```
389+
390+
If we look at the implementation of `httpapi.Is404Error`:
391+
392+
```go
393+
// Is404Error returns true if the given error should return a 404 status code.
394+
// Both actual 404s and unauthorized errors should return 404s to not leak
395+
// information about the existence of resources.
396+
func Is404Error(err error) bool {
397+
if err == nil {
398+
return false
399+
}
400+
401+
// This tests for dbauthz.IsNotAuthorizedError and rbac.IsUnauthorizedError.
402+
if IsUnauthorizedError(err) {
403+
return true
404+
}
405+
return xerrors.Is(err, sql.ErrNoRows)
337406
}
338407
```
339408
340-
`api.Authorize(r, policy.ActionRead, rbac.ResourceFrobulator.WithOwner(key.UserID.String()))`
341-
is specifying that we only want to permit a user to read their own frobulators.
342-
If the requester does not have this permission, we forbid the request. We're
343-
checking the user associated to the API key here because this could also be an
344-
**owner** or **orgAdmin**, and we want to permit those users.
409+
With this, we're able to handle unauthorized access to the resource but return a
410+
`404 Not Found` to not leak the fact that the resources exist but are not
411+
accessible by the given actor.

0 commit comments

Comments
 (0)