Skip to content

Commit 2089006

Browse files
EmyrkEmyrk
authored
feat!: drop reading other 'user' permission (#8650)
* feat: drop reading other 'user' permission Members of the platform can no longer read or list other users. Resources that have "created_by" or "initiated_by" still retain user context, but only include username and avatar url. Attempting to read a user found via those means will result in a 404. * Hide /users page for regular users * make groups a privledged endpoint * Permissions page for template perms * Admin for a given template enables an endpoint for listing users/groups.
1 parent 8649a10 commit 2089006

31 files changed

+579
-119
lines changed

‎coderd/apidoc/docs.go

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

‎coderd/apidoc/swagger.json

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

‎coderd/authorize_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func TestCheckPermissions(t *testing.T) {
103103
Client: orgAdminClient,
104104
UserID: orgAdminUser.ID,
105105
Check: map[string]bool{
106-
readAllUsers: true,
106+
readAllUsers: false,
107107
readMyself: true,
108108
readOwnWorkspaces: true,
109109
readOrgWorkspaces: true,
@@ -115,7 +115,7 @@ func TestCheckPermissions(t *testing.T) {
115115
Client: memberClient,
116116
UserID: memberUser.ID,
117117
Check: map[string]bool{
118-
readAllUsers: true,
118+
readAllUsers: false,
119119
readMyself: true,
120120
readOwnWorkspaces: true,
121121
readOrgWorkspaces: false,

‎coderd/coderdtest/authorize.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (RBACAsserter) convertObjects(t *testing.T, objs ...interface{}) []rbac.Obj
116116
case codersdk.TemplateVersion:
117117
robj = rbac.ResourceTemplate.InOrg(obj.OrganizationID)
118118
case codersdk.User:
119-
robj = rbac.ResourceUser.WithID(obj.ID)
119+
robj = rbac.ResourceUserObject(obj.ID)
120120
case codersdk.Workspace:
121121
robj = rbac.ResourceWorkspace.WithID(obj.ID).InOrg(obj.OrganizationID).WithOwner(obj.OwnerID.String())
122122
default:

‎coderd/database/dbauthz/dbauthz.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -1108,15 +1108,15 @@ func (q *querier) GetProvisionerLogsAfterID(ctx context.Context, arg database.Ge
11081108
}
11091109

11101110
func (q *querier) GetQuotaAllowanceForUser(ctx context.Context, userID uuid.UUID) (int64, error) {
1111-
err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID))
1111+
err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(userID))
11121112
if err != nil {
11131113
return -1, err
11141114
}
11151115
return q.db.GetQuotaAllowanceForUser(ctx, userID)
11161116
}
11171117

11181118
func (q *querier) GetQuotaConsumedForUser(ctx context.Context, userID uuid.UUID) (int64, error) {
1119-
err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID))
1119+
err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(userID))
11201120
if err != nil {
11211121
return -1, err
11221122
}
@@ -1390,7 +1390,7 @@ func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]
13901390
// itself.
13911391
func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) {
13921392
for _, uid := range ids {
1393-
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid)); err != nil {
1393+
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(uid)); err != nil {
13941394
return nil, err
13951395
}
13961396
}
@@ -1899,7 +1899,7 @@ func (q *querier) InsertUserGroupsByName(ctx context.Context, arg database.Inser
18991899

19001900
// TODO: Should this be in system.go?
19011901
func (q *querier) InsertUserLink(ctx context.Context, arg database.InsertUserLinkParams) (database.UserLink, error) {
1902-
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUser.WithID(arg.UserID)); err != nil {
1902+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUserObject(arg.UserID)); err != nil {
19031903
return database.UserLink{}, err
19041904
}
19051905
return q.db.InsertUserLink(ctx, arg)
@@ -2614,24 +2614,24 @@ func (q *querier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTe
26142614
}
26152615

26162616
func (q *querier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateGroup, error) {
2617-
// An actor is authorized to read template group roles if they are authorized to read the template.
2617+
// An actor is authorized to read template group roles if they are authorized to update the template.
26182618
template, err := q.db.GetTemplateByID(ctx, id)
26192619
if err != nil {
26202620
return nil, err
26212621
}
2622-
if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
2622+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
26232623
return nil, err
26242624
}
26252625
return q.db.GetTemplateGroupRoles(ctx, id)
26262626
}
26272627

26282628
func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateUser, error) {
2629-
// An actor is authorized to query template user roles if they are authorized to read the template.
2629+
// An actor is authorized to query template user roles if they are authorized to update the template.
26302630
template, err := q.db.GetTemplateByID(ctx, id)
26312631
if err != nil {
26322632
return nil, err
26332633
}
2634-
if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
2634+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
26352635
return nil, err
26362636
}
26372637
return q.db.GetTemplateUserRoles(ctx, id)

‎coderd/database/dbauthz/dbauthz_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ func (s *MethodTestSuite) TestOrganization() {
521521
ma := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: oa.ID})
522522
mb := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: ob.ID})
523523
check.Args([]uuid.UUID{ma.UserID, mb.UserID}).
524-
Asserts(rbac.ResourceUser.WithID(ma.UserID), rbac.ActionRead, rbac.ResourceUser.WithID(mb.UserID), rbac.ActionRead)
524+
Asserts(rbac.ResourceUserObject(ma.UserID), rbac.ActionRead, rbac.ResourceUserObject(mb.UserID), rbac.ActionRead)
525525
}))
526526
s.Run("GetOrganizationMemberByUserID", s.Subtest(func(db database.Store, check *expects) {
527527
mem := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{})
@@ -698,11 +698,11 @@ func (s *MethodTestSuite) TestTemplate() {
698698
}))
699699
s.Run("GetTemplateGroupRoles", s.Subtest(func(db database.Store, check *expects) {
700700
t1 := dbgen.Template(s.T(), db, database.Template{})
701-
check.Args(t1.ID).Asserts(t1, rbac.ActionRead)
701+
check.Args(t1.ID).Asserts(t1, rbac.ActionUpdate)
702702
}))
703703
s.Run("GetTemplateUserRoles", s.Subtest(func(db database.Store, check *expects) {
704704
t1 := dbgen.Template(s.T(), db, database.Template{})
705-
check.Args(t1.ID).Asserts(t1, rbac.ActionRead)
705+
check.Args(t1.ID).Asserts(t1, rbac.ActionUpdate)
706706
}))
707707
s.Run("GetTemplateVersionByID", s.Subtest(func(db database.Store, check *expects) {
708708
t1 := dbgen.Template(s.T(), db, database.Template{})

‎coderd/database/modelmethods.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -194,14 +194,15 @@ func (w Workspace) LockedRBAC() rbac.Object {
194194
func (m OrganizationMember) RBACObject() rbac.Object {
195195
return rbac.ResourceOrganizationMember.
196196
WithID(m.UserID).
197-
InOrg(m.OrganizationID)
197+
InOrg(m.OrganizationID).
198+
WithOwner(m.UserID.String())
198199
}
199200

200201
func (m GetOrganizationIDsByMemberIDsRow) RBACObject() rbac.Object {
201202
// TODO: This feels incorrect as we are really returning a list of orgmembers.
202203
// This return type should be refactored to return a list of orgmembers, not this
203204
// special type.
204-
return rbac.ResourceUser.WithID(m.UserID)
205+
return rbac.ResourceUserObject(m.UserID)
205206
}
206207

207208
func (o Organization) RBACObject() rbac.Object {
@@ -233,15 +234,15 @@ func (f File) RBACObject() rbac.Object {
233234
// If you are trying to get the RBAC object for the UserData, use
234235
// u.UserDataRBACObject() instead.
235236
func (u User) RBACObject() rbac.Object {
236-
return rbac.ResourceUser.WithID(u.ID)
237+
return rbac.ResourceUserObject(u.ID)
237238
}
238239

239240
func (u User) UserDataRBACObject() rbac.Object {
240241
return rbac.ResourceUserData.WithID(u.ID).WithOwner(u.ID.String())
241242
}
242243

243244
func (u GetUsersRow) RBACObject() rbac.Object {
244-
return rbac.ResourceUser.WithID(u.ID)
245+
return rbac.ResourceUserObject(u.ID)
245246
}
246247

247248
func (u GitSSHKey) RBACObject() rbac.Object {

‎coderd/rbac/object.go

+5
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,11 @@ var (
195195
}
196196
)
197197

198+
// ResourceUserObject is a helper function to create a user object for authz checks.
199+
func ResourceUserObject(userID uuid.UUID) Object {
200+
return ResourceUser.WithID(userID).WithOwner(userID.String())
201+
}
202+
198203
// Object is used to create objects for authz checks when you have none in
199204
// hand to run the check on.
200205
// An example is if you want to list all workspaces, you can create a Object

‎coderd/rbac/regosql/compile_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ neq(input.object.owner, "");
259259
},
260260
VariableConverter: regosql.UserConverter(),
261261
ExpectedSQL: p(
262-
p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = ''") + " AND " + p("'' != ''") + " AND " + p("'' = ''"),
262+
p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = id :: text") + " AND " + p("id :: text != ''") + " AND " + p("'' = ''"),
263263
),
264264
},
265265
}

‎coderd/rbac/regosql/configs.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ func UserConverter() *sqltypes.VariableConverter {
4242
// Users are never owned by an organization, so always return the empty string
4343
// for the org owner.
4444
sqltypes.StringVarMatcher("''", []string{"input", "object", "org_owner"}),
45-
// Users never have an owner, and are only owned site wide.
46-
sqltypes.StringVarMatcher("''", []string{"input", "object", "owner"}),
45+
// Users are always owned by themselves.
46+
sqltypes.StringVarMatcher("id :: text", []string{"input", "object", "owner"}),
4747
)
4848
matcher.RegisterMatcher(
4949
// No ACLs on the user type

‎coderd/rbac/roles.go

+22-14
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,18 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
145145
Name: member,
146146
DisplayName: "",
147147
Site: Permissions(map[string][]Action{
148-
// All users can read all other users and know they exist.
149-
ResourceUser.Type: {ActionRead},
150148
ResourceRoleAssignment.Type: {ActionRead},
151149
// All users can see the provisioner daemons.
152150
ResourceProvisionerDaemon.Type: {ActionRead},
153151
}),
154-
Org: map[string][]Permission{},
155-
User: allPermsExcept(ResourceWorkspaceLocked),
152+
Org: map[string][]Permission{},
153+
User: append(allPermsExcept(ResourceWorkspaceLocked, ResourceUser, ResourceOrganizationMember),
154+
Permissions(map[string][]Action{
155+
// Users cannot do create/update/delete on themselves, but they
156+
// can read their own details.
157+
ResourceUser.Type: {ActionRead},
158+
})...,
159+
),
156160
}.withCachedRegoValue()
157161

158162
auditorRole := Role{
@@ -163,6 +167,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
163167
// are not in.
164168
ResourceTemplate.Type: {ActionRead},
165169
ResourceAuditLog.Type: {ActionRead},
170+
ResourceUser.Type: {ActionRead},
171+
ResourceGroup.Type: {ActionRead},
172+
// Org roles are not really used yet, so grant the perm at the site level.
173+
ResourceOrganizationMember.Type: {ActionRead},
166174
}),
167175
Org: map[string][]Permission{},
168176
User: []Permission{},
@@ -180,6 +188,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
180188
ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
181189
// Needs to read all organizations since
182190
ResourceOrganization.Type: {ActionRead},
191+
ResourceUser.Type: {ActionRead},
192+
ResourceGroup.Type: {ActionRead},
193+
// Org roles are not really used yet, so grant the perm at the site level.
194+
ResourceOrganizationMember.Type: {ActionRead},
183195
}),
184196
Org: map[string][]Permission{},
185197
User: []Permission{},
@@ -249,11 +261,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
249261
Site: []Permission{},
250262
Org: map[string][]Permission{
251263
organizationID: {
252-
{
253-
// All org members can read the other members in their org.
254-
ResourceType: ResourceOrganizationMember.Type,
255-
Action: ActionRead,
256-
},
257264
{
258265
// All org members can read the organization
259266
ResourceType: ResourceOrganization.Type,
@@ -264,13 +271,14 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
264271
ResourceType: ResourceOrgRoleAssignment.Type,
265272
Action: ActionRead,
266273
},
267-
{
268-
ResourceType: ResourceGroup.Type,
269-
Action: ActionRead,
270-
},
271274
},
272275
},
273-
User: []Permission{},
276+
User: []Permission{
277+
{
278+
ResourceType: ResourceOrganizationMember.Type,
279+
Action: ActionRead,
280+
},
281+
},
274282
}
275283
},
276284
}

0 commit comments

Comments
 (0)