feat: implement proper sql filter for users query

Emyrk · Emyrk · commit a9b7704a1949 · 2023-07-12T11:55:41.000-04:00
diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql
@@ -209,6 +209,8 @@ WHERE
 	END
 	-- End of filters
 
+	-- Authorize Filter clause will be injected below in GetAuthorizedUserCount
+	-- @authorize_filter
 ORDER BY
 	-- Deterministic and consistent ordering of all users. This is to ensure consistent pagination.
 	LOWER(username) ASC OFFSET @offset_opt
diff --git a/coderd/rbac/input.json b/coderd/rbac/input.json
@@ -1,33 +1,33 @@
 {
-  "action": "never-match-action",
+  "action": "read",
   "object": {
     "id": "9046b041-58ed-47a3-9c3a-de302577875a",
-    "owner": "00000000-0000-0000-0000-000000000000",
-    "org_owner": "bf7b72bd-a2b1-4ef2-962c-1d698e0483f6",
-    "type": "workspace",
+    "owner": "9046b041-58ed-47a3-9c3a-de302577875a",
+    "org_owner": "00000000-0000-0000-0000-000000000000",
+    "type": "user",
     "acl_user_list": {
-      "f041847d-711b-40da-a89a-ede39f70dc7f": ["create"]
     },
     "acl_group_list": {}
   },
   "subject": {
     "id": "10d03e62-7703-4df5-a358-4f76577d4e2f",
     "roles": [
       {
-        "name": "owner",
-        "display_name": "Owner",
+        "name": "member",
+        "display_name": "Member",
         "site": [
+        ],
+        "org": {},
+        "user": [
           {
             "negate": false,
-            "resource_type": "*",
-            "action": "*"
+            "resource_type": "user",
+            "action": "read"
           }
-        ],
-        "org": {},
-        "user": []
+        ]
       }
     ],
-    "groups": ["b617a647-b5d0-4cbe-9e40-26f89710bf18"],
+    "groups": [],
     "scope": {
       "name": "Scope_all",
       "display_name": "All operations",
diff --git a/coderd/rbac/regosql/compile_test.go b/coderd/rbac/regosql/compile_test.go
@@ -242,6 +242,26 @@ neq(input.object.owner, "");
 				p("false")),
 			VariableConverter: regosql.TemplateConverter(),
 		},
+		{
+			Name: "UserNoOrgOwner",
+			Queries: []string{
+				`input.object.org_owner != ""`,
+			},
+			ExpectedSQL:       p("'' != ''"),
+			VariableConverter: regosql.UserConverter(),
+		},
+		{
+			Name: "UserOwnsSelf",
+			Queries: []string{
+				`"10d03e62-7703-4df5-a358-4f76577d4e2f" = input.object.owner;
+				input.object.owner != "";
+				input.object.org_owner = ""`,
+			},
+			VariableConverter: regosql.UserConverter(),
+			ExpectedSQL: p(
+				p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = id :: text") + " AND " + p("id :: text != ''") + " AND " + p("'' = ''"),
+			),
+		},
 	}
 
 	for _, tc := range testCases {
diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go
@@ -25,8 +25,9 @@ func userACLMatcher(m sqltypes.VariableMatcher) sqltypes.VariableMatcher {
 func UserConverter() *sqltypes.VariableConverter {
 	matcher := sqltypes.NewVariableConverter().RegisterMatcher(
 		resourceIDMatcher(),
-		// Users are never owned by an organization.
-		sqltypes.AlwaysFalse(organizationOwnerMatcher()),
+		// 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 are always owned by themselves.
 		sqltypes.StringVarMatcher("id :: text", []string{"input", "object", "owner"}),
 	)
diff --git a/coderd/rbac/regosql/sqltypes/always_false.go b/coderd/rbac/regosql/sqltypes/always_false.go
@@ -1,64 +1,77 @@
 package sqltypes
 
 import (
+	"strconv"
+
 	"github.com/open-policy-agent/opa/ast"
 )
 
 var (
-	_ Node            = alwaysFalse{}
-	_ VariableMatcher = alwaysFalse{}
+	_ Node            = constBoolean{}
+	_ VariableMatcher = constBoolean{}
 )
 
-type alwaysFalse struct {
-	Matcher VariableMatcher
+type constBoolean struct {
+	Matcher  VariableMatcher
+	constant bool
 
 	InnerNode Node
 }
 
 // AlwaysFalse overrides the inner node with a constant "false".
 func AlwaysFalse(m VariableMatcher) VariableMatcher {
-	return alwaysFalse{
-		Matcher: m,
+	return constBoolean{
+		Matcher:  m,
+		constant: false,
+	}
+}
+
+func AlwaysTrue(m VariableMatcher) VariableMatcher {
+	return constBoolean{
+		Matcher:  m,
+		constant: true,
 	}
 }
 
 // AlwaysFalseNode is mainly used for unit testing to make a Node immediately.
 func AlwaysFalseNode(n Node) Node {
-	return alwaysFalse{
+	return constBoolean{
 		InnerNode: n,
 		Matcher:   nil,
+		constant:  false,
 	}
 }
 
 // UseAs uses a type no one supports to always override with false.
-func (alwaysFalse) UseAs() Node { return alwaysFalse{} }
+func (constBoolean) UseAs() Node { return constBoolean{} }
 
-func (f alwaysFalse) ConvertVariable(rego ast.Ref) (Node, bool) {
+func (f constBoolean) ConvertVariable(rego ast.Ref) (Node, bool) {
 	if f.Matcher != nil {
 		n, ok := f.Matcher.ConvertVariable(rego)
 		if ok {
-			return alwaysFalse{
+			return constBoolean{
 				Matcher:   f.Matcher,
 				InnerNode: n,
+				constant:  f.constant,
 			}, true
 		}
 	}
 
 	return nil, false
 }
 
-func (alwaysFalse) SQLString(_ *SQLGenerator) string {
-	return "false"
+func (c constBoolean) SQLString(_ *SQLGenerator) string {
+	return strconv.FormatBool(c.constant)
 }
 
-func (alwaysFalse) ContainsSQL(_ *SQLGenerator, _ Node) (string, error) {
-	return "false", nil
+func (c constBoolean) ContainsSQL(_ *SQLGenerator, _ Node) (string, error) {
+	return strconv.FormatBool(c.constant), nil
 }
 
-func (alwaysFalse) ContainedInSQL(_ *SQLGenerator, _ Node) (string, error) {
-	return "false", nil
+func (c constBoolean) ContainedInSQL(_ *SQLGenerator, _ Node) (string, error) {
+	return strconv.FormatBool(c.constant), nil
 }
 
-func (alwaysFalse) EqualsSQLString(_ *SQLGenerator, _ bool, _ Node) (string, error) {
-	return "false", nil
+func (c constBoolean) EqualsSQLString(_ *SQLGenerator, _ bool, _ Node) (string, error) {
+	return strconv.FormatBool(c.constant), nil
 }
