- Notifications
You must be signed in to change notification settings - Fork928
chore: Update rego to be partial execution friendly#3449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from17 commits
b3536bc75e3a126cdf575f5eacd04a7c68edf75be5e90ac2d1e774e038917dce139a1fc44d4d119f3557f9dd9aabed9f4f74d90f4fe0d05add5c55cae22f890266963af457b921f4f212c872201c407ab9f9b2d1ab55cf5abf098d44c73704611322c8e26a8510e94bFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -10,9 +10,27 @@ import ( | ||
| "github.com/coder/coder/coderd/rbac" | ||
| ) | ||
| func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Action, objects []O)([]O, error) { | ||
| roles := httpmw.AuthorizationUserRoles(r) | ||
| if len(objects) == 0 { | ||
| return objects, nil | ||
| } | ||
| objecType := objects[0].RBACObject().Type | ||
Emyrk marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| objects, err := rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objecType, objects) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Seems like all of this logic could be in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. You are right about the object type stuff 👍 As for | ||
| if err != nil { | ||
| api.Logger.Error(r.Context(), "filter failed", | ||
| slog.Error(err), | ||
| slog.F("object_type", objecType), | ||
| slog.F("user_id", roles.ID), | ||
| slog.F("username", roles.Username), | ||
| slog.F("route", r.URL.Path), | ||
| slog.F("action", action), | ||
| ) | ||
| // Hide the underlying error in case it has sensitive information | ||
| return nil, xerrors.Errorf("failed to filter requested objects") | ||
Emyrk marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| } | ||
| return objects, nil | ||
| } | ||
| // Authorize will return false if the user is not authorized to do the action. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package rbac | ||
Emyrk marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| import "context" | ||
| type FakeAuthorizer struct { | ||
| AuthFunc func(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error | ||
| } | ||
| func (f FakeAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error { | ||
| return f.AuthFunc(ctx, subjectID, roleNames, action, object) | ||
| } | ||
| func (f FakeAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, action Action, _ string) (PreparedAuthorized, error) { | ||
| return &fakePreparedAuthorizer{ | ||
| Original: f, | ||
| SubjectID: subjectID, | ||
| Roles: roles, | ||
| Action: action, | ||
| }, nil | ||
| } | ||
| type fakePreparedAuthorizer struct { | ||
| Original Authorizer | ||
| SubjectID string | ||
| Roles []string | ||
| Action Action | ||
| } | ||
| func (f fakePreparedAuthorizer) Authorize(ctx context.Context, object Object) error { | ||
| return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Action, object) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -11,24 +11,35 @@ import ( | ||
| type Authorizer interface { | ||
| ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error | ||
| PrepareByRoleName(ctx context.Context, subjectID string, roles []string, action Action, objectType string) (PreparedAuthorized, error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This function is only used internally, so seems like it might not need to be exported like this. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I need it for my Fake authorizer to work for some unit tests. I can't fake it without an interface
Emyrk marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I don't see any use case for exposing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I wish I could, but you can't make generic methods on structs. The nice thing about the generic If I made it a non-generic function, you'd have to take your slice of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Well, that sucks. Go generics continue to disappoint. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Yea, it's really dumb because I don't see why it's not possible. Like I would like to know the objection to it. | ||
| } | ||
| type PreparedAuthorized interface { | ||
| Authorize(ctx context.Context, object Object) error | ||
| } | ||
| // Filter takes in a list of objects, and will filter the list removing all | ||
| // the elements the subject does not have permission for. All objects must be | ||
| // of the same type. | ||
| func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objectType string, objects []O) ([]O, error) { | ||
| filtered := make([]O, 0) | ||
| prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, action, objectType) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("prepare: %w", err) | ||
| } | ||
| for i := range objects { | ||
| object := objects[i] | ||
| rbacObj := object.RBACObject() | ||
| if rbacObj.Type != objectType { | ||
| return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, object.RBACObject().Type) | ||
| } | ||
| err := prepared.Authorize(ctx, rbacObj) | ||
| if err == nil { | ||
| filtered = append(filtered, object) | ||
| } | ||
| } | ||
| return filtered, nil | ||
| } | ||
| // RegoAuthorizer will use a prepared rego query for performing authorize() | ||
| @@ -45,7 +56,7 @@ func NewAuthorizer() (*RegoAuthorizer, error) { | ||
| query, err := rego.New( | ||
| // allowed is the `allow` field from the prepared query. This is the field to check if authorization is | ||
| // granted. | ||
| rego.Query("data.authz.allow"), | ||
| rego.Module("policy.rego", policy), | ||
| ).PrepareForEval(ctx) | ||
| @@ -64,14 +75,11 @@ type authSubject struct { | ||
| // This is the function intended to be used outside this package. | ||
| // The role is fetched from the builtin map located in memory. | ||
| func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error { | ||
| roles, err := RolesByNames(roleNames) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return a.Authorize(ctx, subjectID, roles, action, object) | ||
| } | ||
| @@ -92,18 +100,29 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [ | ||
| return ForbiddenWithInternal(xerrors.Errorf("eval rego: %w", err), input, results) | ||
| } | ||
| if!results.Allowed() { | ||
| return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results) | ||
| } | ||
| return nil | ||
| } | ||
| // Prepare will partially execute the rego policy leaving the object fields unknown (except for the type). | ||
| // This will vastly speed up performance if batch authorization on the same type of objects is needed. | ||
| func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*PartialAuthorizer, error) { | ||
| auth, err := newPartialAuthorizer(ctx, subjectID, roles, action, objectType) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("new partial authorizer: %w", err) | ||
| } | ||
| return auth, nil | ||
| } | ||
| func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error) { | ||
| roles, err := RolesByNames(roleNames) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| returna.Prepare(ctx, subjectID, roles, action, objectType) | ||
| } | ||
Uh oh!
There was an error while loading.Please reload this page.