From 39046c52d0b4d947d9865cee4289d4fe168af6fd Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Wed, 13 Jul 2022 12:14:54 +0200 Subject: [PATCH] Harden RBAC and refactor check tracking - stricter rule checking when multiple roles have permissions on same resource - tracking (prev: evaluation) is refactored to stand out less than previous solution - performance optimization on certain situations (earlier fn return) --- automation/rest.yaml | 4 +- automation/rest/handlers/permissions.go | 12 +- automation/rest/permissions.go | 6 +- automation/rest/request/permissions.go | 18 +- automation/service/access_control.gen.go | 9 +- .../rbac/$component_access_control.go.tpl | 9 +- compose/rest.yaml | 4 +- compose/rest/handlers/permissions.go | 12 +- compose/rest/permissions.go | 6 +- compose/rest/request/permissions.go | 18 +- compose/service/access_control.gen.go | 11 +- federation/rest.yaml | 4 +- federation/rest/handlers/permissions.go | 12 +- federation/rest/permissions.go | 6 +- federation/rest/request/permissions.go | 18 +- federation/service/access_control.gen.go | 9 +- pkg/rbac/evaluate.go | 27 --- pkg/rbac/roles.go | 29 ++- pkg/rbac/roles_test.go | 12 ++ pkg/rbac/ruleset_checks.go | 112 +++++++---- pkg/rbac/ruleset_checks_test.go | 177 +++++++++++++++++- pkg/rbac/service.go | 73 ++------ pkg/rbac/service_alt.go | 4 +- pkg/rbac/trace.go | 74 ++++++++ pkg/rbac/trace_test.go | 60 ++++++ system/rest.yaml | 4 +- system/rest/handlers/permissions.go | 12 +- system/rest/permissions.go | 6 +- system/rest/request/permissions.go | 18 +- system/service/access_control.gen.go | 17 +- system/service/role.go | 15 -- tests/automation/permissions_test.go | 16 ++ tests/compose/permissions_test.go | 16 ++ tests/system/permissions_test.go | 16 ++ 34 files changed, 596 insertions(+), 250 deletions(-) delete mode 100644 pkg/rbac/evaluate.go create mode 100644 pkg/rbac/trace.go create mode 100644 pkg/rbac/trace_test.go diff --git a/automation/rest.yaml b/automation/rest.yaml index ea7d0680f..795f48006 100644 --- a/automation/rest.yaml +++ b/automation/rest.yaml @@ -292,8 +292,8 @@ endpoints: type: string required: false title: Show only rules for a specific resource - - name: evaluate - path: "/evaluate" + - name: trace + path: "/trace" method: GET title: Evaluate rules for given user/role combo parameters: diff --git a/automation/rest/handlers/permissions.go b/automation/rest/handlers/permissions.go index 615b9c658..9b261a83c 100644 --- a/automation/rest/handlers/permissions.go +++ b/automation/rest/handlers/permissions.go @@ -21,7 +21,7 @@ type ( PermissionsAPI interface { List(context.Context, *request.PermissionsList) (interface{}, error) Effective(context.Context, *request.PermissionsEffective) (interface{}, error) - Evaluate(context.Context, *request.PermissionsEvaluate) (interface{}, error) + Trace(context.Context, *request.PermissionsTrace) (interface{}, error) Read(context.Context, *request.PermissionsRead) (interface{}, error) Delete(context.Context, *request.PermissionsDelete) (interface{}, error) Update(context.Context, *request.PermissionsUpdate) (interface{}, error) @@ -31,7 +31,7 @@ type ( Permissions struct { List func(http.ResponseWriter, *http.Request) Effective func(http.ResponseWriter, *http.Request) - Evaluate func(http.ResponseWriter, *http.Request) + Trace func(http.ResponseWriter, *http.Request) Read func(http.ResponseWriter, *http.Request) Delete func(http.ResponseWriter, *http.Request) Update func(http.ResponseWriter, *http.Request) @@ -72,15 +72,15 @@ func NewPermissions(h PermissionsAPI) *Permissions { api.Send(w, r, value) }, - Evaluate: func(w http.ResponseWriter, r *http.Request) { + Trace: func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() - params := request.NewPermissionsEvaluate() + params := request.NewPermissionsTrace() if err := params.Fill(r); err != nil { api.Send(w, r, err) return } - value, err := h.Evaluate(r.Context(), params) + value, err := h.Trace(r.Context(), params) if err != nil { api.Send(w, r, err) return @@ -144,7 +144,7 @@ func (h Permissions) MountRoutes(r chi.Router, middlewares ...func(http.Handler) r.Use(middlewares...) r.Get("/permissions/", h.List) r.Get("/permissions/effective", h.Effective) - r.Get("/permissions/evaluate", h.Evaluate) + r.Get("/permissions/trace", h.Trace) r.Get("/permissions/{roleID}/rules", h.Read) r.Delete("/permissions/{roleID}/rules", h.Delete) r.Patch("/permissions/{roleID}/rules", h.Update) diff --git a/automation/rest/permissions.go b/automation/rest/permissions.go index 11acd00dc..3ae10fd10 100644 --- a/automation/rest/permissions.go +++ b/automation/rest/permissions.go @@ -17,7 +17,7 @@ type ( permissionsAccessController interface { Effective(context.Context, ...rbac.Resource) rbac.EffectiveSet - Evaluate(ctx context.Context, user uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) + Trace(context.Context, uint64, []uint64, ...string) ([]*rbac.Trace, error) List() []map[string]string FindRulesByRoleID(context.Context, uint64) (rbac.RuleSet, error) Grant(ctx context.Context, rr ...*rbac.Rule) error @@ -34,8 +34,8 @@ func (ctrl Permissions) Effective(ctx context.Context, r *request.PermissionsEff return ctrl.ac.Effective(ctx, types.Component{}), nil } -func (ctrl Permissions) Evaluate(ctx context.Context, r *request.PermissionsEvaluate) (interface{}, error) { - return ctrl.ac.Evaluate(ctx, r.UserID, r.RoleID, r.Resource...) +func (ctrl Permissions) Trace(ctx context.Context, r *request.PermissionsTrace) (interface{}, error) { + return ctrl.ac.Trace(ctx, r.UserID, r.RoleID, r.Resource...) } func (ctrl Permissions) List(ctx context.Context, r *request.PermissionsList) (interface{}, error) { diff --git a/automation/rest/request/permissions.go b/automation/rest/request/permissions.go index 12b760af9..a2cc3cd54 100644 --- a/automation/rest/request/permissions.go +++ b/automation/rest/request/permissions.go @@ -44,7 +44,7 @@ type ( Resource string } - PermissionsEvaluate struct { + PermissionsTrace struct { // Resource GET parameter // // Show only rules for a specific resource @@ -139,13 +139,13 @@ func (r *PermissionsEffective) Fill(req *http.Request) (err error) { return err } -// NewPermissionsEvaluate request -func NewPermissionsEvaluate() *PermissionsEvaluate { - return &PermissionsEvaluate{} +// NewPermissionsTrace request +func NewPermissionsTrace() *PermissionsTrace { + return &PermissionsTrace{} } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) Auditable() map[string]interface{} { +func (r PermissionsTrace) Auditable() map[string]interface{} { return map[string]interface{}{ "resource": r.Resource, "userID": r.UserID, @@ -154,22 +154,22 @@ func (r PermissionsEvaluate) Auditable() map[string]interface{} { } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetResource() []string { +func (r PermissionsTrace) GetResource() []string { return r.Resource } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetUserID() uint64 { +func (r PermissionsTrace) GetUserID() uint64 { return r.UserID } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetRoleID() []uint64 { +func (r PermissionsTrace) GetRoleID() []uint64 { return r.RoleID } // Fill processes request and fills internal variables -func (r *PermissionsEvaluate) Fill(req *http.Request) (err error) { +func (r *PermissionsTrace) Fill(req *http.Request) (err error) { { // GET params diff --git a/automation/service/access_control.gen.go b/automation/service/access_control.gen.go index 4093baaa0..70edbf321 100644 --- a/automation/service/access_control.gen.go +++ b/automation/service/access_control.gen.go @@ -23,7 +23,8 @@ type ( } rbacService interface { - Evaluate(rbac.Session, string, rbac.Resource) rbac.Evaluated + Can(rbac.Session, string, rbac.Resource) bool + Trace(rbac.Session, string, rbac.Resource) *rbac.Trace Grant(context.Context, ...*rbac.Rule) error FindRulesByRoleID(roleID uint64) (rr rbac.RuleSet) } @@ -45,7 +46,7 @@ func AccessControl(rms roleMemberSearcher) *accessControl { } func (svc accessControl) can(ctx context.Context, op string, res rbac.Resource) bool { - return svc.rbac.Evaluate(rbac.ContextToSession(ctx), op, res).Can + return svc.rbac.Can(rbac.ContextToSession(ctx), op, res) } // Effective returns a list of effective permissions for all given resource @@ -65,7 +66,7 @@ func (svc accessControl) Effective(ctx context.Context, rr ...rbac.Resource) (ee // Evaluate returns a list of permissions evaluated for the given user/roles combo // // This function is auto-generated -func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) { +func (svc accessControl) Trace(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee []*rbac.Trace, err error) { // Reusing the grant permission since this is who the feature is for if !svc.CanGrant(ctx) { // @todo should be altered to check grant permissions PER resource @@ -111,7 +112,7 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui for _, res := range resources { r := res.RbacResource() for op := range rbacResourceOperations(r) { - ee = append(ee, svc.rbac.Evaluate(session, op, res)) + ee = append(ee, svc.rbac.Trace(session, op, res)) } } diff --git a/codegen/assets/templates/gocode/rbac/$component_access_control.go.tpl b/codegen/assets/templates/gocode/rbac/$component_access_control.go.tpl index 6eca47288..c1003d15f 100644 --- a/codegen/assets/templates/gocode/rbac/$component_access_control.go.tpl +++ b/codegen/assets/templates/gocode/rbac/$component_access_control.go.tpl @@ -22,7 +22,8 @@ type ( } rbacService interface { - Evaluate(rbac.Session, string, rbac.Resource) rbac.Evaluated + Can(rbac.Session, string, rbac.Resource) bool + Trace(rbac.Session, string, rbac.Resource) *rbac.Trace Grant(context.Context, ...*rbac.Rule) error FindRulesByRoleID(roleID uint64) (rr rbac.RuleSet) } @@ -44,7 +45,7 @@ func AccessControl(rms roleMemberSearcher) *accessControl { } func (svc accessControl) can(ctx context.Context, op string, res rbac.Resource) bool { - return svc.rbac.Evaluate(rbac.ContextToSession(ctx), op, res).Can + return svc.rbac.Can(rbac.ContextToSession(ctx), op, res) } // Effective returns a list of effective permissions for all given resource @@ -64,7 +65,7 @@ func (svc accessControl) Effective(ctx context.Context, rr ... rbac.Resource) (e // Evaluate returns a list of permissions evaluated for the given user/roles combo // // This function is auto-generated -func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) { +func (svc accessControl) Trace(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee []*rbac.Trace, err error) { // Reusing the grant permission since this is who the feature is for if !svc.CanGrant(ctx) { // @todo should be altered to check grant permissions PER resource @@ -110,7 +111,7 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui for _, res := range resources { r := res.RbacResource() for op := range rbacResourceOperations(r) { - ee = append(ee, svc.rbac.Evaluate(session, op, res)) + ee = append(ee, svc.rbac.Trace(session, op, res)) } } diff --git a/compose/rest.yaml b/compose/rest.yaml index ba7e6f3e3..b286875ff 100644 --- a/compose/rest.yaml +++ b/compose/rest.yaml @@ -1297,8 +1297,8 @@ endpoints: type: string required: false title: Show only rules for a specific resource - - name: evaluate - path: "/evaluate" + - name: trace + path: "/trace" method: GET title: Evaluate rules for given user/role combo parameters: diff --git a/compose/rest/handlers/permissions.go b/compose/rest/handlers/permissions.go index 8ffa1be23..e4610894d 100644 --- a/compose/rest/handlers/permissions.go +++ b/compose/rest/handlers/permissions.go @@ -21,7 +21,7 @@ type ( PermissionsAPI interface { List(context.Context, *request.PermissionsList) (interface{}, error) Effective(context.Context, *request.PermissionsEffective) (interface{}, error) - Evaluate(context.Context, *request.PermissionsEvaluate) (interface{}, error) + Trace(context.Context, *request.PermissionsTrace) (interface{}, error) Read(context.Context, *request.PermissionsRead) (interface{}, error) Delete(context.Context, *request.PermissionsDelete) (interface{}, error) Update(context.Context, *request.PermissionsUpdate) (interface{}, error) @@ -31,7 +31,7 @@ type ( Permissions struct { List func(http.ResponseWriter, *http.Request) Effective func(http.ResponseWriter, *http.Request) - Evaluate func(http.ResponseWriter, *http.Request) + Trace func(http.ResponseWriter, *http.Request) Read func(http.ResponseWriter, *http.Request) Delete func(http.ResponseWriter, *http.Request) Update func(http.ResponseWriter, *http.Request) @@ -72,15 +72,15 @@ func NewPermissions(h PermissionsAPI) *Permissions { api.Send(w, r, value) }, - Evaluate: func(w http.ResponseWriter, r *http.Request) { + Trace: func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() - params := request.NewPermissionsEvaluate() + params := request.NewPermissionsTrace() if err := params.Fill(r); err != nil { api.Send(w, r, err) return } - value, err := h.Evaluate(r.Context(), params) + value, err := h.Trace(r.Context(), params) if err != nil { api.Send(w, r, err) return @@ -144,7 +144,7 @@ func (h Permissions) MountRoutes(r chi.Router, middlewares ...func(http.Handler) r.Use(middlewares...) r.Get("/permissions/", h.List) r.Get("/permissions/effective", h.Effective) - r.Get("/permissions/evaluate", h.Evaluate) + r.Get("/permissions/trace", h.Trace) r.Get("/permissions/{roleID}/rules", h.Read) r.Delete("/permissions/{roleID}/rules", h.Delete) r.Patch("/permissions/{roleID}/rules", h.Update) diff --git a/compose/rest/permissions.go b/compose/rest/permissions.go index ab7dae5e8..45a95e209 100644 --- a/compose/rest/permissions.go +++ b/compose/rest/permissions.go @@ -17,7 +17,7 @@ type ( permissionsAccessController interface { Effective(context.Context, ...rbac.Resource) rbac.EffectiveSet - Evaluate(ctx context.Context, user uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) + Trace(context.Context, uint64, []uint64, ...string) ([]*rbac.Trace, error) List() []map[string]string FindRulesByRoleID(context.Context, uint64) (rbac.RuleSet, error) Grant(ctx context.Context, rr ...*rbac.Rule) error @@ -34,8 +34,8 @@ func (ctrl Permissions) Effective(ctx context.Context, r *request.PermissionsEff return ctrl.ac.Effective(ctx, types.Component{}), nil } -func (ctrl Permissions) Evaluate(ctx context.Context, r *request.PermissionsEvaluate) (interface{}, error) { - return ctrl.ac.Evaluate(ctx, r.UserID, r.RoleID, r.Resource...) +func (ctrl Permissions) Trace(ctx context.Context, r *request.PermissionsTrace) (interface{}, error) { + return ctrl.ac.Trace(ctx, r.UserID, r.RoleID, r.Resource...) } func (ctrl Permissions) List(ctx context.Context, r *request.PermissionsList) (interface{}, error) { diff --git a/compose/rest/request/permissions.go b/compose/rest/request/permissions.go index 12b760af9..a2cc3cd54 100644 --- a/compose/rest/request/permissions.go +++ b/compose/rest/request/permissions.go @@ -44,7 +44,7 @@ type ( Resource string } - PermissionsEvaluate struct { + PermissionsTrace struct { // Resource GET parameter // // Show only rules for a specific resource @@ -139,13 +139,13 @@ func (r *PermissionsEffective) Fill(req *http.Request) (err error) { return err } -// NewPermissionsEvaluate request -func NewPermissionsEvaluate() *PermissionsEvaluate { - return &PermissionsEvaluate{} +// NewPermissionsTrace request +func NewPermissionsTrace() *PermissionsTrace { + return &PermissionsTrace{} } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) Auditable() map[string]interface{} { +func (r PermissionsTrace) Auditable() map[string]interface{} { return map[string]interface{}{ "resource": r.Resource, "userID": r.UserID, @@ -154,22 +154,22 @@ func (r PermissionsEvaluate) Auditable() map[string]interface{} { } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetResource() []string { +func (r PermissionsTrace) GetResource() []string { return r.Resource } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetUserID() uint64 { +func (r PermissionsTrace) GetUserID() uint64 { return r.UserID } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetRoleID() []uint64 { +func (r PermissionsTrace) GetRoleID() []uint64 { return r.RoleID } // Fill processes request and fills internal variables -func (r *PermissionsEvaluate) Fill(req *http.Request) (err error) { +func (r *PermissionsTrace) Fill(req *http.Request) (err error) { { // GET params diff --git a/compose/service/access_control.gen.go b/compose/service/access_control.gen.go index 5a086ec46..9049f8a6d 100644 --- a/compose/service/access_control.gen.go +++ b/compose/service/access_control.gen.go @@ -23,7 +23,8 @@ type ( } rbacService interface { - Evaluate(rbac.Session, string, rbac.Resource) rbac.Evaluated + Can(rbac.Session, string, rbac.Resource) bool + Trace(rbac.Session, string, rbac.Resource) *rbac.Trace Grant(context.Context, ...*rbac.Rule) error FindRulesByRoleID(roleID uint64) (rr rbac.RuleSet) } @@ -45,7 +46,7 @@ func AccessControl(rms roleMemberSearcher) *accessControl { } func (svc accessControl) can(ctx context.Context, op string, res rbac.Resource) bool { - return svc.rbac.Evaluate(rbac.ContextToSession(ctx), op, res).Can + return svc.rbac.Can(rbac.ContextToSession(ctx), op, res) } // Effective returns a list of effective permissions for all given resource @@ -65,7 +66,7 @@ func (svc accessControl) Effective(ctx context.Context, rr ...rbac.Resource) (ee // Evaluate returns a list of permissions evaluated for the given user/roles combo // // This function is auto-generated -func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) { +func (svc accessControl) Trace(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee []*rbac.Trace, err error) { // Reusing the grant permission since this is who the feature is for if !svc.CanGrant(ctx) { // @todo should be altered to check grant permissions PER resource @@ -73,7 +74,7 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui } var ( - resources []rbac.Resource + resources = svc.Resources() members systemTypes.RoleMemberSet ) if len(rr) > 0 { @@ -111,7 +112,7 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui for _, res := range resources { r := res.RbacResource() for op := range rbacResourceOperations(r) { - ee = append(ee, svc.rbac.Evaluate(session, op, res)) + ee = append(ee, svc.rbac.Trace(session, op, res)) } } diff --git a/federation/rest.yaml b/federation/rest.yaml index 5b0ca77c7..f93e55f13 100644 --- a/federation/rest.yaml +++ b/federation/rest.yaml @@ -519,8 +519,8 @@ endpoints: type: string required: false title: Show only rules for a specific resource - - name: evaluate - path: "/evaluate" + - name: trace + path: "/trace" method: GET title: Evaluate rules for given user/role combo parameters: diff --git a/federation/rest/handlers/permissions.go b/federation/rest/handlers/permissions.go index b48744357..9873cc01a 100644 --- a/federation/rest/handlers/permissions.go +++ b/federation/rest/handlers/permissions.go @@ -21,7 +21,7 @@ type ( PermissionsAPI interface { List(context.Context, *request.PermissionsList) (interface{}, error) Effective(context.Context, *request.PermissionsEffective) (interface{}, error) - Evaluate(context.Context, *request.PermissionsEvaluate) (interface{}, error) + Trace(context.Context, *request.PermissionsTrace) (interface{}, error) Read(context.Context, *request.PermissionsRead) (interface{}, error) Delete(context.Context, *request.PermissionsDelete) (interface{}, error) Update(context.Context, *request.PermissionsUpdate) (interface{}, error) @@ -31,7 +31,7 @@ type ( Permissions struct { List func(http.ResponseWriter, *http.Request) Effective func(http.ResponseWriter, *http.Request) - Evaluate func(http.ResponseWriter, *http.Request) + Trace func(http.ResponseWriter, *http.Request) Read func(http.ResponseWriter, *http.Request) Delete func(http.ResponseWriter, *http.Request) Update func(http.ResponseWriter, *http.Request) @@ -72,15 +72,15 @@ func NewPermissions(h PermissionsAPI) *Permissions { api.Send(w, r, value) }, - Evaluate: func(w http.ResponseWriter, r *http.Request) { + Trace: func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() - params := request.NewPermissionsEvaluate() + params := request.NewPermissionsTrace() if err := params.Fill(r); err != nil { api.Send(w, r, err) return } - value, err := h.Evaluate(r.Context(), params) + value, err := h.Trace(r.Context(), params) if err != nil { api.Send(w, r, err) return @@ -144,7 +144,7 @@ func (h Permissions) MountRoutes(r chi.Router, middlewares ...func(http.Handler) r.Use(middlewares...) r.Get("/permissions/", h.List) r.Get("/permissions/effective", h.Effective) - r.Get("/permissions/evaluate", h.Evaluate) + r.Get("/permissions/trace", h.Trace) r.Get("/permissions/{roleID}/rules", h.Read) r.Delete("/permissions/{roleID}/rules", h.Delete) r.Patch("/permissions/{roleID}/rules", h.Update) diff --git a/federation/rest/permissions.go b/federation/rest/permissions.go index 9c9a2e7d3..478290253 100644 --- a/federation/rest/permissions.go +++ b/federation/rest/permissions.go @@ -17,7 +17,7 @@ type ( permissionsAccessController interface { Effective(context.Context, ...rbac.Resource) rbac.EffectiveSet - Evaluate(ctx context.Context, user uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) + Trace(context.Context, uint64, []uint64, ...string) ([]*rbac.Trace, error) List() []map[string]string FindRulesByRoleID(context.Context, uint64) (rbac.RuleSet, error) Grant(ctx context.Context, rr ...*rbac.Rule) error @@ -34,8 +34,8 @@ func (ctrl Permissions) Effective(ctx context.Context, r *request.PermissionsEff return ctrl.ac.Effective(ctx, types.Component{}), nil } -func (ctrl Permissions) Evaluate(ctx context.Context, r *request.PermissionsEvaluate) (interface{}, error) { - return ctrl.ac.Evaluate(ctx, r.UserID, r.RoleID, r.Resource...) +func (ctrl Permissions) Trace(ctx context.Context, r *request.PermissionsTrace) (interface{}, error) { + return ctrl.ac.Trace(ctx, r.UserID, r.RoleID, r.Resource...) } func (ctrl Permissions) List(ctx context.Context, r *request.PermissionsList) (interface{}, error) { diff --git a/federation/rest/request/permissions.go b/federation/rest/request/permissions.go index 12b760af9..a2cc3cd54 100644 --- a/federation/rest/request/permissions.go +++ b/federation/rest/request/permissions.go @@ -44,7 +44,7 @@ type ( Resource string } - PermissionsEvaluate struct { + PermissionsTrace struct { // Resource GET parameter // // Show only rules for a specific resource @@ -139,13 +139,13 @@ func (r *PermissionsEffective) Fill(req *http.Request) (err error) { return err } -// NewPermissionsEvaluate request -func NewPermissionsEvaluate() *PermissionsEvaluate { - return &PermissionsEvaluate{} +// NewPermissionsTrace request +func NewPermissionsTrace() *PermissionsTrace { + return &PermissionsTrace{} } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) Auditable() map[string]interface{} { +func (r PermissionsTrace) Auditable() map[string]interface{} { return map[string]interface{}{ "resource": r.Resource, "userID": r.UserID, @@ -154,22 +154,22 @@ func (r PermissionsEvaluate) Auditable() map[string]interface{} { } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetResource() []string { +func (r PermissionsTrace) GetResource() []string { return r.Resource } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetUserID() uint64 { +func (r PermissionsTrace) GetUserID() uint64 { return r.UserID } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetRoleID() []uint64 { +func (r PermissionsTrace) GetRoleID() []uint64 { return r.RoleID } // Fill processes request and fills internal variables -func (r *PermissionsEvaluate) Fill(req *http.Request) (err error) { +func (r *PermissionsTrace) Fill(req *http.Request) (err error) { { // GET params diff --git a/federation/service/access_control.gen.go b/federation/service/access_control.gen.go index 6c1b783f1..ac983e306 100644 --- a/federation/service/access_control.gen.go +++ b/federation/service/access_control.gen.go @@ -23,7 +23,8 @@ type ( } rbacService interface { - Evaluate(rbac.Session, string, rbac.Resource) rbac.Evaluated + Can(rbac.Session, string, rbac.Resource) bool + Trace(rbac.Session, string, rbac.Resource) *rbac.Trace Grant(context.Context, ...*rbac.Rule) error FindRulesByRoleID(roleID uint64) (rr rbac.RuleSet) } @@ -45,7 +46,7 @@ func AccessControl(rms roleMemberSearcher) *accessControl { } func (svc accessControl) can(ctx context.Context, op string, res rbac.Resource) bool { - return svc.rbac.Evaluate(rbac.ContextToSession(ctx), op, res).Can + return svc.rbac.Can(rbac.ContextToSession(ctx), op, res) } // Effective returns a list of effective permissions for all given resource @@ -65,7 +66,7 @@ func (svc accessControl) Effective(ctx context.Context, rr ...rbac.Resource) (ee // Evaluate returns a list of permissions evaluated for the given user/roles combo // // This function is auto-generated -func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) { +func (svc accessControl) Trace(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee []*rbac.Trace, err error) { // Reusing the grant permission since this is who the feature is for if !svc.CanGrant(ctx) { // @todo should be altered to check grant permissions PER resource @@ -111,7 +112,7 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui for _, res := range resources { r := res.RbacResource() for op := range rbacResourceOperations(r) { - ee = append(ee, svc.rbac.Evaluate(session, op, res)) + ee = append(ee, svc.rbac.Trace(session, op, res)) } } diff --git a/pkg/rbac/evaluate.go b/pkg/rbac/evaluate.go deleted file mode 100644 index 7dda038e4..000000000 --- a/pkg/rbac/evaluate.go +++ /dev/null @@ -1,27 +0,0 @@ -package rbac - -type ( - Evaluated struct { - Resource string `json:"resource"` - Operation string `json:"operation"` - Access Access `json:"-"` - Can bool `json:"can"` - Step explanation `json:"step"` - - RoleID uint64 `json:"roleID,string,omitempty"` - Rule *Rule `json:"rule,omitempty"` - - Default *Evaluated `json:"default,omitempty"` - } - - EvaluatedSet []Evaluated - - explanation string -) - -const ( - stepIntegrity explanation = "integrity" - stepBypass explanation = "bypass" - stepRuleless explanation = "ruleless" - stepEvaluated explanation = "evaluated" -) diff --git a/pkg/rbac/roles.go b/pkg/rbac/roles.go index 623983b10..77f9c8e38 100644 --- a/pkg/rbac/roles.go +++ b/pkg/rbac/roles.go @@ -121,6 +121,34 @@ func getContextRoles(s Session, res Resource, preloadedRoles []*Role) (out partR scope = make(map[string]interface{}) ) + out = [roleKinds]map[uint64]bool{} + + { + // if this is an anonymous user (has one role of kind anonymous) + // ensure role-kind integrity and skip complex checks + for _, r := range preloadedRoles { + if !mm[r.id] { + // skip roles that are not in the security session + // skip all other types of roles that user from session is not member of + continue + } + + if r.kind != AnonymousRole { + continue + } + + if out[AnonymousRole] == nil { + out[AnonymousRole] = make(map[uint64]bool) + } + + out[AnonymousRole][r.id] = true + } + + if len(out[AnonymousRole]) > 0 { + return + } + } + if d, ok := res.(resourceDicter); ok { // if resource implements Dict() fn, we can use it to // collect attributes, used for expression evaluation and contextual role gathering @@ -129,7 +157,6 @@ func getContextRoles(s Session, res Resource, preloadedRoles []*Role) (out partR scope["userID"] = s.Identity() - out = [roleKinds]map[uint64]bool{} for _, r := range preloadedRoles { if r.kind == ContextRole { if len(r.crtypes) == 0 || !r.crtypes[ResourceType(res.RbacResource())] { diff --git a/pkg/rbac/roles_test.go b/pkg/rbac/roles_test.go index 5ab539179..f6172cafd 100644 --- a/pkg/rbac/roles_test.go +++ b/pkg/rbac/roles_test.go @@ -76,6 +76,18 @@ func Test_getContextRoles(t *testing.T) { }, []*Role{{id: 1, kind: BypassRole}, {id: 2, kind: ContextRole}}, }, + { + "anonymous role mixed with matching contextual", + []uint64{1, 2}, + tres, + []*Role{ + {id: 1, kind: AnonymousRole}, + {id: 2, kind: ContextRole, check: dyCheck(true), crtypes: map[string]bool{tres.RbacResource(): true}}, + {id: 3, kind: ContextRole, check: dyCheck(false), crtypes: map[string]bool{tres.RbacResource(): true}}, + {id: 4, kind: ContextRole, check: dyCheck(true)}, + }, + []*Role{{id: 1, kind: AnonymousRole}}, + }, } ) diff --git a/pkg/rbac/ruleset_checks.go b/pkg/rbac/ruleset_checks.go index 2c4956e78..cf4a64038 100644 --- a/pkg/rbac/ruleset_checks.go +++ b/pkg/rbac/ruleset_checks.go @@ -4,72 +4,120 @@ import ( "sort" ) -func check(indexedRules OptRuleSet, rolesByKind partRoles, op, res string) Access { - a, _, _ := evaluate(indexedRules, rolesByKind, op, res, false) - return a -} +// function checks all given rules +// +// - indexRules are rules optimized for quick lookup rules ar grouped in 2 levels: +// by operation, and role, containing slice of rules (for specific operation and role) +// at the deepest level +// +// - rolesByKind are roles optimized for quick lookup +// roles are grouped by kind and each kind contains fast-lookup (map[role-id]bool) +// +// - op and res represent operation and resource that are checked +// +// - trace is optional; when not nil, function will update trace stuct +// with information as it traverses and checks the rules +// +func check(indexedRules OptRuleSet, rolesByKind partRoles, op, res string, trace *Trace) Access { + baseTraceInfo(trace, res, op, rolesByKind) -func evaluate(indexedRules OptRuleSet, rolesByKind partRoles, op, res string, parent bool) (Access, *Rule, explanation) { if member(rolesByKind, AnonymousRole) && len(rolesByKind) > 1 { // Integrity check; when user is member of anonymous role // should not be member of any other type of role - - return Deny, nil, stepIntegrity + return resolve(trace, Deny, failedIntegrityCheck) } if member(rolesByKind, BypassRole) { // if user has at least one bypass role, we allow access - return Allow, nil, stepBypass + return resolve(trace, Allow, bypassRoleMembership) } if len(indexedRules) == 0 { - // no rules no access - return Inherit, nil, stepRuleless + // no rules to check + return resolve(trace, Inherit, noRules) } - var rules RuleSet + var ( + match *Rule + allowed bool + ) + + // + if trace != nil { + // from this point on, there is a chance trace (if set) + // will contain some rules. + // + // Stable order needs to be ensured: there is no production + // code that relies on that but tests might fail and API + // response would be flaky. + defer sortTraceRules(trace) + } // Priority is important here. We want to have // stable RBAC check behaviour and ability // to override allow/deny depending on how niche the role (type) is: // - context (eg owners) are more niche than common - // - rules for common roles are more important than + // - rules for common roles are more important than authenticated and anonymous role types + // + // Note that bypass roles are intentionally ignored here; if user is member of + // bypass role there is no need to check any other rule for _, kind := range []roleKind{ContextRole, CommonRole, AuthenticatedRole, AnonymousRole} { - // no roles if this kind + // not a member of any role of this kind if len(rolesByKind[kind]) == 0 { continue } - rules = nil - for roleID, r := range indexedRules[op] { + // reset allowed to false + // for each role kind + allowed = false + + for roleID, rr := range indexedRules[op] { if !rolesByKind[kind][roleID] { continue } - rules = append(rules, r...) - } - // When evaluating access for parent, omit the exact tule - if parent { - nr := make(RuleSet, 0, len(rules)) - for _, r := range rules { - if r.Resource != res { - nr = append(nr, r) - } + if len(rr) == 0 { + // no rules found + continue + } + + // check all rules for each role the security-context + if match = findRuleByResOp(rr, op, res); match.Access == Inherit { + // no rules match + continue + } + + if trace != nil { + // if trace is enabled, append + // each matching rule + trace.Rules = append(trace.Rules, match) + } + + if match.Access == Deny { + // if we stumble upon Deny we short-circuit the check + return resolve(trace, Deny, "") + } + + if match.Access == Allow { + // allow rule found, we need to check rules on other roles + // before we allow it + allowed = true } - rules = nr } - r, access := checkRulesByResource(rules, op, res) - if access != Inherit { - return access, r, stepEvaluated + if allowed { + // at least one of the roles (per role type) in the security context + // allows operation on a resource + return resolve(trace, Allow, "") } } - return Inherit, nil, stepRuleless + // No rule matched + return resolve(trace, Inherit, noMatch) } // Check given resource match and operation on all given rules -func checkRulesByResource(set RuleSet, op, res string) (*Rule, Access) { +func findRuleByResOp(set RuleSet, op, res string) *Rule { // Make sure rules are always sorted (by level) // to avoid any kind of unstable behaviour sort.Sort(set) @@ -84,11 +132,11 @@ func checkRulesByResource(set RuleSet, op, res string) (*Rule, Access) { } if r.Access != Inherit { - return r, r.Access + return r } } - return nil, Inherit + return &Rule{Access: Inherit} } // at least one of the roles must be set to true diff --git a/pkg/rbac/ruleset_checks_test.go b/pkg/rbac/ruleset_checks_test.go index 398206231..d52a7018d 100644 --- a/pkg/rbac/ruleset_checks_test.go +++ b/pkg/rbac/ruleset_checks_test.go @@ -48,7 +48,6 @@ func Test_check(t *testing.T) { "", "", []*Role{ - {id: 1, kind: CommonRole}, }, []*Rule{ @@ -56,6 +55,34 @@ func Test_check(t *testing.T) { {RoleID: 2, Access: Deny}, }, }, + { + "multiple matching roles of same kind with deny", + Deny, + "", + "", + []*Role{ + {id: 1, kind: CommonRole}, + {id: 2, kind: CommonRole}, + }, + []*Rule{ + {RoleID: 1, Access: Allow}, + {RoleID: 2, Access: Deny}, + }, + }, + { + "multiple matching matching roles of different with deny last", + Allow, + "", + "", + []*Role{ + {id: 1, kind: CommonRole}, + {id: 2, kind: AuthenticatedRole}, + }, + []*Rule{ + {RoleID: 1, Access: Allow}, + {RoleID: 2, Access: Deny}, + }, + }, { "complex inheritance", Deny, @@ -77,17 +104,147 @@ func Test_check(t *testing.T) { for _, c := range cc { t.Run(c.name, func(t *testing.T) { - require.Equal(t, c.exp.String(), check(indexRules(c.set), partitionRoles(c.rr...), c.op, c.res).String()) + require.Equal(t, c.exp.String(), check(indexRules(c.set), partitionRoles(c.rr...), c.op, c.res, nil).String()) + }) + } +} + +func Test_checkWithTrace(t *testing.T) { + var ( + trace *Trace + + cc = []struct { + name string + res string + exp Access + rr []*Role + set RuleSet + trace *Trace + }{ + { + "fail on integrity check (multiple anonymous roles)", + "res-trace", + Deny, + []*Role{ + {id: 1, kind: AnonymousRole}, + {id: 2, kind: CommonRole}, + {id: 3, kind: CommonRole}, + }, + nil, + &Trace{ + Resource: "res-trace", + Operation: "op-trace", + Access: Deny, + Roles: []uint64{1, 2, 3}, + Rules: nil, + Resolution: failedIntegrityCheck, + }, + }, + { + "allow when checking with bypass roles", + "res-trace", + Allow, + []*Role{ + {id: 1, kind: BypassRole}, + }, + nil, + &Trace{ + Resource: "res-trace", + Operation: "op-trace", + Access: Allow, + Roles: []uint64{1}, + Rules: nil, + Resolution: bypassRoleMembership, + }, + }, + { + "no rules", + "res-trace", + Allow, + []*Role{ + {id: 1, kind: CommonRole}, + }, + nil, + &Trace{ + Resource: "res-trace", + Operation: "op-trace", + Access: Inherit, + Roles: []uint64{1}, + Rules: nil, + Resolution: noRules, + }, + }, + { + "multi-role", + "res-trace", + Allow, + []*Role{ + {id: 1, kind: CommonRole}, + {id: 2, kind: CommonRole}, + {id: 3, kind: AuthenticatedRole}, + }, + RuleSet{ + AllowRule(1, "res-trace", "op-trace"), + AllowRule(2, "res-trace", "op-trace"), + AllowRule(3, "res-trace", "op-trace"), + AllowRule(1, "res-trace-2", "op-trace"), + AllowRule(2, "res-trace", "op-trace-2"), + }, + &Trace{ + Resource: "res-trace", + Operation: "op-trace", + Access: Allow, + Roles: []uint64{1, 2, 3}, + Rules: RuleSet{ + AllowRule(1, "res-trace", "op-trace"), + AllowRule(2, "res-trace", "op-trace"), + }, + }, + }, + { + "nested resource", + "res-trace/2", + Allow, + []*Role{ + {id: 1, kind: CommonRole}, + {id: 2, kind: CommonRole}, + {id: 3, kind: AuthenticatedRole}, + }, + RuleSet{ + AllowRule(1, "res-trace/*", "op-trace"), + AllowRule(2, "res-trace/*", "op-trace"), + AllowRule(2, "res-trace/1", "op-trace"), + }, + &Trace{ + Resource: "res-trace/2", + Operation: "op-trace", + Access: Allow, + Roles: []uint64{1, 2, 3}, + Rules: RuleSet{ + AllowRule(1, "res-trace/*", "op-trace"), + AllowRule(2, "res-trace/*", "op-trace"), + }, + }, + }, + } + ) + + for _, c := range cc { + t.Run(c.name, func(t *testing.T) { + trace = new(Trace) + check(indexRules(c.set), partitionRoles(c.rr...), "op-trace", c.res, trace) + require.Equal(t, c.trace, trace) + }) } } //cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz -//Benchmark_Check100-16 15626196 88.85 ns/op -//Benchmark_Check1000-16 15976252 74.09 ns/op -//Benchmark_Check10000-16 15025586 78.12 ns/op -//Benchmark_Check100000-16 13760616 84.70 ns/op -//Benchmark_Check1000000-16 2602420 415.8 ns/op +//Benchmark_Check100-16 12395438 95.24 ns/op +//Benchmark_Check1000-16 12507883 96.34 ns/op +//Benchmark_Check10000-16 11788594 96.85 ns/op +//Benchmark_Check100000-16 11679951 100.1 ns/op +//Benchmark_Check1000000-16 4670353 287.3 ns/op func benchmarkCheck(b *testing.B, c int) { var ( // resting with 50 roles @@ -117,7 +274,7 @@ func benchmarkCheck(b *testing.B, c int) { b.StartTimer() for n := 0; n < b.N; n++ { - check(iRules, pr, "res-0", "op-0") + check(iRules, pr, "res-0", "op-0", nil) } b.StopTimer() @@ -155,8 +312,8 @@ func Test_checkRulesByResource(t *testing.T) { for _, c := range cc { t.Run(c.res, func(t *testing.T) { - _, a := checkRulesByResource(c.set, c.op, c.res) - require.Equal(t, c.exp.String(), a.String()) + a := findRuleByResOp(c.set, c.op, c.res) + require.Equal(t, c.exp.String(), a.Access.String()) }) } } diff --git a/pkg/rbac/service.go b/pkg/rbac/service.go index 876fdd346..f3da56bd7 100644 --- a/pkg/rbac/service.go +++ b/pkg/rbac/service.go @@ -92,13 +92,14 @@ func (svc *service) Can(ses Session, op string, res Resource) bool { // Check verifies if role has access to perform an operation on a resource // // See RuleSet's Check() func for details -func (svc *service) Check(ses Session, op string, res Resource) (v Access) { +func (svc *service) Check(ses Session, op string, res Resource) (a Access) { var ( - eval, fRoles = svc.evaluate(ses, op, res, false) - access = eval.Access + fRoles = getContextRoles(ses, res, svc.roles) ) - svc.logger.Debug(access.String()+" "+op+" for "+res.RbacResource(), + a = check(svc.indexed, fRoles, op, res.RbacResource(), nil) + + svc.logger.Debug(a.String()+" "+op+" for "+res.RbacResource(), append( fRoles.LogFields(), zap.Uint64("identity", ses.Identity()), @@ -107,69 +108,19 @@ func (svc *service) Check(ses Session, op string, res Resource) (v Access) { )..., ) - return access + return } -// Evaluate access for the given session, operation and resource -// -// The evaluation outputs verbose details to assist the UI. -func (svc *service) Evaluate(ses Session, op string, res Resource) Evaluated { +// Trace checks RBAC rules and returns all decision trace log +func (svc *service) Trace(ses Session, op string, res Resource) *Trace { var ( - eval, fRoles = svc.evaluate(ses, op, res, true) + t = new(Trace) + fRoles = getContextRoles(ses, res, svc.roles) ) - svc.logger.Debug(eval.Access.String()+" "+op+" for "+res.RbacResource(), - append( - fRoles.LogFields(), - zap.Uint64("identity", ses.Identity()), - zap.Any("indexed", len(svc.indexed)), - zap.Any("rules", len(svc.rules)), - )..., - ) + _ = check(svc.indexed, fRoles, op, res.RbacResource(), t) - return eval -} - -func (svc *service) evaluate(ses Session, op string, res Resource, inclParent bool) (e Evaluated, _ partRoles) { - svc.l.RLock() - defer svc.l.RUnlock() - - var ( - fRoles = getContextRoles(ses, res, svc.roles) - access, rule, expl = evaluate(svc.indexed, fRoles, op, res.RbacResource(), false) - ) - - // Check the requested resource - e = Evaluated{ - Resource: res.RbacResource(), - Operation: op, - - Access: access, - Can: access == Allow, - Rule: rule, - Step: expl, - } - if rule != nil { - e.RoleID = rule.RoleID - } - - // Check the parent resource - if inclParent { - access, rule, expl = evaluate(svc.indexed, fRoles, op, res.RbacResource(), true) - e.Default = &Evaluated{ - Resource: res.RbacResource(), - Operation: op, - Access: access, - Can: access == Allow, - Rule: rule, - Step: expl, - } - if rule != nil { - e.Default.RoleID = rule.RoleID - } - } - - return e, fRoles + return t } // Grant appends and/or overwrites internal rules slice diff --git a/pkg/rbac/service_alt.go b/pkg/rbac/service_alt.go index 958c7e8b2..5fe895009 100644 --- a/pkg/rbac/service_alt.go +++ b/pkg/rbac/service_alt.go @@ -25,8 +25,8 @@ func (ServiceAllowAll) Grant(context.Context, ...*Rule) error { return nil } -func (ServiceAllowAll) Evaluate(Session, string, Resource) Evaluated { - return Evaluated{Access: Allow, Can: true} +func (ServiceAllowAll) Trace(Session, string, Resource) *Trace { + return nil } func (ServiceAllowAll) CloneRulesByRoleID(context.Context, uint64, ...uint64) error { diff --git a/pkg/rbac/trace.go b/pkg/rbac/trace.go new file mode 100644 index 000000000..c8e95bee8 --- /dev/null +++ b/pkg/rbac/trace.go @@ -0,0 +1,74 @@ +package rbac + +import "sort" + +type ( + resolution string + + Trace struct { + Resource string `json:"resource"` + Operation string `json:"operation"` + Access Access `json:"access"` + Roles []uint64 `json:"roles"` + Rules []*Rule `json:"rules,omitempty"` + Resolution resolution `json:"resolution,omitempty"` + } +) + +const ( + failedIntegrityCheck resolution = "failed-integrity-check" + bypassRoleMembership resolution = "bypass-role-membership" + noRules resolution = "no-rules" + noMatch resolution = "no-match" +) + +// baseTraceInfo updates given check trace struct +// +// If Trace is nil, function terminates early and does not panic +func baseTraceInfo(t *Trace, res, op string, rolesByKind partRoles) { + if t == nil { + return + } + + t.Resource = res + t.Operation = op + + for _, rr := range rolesByKind { + for r := range rr { + t.Roles = append(t.Roles, r) + } + } + + sort.Slice(t.Roles, func(i, j int) bool { + return t.Roles[i] < t.Roles[j] + }) +} + +// resolve updates access and resolution and returns access info unmodified +// if nil is passed for Trace, function silently ignores that +func resolve(t *Trace, access Access, res resolution) Access { + if t != nil { + t.Access = access + t.Resolution = res + } + + return access +} + +// ensure stable order of trace rules +func sortTraceRules(trace *Trace) { + // using custom sorting (and not RulSet implementation) because + // we might have multiple roles per resource. + sort.Slice(trace.Rules, func(i, j int) bool { + var ( + rli = level(trace.Rules[i].Resource) + rlj = level(trace.Rules[j].Resource) + ) + + if rli != rlj { + return rli < rlj + } + + return trace.Rules[i].RoleID < trace.Rules[j].RoleID + }) +} diff --git a/pkg/rbac/trace_test.go b/pkg/rbac/trace_test.go new file mode 100644 index 000000000..e7b309712 --- /dev/null +++ b/pkg/rbac/trace_test.go @@ -0,0 +1,60 @@ +package rbac + +import ( + "testing" +) + +func TestEvaluate(t *testing.T) { + //var ( + // rC1 = &Role{id: 1, kind: CommonRole} + // rAnon = &Role{id: 100, kind: AnonymousRole} + // rAuth = &Role{id: 200, kind: AuthenticatedRole} + // rCtx1 = &Role{id: 300, kind: ContextRole} + // rBps1 = &Role{id: 400, kind: BypassRole} + // + // allRoles = []*Role{rC1, rAnon, rAuth, rCtx1, rBps1} + // _ = allRoles + // + // res1 = "corteza::test:resource1" + // + // tcc = []struct { + // name string + // op string + // res string + // should Access + // roles []*Role + // rules RuleSet + // }{ + // { + // name: "base", + // op: "READ", + // res: res1, + // roles: []*Role{rAuth}, + // rules: RuleSet{ + // &Rule{RoleID: rAuth.id, Resource: res1, Operation: "READ", Access: Allow}, + // }, + // + // should: Allow, + // }, + // } + //) + // + //for _, tc := range tcc { + // t.Run(tc.name, func(t *testing.T) { + // var ( + // req = require.New(t) + // + // partitionedRoles = partitionRoles(tc.roles...) + // indexedRules = indexRules(tc.rules) + // + // _access, _rule, _expl = evaluate(indexedRules, partitionedRoles, tc.op, tc.res, false) + // ) + // + // _ = _access + // _ = _rule + // _ = _expl + // + // req.Equal(tc.should, _access) + // }) + //} +} diff --git a/system/rest.yaml b/system/rest.yaml index 6c5819f70..5002ae02e 100644 --- a/system/rest.yaml +++ b/system/rest.yaml @@ -1236,8 +1236,8 @@ endpoints: type: string required: false title: Show only rules for a specific resource - - name: evaluate - path: "/evaluate" + - name: trace + path: "/trace" method: GET title: Evaluate rules for given user/role combo parameters: diff --git a/system/rest/handlers/permissions.go b/system/rest/handlers/permissions.go index ac002d827..097738266 100644 --- a/system/rest/handlers/permissions.go +++ b/system/rest/handlers/permissions.go @@ -21,7 +21,7 @@ type ( PermissionsAPI interface { List(context.Context, *request.PermissionsList) (interface{}, error) Effective(context.Context, *request.PermissionsEffective) (interface{}, error) - Evaluate(context.Context, *request.PermissionsEvaluate) (interface{}, error) + Trace(context.Context, *request.PermissionsTrace) (interface{}, error) Read(context.Context, *request.PermissionsRead) (interface{}, error) Delete(context.Context, *request.PermissionsDelete) (interface{}, error) Update(context.Context, *request.PermissionsUpdate) (interface{}, error) @@ -31,7 +31,7 @@ type ( Permissions struct { List func(http.ResponseWriter, *http.Request) Effective func(http.ResponseWriter, *http.Request) - Evaluate func(http.ResponseWriter, *http.Request) + Trace func(http.ResponseWriter, *http.Request) Read func(http.ResponseWriter, *http.Request) Delete func(http.ResponseWriter, *http.Request) Update func(http.ResponseWriter, *http.Request) @@ -72,15 +72,15 @@ func NewPermissions(h PermissionsAPI) *Permissions { api.Send(w, r, value) }, - Evaluate: func(w http.ResponseWriter, r *http.Request) { + Trace: func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() - params := request.NewPermissionsEvaluate() + params := request.NewPermissionsTrace() if err := params.Fill(r); err != nil { api.Send(w, r, err) return } - value, err := h.Evaluate(r.Context(), params) + value, err := h.Trace(r.Context(), params) if err != nil { api.Send(w, r, err) return @@ -144,7 +144,7 @@ func (h Permissions) MountRoutes(r chi.Router, middlewares ...func(http.Handler) r.Use(middlewares...) r.Get("/permissions/", h.List) r.Get("/permissions/effective", h.Effective) - r.Get("/permissions/evaluate", h.Evaluate) + r.Get("/permissions/trace", h.Trace) r.Get("/permissions/{roleID}/rules", h.Read) r.Delete("/permissions/{roleID}/rules", h.Delete) r.Patch("/permissions/{roleID}/rules", h.Update) diff --git a/system/rest/permissions.go b/system/rest/permissions.go index 5d4a19944..c7a2ca0ca 100644 --- a/system/rest/permissions.go +++ b/system/rest/permissions.go @@ -17,7 +17,7 @@ type ( permissionsAccessController interface { Effective(context.Context, ...rbac.Resource) rbac.EffectiveSet - Evaluate(ctx context.Context, user uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) + Trace(context.Context, uint64, []uint64, ...string) ([]*rbac.Trace, error) List() []map[string]string FindRulesByRoleID(context.Context, uint64) (rbac.RuleSet, error) Grant(ctx context.Context, rr ...*rbac.Rule) error @@ -34,8 +34,8 @@ func (ctrl Permissions) Effective(ctx context.Context, r *request.PermissionsEff return ctrl.ac.Effective(ctx, types.Component{}), nil } -func (ctrl Permissions) Evaluate(ctx context.Context, r *request.PermissionsEvaluate) (interface{}, error) { - return ctrl.ac.Evaluate(ctx, r.UserID, r.RoleID, r.Resource...) +func (ctrl Permissions) Trace(ctx context.Context, r *request.PermissionsTrace) (interface{}, error) { + return ctrl.ac.Trace(ctx, r.UserID, r.RoleID, r.Resource...) } func (ctrl Permissions) List(ctx context.Context, r *request.PermissionsList) (interface{}, error) { diff --git a/system/rest/request/permissions.go b/system/rest/request/permissions.go index 12b760af9..a2cc3cd54 100644 --- a/system/rest/request/permissions.go +++ b/system/rest/request/permissions.go @@ -44,7 +44,7 @@ type ( Resource string } - PermissionsEvaluate struct { + PermissionsTrace struct { // Resource GET parameter // // Show only rules for a specific resource @@ -139,13 +139,13 @@ func (r *PermissionsEffective) Fill(req *http.Request) (err error) { return err } -// NewPermissionsEvaluate request -func NewPermissionsEvaluate() *PermissionsEvaluate { - return &PermissionsEvaluate{} +// NewPermissionsTrace request +func NewPermissionsTrace() *PermissionsTrace { + return &PermissionsTrace{} } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) Auditable() map[string]interface{} { +func (r PermissionsTrace) Auditable() map[string]interface{} { return map[string]interface{}{ "resource": r.Resource, "userID": r.UserID, @@ -154,22 +154,22 @@ func (r PermissionsEvaluate) Auditable() map[string]interface{} { } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetResource() []string { +func (r PermissionsTrace) GetResource() []string { return r.Resource } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetUserID() uint64 { +func (r PermissionsTrace) GetUserID() uint64 { return r.UserID } // Auditable returns all auditable/loggable parameters -func (r PermissionsEvaluate) GetRoleID() []uint64 { +func (r PermissionsTrace) GetRoleID() []uint64 { return r.RoleID } // Fill processes request and fills internal variables -func (r *PermissionsEvaluate) Fill(req *http.Request) (err error) { +func (r *PermissionsTrace) Fill(req *http.Request) (err error) { { // GET params diff --git a/system/service/access_control.gen.go b/system/service/access_control.gen.go index 0896ddb7a..1c073fcd3 100644 --- a/system/service/access_control.gen.go +++ b/system/service/access_control.gen.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "github.com/cortezaproject/corteza-server/pkg/actionlog" + internalAuth "github.com/cortezaproject/corteza-server/pkg/auth" "github.com/cortezaproject/corteza-server/pkg/rbac" "github.com/cortezaproject/corteza-server/system/types" systemTypes "github.com/cortezaproject/corteza-server/system/types" @@ -23,7 +24,8 @@ type ( } rbacService interface { - Evaluate(rbac.Session, string, rbac.Resource) rbac.Evaluated + Can(rbac.Session, string, rbac.Resource) bool + Trace(rbac.Session, string, rbac.Resource) *rbac.Trace Grant(context.Context, ...*rbac.Rule) error FindRulesByRoleID(roleID uint64) (rr rbac.RuleSet) } @@ -45,7 +47,7 @@ func AccessControl(rms roleMemberSearcher) *accessControl { } func (svc accessControl) can(ctx context.Context, op string, res rbac.Resource) bool { - return svc.rbac.Evaluate(rbac.ContextToSession(ctx), op, res).Can + return svc.rbac.Can(rbac.ContextToSession(ctx), op, res) } // Effective returns a list of effective permissions for all given resource @@ -65,7 +67,7 @@ func (svc accessControl) Effective(ctx context.Context, rr ...rbac.Resource) (ee // Evaluate returns a list of permissions evaluated for the given user/roles combo // // This function is auto-generated -func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee rbac.EvaluatedSet, err error) { +func (svc accessControl) Trace(ctx context.Context, userID uint64, roles []uint64, rr ...string) (ee []*rbac.Trace, err error) { // Reusing the grant permission since this is who the feature is for if !svc.CanGrant(ctx) { // @todo should be altered to check grant permissions PER resource @@ -89,7 +91,7 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui if userID != 0 { if len(roles) > 0 { // should be prevented on the client - return nil, fmt.Errorf("userID and roles are mutually exclusive") + return nil, fmt.Errorf("userID and roles parameters are mutually exclusive") } members, _, err = svc.store.SearchRoleMembers(ctx, systemTypes.RoleMemberFilter{UserID: userID}) @@ -100,6 +102,11 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui for _, m := range members { roles = append(roles, m.RoleID) } + + // make sure we append all "authenticated" roles + for _, r := range internalAuth.AuthenticatedRoles() { + roles = append(roles, r.ID) + } } if len(roles) == 0 { @@ -111,7 +118,7 @@ func (svc accessControl) Evaluate(ctx context.Context, userID uint64, roles []ui for _, res := range resources { r := res.RbacResource() for op := range rbacResourceOperations(r) { - ee = append(ee, svc.rbac.Evaluate(session, op, res)) + ee = append(ee, svc.rbac.Trace(session, op, res)) } } diff --git a/system/service/role.go b/system/service/role.go index bc50667cc..b438fae0a 100644 --- a/system/service/role.go +++ b/system/service/role.go @@ -979,18 +979,3 @@ func UpdateRbacRoles(ctx context.Context, log *zap.Logger, ru rbacRoleUpdater, b ru.UpdateRoles(rr...) return nil } - -func RolesForUser(s store.Storer) func(ctx context.Context, userID uint64) ([]uint64, error) { - return func(ctx context.Context, userID uint64) ([]uint64, error) { - rr, _, err := store.SearchRoles(ctx, s, types.RoleFilter{MemberID: userID}) - if err != nil { - return nil, err - } - - out := make([]uint64, len(rr)) - for i, r := range rr { - out[i] = r.ID - } - return out, nil - } -} diff --git a/tests/automation/permissions_test.go b/tests/automation/permissions_test.go index e4641449f..232cd2892 100644 --- a/tests/automation/permissions_test.go +++ b/tests/automation/permissions_test.go @@ -97,3 +97,19 @@ func TestPermissionsDelete(t *testing.T) { h.a.True(r.Access == rbac.Inherit) } } + +func TestPermissionsTrace(t *testing.T) { + h := newHelper(t) + + helpers.AllowMe(h, types.ComponentRbacResource(), "grant") + + h.apiInit(). + Get("/permissions/trace"). + Query("roleID[]", "1"). + Header("Accept", "application/json"). + Expect(t). + Status(http.StatusOK). + Assert(helpers.AssertNoErrors). + Assert(jsonpath.Present(`$.response`)). + End() +} diff --git a/tests/compose/permissions_test.go b/tests/compose/permissions_test.go index 500bcd40a..108917332 100644 --- a/tests/compose/permissions_test.go +++ b/tests/compose/permissions_test.go @@ -103,3 +103,19 @@ func TestPermissionsDelete(t *testing.T) { h.a.True(r.Access == rbac.Inherit) } } + +func TestPermissionsTrace(t *testing.T) { + h := newHelper(t) + + helpers.AllowMe(h, types.ComponentRbacResource(), "grant") + + h.apiInit(). + Get("/permissions/trace"). + Query("roleID[]", "1"). + Header("Accept", "application/json"). + Expect(t). + Status(http.StatusOK). + Assert(helpers.AssertNoErrors). + Assert(jsonpath.Present(`$.response`)). + End() +} diff --git a/tests/system/permissions_test.go b/tests/system/permissions_test.go index 30e8cba82..e079e7e0a 100644 --- a/tests/system/permissions_test.go +++ b/tests/system/permissions_test.go @@ -99,6 +99,22 @@ func TestPermissionsDelete(t *testing.T) { } } +func TestPermissionsTrace(t *testing.T) { + h := newHelper(t) + + helpers.AllowMe(h, types.ComponentRbacResource(), "grant") + + h.apiInit(). + Get("/permissions/trace"). + Query("roleID[]", "1"). + Header("Accept", "application/json"). + Expect(t). + Status(http.StatusOK). + Assert(helpers.AssertNoErrors). + Assert(jsonpath.Present(`$.response`)). + End() +} + func TestPermissionsCloneToSingleRole(t *testing.T) { h := newHelper(t) p := rbac.Global()