Cleanup and comments

coder · Emyrk · Nov 28, 2022 · Nov 20, 2022 · Nov 20, 2022 · Nov 20, 2022
commit 203bc7cebc3e9c5de27a042d9519a4b89ceddfd3
diff --git a/coderd/rbac/regosql/aclGroupVar.go b/coderd/rbac/regosql/aclGroupVar.go
@@ -9,9 +9,6 @@ import (
 )
 
 var _ sqltypes.VariableMatcher = ACLGroupVar{}
-
-// ACLGroupVar is also the Node type to reduce the number of types that we need
-// to export.
 var _ sqltypes.Node = ACLGroupVar{}
 
 // ACLGroupVar is a variable matcher that handles group_acl and user_acl.
@@ -23,11 +20,9 @@ var _ sqltypes.Node = ACLGroupVar{}
 //
 // This is a custom variable matcher as json objects have arbitrary complexity.
 type ACLGroupVar struct {
-	StructSQL  string
+	StructSQL string
+	// input.object.group_acl -> ["input", "object", "group_acl"]
 	StructPath []string
-	// DenyAll is helpful for when we don't care about ACL groups.
-	// We need to default to denying access.
-	DenyAll bool
 
 	// FieldReference handles referencing the subfields, which could be
 	// more variables. We pass one in as the global one might not be correctly
@@ -45,15 +40,6 @@ func ACLGroupMatcher(fieldRefernce sqltypes.VariableMatcher, structSQL string, s
 
 func (ACLGroupVar) UseAs() sqltypes.Node { return ACLGroupVar{} }
 
-// Disable is a helper to disable the ACL group matching in the SQL generation.
-// This is because some tables do not have ACL columns, and in this case we
-// do not want to grant access based on columns that do not exist.
-// This replaces any clause with "group_acl" or "user_acl" with "false".
-func (g *ACLGroupVar) Disable() *ACLGroupVar {
-	g.DenyAll = true
-	return g
-}
-
 func (g ACLGroupVar) ConvertVariable(rego ast.Ref) (sqltypes.Node, bool) {
 	// "left" will be a map of group names to actions in rego.
 	//	{
@@ -65,7 +51,6 @@ func (g ACLGroupVar) ConvertVariable(rego ast.Ref) (sqltypes.Node, bool) {
 	}
 
 	aclGrp := ACLGroupVar{
-		DenyAll:        g.DenyAll,
 		StructSQL:      g.StructSQL,
 		StructPath:     g.StructPath,
 		FieldReference: g.FieldReference,
@@ -101,17 +86,10 @@ func (g ACLGroupVar) ConvertVariable(rego ast.Ref) (sqltypes.Node, bool) {
 }
 
 func (g ACLGroupVar) SQLString(cfg *sqltypes.SQLGenerator) string {
-	if g.DenyAll {
-		return "false"
-	}
 	return fmt.Sprintf("%s->%s", g.StructSQL, g.GroupNode.SQLString(cfg))
 }
 
 func (g ACLGroupVar) ContainsSQL(cfg *sqltypes.SQLGenerator, other sqltypes.Node) (string, error) {
-	if g.DenyAll {
-		return "false", nil
-	}
-
 	switch other.UseAs().(type) {
 	// Only supports containing other strings.
 	case sqltypes.AstString:

diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go
@@ -2,17 +2,31 @@ package regosql
 
 import "github.com/coder/coder/coderd/rbac/regosql/sqltypes"
 
-// For the templates table
+func organizationOwnerMatcher() sqltypes.VariableMatcher {
+	return sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"})
+}
+
+func userOwnerMatcher() sqltypes.VariableMatcher {
+	return sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"})
+}
+
+func groupACLMatcher(m sqltypes.VariableMatcher) sqltypes.VariableMatcher {
+	return ACLGroupMatcher(m, "group_acl", []string{"input", "object", "acl_group_list"})
+}
+
+func userACLMatcher(m sqltypes.VariableMatcher) sqltypes.VariableMatcher {
+	return ACLGroupMatcher(m, "user_acl", []string{"input", "object", "acl_user_list"})
+}
+
 func TemplateConverter() *sqltypes.VariableConverter {
 	matcher := sqltypes.NewVariableConverter().RegisterMatcher(
-		// Basic strings
-		sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"}),
+		organizationOwnerMatcher(),
 		// Templates have no user owner, only owner by an organization.
-		sqltypes.AlwaysFalse(sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"})),
+		sqltypes.AlwaysFalse(userOwnerMatcher()),
 	)
 	matcher.RegisterMatcher(
-		ACLGroupMatcher(matcher, "group_acl", []string{"input", "object", "acl_group_list"}),
-		ACLGroupMatcher(matcher, "user_acl", []string{"input", "object", "acl_user_list"}),
+		groupACLMatcher(matcher),
+		userACLMatcher(matcher),
 	)
 	return matcher
 }
@@ -21,27 +35,25 @@ func TemplateConverter() *sqltypes.VariableConverter {
 // group or user ACL columns.
 func NoACLConverter() *sqltypes.VariableConverter {
 	matcher := sqltypes.NewVariableConverter().RegisterMatcher(
-		// Basic strings
-		sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"}),
-		sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"}),
+		organizationOwnerMatcher(),
+		userOwnerMatcher(),
 	)
 	matcher.RegisterMatcher(
-		sqltypes.AlwaysFalse(ACLGroupMatcher(matcher, "group_acl", []string{"input", "object", "acl_group_list"})),
-		sqltypes.AlwaysFalse(ACLGroupMatcher(matcher, "user_acl", []string{"input", "object", "acl_user_list"})),
+		sqltypes.AlwaysFalse(groupACLMatcher(matcher)),
+		sqltypes.AlwaysFalse(userACLMatcher(matcher)),
 	)
 
 	return matcher
 }
 
 func DefaultVariableConverter() *sqltypes.VariableConverter {
 	matcher := sqltypes.NewVariableConverter().RegisterMatcher(
-		// Basic strings
-		sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"}),
-		sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"}),
+		organizationOwnerMatcher(),
+		userOwnerMatcher(),
 	)
 	matcher.RegisterMatcher(
-		ACLGroupMatcher(matcher, "group_acl", []string{"input", "object", "acl_group_list"}),
-		ACLGroupMatcher(matcher, "user_acl", []string{"input", "object", "acl_user_list"}),
+		groupACLMatcher(matcher),
+		userACLMatcher(matcher),
 	)
 
 	return matcher

diff --git a/coderd/rbac/regosql/sqltypes/alwaysFalse.go b/coderd/rbac/regosql/sqltypes/alwaysFalse.go
@@ -13,14 +13,15 @@ type alwaysFalse struct {
 	InnerNode Node
 }
 
-func AlwaysFalse(m VariableMatcher) alwaysFalse {
+// AlwaysFalse overrides the inner node with a constant "false".
+func AlwaysFalse(m VariableMatcher) VariableMatcher {
 	return alwaysFalse{
 		Matcher: m,
 	}
 }
 
 // AlwaysFalseNode is mainly used for unit testing to make a Node immediately.
-func AlwaysFalseNode(n Node) alwaysFalse {
+func AlwaysFalseNode(n Node) Node {
 	return alwaysFalse{
 		InnerNode: n,
 		Matcher:   nil,

diff --git a/coderd/rbac/regosql/sqltypes/array.go b/coderd/rbac/regosql/sqltypes/array.go
@@ -26,9 +26,6 @@ func Array(source RegoSource, nodes ...Node) (Node, error) {
 func (ASTArray) UseAs() Node { return ASTArray{} }
 
 func (a ASTArray) ContainsSQL(cfg *SQLGenerator, needle Node) (string, error) {
-	// TODO: Handle ASTArray.Contains(ASTArray). Must handle types correctly.
-	// 	Should implement as strict subset.
-
 	// If we have no elements in our set, then our needle is never in the set.
 	if len(a.Value) == 0 {
 		return "false", nil

diff --git a/coderd/rbac/regosql/sqltypes/member.go b/coderd/rbac/regosql/sqltypes/member.go
@@ -18,16 +18,11 @@ type SupportsContainedIn interface {
 
 var _ BooleanNode = memberOf{}
 var _ Node = memberOf{}
-
-//var _ SupportsMemberOf = memberOf{}
+var _ SupportsEquality = memberOf{}
 
 type memberOf struct {
 	Needle   Node
 	Haystack Node
-
-	// Not just inverses the result of the comparison. We could implement this
-	// as a Not node wrapping the equality, but this is more efficient.
-	Not bool
 }
 
 func MemberOf(needle, haystack Node) BooleanNode {