coder · Emyrk · Jul 26, 2023 · Jul 21, 2023 · Jul 25, 2023 · Jul 25, 2023
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/authorize_test.go b/coderd/authorize_test.go
@@ -103,7 +103,7 @@ func TestCheckPermissions(t *testing.T) {
 			Client: orgAdminClient,
 			UserID: orgAdminUser.ID,
 			Check: map[string]bool{
-				readAllUsers:           true,
+				readAllUsers:           false,
 				readMyself:             true,
 				readOwnWorkspaces:      true,
 				readOrgWorkspaces:      true,
@@ -115,7 +115,7 @@ func TestCheckPermissions(t *testing.T) {
 			Client: memberClient,
 			UserID: memberUser.ID,
 			Check: map[string]bool{
-				readAllUsers:           true,
+				readAllUsers:           false,
 				readMyself:             true,
 				readOwnWorkspaces:      true,
 				readOrgWorkspaces:      false,

diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go
@@ -116,7 +116,7 @@ func (RBACAsserter) convertObjects(t *testing.T, objs ...interface{}) []rbac.Obj
 		case codersdk.TemplateVersion:
 			robj = rbac.ResourceTemplate.InOrg(obj.OrganizationID)
 		case codersdk.User:
-			robj = rbac.ResourceUser.WithID(obj.ID)
+			robj = rbac.ResourceUserObject(obj.ID)
 		case codersdk.Workspace:
 			robj = rbac.ResourceWorkspace.WithID(obj.ID).InOrg(obj.OrganizationID).WithOwner(obj.OwnerID.String())
 		default:

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -1108,15 +1108,15 @@ func (q *querier) GetProvisionerLogsAfterID(ctx context.Context, arg database.Ge
 }
 
 func (q *querier) GetQuotaAllowanceForUser(ctx context.Context, userID uuid.UUID) (int64, error) {
-	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID))
+	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(userID))
 	if err != nil {
 		return -1, err
 	}
 	return q.db.GetQuotaAllowanceForUser(ctx, userID)
 }
 
 func (q *querier) GetQuotaConsumedForUser(ctx context.Context, userID uuid.UUID) (int64, error) {
-	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID))
+	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(userID))
 	if err != nil {
 		return -1, err
 	}
@@ -1390,7 +1390,7 @@ func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]
 // itself.
 func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) {
 	for _, uid := range ids {
-		if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid)); err != nil {
+		if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(uid)); err != nil {
 			return nil, err
 		}
 	}
@@ -1899,7 +1899,7 @@ func (q *querier) InsertUserGroupsByName(ctx context.Context, arg database.Inser
 
 // TODO: Should this be in system.go?
 func (q *querier) InsertUserLink(ctx context.Context, arg database.InsertUserLinkParams) (database.UserLink, error) {
-	if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUser.WithID(arg.UserID)); err != nil {
+	if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUserObject(arg.UserID)); err != nil {
 		return database.UserLink{}, err
 	}
 	return q.db.InsertUserLink(ctx, arg)
@@ -2614,24 +2614,24 @@ func (q *querier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTe
 }
 
 func (q *querier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateGroup, error) {
-	// An actor is authorized to read template group roles if they are authorized to read the template.
+	// An actor is authorized to read template group roles if they are authorized to update the template.
 	template, err := q.db.GetTemplateByID(ctx, id)
 	if err != nil {
 		return nil, err
 	}
-	if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
+	if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
 		return nil, err
 	}
 	return q.db.GetTemplateGroupRoles(ctx, id)
 }
 
 func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateUser, error) {
-	// An actor is authorized to query template user roles if they are authorized to read the template.
+	// An actor is authorized to query template user roles if they are authorized to update the template.
 	template, err := q.db.GetTemplateByID(ctx, id)
 	if err != nil {
 		return nil, err
 	}
-	if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
+	if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
 		return nil, err
 	}
 	return q.db.GetTemplateUserRoles(ctx, id)

diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -521,7 +521,7 @@ func (s *MethodTestSuite) TestOrganization() {
 		ma := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: oa.ID})
 		mb := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: ob.ID})
 		check.Args([]uuid.UUID{ma.UserID, mb.UserID}).
-			Asserts(rbac.ResourceUser.WithID(ma.UserID), rbac.ActionRead, rbac.ResourceUser.WithID(mb.UserID), rbac.ActionRead)
+			Asserts(rbac.ResourceUserObject(ma.UserID), rbac.ActionRead, rbac.ResourceUserObject(mb.UserID), rbac.ActionRead)
 	}))
 	s.Run("GetOrganizationMemberByUserID", s.Subtest(func(db database.Store, check *expects) {
 		mem := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{})
@@ -698,11 +698,11 @@ func (s *MethodTestSuite) TestTemplate() {
 	}))
 	s.Run("GetTemplateGroupRoles", s.Subtest(func(db database.Store, check *expects) {
 		t1 := dbgen.Template(s.T(), db, database.Template{})
-		check.Args(t1.ID).Asserts(t1, rbac.ActionRead)
+		check.Args(t1.ID).Asserts(t1, rbac.ActionUpdate)
 	}))
 	s.Run("GetTemplateUserRoles", s.Subtest(func(db database.Store, check *expects) {
 		t1 := dbgen.Template(s.T(), db, database.Template{})
-		check.Args(t1.ID).Asserts(t1, rbac.ActionRead)
+		check.Args(t1.ID).Asserts(t1, rbac.ActionUpdate)
 	}))
 	s.Run("GetTemplateVersionByID", s.Subtest(func(db database.Store, check *expects) {
 		t1 := dbgen.Template(s.T(), db, database.Template{})

diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go
@@ -194,14 +194,15 @@ func (w Workspace) LockedRBAC() rbac.Object {
 func (m OrganizationMember) RBACObject() rbac.Object {
 	return rbac.ResourceOrganizationMember.
 		WithID(m.UserID).
-		InOrg(m.OrganizationID)
+		InOrg(m.OrganizationID).
+		WithOwner(m.UserID.String())
 }
 
 func (m GetOrganizationIDsByMemberIDsRow) RBACObject() rbac.Object {
 	// TODO: This feels incorrect as we are really returning a list of orgmembers.
 	// This return type should be refactored to return a list of orgmembers, not this
 	// special type.
-	return rbac.ResourceUser.WithID(m.UserID)
+	return rbac.ResourceUserObject(m.UserID)
 }
 
 func (o Organization) RBACObject() rbac.Object {
@@ -233,15 +234,15 @@ func (f File) RBACObject() rbac.Object {
 // If you are trying to get the RBAC object for the UserData, use
 // u.UserDataRBACObject() instead.
 func (u User) RBACObject() rbac.Object {
-	return rbac.ResourceUser.WithID(u.ID)
+	return rbac.ResourceUserObject(u.ID)
 }
 
 func (u User) UserDataRBACObject() rbac.Object {
 	return rbac.ResourceUserData.WithID(u.ID).WithOwner(u.ID.String())
 }
 
 func (u GetUsersRow) RBACObject() rbac.Object {
-	return rbac.ResourceUser.WithID(u.ID)
+	return rbac.ResourceUserObject(u.ID)
 }
 
 func (u GitSSHKey) RBACObject() rbac.Object {

diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go
@@ -195,6 +195,11 @@ var (
 	}
 )
 
+// ResourceUserObject is a helper function to create a user object for authz checks.
+func ResourceUserObject(userID uuid.UUID) Object {
+	return ResourceUser.WithID(userID).WithOwner(userID.String())
+}
+
 // Object is used to create objects for authz checks when you have none in
 // hand to run the check on.
 // An example is if you want to list all workspaces, you can create a Object

diff --git a/coderd/rbac/regosql/compile_test.go b/coderd/rbac/regosql/compile_test.go
@@ -259,7 +259,7 @@ neq(input.object.owner, "");
 			},
 			VariableConverter: regosql.UserConverter(),
 			ExpectedSQL: p(
-				p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = ''") + " AND " + p("'' != ''") + " AND " + p("'' = ''"),
+				p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = id :: text") + " AND " + p("id :: text != ''") + " AND " + p("'' = ''"),
 			),
 		},
 	}

diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go
@@ -42,8 +42,8 @@ func UserConverter() *sqltypes.VariableConverter {
 		// Users are never owned by an organization, so always return the empty string
 		// for the org owner.
 		sqltypes.StringVarMatcher("''", []string{"input", "object", "org_owner"}),
-		// Users never have an owner, and are only owned site wide.
-		sqltypes.StringVarMatcher("''", []string{"input", "object", "owner"}),
+		// Users are always owned by themselves.
+		sqltypes.StringVarMatcher("id :: text", []string{"input", "object", "owner"}),
 	)
 	matcher.RegisterMatcher(
 		// No ACLs on the user type

diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go
@@ -145,14 +145,18 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 		Name:        member,
 		DisplayName: "",
 		Site: Permissions(map[string][]Action{
-			// All users can read all other users and know they exist.
-			ResourceUser.Type:           {ActionRead},
 			ResourceRoleAssignment.Type: {ActionRead},
 			// All users can see the provisioner daemons.
 			ResourceProvisionerDaemon.Type: {ActionRead},
 		}),
-		Org:  map[string][]Permission{},
-		User: allPermsExcept(ResourceWorkspaceLocked),
+		Org: map[string][]Permission{},
+		User: append(allPermsExcept(ResourceWorkspaceLocked, ResourceUser, ResourceOrganizationMember),
+			Permissions(map[string][]Action{
+				// Users cannot do create/update/delete on themselves, but they
+				// can read their own details.
+				ResourceUser.Type: {ActionRead},
+			})...,
+		),
 	}.withCachedRegoValue()
 
 	auditorRole := Role{
@@ -163,6 +167,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 			// are not in.
 			ResourceTemplate.Type: {ActionRead},
 			ResourceAuditLog.Type: {ActionRead},
+			ResourceUser.Type:     {ActionRead},
+			ResourceGroup.Type:    {ActionRead},
+			// Org roles are not really used yet, so grant the perm at the site level.
+			ResourceOrganizationMember.Type: {ActionRead},
 		}),
 		Org:  map[string][]Permission{},
 		User: []Permission{},
@@ -180,6 +188,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 			ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
 			// Needs to read all organizations since
 			ResourceOrganization.Type: {ActionRead},
+			ResourceUser.Type:         {ActionRead},
+			ResourceGroup.Type:        {ActionRead},
+			// Org roles are not really used yet, so grant the perm at the site level.
+			ResourceOrganizationMember.Type: {ActionRead},
 		}),
 		Org:  map[string][]Permission{},
 		User: []Permission{},
@@ -249,11 +261,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 				Site:        []Permission{},
 				Org: map[string][]Permission{
 					organizationID: {
-						{
-							// All org members can read the other members in their org.
-							ResourceType: ResourceOrganizationMember.Type,
-							Action:       ActionRead,
-						},
 						{
 							// All org members can read the organization
 							ResourceType: ResourceOrganization.Type,
@@ -264,13 +271,14 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
 							ResourceType: ResourceOrgRoleAssignment.Type,
 							Action:       ActionRead,
 						},
-						{
-							ResourceType: ResourceGroup.Type,
-							Action:       ActionRead,
-						},
 					},
 				},
-				User: []Permission{},
+				User: []Permission{
+					{
+						ResourceType: ResourceOrganizationMember.Type,
+						Action:       ActionRead,
+					},
+				},
 			}
 		},
 	}