synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-08 11:39:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat!: drop reading other 'user' permission (#8650)

Browse Source 
* 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.
This commit is contained in:
Steven Masley
2023-07-26 10:33:48 -04:00
committed by GitHub
parent 8649a10441
commit 2089006fbc
31 changed files with 585 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
coderd/apidoc/docs.go generated
View File

@ -2109,6 +2109,44 @@ const docTemplate = `{
}
}
},
"/templates/{template}/acl/available": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": [
"application/json"
],
"tags": [
"Enterprise"
],
"summary": "Get template available acl users/groups",
"operationId": "get-template-available-acl-usersgroups",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Template ID",
"name": "template",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.ACLAvailable"
}
}
}
}
}
},
"/templates/{template}/daus": {
"get": {
"security": [
@ -6619,6 +6657,23 @@ const docTemplate = `{
}
}
},
"codersdk.ACLAvailable": {
"type": "object",
"properties": {
"groups": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.Group"
}
},
"users": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.User"
}
}
}
},
"codersdk.APIKey": {
"type": "object",
"required": [

51
coderd/apidoc/swagger.json generated
View File

@ -1841,6 +1841,40 @@
}
}
},
"/templates/{template}/acl/available": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": ["application/json"],
"tags": ["Enterprise"],
"summary": "Get template available acl users/groups",
"operationId": "get-template-available-acl-usersgroups",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Template ID",
"name": "template",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.ACLAvailable"
}
}
}
}
}
},
"/templates/{template}/daus": {
"get": {
"security": [
@ -5879,6 +5913,23 @@
}
}
},
"codersdk.ACLAvailable": {
"type": "object",
"properties": {
"groups": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.Group"
}
},
"users": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.User"
}
}
}
},
"codersdk.APIKey": {
"type": "object",
"required": [

4
coderd/authorize_test.go
View File

@ -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,

2
coderd/coderdtest/authorize.go
View File

@ -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:

16
coderd/database/dbauthz/dbauthz.go
View File

@ -1108,7 +1108,7 @@ 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
}
@ -1116,7 +1116,7 @@ func (q *querier) GetQuotaAllowanceForUser(ctx context.Context, userID uuid.UUID
}
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)

6
coderd/database/dbauthz/dbauthz_test.go
View File

@ -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{})

9
coderd/database/modelmethods.go
View File

@ -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,7 +234,7 @@ 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 {
@ -241,7 +242,7 @@ func (u User) UserDataRBACObject() rbac.Object {
}
func (u GetUsersRow) RBACObject() rbac.Object {
return rbac.ResourceUser.WithID(u.ID)
return rbac.ResourceUserObject(u.ID)
}
func (u GitSSHKey) RBACObject() rbac.Object {

5
coderd/rbac/object.go
View File

@ -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

2
coderd/rbac/regosql/compile_test.go
View File

@ -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("'' = ''"),
),
},
}

4
coderd/rbac/regosql/configs.go
View File

@ -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

36
coderd/rbac/roles.go
View File

@ -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,
},
},
}
},
}

18
coderd/rbac/roles_test.go
View File

@ -106,10 +106,10 @@ func TestRolePermissions(t *testing.T) {
{
Name: "MyUser",
Actions: []rbac.Action{rbac.ActionRead},
Resource: rbac.ResourceUser.WithID(currentUser),
Resource: rbac.ResourceUserObject(currentUser),
AuthorizeMap: map[bool][]authSubject{
true: {owner, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin, userAdmin},
false: {},
true: {orgMemberMe, owner, memberMe, templateAdmin, userAdmin},
false: {otherOrgMember, otherOrgAdmin, orgAdmin},
},
},
{
@ -281,7 +281,7 @@ func TestRolePermissions(t *testing.T) {
{
Name: "ManageOrgMember",
Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID),
Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()),
AuthorizeMap: map[bool][]authSubject{
true: {owner, orgAdmin, userAdmin},
false: {orgMemberMe, memberMe, otherOrgAdmin, otherOrgMember, templateAdmin},
@ -290,10 +290,10 @@ func TestRolePermissions(t *testing.T) {
{
Name: "ReadOrgMember",
Actions: []rbac.Action{rbac.ActionRead},
Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID),
Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()),
AuthorizeMap: map[bool][]authSubject{
true: {owner, orgAdmin, orgMemberMe, userAdmin},
false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin},
true: {owner, orgAdmin, userAdmin, orgMemberMe, templateAdmin},
false: {memberMe, otherOrgAdmin, otherOrgMember},
},
},
{
@ -314,8 +314,8 @@ func TestRolePermissions(t *testing.T) {
Actions: []rbac.Action{rbac.ActionRead},
Resource: rbac.ResourceGroup.WithID(groupID).InOrg(orgID),
AuthorizeMap: map[bool][]authSubject{
true: {owner, orgAdmin, userAdmin, orgMemberMe},
false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin},
true: {owner, orgAdmin, userAdmin, templateAdmin},
false: {memberMe, otherOrgAdmin, orgMemberMe, otherOrgMember},
},
},
{

95
coderd/users.go
View File

@ -183,57 +183,11 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
// @Router /users [get]
func (api *API) users(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
query := r.URL.Query().Get("q")
params, errs := searchquery.Users(query)
if len(errs) > 0 {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid user search query.",
Validations: errs,
})
return
}
paginationParams, ok := parsePagination(rw, r)
users, userCount, ok := api.GetUsers(rw, r)
if !ok {
return
}
userRows, err := api.Database.GetUsers(ctx, database.GetUsersParams{
AfterID: paginationParams.AfterID,
Search: params.Search,
Status: params.Status,
RbacRole: params.RbacRole,
LastSeenBefore: params.LastSeenBefore,
LastSeenAfter: params.LastSeenAfter,
OffsetOpt: int32(paginationParams.Offset),
LimitOpt: int32(paginationParams.Limit),
})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching users.",
Detail: err.Error(),
})
return
}
// GetUsers does not return ErrNoRows because it uses a window function to get the count.
// So we need to check if the userRows is empty and return an empty array if so.
if len(userRows) == 0 {
httpapi.Write(ctx, rw, http.StatusOK, codersdk.GetUsersResponse{
Users: []codersdk.User{},
Count: 0,
})
return
}
users, err := AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, database.ConvertUserRows(userRows))
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching users.",
Detail: err.Error(),
})
return
}
userIDs := make([]uuid.UUID, 0, len(users))
for _, user := range users {
userIDs = append(userIDs, user.ID)
@ -257,10 +211,55 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) {
render.Status(r, http.StatusOK)
render.JSON(rw, r, codersdk.GetUsersResponse{
Users: convertUsers(users, organizationIDsByUserID),
Count: int(userRows[0].Count),
Count: int(userCount),
})
}
func (api *API) GetUsers(rw http.ResponseWriter, r *http.Request) ([]database.User, int64, bool) {
ctx := r.Context()
query := r.URL.Query().Get("q")
params, errs := searchquery.Users(query)
if len(errs) > 0 {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid user search query.",
Validations: errs,
})
return nil, -1, false
}
paginationParams, ok := parsePagination(rw, r)
if !ok {
return nil, -1, false
}
userRows, err := api.Database.GetUsers(ctx, database.GetUsersParams{
AfterID: paginationParams.AfterID,
Search: params.Search,
Status: params.Status,
RbacRole: params.RbacRole,
LastSeenBefore: params.LastSeenBefore,
LastSeenAfter: params.LastSeenAfter,
OffsetOpt: int32(paginationParams.Offset),
LimitOpt: int32(paginationParams.Limit),
})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching users.",
Detail: err.Error(),
})
return nil, -1, false
}
// GetUsers does not return ErrNoRows because it uses a window function to get the count.
// So we need to check if the userRows is empty and return an empty array if so.
if len(userRows) == 0 {
return []database.User{}, 0, true
}
users := database.ConvertUserRows(userRows)
return users, userRows[0].Count, true
}
// Creates a new user.
//
// @Summary Create new user

2
coderd/users_test.go
View File

@ -1431,7 +1431,7 @@ func TestGetUsersPagination(t *testing.T) {
require.Equal(t, res.Count, 2)
// if offset is higher than the count postgres returns an empty array
// and not an ErrNoRows error. This also means the count must be 0.
// and not an ErrNoRows error.
res, err = client.Users(ctx, codersdk.UsersRequest{
Pagination: codersdk.Pagination{
Offset: 3,