Skip to content

Commit 5de395a

Browse files
EmyrkEmyrk
committed
Make filter ignore owner, not org
1 parent 618904e commit 5de395a

File tree

16 files changed

+274
-56
lines changed

16 files changed

+274
-56
lines changed

‎coderd/rbac/input.json

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
{
2+
"action":"read",
3+
"object":{
4+
"owner":"d5389ccc-57a4-4b13-8c3f-31747bcdc9f1",
5+
"org_owner":"",
6+
"type":"file",
7+
"acl_user_list":null,
8+
"acl_group_list":null
9+
},
10+
"subject":{
11+
"id":"d5389ccc-57a4-4b13-8c3f-31747bcdc9f1",
12+
"roles":[
13+
{
14+
"name":"owner",
15+
"display_name":"Owner",
16+
"site":[
17+
{
18+
"negate":false,
19+
"resource_type":"*",
20+
"action":"*"
21+
}
22+
],
23+
"org":null,
24+
"user":null
25+
},
26+
{
27+
"name":"member",
28+
"display_name":"",
29+
"site":[
30+
{
31+
"negate":false,
32+
"resource_type":"user",
33+
"action":"read"
34+
},
35+
{
36+
"negate":false,
37+
"resource_type":"assign_role",
38+
"action":"read"
39+
},
40+
{
41+
"negate":false,
42+
"resource_type":"provisioner_daemon",
43+
"action":"read"
44+
}
45+
],
46+
"org":null,
47+
"user":[
48+
{
49+
"negate":false,
50+
"resource_type":"*",
51+
"action":"*"
52+
}
53+
]
54+
},
55+
{
56+
"name":"organization-admin:05f58202-4bfc-43ce-9ba4-5ff6e0174a71",
57+
"display_name":"Organization Admin",
58+
"site":null,
59+
"org":{
60+
"05f58202-4bfc-43ce-9ba4-5ff6e0174a71":[
61+
{
62+
"negate":false,
63+
"resource_type":"*",
64+
"action":"*"
65+
}
66+
]
67+
},
68+
"user":null
69+
},
70+
{
71+
"name":"organization-member:05f58202-4bfc-43ce-9ba4-5ff6e0174a71",
72+
"display_name":"",
73+
"site":null,
74+
"org":{
75+
"05f58202-4bfc-43ce-9ba4-5ff6e0174a71":[
76+
{
77+
"negate":false,
78+
"resource_type":"organization_member",
79+
"action":"read"
80+
},
81+
{
82+
"negate":false,
83+
"resource_type":"organization",
84+
"action":"read"
85+
},
86+
{
87+
"negate":false,
88+
"resource_type":"assign_org_role",
89+
"action":"read"
90+
},
91+
{
92+
"negate":false,
93+
"resource_type":"group",
94+
"action":"read"
95+
}
96+
]
97+
},
98+
"user":null
99+
}
100+
],
101+
"groups":null,
102+
"scope":{
103+
"name":"Scope_all",
104+
"display_name":"All operations",
105+
"site":[
106+
{
107+
"negate":false,
108+
"resource_type":"*",
109+
"action":"*"
110+
}
111+
],
112+
"org":{
113+
114+
},
115+
"user":[
116+
117+
]
118+
}
119+
}
120+
}

‎coderd/rbac/input2.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"action":"create","object":{"owner":"me","org_owner":"","type":"workspace","acl_user_list":null,"acl_group_list":null},"subject":{"id":"me","roles":[{"name":"deny-all","display_name":"","site":[{"negate":true,"resource_type":"*","action":"*"}],"org":null,"user":null}],"groups":null,"scope":{"name":"Scope_all","display_name":"All operations","site":[{"negate":false,"resource_type":"*","action":"*"}],"org":{},"user":[]}}}

‎coderd/rbac/memprofile.out

25.4 KB
Binary file not shown.

‎coderd/rbac/partial.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"object":{
3+
"owner":"me",
4+
"org_owner":"b8ec52c9-0691-41e5-acd3-9ac5604d17c2",
5+
"type":"workspace"
6+
}
7+
}

‎coderd/rbac/partial.rego

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package partial

‎coderd/rbac/profile.out

46.5 KB
Binary file not shown.

‎coderd/rbac/rbac.test

24 MB
Binary file not shown.

‎coderd/rbac/regosql/alwaysfalse.go

Lines changed: 0 additions & 46 deletions
This file was deleted.

‎coderd/rbac/regosql/compile_test.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func TestRegoQueries(t *testing.T) {
196196
VariableConverter: regosql.NoACLConverter(),
197197
},
198198
{
199-
Name: "EmptyACLList",
199+
Name: "EmptyACLListNoACLs",
200200
Queries: []string{
201201
`input.object.org_owner != "";
202202
input.object.org_owner in set();
@@ -216,6 +216,20 @@ func TestRegoQueries(t *testing.T) {
216216
p("user_acl->'me' ? '*'")),
217217
VariableConverter: regosql.DefaultVariableConverter(),
218218
},
219+
{
220+
Name: "TemplateOwner",
221+
Queries: []string{
222+
`neq(input.object.org_owner, "");
223+
internal.member_2(input.object.org_owner, {"3bf82434-e40b-44ae-b3d8-d0115bba9bad", "5630fda3-26ab-462c-9014-a88a62d7a415", "c304877a-bc0d-4e9b-9623-a38eae412929"});
224+
neq(input.object.owner, "");
225+
"806dd721-775f-4c85-9ce3-63fbbd975954" = input.object.owner`,
226+
},
227+
ExpectedSQL: p(p("organization_id :: text != ''") + " AND " +
228+
p("organization_id :: text = ANY(ARRAY ['3bf82434-e40b-44ae-b3d8-d0115bba9bad','5630fda3-26ab-462c-9014-a88a62d7a415','c304877a-bc0d-4e9b-9623-a38eae412929'])") + " AND " +
229+
p("false") + " AND " +
230+
p("false")),
231+
VariableConverter: regosql.TemplateConverter(),
232+
},
219233
}
220234

221235
for _, tc := range testCases {

‎coderd/rbac/regosql/configs.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import "github.com/coder/coder/coderd/rbac/regosql/sqltypes"
66
func TemplateConverter() *sqltypes.VariableConverter {
77
matcher := sqltypes.NewVariableConverter().RegisterMatcher(
88
// Basic strings
9-
AlwaysFalse(sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"})),
10-
sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"}),
9+
sqltypes.StringVarMatcher("organization_id :: text", []string{"input", "object", "org_owner"}),
10+
sqltypes.AlwaysFalse(sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"})),
1111
)
1212
matcher.RegisterMatcher(
1313
ACLGroupMatcher(matcher, "group_acl", []string{"input", "object", "acl_group_list"}),
@@ -25,8 +25,8 @@ func NoACLConverter() *sqltypes.VariableConverter {
2525
sqltypes.StringVarMatcher("owner_id :: text", []string{"input", "object", "owner"}),
2626
)
2727
matcher.RegisterMatcher(
28-
AlwaysFalse(ACLGroupMatcher(matcher, "group_acl", []string{"input", "object", "acl_group_list"})),
29-
AlwaysFalse(ACLGroupMatcher(matcher, "user_acl", []string{"input", "object", "acl_user_list"})),
28+
sqltypes.AlwaysFalse(ACLGroupMatcher(matcher, "group_acl", []string{"input", "object", "acl_group_list"})),
29+
sqltypes.AlwaysFalse(ACLGroupMatcher(matcher, "user_acl", []string{"input", "object", "acl_user_list"})),
3030
)
3131

3232
return matcher

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package sqltypes
2+
3+
import (
4+
"github.com/open-policy-agent/opa/ast"
5+
)
6+
7+
var _ Node = alwaysFalse{}
8+
var _ VariableMatcher = alwaysFalse{}
9+
10+
type alwaysFalse struct {
11+
Matcher VariableMatcher
12+
13+
InnerNode Node
14+
}
15+
16+
func AlwaysFalse(m VariableMatcher) alwaysFalse {
17+
return alwaysFalse{
18+
Matcher: m,
19+
}
20+
}
21+
22+
// AlwaysFalseNode is mainly used for unit testing to make a Node immediately.
23+
func AlwaysFalseNode(n Node) alwaysFalse {
24+
return alwaysFalse{
25+
InnerNode: n,
26+
Matcher: nil,
27+
}
28+
}
29+
30+
// UseAs uses a type no one supports to always override with false.
31+
func (f alwaysFalse) UseAs() Node { return alwaysFalse{} }
32+
func (f alwaysFalse) ConvertVariable(rego ast.Ref) (Node, bool) {
33+
if f.Matcher != nil {
34+
n, ok := f.Matcher.ConvertVariable(rego)
35+
if ok {
36+
return alwaysFalse{
37+
Matcher: f.Matcher,
38+
InnerNode: n,
39+
}, true
40+
}
41+
}
42+
43+
return nil, false
44+
}
45+
46+
func (f alwaysFalse) SQLString(_ *SQLGenerator) string {
47+
return "false"
48+
}
49+
50+
func (f alwaysFalse) ContainsSQL(_ *SQLGenerator, _ Node) (string, error) {
51+
return "false", nil
52+
}
53+
54+
func (f alwaysFalse) ContainedInSQL(_ *SQLGenerator, _ Node) (string, error) {
55+
return "false", nil
56+
}
57+
58+
func (f alwaysFalse) EqualsSQLString(_ *SQLGenerator, not bool, _ Node) (string, error) {
59+
return "false", nil
60+
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ func (a ASTArray) ContainsSQL(cfg *SQLGenerator, needle Node) (string, error) {
3737
// This condition supports any contains function if the needle type is
3838
// the same as the ASTArray element type.
3939
if reflect.TypeOf(a.MyType().UseAs()) != reflect.TypeOf(needle.UseAs()) {
40-
cfg.AddError(fmt.Errorf("array contains %q: type mismatch (%T, %T)",
41-
a.Source, a.MyType(), needle))
4240
return "ArrayContainsError", fmt.Errorf("array contains %q: type mismatch (%T, %T)",
4341
a.Source, a.MyType(), needle)
4442
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,22 @@ func TestEquality(t *testing.T) {
9494
),
9595
ExpectedSQL: "(('foo' = ANY(ARRAY ['foo','bar'])) != false) = (true = (2 = ANY(ARRAY [5,2])))",
9696
},
97+
{
98+
Name: "AlwaysFalse=String",
99+
Equality: sqltypes.Equality(false,
100+
sqltypes.AlwaysFalseNode(sqltypes.String("foo")),
101+
sqltypes.String("foo"),
102+
),
103+
ExpectedSQL: "false",
104+
},
105+
{
106+
Name: "String=AlwaysFalse",
107+
Equality: sqltypes.Equality(false,
108+
sqltypes.String("foo"),
109+
sqltypes.AlwaysFalseNode(sqltypes.String("foo")),
110+
),
111+
ExpectedSQL: "false",
112+
},
97113
}
98114

99115
for _, tc := range testCases {

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ type SupportsContains interface {
1010
ContainsSQL(cfg *SQLGenerator, other Node) (string, error)
1111
}
1212

13+
// SupportsContainedIn is the inverse of SupportsContains. It is implemented
14+
// from the "needle" rather than the haystack.
15+
type SupportsContainedIn interface {
16+
ContainedInSQL(cfg *SQLGenerator, other Node) (string, error)
17+
}
18+
1319
var _ BooleanNode = memberOf{}
1420
var _ Node = memberOf{}
1521

@@ -44,6 +50,13 @@ func (e memberOf) SQLString(cfg *SQLGenerator) string {
4450
}
4551
}
4652

53+
if sc, ok := e.Needle.(SupportsContainedIn); ok {
54+
v, err := sc.ContainedInSQL(cfg, e.Haystack)
55+
if err == nil {
56+
return v
57+
}
58+
}
59+
4760
cfg.AddError(fmt.Errorf("unsupported contains: %T contains %T", e.Haystack, e.Needle))
4861
return "MemberOfError"
4962
}

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,28 @@ func TestMembership(t *testing.T) {
5656
),
5757
ExpectedSQL: "false",
5858
},
59+
{
60+
Name: "AlwaysFalseMember",
61+
Membership: sqltypes.MemberOf(
62+
sqltypes.AlwaysFalseNode(sqltypes.Bool(true)),
63+
must(sqltypes.Array("",
64+
sqltypes.Bool(false),
65+
sqltypes.Bool(true),
66+
)),
67+
),
68+
ExpectedSQL: "false",
69+
},
70+
{
71+
Name: "AlwaysFalseArray",
72+
Membership: sqltypes.MemberOf(
73+
sqltypes.Bool(true),
74+
sqltypes.AlwaysFalseNode(must(sqltypes.Array("",
75+
sqltypes.Bool(false),
76+
sqltypes.Bool(true),
77+
))),
78+
),
79+
ExpectedSQL: "false",
80+
},
5981

6082
// Errors
6183
{
@@ -76,9 +98,10 @@ func TestMembership(t *testing.T) {
7698
gen := sqltypes.NewSQLGenerator()
7799
found := tc.Membership.SQLString(gen)
78100
if tc.ExpectedErrors > 0 {
79-
require.Equal(t, tc.ExpectedErrors, len(gen.Errors()), "expected AstNumber of errors")
101+
require.Equal(t, tc.ExpectedErrors, len(gen.Errors()), "expected some errors")
80102
} else {
81103
require.Equal(t, tc.ExpectedSQL, found, "expected sql")
104+
require.Equal(t, tc.ExpectedErrors, 0, "expected no errors")
82105
}
83106
})
84107
}

0 commit comments

Comments
 (0)