Skip to content

Commit 216720c

Browse files
dannykoppingdannykopping
committed
Merge branch 'cj/howto-add-rbac-frobulation' of github.com:coder/coder into cj/howto-add-rbac-frobulation
2 parents d17b7ab + 47d9477 commit 216720c

File tree

7 files changed

+70
-57
lines changed

7 files changed

+70
-57
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,10 +1484,7 @@ func (q *querier) GetFileTemplates(ctx context.Context, fileID uuid.UUID) ([]dat
14841484
}
14851485

14861486
func (q *querier) GetFrobulators(ctx context.Context, arg database.GetFrobulatorsParams) ([]database.Frobulator, error) {
1487-
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceFrobulator.WithOwner(arg.UserID.String()).InOrg(arg.OrgID)); err != nil {
1488-
return nil, err
1489-
}
1490-
return q.db.GetFrobulators(ctx, arg)
1487+
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetFrobulators)(ctx, arg)
14911488
}
14921489

14931490
func (q *querier) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (database.GitSSHKey, error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2791,17 +2791,20 @@ func (s *MethodTestSuite) TestNotifications() {
27912791

27922792
func (s *MethodTestSuite) TestFrobulators() {
27932793
s.Run("GetFrobulators", s.Subtest(func(db database.Store, check *expects) {
2794-
user := dbgen.User(s.T(), db, database.User{})
2794+
// Pre-requisite: create two users and an organization.
2795+
u1 := dbgen.User(s.T(), db, database.User{})
2796+
u2 := dbgen.User(s.T(), db, database.User{})
27952797
org := dbgen.Organization(s.T(), db, database.Organization{})
2798+
// Create a few frobulator resources: two owned by u1, one owned by u2.
2799+
fr1 := dbgen.Frobulator(s.T(), db, database.Frobulator{UserID: u1.ID, OrgID: org.ID})
2800+
fr2 := dbgen.Frobulator(s.T(), db, database.Frobulator{UserID: u1.ID, OrgID: org.ID})
2801+
_ = dbgen.Frobulator(s.T(), db, database.Frobulator{UserID: u2.ID, OrgID: org.ID})
2802+
// Assert that calling GetFrobulators with a given user and org ID records a
2803+
// read action on each of the resources owned by that user.
27962804
check.Args(database.GetFrobulatorsParams{
2797-
UserID: user.ID,
2805+
UserID: u1.ID,
27982806
OrgID: org.ID,
2799-
}).Asserts(
2800-
rbac.ResourceFrobulator.
2801-
WithOwner(user.ID.String()).
2802-
InOrg(org.ID),
2803-
policy.ActionRead,
2804-
)
2807+
}).Asserts(fr1, policy.ActionRead, fr2, policy.ActionRead)
28052808
}))
28062809
s.Run("InsertFrobulator", s.Subtest(func(db database.Store, check *expects) {
28072810
user := dbgen.User(s.T(), db, database.User{})

‎coderd/database/dbgen/dbgen.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -895,9 +895,10 @@ func CustomRole(t testing.TB, db database.Store, seed database.CustomRole) datab
895895

896896
func Frobulator(t testing.TB, db database.Store, orig database.Frobulator) database.Frobulator {
897897
frob, err := db.InsertFrobulator(genCtx, database.InsertFrobulatorParams{
898-
UserID: orig.UserID,
899-
OrgID: orig.OrgID,
900-
ModelNumber: orig.ModelNumber,
898+
ID: takeFirst(orig.ID, uuid.New()),
899+
UserID: takeFirst(orig.UserID, uuid.New()),
900+
OrgID: takeFirst(orig.OrgID, uuid.New()),
901+
ModelNumber: takeFirst(orig.ModelNumber, testutil.GetRandomName(t)),
901902
})
902903
require.NoError(t, err, "insert frobulator")
903904
return frob

‎coderd/database/queries.sql.go

Lines changed: 8 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/frobulators.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ WHERE user_id = $1 AND org_id = $2;
55

66
-- name: InsertFrobulator :one
77
INSERT INTO frobulators (id, user_id, org_id, model_number)
8-
VALUES (gen_random_uuid(), $1, $2, $3)
8+
VALUES ($1, $2, $3, $4)
99
RETURNING *;
1010

1111
-- name: DeleteFrobulator :exec

‎coderd/frobulators.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,15 @@ func (api *API) createFrobulator(rw http.ResponseWriter, r *http.Request) {
7171
}
7272

7373
frob, err := api.Database.InsertFrobulator(ctx, database.InsertFrobulatorParams{
74+
ID: uuid.New(),
7475
UserID: member.UserID,
7576
OrgID: org.ID,
7677
ModelNumber: req.ModelNumber,
7778
})
79+
if httpapi.Is404Error(err) { // Catches forbidden errors as well
80+
httpapi.ResourceNotFound(rw)
81+
return
82+
}
7883
if err != nil {
7984
httpapi.InternalServerError(rw, err)
8085
return
@@ -113,6 +118,10 @@ func (api *API) deleteFrobulator(rw http.ResponseWriter, r *http.Request) {
113118
UserID: member.UserID,
114119
OrgID: org.ID,
115120
})
121+
if httpapi.Is404Error(err) { // Catches forbidden errors as well
122+
httpapi.ResourceNotFound(rw)
123+
return
124+
}
116125
if err != nil {
117126
httpapi.InternalServerError(rw, err)
118127
return

‎coderd/frobulators_test.go

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -119,37 +119,37 @@ func TestFrobulators(t *testing.T) {
119119
t.Parallel()
120120

121121
tests := []struct {
122-
name string
123-
member codersdk.User
124-
memberOrg database.Organization
125-
adminOrg database.Organization
126-
role rbac.RoleIdentifier
127-
expectedFrob uuid.UUID
128-
expectedErr string
122+
name string
123+
member codersdk.User
124+
memberOrg database.Organization
125+
adminOrg database.Organization
126+
role rbac.RoleIdentifier
127+
expectedFrobIDs []uuid.UUID
128+
expectedErr string
129129
}{
130130
{
131-
name: "owner, same org",
132-
member: member1,
133-
memberOrg: org1,
134-
adminOrg: org1,
135-
role: rbac.RoleOwner(),
136-
expectedFrob: frobulatorID,
131+
name: "owner, same org",
132+
member: member1,
133+
memberOrg: org1,
134+
adminOrg: org1,
135+
role: rbac.RoleOwner(),
136+
expectedFrobIDs: []uuid.UUID{frobulatorID},
137137
},
138138
{
139-
name: "owner, different org",
140-
member: member2,
141-
memberOrg: org2,
142-
adminOrg: org1,
143-
role: rbac.RoleOwner(),
144-
expectedFrob: frobulator2ID,
139+
name: "owner, different org",
140+
member: member2,
141+
memberOrg: org2,
142+
adminOrg: org1,
143+
role: rbac.RoleOwner(),
144+
expectedFrobIDs: []uuid.UUID{frobulator2ID},
145145
},
146146
{
147-
name: "org admin, same org",
148-
member: member2,
149-
memberOrg: org2,
150-
adminOrg: org2,
151-
role: rbac.ScopedRoleOrgAdmin(org2.ID),
152-
expectedFrob: frobulator2ID,
147+
name: "org admin, same org",
148+
member: member2,
149+
memberOrg: org2,
150+
adminOrg: org2,
151+
role: rbac.ScopedRoleOrgAdmin(org2.ID),
152+
expectedFrobIDs: []uuid.UUID{frobulator2ID},
153153
},
154154
{
155155
// Org admins do not have permission outside of their own org.
@@ -162,12 +162,13 @@ func TestFrobulators(t *testing.T) {
162162
},
163163
{
164164
// User admins do not have permissions even inside their own org.
165-
name: "user admin, same org",
166-
member: member2,
167-
memberOrg: org2,
168-
adminOrg: org2,
169-
role: rbac.ScopedRoleOrgUserAdmin(org2.ID),
170-
expectedErr: "500: An internal server error occurred",
165+
// They will simply not see any frobulators to which they do not have access.
166+
name: "user admin, same org",
167+
member: member2,
168+
memberOrg: org2,
169+
adminOrg: org2,
170+
role: rbac.ScopedRoleOrgUserAdmin(org2.ID),
171+
expectedFrobIDs: []uuid.UUID{},
171172
},
172173
}
173174

@@ -187,17 +188,13 @@ func TestFrobulators(t *testing.T) {
187188
return
188189
}
189190

190-
// Then: the expected frobulator should be returned
191+
// Otherwise: the expected frobulator(s) should be returned
191192
require.NoError(t, err)
192-
require.Len(t, frobs, 1)
193-
194-
var found bool
193+
actualFrobIDs := make([]uuid.UUID, 0, len(frobs))
195194
for _, f := range frobs {
196-
if f.ID == tc.expectedFrob {
197-
found = true
198-
}
195+
actualFrobIDs = append(actualFrobIDs, f.ID)
199196
}
200-
require.True(t, found, "reference frobulator not found")
197+
require.ElementsMatch(t, tc.expectedFrobIDs, actualFrobIDs, "expected frobulator IDs not found")
201198
})
202199
}
203200
})

0 commit comments

Comments
 (0)