Skip to content

Commit

Permalink
chore: Drop resource_id support in rbac system (#3426)
Browse files Browse the repository at this point in the history
  • Loading branch information
Emyrk committed Aug 9, 2022
1 parent ccf6f4e commit db665e7
Show file tree
Hide file tree
Showing 17 changed files with 460 additions and 471 deletions.
53 changes: 25 additions & 28 deletions coderd/coderd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)

// Some quick reused objects
workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String())
workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithOwner(workspace.OwnerID.String())

// skipRoutes allows skipping routes from being checked.
skipRoutes := map[string]string{
Expand Down Expand Up @@ -346,107 +346,107 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
"GET:/api/v2/organizations/{organization}/templates": {
StatusCode: http.StatusOK,
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"POST:/api/v2/organizations/{organization}/templates": {
AssertAction: rbac.ActionCreate,
AssertObject: rbac.ResourceTemplate.InOrg(organization.ID),
},
"DELETE:/api/v2/templates/{template}": {
AssertAction: rbac.ActionDelete,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"GET:/api/v2/templates/{template}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile},
"GET:/api/v2/files/{fileHash}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash),
AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()),
},
"GET:/api/v2/templates/{template}/versions": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"PATCH:/api/v2/templates/{template}/versions": {
AssertAction: rbac.ActionUpdate,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"GET:/api/v2/templates/{template}/versions/{templateversionname}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"PATCH:/api/v2/templateversions/{templateversion}/cancel": {
AssertAction: rbac.ActionUpdate,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}/logs": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}/parameters": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}/resources": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}/schema": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"POST:/api/v2/templateversions/{templateversion}/dry-run": {
// The first check is to read the template
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/resources": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
},
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/logs": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
},
"PATCH:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/cancel": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
},
"GET:/api/v2/provisionerdaemons": {
StatusCode: http.StatusOK,
AssertObject: rbac.ResourceProvisionerDaemon.WithID(provisionerds[0].ID.String()),
AssertObject: rbac.ResourceProvisionerDaemon,
},

"POST:/api/v2/parameters/{scope}/{id}": {
AssertAction: rbac.ActionUpdate,
AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate,
},
"GET:/api/v2/parameters/{scope}/{id}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate,
},
"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {
AssertAction: rbac.ActionUpdate,
AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate,
},
"GET:/api/v2/organizations/{organization}/templates/{templatename}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
},
"POST:/api/v2/organizations/{organization}/workspaces": {
AssertAction: rbac.ActionCreate,
// No ID when creating
AssertObject: workspaceRBACObj.WithID(""),
AssertObject: workspaceRBACObj,
},
"GET:/api/v2/workspaces/{workspace}/watch": {
AssertAction: rbac.ActionRead,
Expand Down Expand Up @@ -546,9 +546,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
if routeAssertions.AssertObject.OrgID != "" {
assert.Equal(t, routeAssertions.AssertObject.OrgID, authorizer.Called.Object.OrgID, "resource org")
}
if routeAssertions.AssertObject.ResourceID != "" {
assert.Equal(t, routeAssertions.AssertObject.ResourceID, authorizer.Called.Object.ResourceID, "resource ID")
}
}
} else {
assert.Nil(t, authorizer.Called, "authorize not expected")
Expand Down
9 changes: 7 additions & 2 deletions coderd/coderdtest/coderdtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,15 @@ func CreateFirstUser(t *testing.T, client *codersdk.Client) codersdk.CreateFirst

// CreateAnotherUser creates and authenticates a new user.
func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, roles ...string) *codersdk.Client {
userClient, _ := createAnotherUserRetry(t, client, organizationID, 5, roles...)
return userClient
}

func CreateAnotherUserWithUser(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, roles ...string) (*codersdk.Client, codersdk.User) {
return createAnotherUserRetry(t, client, organizationID, 5, roles...)
}

func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, retries int, roles ...string) *codersdk.Client {
func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, retries int, roles ...string) (*codersdk.Client, codersdk.User) {
req := codersdk.CreateUserRequest{
Email: namesgenerator.GetRandomName(10) + "@coder.com",
Username: randomUsername(),
Expand Down Expand Up @@ -337,7 +342,7 @@ func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationI
require.NoError(t, err, "update org membership roles")
}
}
return other
return other, user
}

// CreateTemplateVersion creates a template import provisioner job
Expand Down
20 changes: 10 additions & 10 deletions coderd/database/modelmethods.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,37 @@ import (
)

func (t Template) RBACObject() rbac.Object {
return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.ID.String())
return rbac.ResourceTemplate.InOrg(t.OrganizationID)
}

func (t TemplateVersion) RBACObject() rbac.Object {
// Just use the parent template resource for controlling versions
return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.TemplateID.UUID.String())
return rbac.ResourceTemplate.InOrg(t.OrganizationID)
}

func (w Workspace) RBACObject() rbac.Object {
return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithID(w.ID.String()).WithOwner(w.OwnerID.String())
return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String())
}

func (m OrganizationMember) RBACObject() rbac.Object {
return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID).WithID(m.UserID.String())
return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID)
}

func (o Organization) RBACObject() rbac.Object {
return rbac.ResourceOrganization.InOrg(o.ID).WithID(o.ID.String())
return rbac.ResourceOrganization.InOrg(o.ID)
}

func (d ProvisionerDaemon) RBACObject() rbac.Object {
return rbac.ResourceProvisionerDaemon.WithID(d.ID.String())
func (ProvisionerDaemon) RBACObject() rbac.Object {
return rbac.ResourceProvisionerDaemon
}

func (f File) RBACObject() rbac.Object {
return rbac.ResourceFile.WithID(f.Hash).WithOwner(f.CreatedBy.String())
return rbac.ResourceFile.WithOwner(f.CreatedBy.String())
}

// RBACObject returns the RBAC object for the site wide user resource.
// If you are trying to get the RBAC object for the UserData, use
// rbac.ResourceUserData
func (u User) RBACObject() rbac.Object {
return rbac.ResourceUser.WithID(u.ID.String())
func (User) RBACObject() rbac.Object {
return rbac.ResourceUser
}
2 changes: 1 addition & 1 deletion coderd/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
}

if !api.Authorize(r, rbac.ActionRead,
rbac.ResourceFile.WithOwner(file.CreatedBy.String()).WithID(file.Hash)) {
rbac.ResourceFile.WithOwner(file.CreatedBy.String())) {
// Return 404 to not leak the file exists
httpapi.ResourceNotFound(rw)
return
Expand Down
25 changes: 16 additions & 9 deletions coderd/members.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
organization := httpmw.OrganizationParam(r)
member := httpmw.OrganizationMemberParam(r)
apiKey := httpmw.APIKey(r)
actorRoles := httpmw.AuthorizationUserRoles(r)

if apiKey.UserID == member.UserID {
httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
Expand All @@ -37,16 +38,22 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
// The org-member role is always implied.
impliedTypes := append(params.Roles, rbac.RoleOrgMember(organization.ID))
added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes)
for _, roleName := range added {
// Assigning a role requires the create permission.
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
httpapi.Forbidden(rw)
return
}

// Assigning a role requires the create permission.
if len(added) > 0 && !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
httpapi.Forbidden(rw)
return
}
for _, roleName := range removed {
// Removing a role requires the delete permission.
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {

// Removing a role requires the delete permission.
if len(removed) > 0 && !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
httpapi.Forbidden(rw)
return
}

// Just treat adding & removing as "assigning" for now.
for _, roleName := range append(added, removed...) {
if !rbac.CanAssignRole(actorRoles.Roles, roleName) {
httpapi.Forbidden(rw)
return
}
Expand Down
3 changes: 1 addition & 2 deletions coderd/organizations.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ func (api *API) organization(rw http.ResponseWriter, r *http.Request) {
organization := httpmw.OrganizationParam(r)

if !api.Authorize(r, rbac.ActionRead, rbac.ResourceOrganization.
InOrg(organization.ID).
WithID(organization.ID.String())) {
InOrg(organization.ID)) {
httpapi.ResourceNotFound(rw)
return
}
Expand Down

0 comments on commit db665e7

Please sign in to comment.