Skip to content

Commit 6cdf575

Browse files
EmyrkEmyrk
committed
Add partial filtering and evaluation
Unfortunately this does slow down the unit tests :(
1 parent 75e3a12 commit 6cdf575

File tree

4 files changed

+176
-44
lines changed

4 files changed

+176
-44
lines changed

‎coderd/rbac/authz.go

Lines changed: 92 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ import (
1111

1212
type Authorizer interface {
1313
ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error
14+
PrepareByRoleName(ctx context.Context, subjectID string, roles []string, action Action, objectType string) (PreparedAuthorized, error)
15+
}
16+
17+
type PreparedAuthorized interface {
18+
Authorize(ctx context.Context, object Object) error
1419
}
1520

1621
// Filter takes in a list of objects, and will filter the list removing all
@@ -31,10 +36,26 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub
3136
return filtered
3237
}
3338

39+
func FilterPart[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objectType string, objects []O) []O {
40+
filtered := make([]O, 0)
41+
prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, action, objectType)
42+
if err != nil {
43+
return filtered
44+
}
45+
46+
for i := range objects {
47+
object := objects[i]
48+
err := prepared.Authorize(ctx, object.RBACObject())
49+
if err == nil {
50+
filtered = append(filtered, object)
51+
}
52+
}
53+
return filtered
54+
}
55+
3456
// RegoAuthorizer will use a prepared rego query for performing authorize()
3557
type RegoAuthorizer struct {
36-
query rego.PreparedEvalQuery
37-
partial rego.PreparedPartialQuery
58+
query rego.PreparedEvalQuery
3859
}
3960

4061
// Load the policy from policy.rego in this directory.
@@ -46,23 +67,14 @@ func NewAuthorizer() (*RegoAuthorizer, error) {
4667
query, err := rego.New(
4768
// allowed is the `allow` field from the prepared query. This is the field to check if authorization is
4869
// granted.
49-
rego.Query("allowed = data.authz.allow"),
70+
rego.Query("data.authz.allow"),
5071
rego.Module("policy.rego", policy),
5172
).PrepareForEval(ctx)
5273

53-
partial, err := rego.New(
54-
rego.Query("allowed = data.authz.allow"),
55-
rego.Module("policy.rego", policy),
56-
rego.Unknowns([]string{
57-
"input.object.owner",
58-
"input.object.org_owner",
59-
}),
60-
).PrepareForPartial(ctx)
61-
6274
if err != nil {
6375
return nil, xerrors.Errorf("prepare query: %w", err)
6476
}
65-
return &RegoAuthorizer{query: query, partial: partial}, nil
77+
return &RegoAuthorizer{query: query}, nil
6678
}
6779

6880
type authSubject struct {
@@ -74,14 +86,11 @@ type authSubject struct {
7486
// This is the function intended to be used outside this package.
7587
// The role is fetched from the builtin map located in memory.
7688
func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error {
77-
roles := make([]Role, 0, len(roleNames))
78-
for _, n := range roleNames {
79-
r, err := RoleByName(n)
80-
if err != nil {
81-
return xerrors.Errorf("get role permissions: %w", err)
82-
}
83-
roles = append(roles, r)
89+
roles, err := a.rolesByNames(roleNames)
90+
if err != nil {
91+
return err
8492
}
93+
8594
return a.Authorize(ctx, subjectID, roles, action, object)
8695
}
8796

@@ -102,28 +111,23 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [
102111
return ForbiddenWithInternal(xerrors.Errorf("eval rego: %w", err), input, results)
103112
}
104113

105-
if len(results) != 1 {
106-
return ForbiddenWithInternal(xerrors.Errorf("expect only 1 result, got %d", len(results)), input, results)
114+
if !results.Allowed() {
115+
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
107116
}
108117

109-
allowedResult, ok := (results[0].Bindings["allowed"]).(bool)
110-
if !ok {
111-
return ForbiddenWithInternal(xerrors.Errorf("expected allowed to be a bool but got %T", allowedResult), input, results)
112-
}
118+
return nil
119+
}
113120

114-
if !allowedResult {
115-
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
121+
func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error) {
122+
roles, err := a.rolesByNames(roleNames)
123+
if err != nil {
124+
return nil, err
116125
}
117126

118-
return nil
127+
return a.Prepare(ctx, subjectID, roles, action, objectType)
119128
}
120129

121-
// CheckPartial will not authorize the request. This function is to be used for unit testing to verify the rego policy
122-
// can be converted into ONLY queries. This ensures we can convert the queries into SQL WHERE clauses in the future.
123-
// If this function returns an error, then there is a set of inputs that also returns support rules, which cannot
124-
// be converted.
125-
// This function will not be used to actually perform authorization on partial queries.
126-
func (a RegoAuthorizer) CheckPartial(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*rego.PartialQueries, interface{}, error) {
130+
func (a RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*partialAuthorizer, error) {
127131
input := map[string]interface{}{
128132
"subject": authSubject{
129133
ID: subjectID,
@@ -134,6 +138,57 @@ func (a RegoAuthorizer) CheckPartial(ctx context.Context, subjectID string, role
134138
},
135139
"action": action,
136140
}
137-
result, err := a.partial.Partial(ctx, rego.EvalInput(input))
138-
return result, input, err
141+
142+
rego := rego.New(
143+
rego.Query("data.authz.allow"),
144+
rego.Module("policy.rego", policy),
145+
rego.Unknowns([]string{
146+
"input.object.owner",
147+
"input.object.org_owner",
148+
}),
149+
rego.Input(input),
150+
)
151+
152+
auth, err := newPartialAuthorizer(ctx, rego, input)
153+
if err != nil {
154+
return nil, xerrors.Errorf("new partial authorizer: %w", err)
155+
}
156+
157+
return auth, nil
158+
}
159+
160+
func (a RegoAuthorizer) rolesByNames(roleNames []string) ([]Role, error) {
161+
roles := make([]Role, 0, len(roleNames))
162+
for _, n := range roleNames {
163+
r, err := RoleByName(n)
164+
if err != nil {
165+
return nil, xerrors.Errorf("get role permissions: %w", err)
166+
}
167+
roles = append(roles, r)
168+
}
169+
return roles, nil
170+
}
171+
172+
// CheckPartial will not authorize the request. This function is to be used for unit testing to verify the rego policy
173+
// can be converted into ONLY queries. This ensures we can convert the queries into SQL WHERE clauses in the future.
174+
// If this function returns an error, then there is a set of inputs that also returns support rules, which cannot
175+
// be converted.
176+
// Unfortunately we cannot reuse a.Prepare for this purpose as a Partial() requires an expression,
177+
// whereas a PartialResult() requires a reference. It's an annoying detail that we need to work around.
178+
func (a RegoAuthorizer) CheckPartial(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*rego.PartialQueries, interface{}, error) {
179+
partAuth, err := a.Prepare(ctx, subjectID, roles, action, objectType)
180+
if err != nil {
181+
return nil, nil, err
182+
}
183+
184+
result, err := rego.New(
185+
rego.Query("true = data.authz.allow"),
186+
rego.Module("policy.rego", policy),
187+
rego.Unknowns([]string{
188+
"input.object.owner",
189+
"input.object.org_owner",
190+
}),
191+
rego.Input(partAuth.Input),
192+
).Partial(ctx)
193+
return result, partAuth.Input, err
139194
}

‎coderd/rbac/authz_test.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -574,33 +574,33 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes
574574
for _, a := range c.actions {
575575
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
576576
defer cancel()
577-
err := authorizer.Authorize(ctx, subject.UserID, subject.Roles, a, c.resource)
577+
authError := authorizer.Authorize(ctx, subject.UserID, subject.Roles, a, c.resource)
578578
if c.allow {
579-
if err != nil {
579+
if authError != nil {
580580
var uerr *rbac.UnauthorizedError
581-
xerrors.As(err, &uerr)
581+
xerrors.As(authError, &uerr)
582582
d, _ := json.Marshal(uerr.Input())
583583
t.Logf("input: %s", string(d))
584584
t.Logf("internal error: %+v", uerr.Internal().Error())
585585
t.Logf("output: %+v", uerr.Output())
586586
}
587-
require.NoError(t, err, "expected no error for testcase action %s", a)
587+
require.NoError(t, authError, "expected no error for testcase action %s", a)
588588
continue
589589
}
590590

591-
if err == nil {
591+
if authError == nil {
592592
d, _ := json.Marshal(map[string]interface{}{
593593
"subject": subject,
594594
"object": c.resource,
595595
"action": a,
596596
})
597597
t.Log(string(d))
598598
}
599-
require.Error(t, err, "expected unauthorized")
599+
require.Error(t, authError, "expected unauthorized")
600600

601601
// Also check the rego policy can form a valid partial query result.
602602
result, input, err := authorizer.CheckPartial(ctx, subject.UserID, subject.Roles, a, c.resource.Type)
603-
require.NoError(t, err, "check partial")
603+
require.NoError(t, err, "run partial")
604604
if len(result.Support) > 0 {
605605
d, _ := json.Marshal(input)
606606
t.Logf("input: %s", string(d))
@@ -611,6 +611,17 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes
611611
t.Logf("support: %+v", s.String())
612612
}
613613
}
614+
require.Equal(t, 0, len(result.Support), "expected 0 support rules")
615+
616+
partialAuther, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, a, c.resource.Type)
617+
require.NoError(t, err, "make prepared authorizer")
618+
619+
partialErr := partialAuther.Authorize(ctx, c.resource)
620+
if authError != nil {
621+
require.Error(t, partialErr, "partial error blocked valid request (false negative)")
622+
} else {
623+
require.NoError(t, partialErr, "partial allowed invalid request (false positive)")
624+
}
614625
}
615626
})
616627
}

‎coderd/rbac/fake_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,23 @@ type fakeAuthorizer struct {
1313
func (f fakeAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error {
1414
return f.AuthFunc(ctx, subjectID, roleNames, action, object)
1515
}
16+
17+
func (f fakeAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roles []string, action rbac.Action, objectType string) (rbac.PreparedAuthorized, error) {
18+
return &fakePreparedAuthorizer{
19+
Original: f,
20+
SubjectID: subjectID,
21+
Roles: roles,
22+
Action: action,
23+
}, nil
24+
}
25+
26+
type fakePreparedAuthorizer struct {
27+
Original rbac.Authorizer
28+
SubjectID string
29+
Roles []string
30+
Action rbac.Action
31+
}
32+
33+
func (f fakePreparedAuthorizer) Authorize(ctx context.Context, object rbac.Object) error {
34+
return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Action, object)
35+
}

‎coderd/rbac/partial.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package rbac
2+
3+
import (
4+
"context"
5+
6+
"golang.org/x/xerrors"
7+
8+
"github.com/open-policy-agent/opa/rego"
9+
)
10+
11+
type partialAuthorizer struct {
12+
// PartialRego is mainly used for unit testing. It is the rego source policy.
13+
PartialRego *rego.Rego
14+
PartialResult rego.PartialResult
15+
Input map[string]interface{}
16+
}
17+
18+
func newPartialAuthorizer(ctx context.Context, partialRego *rego.Rego, input map[string]interface{}) (*partialAuthorizer, error) {
19+
pResult, err := partialRego.PartialResult(ctx)
20+
if err != nil {
21+
return nil, xerrors.Errorf("partial results: %w", err)
22+
}
23+
24+
return &partialAuthorizer{
25+
PartialRego: partialRego,
26+
PartialResult: pResult,
27+
Input: input,
28+
}, nil
29+
}
30+
31+
// Authorize authorizes a single object
32+
func (a partialAuthorizer) Authorize(ctx context.Context, object Object) error {
33+
results, err := a.PartialResult.Rego(rego.Input(
34+
map[string]interface{}{
35+
"object": object,
36+
})).Eval(ctx)
37+
if err != nil {
38+
return ForbiddenWithInternal(xerrors.Errorf("eval prepared"), a.Input, results)
39+
}
40+
41+
if !results.Allowed() {
42+
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), a.Input, results)
43+
}
44+
45+
return nil
46+
}

0 commit comments

Comments
 (0)