Skip to content

Commit b9b1c4e

Browse files
EmyrkEmyrk
committed
Handle empty arrays
1 parent de198c5 commit b9b1c4e

File tree

3 files changed

+45
-12
lines changed

3 files changed

+45
-12
lines changed

‎coderd/rbac/regosql/compile_test.go

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import (
1515
// TestRegoQueriesNoVariables handles cases without variables. These should be
1616
// very simple and straight forward.
1717
func TestRegoQueries(t *testing.T) {
18+
p := func(v string) string {
19+
return "(" + v + ")"
20+
}
21+
1822
testCases := []struct {
1923
Name string
2024
Queries []string
@@ -50,7 +54,7 @@ func TestRegoQueries(t *testing.T) {
5054
"(1 != 2) = true",
5155
"5 == 5",
5256
},
53-
ExpectedSQL: "(((1 != 2) = true) OR (5 = 5))",
57+
ExpectedSQL: p("((1 != 2) = true) OR (5 = 5)"),
5458
},
5559
// Variables
5660
{
@@ -59,7 +63,7 @@ func TestRegoQueries(t *testing.T) {
5963
Queries: []string{
6064
`input.x = "hello_world"`,
6165
},
62-
ExpectedSQL: "(only_var = 'hello_world')",
66+
ExpectedSQL: p("only_var = 'hello_world'"),
6367
VariableConverter: sqltypes.NewVariableConverter().RegisterMatcher(
6468
sqltypes.StringVarMatcher("only_var", []string{
6569
"input", "x",
@@ -96,19 +100,19 @@ func TestRegoQueries(t *testing.T) {
96100
Queries: []string{
97101
`input.object.org_owner in {"a", "b", "c"}`,
98102
},
99-
ExpectedSQL: "(organization_id :: text = ANY(ARRAY ['a','b','c']))",
103+
ExpectedSQL: p("organization_id :: text = ANY(ARRAY ['a','b','c'])"),
100104
VariableConverter: regosql.DefaultVariableConverter(),
101105
},
102106
{
103107
Name: "SetDereference",
104108
Queries: []string{`"*" in input.object.acl_group_list[input.object.org_owner]`},
105-
ExpectedSQL: "(group_acl->organization_id :: text ? '*')",
109+
ExpectedSQL: p("group_acl->organization_id :: text ? '*'"),
106110
VariableConverter: regosql.DefaultVariableConverter(),
107111
},
108112
{
109113
Name: "JsonbLiteralDereference",
110114
Queries: []string{`"*" in input.object.acl_group_list["4d30d4a8-b87d-45ac-b0d4-51b2e68e7e75"]`},
111-
ExpectedSQL: "(group_acl->'4d30d4a8-b87d-45ac-b0d4-51b2e68e7e75' ? '*')",
115+
ExpectedSQL: p("group_acl->'4d30d4a8-b87d-45ac-b0d4-51b2e68e7e75' ? '*'"),
112116
VariableConverter: regosql.DefaultVariableConverter(),
113117
},
114118
{
@@ -134,16 +138,15 @@ func TestRegoQueries(t *testing.T) {
134138
`"*" in input.object.acl_group_list["4d30d4a8-b87d-45ac-b0d4-51b2e68e7e75"]`,
135139
},
136140
// Special case where the bool is wrapped
137-
ExpectedSQL: "((false) OR (false))",
141+
ExpectedSQL: p("(false) OR (false)"),
138142
VariableConverter: regosql.NoACLConverter(),
139143
},
140144
{
141145
Name: "TwoExpressions",
142146
Queries: []string{
143147
`true; true`,
144148
},
145-
// Special case where the bool is wrapped
146-
ExpectedSQL: "(true AND true)",
149+
ExpectedSQL: p("true AND true"),
147150
VariableConverter: regosql.DefaultVariableConverter(),
148151
},
149152

@@ -155,7 +158,6 @@ func TestRegoQueries(t *testing.T) {
155158
`"05f58202-4bfc-43ce-9ba4-5ff6e0174a71" = input.object.org_owner`,
156159
`"read" in input.object.acl_user_list["d5389ccc-57a4-4b13-8c3f-31747bcdc9f1"]`,
157160
},
158-
// Special case where the bool is wrapped
159161
ExpectedSQL: "true",
160162
VariableConverter: regosql.NoACLConverter(),
161163
},
@@ -167,7 +169,6 @@ func TestRegoQueries(t *testing.T) {
167169
input.object.owner != "";
168170
"d5389ccc-57a4-4b13-8c3f-31747bcdc9f1" = input.object.owner`,
169171
},
170-
// Special case where the bool is wrapped
171172
ExpectedSQL: "((organization_id :: text != '') AND " +
172173
"(organization_id :: text = ANY(ARRAY ['05f58202-4bfc-43ce-9ba4-5ff6e0174a71'])) AND " +
173174
"(owner_id :: text != '') AND " +
@@ -180,7 +181,6 @@ func TestRegoQueries(t *testing.T) {
180181
`"read" in input.object.acl_user_list["d5389ccc-57a4-4b13-8c3f-31747bcdc9f1"]`,
181182
`"*" in input.object.acl_user_list["d5389ccc-57a4-4b13-8c3f-31747bcdc9f1"]`,
182183
},
183-
// Special case where the bool is wrapped
184184
ExpectedSQL: "((user_acl->'d5389ccc-57a4-4b13-8c3f-31747bcdc9f1' ? 'read') OR " +
185185
"(user_acl->'d5389ccc-57a4-4b13-8c3f-31747bcdc9f1' ? '*'))",
186186
VariableConverter: regosql.DefaultVariableConverter(),
@@ -192,10 +192,30 @@ func TestRegoQueries(t *testing.T) {
192192
input.object.org_owner in {"05f58202-4bfc-43ce-9ba4-5ff6e0174a71"};
193193
"read" in input.object.acl_group_list[input.object.org_owner]`,
194194
},
195-
// Special case where the bool is wrapped
196195
ExpectedSQL: "((organization_id :: text != '') AND (organization_id :: text = ANY(ARRAY ['05f58202-4bfc-43ce-9ba4-5ff6e0174a71'])) AND (false))",
197196
VariableConverter: regosql.NoACLConverter(),
198197
},
198+
{
199+
Name: "EmptyACLList",
200+
Queries: []string{
201+
`input.object.org_owner != "";
202+
input.object.org_owner in set();
203+
"create" in input.object.acl_group_list[input.object.org_owner]`,
204+
205+
`input.object.org_owner != "";
206+
input.object.org_owner in set();
207+
"*" in input.object.acl_group_list[input.object.org_owner]`,
208+
209+
`"create" in input.object.acl_user_list.me`,
210+
211+
`"*" in input.object.acl_user_list.me`,
212+
},
213+
ExpectedSQL: p(p("(organization_id :: text != '') AND (false) AND (group_acl->organization_id :: text ? 'create')") + " OR " +
214+
p("(organization_id :: text != '') AND (false) AND (group_acl->organization_id :: text ? '*')") + " OR " +
215+
p("user_acl->'me' ? 'create'") + " OR " +
216+
p("user_acl->'me' ? '*'")),
217+
VariableConverter: regosql.DefaultVariableConverter(),
218+
},
199219
}
200220

201221
for _, tc := range testCases {

‎coderd/rbac/regosql/sqltypes/array.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ func (a ASTArray) ContainsSQL(cfg *SQLGenerator, needle Node) (string, error) {
2929
// TODO: Handle ASTArray.Contains(ASTArray). Must handle types correctly.
3030
// Should implement as strict subset.
3131

32+
// If we have no elements in our set, then our needle is never in the set.
33+
if len(a.Value) == 0 {
34+
return "false", nil
35+
}
36+
3237
// This condition supports any contains function if the needle type is
3338
// the same as the ASTArray element type.
3439
if reflect.TypeOf(a.MyType().UseAs()) != reflect.TypeOf(needle.UseAs()) {

‎coderd/rbac/regosql/sqltypes/member_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,14 @@ func TestMembership(t *testing.T) {
4848
),
4949
ExpectedSQL: "true = ANY(ARRAY [false,true])",
5050
},
51+
{
52+
Name: "EmptyArray",
53+
Membership: sqltypes.MemberOf(
54+
sqltypes.Bool(true),
55+
must(sqltypes.Array("")),
56+
),
57+
ExpectedSQL: "false",
58+
},
5159

5260
// Errors
5361
{

0 commit comments

Comments
 (0)