authzquery: fixes to templates and parameters

johnstcn · johnstcn · commit 69a6346f20cf · 2023-02-01T12:09:28.000Z
- add doc comment to authorizedQueryWithRelated
 - handle sql.ErrNoRows in parameterRBACResource()
 - fix incorrect logic in GetTemplateVersionByOrganizationAndName
diff --git a/coderd/authzquery/authz.go b/coderd/authzquery/authz.go
@@ -219,6 +219,12 @@ func authorizedFetchSet[ArgumentType any, ObjectType rbac.Objecter,
 	}
 }
 
+// authorizedQueryWithRelated performs the same function as authorizedQuery, except that
+// RBAC checks are performed on the result of relatedFunc() instead of the result of fetch().
+// This is useful for cases where ObjectType does not implement RBACObjecter.
+// For example, a TemplateVersion object does not implement RBACObjecter, but it is
+// related to a Template object, which does. Thus, any operations on a TemplateVersion
+// are predicated on the RBAC permissions of the related Template object.
 func authorizedQueryWithRelated[ObjectType any, ArgumentType any, Related rbac.Objecter](
 	// Arguments
 	authorizer rbac.Authorizer,
diff --git a/coderd/authzquery/parameters.go b/coderd/authzquery/parameters.go
@@ -2,6 +2,8 @@ package authzquery
 
 import (
 	"context"
+	"database/sql"
+	"errors"
 
 	"github.com/google/uuid"
 	"golang.org/x/xerrors"
@@ -20,6 +22,11 @@ func (q *AuthzQuerier) parameterRBACResource(ctx context.Context, scope database
 		var version database.TemplateVersion
 		version, err = q.database.GetTemplateVersionByJobID(ctx, scopeID)
 		if err != nil {
+			if errors.Is(err, sql.ErrNoRows) {
+				// Template version does not exist yet, fall back to rbac.ResourceTemplate
+				resource = rbac.ResourceTemplate
+				err = nil
+			}
 			break
 		}
 		var template database.Template
diff --git a/coderd/authzquery/template.go b/coderd/authzquery/template.go
@@ -96,13 +96,11 @@ func (q *AuthzQuerier) GetTemplateVersionByOrganizationAndName(ctx context.Conte
 	// An actor can read the template version if they can read the related template in the organization.
 	fetchRelated := func(tv database.TemplateVersion, p database.GetTemplateVersionByOrganizationAndNameParams) (rbac.Objecter, error) {
 		if !tv.TemplateID.Valid {
-			// If no linked template exists, check if the actor can read a template in the organization.
+			// If no linked template exists, check if the actor can read
+			// any template in the organization.
 			return rbac.ResourceTemplate.InOrg(p.OrganizationID), nil
 		}
-		return q.database.GetTemplateByOrganizationAndName(ctx, database.GetTemplateByOrganizationAndNameParams{
-			OrganizationID: arg.OrganizationID,
-			Name:           tv.Name,
-		})
+		return q.database.GetTemplateByID(ctx, tv.TemplateID.UUID)
 	}
 
 	return authorizedQueryWithRelated(

Original file line number	Diff line number	Diff line change
`@@ -219,6 +219,12 @@ func authorizedFetchSet[ArgumentType any, ObjectType rbac.Objecter,`
`219`	`219`	`}`
`220`	`220`	`}`
`221`	`221`
	`222`	`+// authorizedQueryWithRelated performs the same function as authorizedQuery, except that`
	`223`	`+// RBAC checks are performed on the result of relatedFunc() instead of the result of fetch().`
	`224`	`+// This is useful for cases where ObjectType does not implement RBACObjecter.`
	`225`	`+// For example, a TemplateVersion object does not implement RBACObjecter, but it is`
	`226`	`+// related to a Template object, which does. Thus, any operations on a TemplateVersion`
	`227`	`+// are predicated on the RBAC permissions of the related Template object.`
`222`	`228`	`func authorizedQueryWithRelated[ObjectType any, ArgumentType any, Related rbac.Objecter](`
`223`	`229`	`// Arguments`
`224`	`230`	`authorizer rbac.Authorizer,`