Skip to content

Commit 944755e

Browse files
EmyrkEmyrk
committed
Comments
1 parent b252b79 commit 944755e

File tree

6 files changed

+55
-48
lines changed

6 files changed

+55
-48
lines changed

‎coderd/database/bindvars.go

Whitespace-only changes.

‎coderd/database/modelqueries.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,15 +261,19 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
261261
return nil, xerrors.Errorf("insert authorized filter: %w", err)
262262
}
263263

264+
// SQLx expects :arg-named arguments, but we use @arg-named arguments. So
265+
// switch them. Also any ':' in comments breaks this...
264266
filtered = strings.ReplaceAll(filtered, "@", ":")
265267
query, args, err := q.sdb.BindNamed(filtered, arg)
266268
if err != nil {
267269
return nil, xerrors.Errorf("bind named: %w", err)
268270
}
271+
// So SQLx treats '::' as escaping a ":". So we need to unescape it??
269272
query = strings.ReplaceAll(query, ":", "::")
270273

271274
// The name comment is for metric tracking
272-
//query = fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s", query)
275+
// Must add after sqlx or else it breaks 'BindNamed'
276+
query = fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s", query)
273277
var items []WorkspaceWithData
274278
err = q.sdb.SelectContext(ctx, &items, query, args...)
275279
if err != nil {

‎coderd/rbac/authz_internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1012,7 +1012,7 @@ func testAuthorize(t *testing.T, name string, subject Subject, sets ...[]authTes
10121012

10131013
// Ensure the partial can compile to a SQL clause.
10141014
// This does not guarantee that the clause is valid SQL.
1015-
_, err = Compile(ConfigWithACL(), partialAuthz)
1015+
_, err = Compile(ConfigWithACL(""), partialAuthz)
10161016
require.NoError(t, err, "compile prepared authorizer")
10171017

10181018
// Also check the rego policy can form a valid partial query result.

‎coderd/rbac/query.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ type authorizedSQLFilter struct {
2020
auth *PartialAuthorizer
2121
}
2222

23-
func ConfigWithACL() regosql.ConvertConfig {
23+
func ConfigWithACL(prefix string) regosql.ConvertConfig {
2424
return regosql.ConvertConfig{
25-
VariableConverter: regosql.DefaultVariableConverter(),
25+
VariableConverter: regosql.DefaultVariableConverter(prefix),
2626
}
2727
}
2828

29-
func ConfigWithoutACL() regosql.ConvertConfig {
29+
func ConfigWithoutACL(prefix string) regosql.ConvertConfig {
3030
return regosql.ConvertConfig{
31-
VariableConverter: regosql.NoACLConverter(),
31+
VariableConverter: regosql.NoACLConverter(prefix),
3232
}
3333
}
3434

‎coderd/rbac/regosql/compile_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,13 @@ func TestRegoQueries(t *testing.T) {
7979
`"read" in input.object.acl_group_list.allUsers`,
8080
},
8181
ExpectedSQL: "(group_acl->'allUsers' ? 'read')",
82-
VariableConverter: regosql.DefaultVariableConverter(),
82+
VariableConverter: regosql.DefaultVariableConverter(""),
8383
},
8484
{
8585
Name: "GroupWildcard",
8686
Queries: []string{`"*" in input.object.acl_group_list.allUsers`},
8787
ExpectedSQL: "(group_acl->'allUsers' ? '*')",
88-
VariableConverter: regosql.DefaultVariableConverter(),
88+
VariableConverter: regosql.DefaultVariableConverter(""),
8989
},
9090
{
9191
// Always return a constant string for all variables.
@@ -94,27 +94,27 @@ func TestRegoQueries(t *testing.T) {
9494
`"read" in input.object.acl_group_list[input.object.org_owner]`,
9595
},
9696
ExpectedSQL: "(group_acl->organization_id :: text ? 'read')",
97-
VariableConverter: regosql.DefaultVariableConverter(),
97+
VariableConverter: regosql.DefaultVariableConverter(""),
9898
},
9999
{
100100
Name: "VarInArray",
101101
Queries: []string{
102102
`input.object.org_owner in {"a", "b", "c"}`,
103103
},
104104
ExpectedSQL: p("organization_id :: text = ANY(ARRAY ['a','b','c'])"),
105-
VariableConverter: regosql.DefaultVariableConverter(),
105+
VariableConverter: regosql.DefaultVariableConverter(""),
106106
},
107107
{
108108
Name: "SetDereference",
109109
Queries: []string{`"*" in input.object.acl_group_list[input.object.org_owner]`},
110110
ExpectedSQL: p("group_acl->organization_id :: text ? '*'"),
111-
VariableConverter: regosql.DefaultVariableConverter(),
111+
VariableConverter: regosql.DefaultVariableConverter(""),
112112
},
113113
{
114114
Name: "JsonbLiteralDereference",
115115
Queries: []string{`"*" in input.object.acl_group_list["4d30d4a8-b87d-45ac-b0d4-51b2e68e7e75"]`},
116116
ExpectedSQL: p("group_acl->'4d30d4a8-b87d-45ac-b0d4-51b2e68e7e75' ? '*'"),
117-
VariableConverter: regosql.DefaultVariableConverter(),
117+
VariableConverter: regosql.DefaultVariableConverter(""),
118118
},
119119
{
120120
Name: "Complex",
@@ -130,7 +130,7 @@ func TestRegoQueries(t *testing.T) {
130130
`(organization_id :: text != '') OR ` +
131131
`(group_acl->'allUsers' ? 'read') OR ` +
132132
`(user_acl->'me' ? 'read'))`,
133-
VariableConverter: regosql.DefaultVariableConverter(),
133+
VariableConverter: regosql.DefaultVariableConverter(""),
134134
},
135135
{
136136
Name: "NoACLs",
@@ -140,7 +140,7 @@ func TestRegoQueries(t *testing.T) {
140140
},
141141
// Special case where the bool is wrapped
142142
ExpectedSQL: p("(false) OR (false)"),
143-
VariableConverter: regosql.NoACLConverter(),
143+
VariableConverter: regosql.NoACLConverter(""),
144144
},
145145
{
146146
Name: "AllowList",
@@ -151,15 +151,15 @@ func TestRegoQueries(t *testing.T) {
151151
// Special case where the bool is wrapped
152152
ExpectedSQL: p(`(id :: text != '') OR ` +
153153
`(id :: text = ANY(ARRAY ['9046b041-58ed-47a3-9c3a-de302577875a']))`),
154-
VariableConverter: regosql.NoACLConverter(),
154+
VariableConverter: regosql.NoACLConverter(""),
155155
},
156156
{
157157
Name: "TwoExpressions",
158158
Queries: []string{
159159
`true; true`,
160160
},
161161
ExpectedSQL: p("true AND true"),
162-
VariableConverter: regosql.DefaultVariableConverter(),
162+
VariableConverter: regosql.DefaultVariableConverter(""),
163163
},
164164

165165
// Actual vectors from production
@@ -171,7 +171,7 @@ func TestRegoQueries(t *testing.T) {
171171
`"read" in input.object.acl_user_list["d5389ccc-57a4-4b13-8c3f-31747bcdc9f1"]`,
172172
},
173173
ExpectedSQL: "true",
174-
VariableConverter: regosql.NoACLConverter(),
174+
VariableConverter: regosql.NoACLConverter(""),
175175
},
176176
{
177177
Name: "OrgAdmin",
@@ -185,7 +185,7 @@ func TestRegoQueries(t *testing.T) {
185185
"(organization_id :: text = ANY(ARRAY ['05f58202-4bfc-43ce-9ba4-5ff6e0174a71'])) AND " +
186186
"(owner_id :: text != '') AND " +
187187
"('d5389ccc-57a4-4b13-8c3f-31747bcdc9f1' = owner_id :: text))",
188-
VariableConverter: regosql.DefaultVariableConverter(),
188+
VariableConverter: regosql.DefaultVariableConverter(""),
189189
},
190190
{
191191
Name: "UserACLAllow",
@@ -195,7 +195,7 @@ func TestRegoQueries(t *testing.T) {
195195
},
196196
ExpectedSQL: "((user_acl->'d5389ccc-57a4-4b13-8c3f-31747bcdc9f1' ? 'read') OR " +
197197
"(user_acl->'d5389ccc-57a4-4b13-8c3f-31747bcdc9f1' ? '*'))",
198-
VariableConverter: regosql.DefaultVariableConverter(),
198+
VariableConverter: regosql.DefaultVariableConverter(""),
199199
},
200200
{
201201
Name: "NoACLConfig",
@@ -205,7 +205,7 @@ func TestRegoQueries(t *testing.T) {
205205
"read" in input.object.acl_group_list[input.object.org_owner]`,
206206
},
207207
ExpectedSQL: "((organization_id :: text != '') AND (organization_id :: text = ANY(ARRAY ['05f58202-4bfc-43ce-9ba4-5ff6e0174a71'])) AND (false))",
208-
VariableConverter: regosql.NoACLConverter(),
208+
VariableConverter: regosql.NoACLConverter(""),
209209
},
210210
{
211211
Name: "EmptyACLListNoACLs",
@@ -226,7 +226,7 @@ func TestRegoQueries(t *testing.T) {
226226
p("(organization_id :: text != '') AND (false) AND (group_acl->organization_id :: text ? '*')") + " OR " +
227227
p("user_acl->'me' ? 'create'") + " OR " +
228228
p("user_acl->'me' ? '*'")),
229-
VariableConverter: regosql.DefaultVariableConverter(),
229+
VariableConverter: regosql.DefaultVariableConverter(""),
230230
},
231231
{
232232
Name: "TemplateOwner",

‎coderd/rbac/regosql/configs.go

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,65 +2,68 @@ package regosql
22

33
import "github.com/coder/coder/coderd/rbac/regosql/sqltypes"
44

5-
func resourceIDMatcher() sqltypes.VariableMatcher {
6-
return sqltypes.StringVarMatcher("id :: text", []string{"input", "object", "id"})
5+
func resourceIDMatcher(prefix string) sqltypes.VariableMatcher {
6+
return sqltypes.StringVarMatcher(prefix+"id :: text", []string{"input", "object", "id"})
77
}
88

9-
func organizationOwnerMatcher() sqltypes.VariableMatcher {
10-
return sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"})
9+
func organizationOwnerMatcher(prefix string) sqltypes.VariableMatcher {
10+
return sqltypes.StringVarMatcher(prefix+"organization_id :: text", []string{"input", "object", "org_owner"})
1111
}
1212

13-
func userOwnerMatcher() sqltypes.VariableMatcher {
14-
return sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"})
13+
func userOwnerMatcher(prefix string) sqltypes.VariableMatcher {
14+
return sqltypes.StringVarMatcher(prefix+"owner_id :: text", []string{"input", "object", "owner"})
1515
}
1616

17-
func groupACLMatcher(m sqltypes.VariableMatcher) sqltypes.VariableMatcher {
18-
return ACLGroupMatcher(m, "group_acl", []string{"input", "object", "acl_group_list"})
17+
func groupACLMatcher(prefix string, m sqltypes.VariableMatcher) sqltypes.VariableMatcher {
18+
return ACLGroupMatcher(m, prefix+"group_acl", []string{"input", "object", "acl_group_list"})
1919
}
2020

21-
func userACLMatcher(m sqltypes.VariableMatcher) sqltypes.VariableMatcher {
22-
return ACLGroupMatcher(m, "user_acl", []string{"input", "object", "acl_user_list"})
21+
func userACLMatcher(prefix string, m sqltypes.VariableMatcher) sqltypes.VariableMatcher {
22+
return ACLGroupMatcher(m, prefix+"user_acl", []string{"input", "object", "acl_user_list"})
2323
}
2424

2525
func TemplateConverter() *sqltypes.VariableConverter {
26+
prefix := ""
2627
matcher := sqltypes.NewVariableConverter().RegisterMatcher(
27-
resourceIDMatcher(),
28-
organizationOwnerMatcher(),
28+
resourceIDMatcher(prefix),
29+
organizationOwnerMatcher(prefix),
2930
// Templates have no user owner, only owner by an organization.
30-
sqltypes.AlwaysFalse(userOwnerMatcher()),
31+
sqltypes.AlwaysFalse(userOwnerMatcher(prefix)),
3132
)
3233
matcher.RegisterMatcher(
33-
groupACLMatcher(matcher),
34-
userACLMatcher(matcher),
34+
groupACLMatcher(prefix, matcher),
35+
userACLMatcher(prefix, matcher),
3536
)
3637
return matcher
3738
}
3839

3940
// NoACLConverter should be used when the target SQL table does not contain
4041
// group or user ACL columns.
41-
func NoACLConverter() *sqltypes.VariableConverter {
42+
//
43+
// prefix allows namespacing the generated sql columns.
44+
func NoACLConverter(prefix string) *sqltypes.VariableConverter {
4245
matcher := sqltypes.NewVariableConverter().RegisterMatcher(
43-
resourceIDMatcher(),
44-
organizationOwnerMatcher(),
45-
userOwnerMatcher(),
46+
resourceIDMatcher(prefix),
47+
organizationOwnerMatcher(prefix),
48+
userOwnerMatcher(prefix),
4649
)
4750
matcher.RegisterMatcher(
48-
sqltypes.AlwaysFalse(groupACLMatcher(matcher)),
49-
sqltypes.AlwaysFalse(userACLMatcher(matcher)),
51+
sqltypes.AlwaysFalse(groupACLMatcher(prefix, matcher)),
52+
sqltypes.AlwaysFalse(userACLMatcher(prefix, matcher)),
5053
)
5154

5255
return matcher
5356
}
5457

55-
func DefaultVariableConverter() *sqltypes.VariableConverter {
58+
func DefaultVariableConverter(prefix string) *sqltypes.VariableConverter {
5659
matcher := sqltypes.NewVariableConverter().RegisterMatcher(
57-
resourceIDMatcher(),
58-
organizationOwnerMatcher(),
59-
userOwnerMatcher(),
60+
resourceIDMatcher(prefix),
61+
organizationOwnerMatcher(prefix),
62+
userOwnerMatcher(prefix),
6063
)
6164
matcher.RegisterMatcher(
62-
groupACLMatcher(matcher),
63-
userACLMatcher(matcher),
65+
groupACLMatcher(prefix, matcher),
66+
userACLMatcher(prefix, matcher),
6467
)
6568

6669
return matcher

0 commit comments

Comments
 (0)