coder · Emyrk · May 24, 2022 · May 23, 2022 · May 23, 2022 · May 23, 2022
diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -12,9 +12,14 @@ import (
 	"github.com/coder/coder/coderd/rbac"
 )
 
-func (api *api) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.Action, object rbac.Object) bool {
+func AuthorizeFilter[O rbac.Objecter](api *api, r *http.Request, action rbac.Action, objects []O) []O {
 	roles := httpmw.UserRoles(r)
-	err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object)
+	return rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objects)
+}
+
+func (api *api) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.Action, object rbac.Objecter) bool {
+	roles := httpmw.UserRoles(r)
+	err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject())
 	if err != nil {
 		httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
 			Message: err.Error(),

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -186,6 +186,7 @@ func newRouter(options *Options, a *api) chi.Router {
 		r.Route("/templates/{template}", func(r chi.Router) {
 			r.Use(
 				apiKeyMiddleware,
+				authRolesMiddleware,
 				httpmw.ExtractTemplateParam(options.Database),
 			)
 

diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go
@@ -100,8 +100,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 
 		"PUT:/api/v2/organizations/{organization}/members/{user}/roles":     {NoAuthorize: true},
 		"GET:/api/v2/organizations/{organization}/provisionerdaemons":       {NoAuthorize: true},
-		"POST:/api/v2/organizations/{organization}/templates":               {NoAuthorize: true},
-		"GET:/api/v2/organizations/{organization}/templates":                {NoAuthorize: true},
 		"GET:/api/v2/organizations/{organization}/templates/{templatename}": {NoAuthorize: true},
 		"POST:/api/v2/organizations/{organization}/templateversions":        {NoAuthorize: true},
 		"POST:/api/v2/organizations/{organization}/workspaces":              {NoAuthorize: true},
@@ -110,8 +108,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 		"GET:/api/v2/parameters/{scope}/{id}":           {NoAuthorize: true},
 		"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {NoAuthorize: true},
 
-		"DELETE:/api/v2/templates/{template}":                             {NoAuthorize: true},
-		"GET:/api/v2/templates/{template}":                                {NoAuthorize: true},
 		"GET:/api/v2/templates/{template}/versions":                       {NoAuthorize: true},
 		"PATCH:/api/v2/templates/{template}/versions":                     {NoAuthorize: true},
 		"GET:/api/v2/templates/{template}/versions/{templateversionname}": {NoAuthorize: true},
@@ -185,7 +181,23 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 			AssertAction: rbac.ActionRead,
 			AssertObject: workspaceRBACObj,
 		},
-
+		"GET:/api/v2/organizations/{organization}/templates": {
+			StatusCode:   http.StatusOK,
+			AssertAction: rbac.ActionRead,
+			AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
+		},
+		"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()),
+		},
+		"GET:/api/v2/templates/{template}": {
+			AssertAction: rbac.ActionRead,
+			AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
+		},
 		"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)},
@@ -226,6 +238,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 			route = strings.ReplaceAll(route, "{workspacebuild}", workspace.LatestBuild.ID.String())
 			route = strings.ReplaceAll(route, "{workspacename}", workspace.Name)
 			route = strings.ReplaceAll(route, "{workspacebuildname}", workspace.LatestBuild.Name)
+			route = strings.ReplaceAll(route, "{template}", template.ID.String())
 			route = strings.ReplaceAll(route, "{hash}", file.Hash)
 
 			resp, err := client.Request(context.Background(), method, route, nil)

diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go
@@ -0,0 +1,19 @@
+package database
+
+import "github.com/coder/coder/coderd/rbac"
+
+func (t Template) RBACObject() rbac.Object {
+	return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.ID.String())
+}
+
+func (w Workspace) RBACObject() rbac.Object {
+	return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithID(w.ID.String()).WithOwner(w.OwnerID.String())
+}
+
+func (m OrganizationMember) RBACObject() rbac.Object {
+	return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID).WithID(m.UserID.String())
+}
+
+func (o Organization) RBACObject() rbac.Object {
+	return rbac.ResourceOrganization.InOrg(o.ID).WithID(o.ID.String())
+}
diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go
@@ -3,7 +3,6 @@ package rbac
 import (
 	"context"
 	_ "embed"
-
 	"golang.org/x/xerrors"
 
 	"github.com/open-policy-agent/opa/rego"
@@ -13,6 +12,24 @@ type Authorizer interface {
 	ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error
 }
 
+// Filter takes in a list of objects, and will filter the list removing all
+// the elements the subject does not have permission for.
+// Filter does not allocate a new slice, and will use the existing one
+// passed in. This can cause memory leaks if the slice is held for a prolonged
+// period of time.
+func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objects []O) []O {
+	filtered := make([]O, 0)
+
+	for i := range objects {
+		object := objects[i]
+		err := auth.ByRoleName(ctx, subjID, subjRoles, action, object.RBACObject())
+		if err == nil {
+			filtered = append(filtered, object)
+		}
+	}
+	return filtered
+}
+
 // RegoAuthorizer will use a prepared rego query for performing authorize()
 type RegoAuthorizer struct {
 	query rego.PreparedEvalQuery

diff --git a/coderd/rbac/authz_test.go b/coderd/rbac/authz_test.go
@@ -4,13 +4,12 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strconv"
 	"testing"
 
 	"github.com/google/uuid"
-
-	"golang.org/x/xerrors"
-
 	"github.com/stretchr/testify/require"
+	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/coderd/rbac"
 )
@@ -24,6 +23,94 @@ type subject struct {
 	Roles []rbac.Role `json:"roles"`
 }
 
+func TestFilter(t *testing.T) {
+	t.Parallel()
+
+	objectList := make([]rbac.Object, 0)
+	workspaceList := make([]rbac.Object, 0)
+	fileList := make([]rbac.Object, 0)
+	for i := 0; i < 10; i++ {
+		idxStr := strconv.Itoa(i)
+		workspace := rbac.ResourceWorkspace.WithID(idxStr).WithOwner("me")
+		file := rbac.ResourceFile.WithID(idxStr).WithOwner("me")
+
+		workspaceList = append(workspaceList, workspace)
+		fileList = append(fileList, file)
+
+		objectList = append(objectList, workspace)
+		objectList = append(objectList, file)
+	}
+
+	// copyList is to prevent tests from sharing the same slice
+	copyList := func(list []rbac.Object) []rbac.Object {
+		tmp := make([]rbac.Object, len(list))
+		copy(tmp, list)
+		return tmp
+	}
+
+	testCases := []struct {
+		Name     string
+		List     []rbac.Object
+		Expected []rbac.Object
+		Auth     func(o rbac.Object) error
+	}{
+		{
+			Name:     "FilterWorkspaceType",
+			List:     copyList(objectList),
+			Expected: copyList(workspaceList),
+			Auth: func(o rbac.Object) error {
+				if o.Type != rbac.ResourceWorkspace.Type {
+					return xerrors.New("only workspace")
+				}
+				return nil
+			},
+		},
+		{
+			Name:     "FilterFileType",
+			List:     copyList(objectList),
+			Expected: copyList(fileList),
+			Auth: func(o rbac.Object) error {
+				if o.Type != rbac.ResourceFile.Type {
+					return xerrors.New("only file")
+				}
+				return nil
+			},
+		},
+		{
+			Name:     "FilterAll",
+			List:     copyList(objectList),
+			Expected: []rbac.Object{},
+			Auth: func(o rbac.Object) error {
+				return xerrors.New("always fail")
+			},
+		},
+		{
+			Name:     "FilterNone",
+			List:     copyList(objectList),
+			Expected: copyList(objectList),
+			Auth: func(o rbac.Object) error {
+				return nil
+			},
+		},
+	}
+
+	for _, c := range testCases {
+		c := c
+		t.Run(c.Name, func(t *testing.T) {
+			t.Parallel()
+			authorizer := fakeAuthorizer{
+				AuthFunc: func(_ context.Context, _ string, _ []string, _ rbac.Action, object rbac.Object) error {
+					return c.Auth(object)
+				},
+			}
+
+			filtered := rbac.Filter(context.Background(), authorizer, "me", []string{}, rbac.ActionRead, c.List)
+			require.ElementsMatch(t, c.Expected, filtered, "expect same list")
+			require.Equal(t, len(c.Expected), len(filtered), "same length list")
+		})
+	}
+}
+
 // TestAuthorizeDomain test the very basic roles that are commonly used.
 func TestAuthorizeDomain(t *testing.T) {
 	t.Parallel()

diff --git a/coderd/rbac/fake_test.go b/coderd/rbac/fake_test.go
@@ -0,0 +1,15 @@
+package rbac_test
+
+import (
+	"context"
+
+	"github.com/coder/coder/coderd/rbac"
+)
+
+type fakeAuthorizer struct {
+	AuthFunc func(ctx context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error
+}
+
+func (f fakeAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error {
+	return f.AuthFunc(ctx, subjectID, roleNames, action, object)
+}
diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go
@@ -6,6 +6,11 @@ import (
 
 const WildcardSymbol = "*"
 
+// Objecter returns the RBAC object for itself.
+type Objecter interface {
+	RBACObject() Object
+}
+
 // Resources are just typed objects. Making resources this way allows directly
 // passing them into an Authorize function and use the chaining api.
 var (
@@ -99,6 +104,10 @@ type Object struct {
 	// TODO: SharedUsers?
 }
 
+func (z Object) RBACObject() Object {
+	return z
+}
+
 // All returns an object matching all resources of the same type.
 func (z Object) All() Object {
 	return Object{

diff --git a/coderd/templates.go b/coderd/templates.go
@@ -13,6 +13,7 @@ import (
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/httpapi"
 	"github.com/coder/coder/coderd/httpmw"
+	"github.com/coder/coder/coderd/rbac"
 	"github.com/coder/coder/codersdk"
 )
 
@@ -30,6 +31,11 @@ func (api *api) template(rw http.ResponseWriter, r *http.Request) {
 		})
 		return
 	}
+
+	if !api.Authorize(rw, r, rbac.ActionRead, template) {
+		return
+	}
+
 	count := uint32(0)
 	if len(workspaceCounts) > 0 {
 		count = uint32(workspaceCounts[0].Count)
@@ -40,6 +46,9 @@ func (api *api) template(rw http.ResponseWriter, r *http.Request) {
 
 func (api *api) deleteTemplate(rw http.ResponseWriter, r *http.Request) {
 	template := httpmw.TemplateParam(r)
+	if !api.Authorize(rw, r, rbac.ActionDelete, template) {
+		return
+	}
 
 	workspaces, err := api.Database.GetWorkspacesByTemplateID(r.Context(), database.GetWorkspacesByTemplateIDParams{
 		TemplateID: template.ID,
@@ -77,10 +86,14 @@ func (api *api) deleteTemplate(rw http.ResponseWriter, r *http.Request) {
 // Create a new template in an organization.
 func (api *api) postTemplateByOrganization(rw http.ResponseWriter, r *http.Request) {
 	var createTemplate codersdk.CreateTemplateRequest
+	organization := httpmw.OrganizationParam(r)
+	if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceTemplate.InOrg(organization.ID)) {
+		return
+	}
+
 	if !httpapi.Read(rw, r, &createTemplate) {
 		return
 	}
-	organization := httpmw.OrganizationParam(r)
 	_, err := api.Database.GetTemplateByOrganizationAndName(r.Context(), database.GetTemplateByOrganizationAndNameParams{
 		OrganizationID: organization.ID,
 		Name:           createTemplate.Name,
@@ -194,7 +207,12 @@ func (api *api) templatesByOrganization(rw http.ResponseWriter, r *http.Request)
 		})
 		return
 	}
+
+	// Filter templates based on rbac permissions
+	templates = AuthorizeFilter(api, r, rbac.ActionRead, templates)
+
 	templateIDs := make([]uuid.UUID, 0, len(templates))
+
 	for _, template := range templates {
 		templateIDs = append(templateIDs, template.ID)
 	}
@@ -233,6 +251,10 @@ func (api *api) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
 		return
 	}
 
+	if !api.Authorize(rw, r, rbac.ActionRead, template) {
+		return
+	}
+
 	workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(r.Context(), []uuid.UUID{template.ID})
 	if errors.Is(err, sql.ErrNoRows) {
 		err = nil