From d14b26923eedaa580c2372e66b41a4e7b4add1c8 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Sun, 6 Sep 2020 16:41:17 +0200 Subject: [PATCH] Remove (repository-layer) resource filtering --- compose/service/access_control.go | 35 -------- compose/service/chart.go | 3 - compose/service/module.go | 3 - compose/service/namespace.go | 2 - compose/service/page.go | 3 - messaging/service/access_control.go | 17 ---- pkg/permissions/filter.go | 128 ---------------------------- pkg/permissions/filter_test.go | 51 ----------- pkg/permissions/service.go | 17 ---- pkg/permissions/service_alt.go | 4 - system/service/access_control.go | 37 -------- system/service/application.go | 3 - system/service/role.go | 2 - system/service/user.go | 5 -- 14 files changed, 310 deletions(-) delete mode 100644 pkg/permissions/filter.go delete mode 100644 pkg/permissions/filter_test.go diff --git a/compose/service/access_control.go b/compose/service/access_control.go index 1dfdf07d9..9d5ba8670 100644 --- a/compose/service/access_control.go +++ b/compose/service/access_control.go @@ -18,7 +18,6 @@ type ( Can([]uint64, permissions.Resource, permissions.Operation, ...permissions.CheckAccessFunc) bool Grant(context.Context, permissions.Whitelist, ...*permissions.Rule) error FindRulesByRoleID(roleID uint64) (rr permissions.RuleSet) - ResourceFilter([]uint64, permissions.Resource, permissions.Operation, permissions.Access) *permissions.ResourceFilter } secureResource interface { @@ -71,10 +70,6 @@ func (svc accessControl) CanReadNamespace(ctx context.Context, r *types.Namespac return svc.can(ctx, r, "read", permissions.Allowed) } -func (svc accessControl) FilterReadableNamespaces(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.NamespacePermissionResource, "read", permissions.Deny) -} - func (svc accessControl) CanUpdateNamespace(ctx context.Context, r *types.Namespace) bool { return svc.can(ctx, r, "update") } @@ -95,10 +90,6 @@ func (svc accessControl) CanReadModule(ctx context.Context, r *types.Module) boo return svc.can(ctx, r, "read") } -func (svc accessControl) FilterReadableModules(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.ModulePermissionResource, "read", permissions.Deny) -} - func (svc accessControl) CanUpdateModule(ctx context.Context, r *types.Module) bool { return svc.can(ctx, r, "update") } @@ -143,10 +134,6 @@ func (svc accessControl) CanReadChart(ctx context.Context, r *types.Chart) bool return svc.can(ctx, r, "read") } -func (svc accessControl) FilterReadableCharts(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.ChartPermissionResource, "read", permissions.Deny) -} - func (svc accessControl) CanUpdateChart(ctx context.Context, r *types.Chart) bool { return svc.can(ctx, r, "update") } @@ -163,10 +150,6 @@ func (svc accessControl) CanReadPage(ctx context.Context, r *types.Page) bool { return svc.can(ctx, r, "read") } -func (svc accessControl) FilterReadablePages(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.PagePermissionResource, "read", permissions.Deny) -} - func (svc accessControl) CanUpdatePage(ctx context.Context, r *types.Page) bool { return svc.can(ctx, r, "update") } @@ -193,24 +176,6 @@ func (svc accessControl) can(ctx context.Context, res secureResource, op permiss ) } -func (svc accessControl) filter(ctx context.Context, res permissions.Resource, op permissions.Operation, a permissions.Access) *permissions.ResourceFilter { - var u = auth.GetIdentityFromContext(ctx) - - if auth.IsSuperUser(u) { - // Temp solution to allow migration from passing context to ResourceFilter - //and checking "superuser" privileges there - // to more sustainable solution (eg: creating super-role with allow-all) - return permissions.NewSuperuserFilter() - } - - return svc.permissions.ResourceFilter( - append(u.Roles(), res.DynamicRoles(u.Identity())...), - res, - op, - a, - ) -} - func (svc accessControl) Grant(ctx context.Context, rr ...*permissions.Rule) error { if !svc.CanGrant(ctx) { return AccessControlErrNotAllowedToSetPermissions() diff --git a/compose/service/chart.go b/compose/service/chart.go index 1c818720e..3fc31cf1f 100644 --- a/compose/service/chart.go +++ b/compose/service/chart.go @@ -7,7 +7,6 @@ import ( "github.com/cortezaproject/corteza-server/pkg/actionlog" "github.com/cortezaproject/corteza-server/pkg/handle" "github.com/cortezaproject/corteza-server/pkg/id" - "github.com/cortezaproject/corteza-server/pkg/permissions" "github.com/cortezaproject/corteza-server/store" ) @@ -25,8 +24,6 @@ type ( CanReadChart(context.Context, *types.Chart) bool CanUpdateChart(context.Context, *types.Chart) bool CanDeleteChart(context.Context, *types.Chart) bool - - FilterReadableCharts(ctx context.Context) *permissions.ResourceFilter } ChartService interface { diff --git a/compose/service/module.go b/compose/service/module.go index 42904d4e9..c287a6aa2 100644 --- a/compose/service/module.go +++ b/compose/service/module.go @@ -10,7 +10,6 @@ import ( "github.com/cortezaproject/corteza-server/pkg/eventbus" "github.com/cortezaproject/corteza-server/pkg/handle" "github.com/cortezaproject/corteza-server/pkg/id" - "github.com/cortezaproject/corteza-server/pkg/permissions" "github.com/cortezaproject/corteza-server/store" "sort" "strconv" @@ -31,8 +30,6 @@ type ( CanReadModule(context.Context, *types.Module) bool CanUpdateModule(context.Context, *types.Module) bool CanDeleteModule(context.Context, *types.Module) bool - - FilterReadableModules(ctx context.Context) *permissions.ResourceFilter } ModuleService interface { diff --git a/compose/service/namespace.go b/compose/service/namespace.go index 2535b6653..6ba3f8c20 100644 --- a/compose/service/namespace.go +++ b/compose/service/namespace.go @@ -29,8 +29,6 @@ type ( CanDeleteNamespace(context.Context, *types.Namespace) bool Grant(ctx context.Context, rr ...*permissions.Rule) error - - FilterReadableNamespaces(ctx context.Context) *permissions.ResourceFilter } NamespaceService interface { diff --git a/compose/service/page.go b/compose/service/page.go index 8311e54a7..cd09114d3 100644 --- a/compose/service/page.go +++ b/compose/service/page.go @@ -8,7 +8,6 @@ import ( "github.com/cortezaproject/corteza-server/pkg/actionlog" "github.com/cortezaproject/corteza-server/pkg/eventbus" "github.com/cortezaproject/corteza-server/pkg/handle" - "github.com/cortezaproject/corteza-server/pkg/permissions" "github.com/cortezaproject/corteza-server/store" ) @@ -27,8 +26,6 @@ type ( CanReadPage(context.Context, *types.Page) bool CanUpdatePage(context.Context, *types.Page) bool CanDeletePage(context.Context, *types.Page) bool - - FilterReadablePages(ctx context.Context) *permissions.ResourceFilter } PageService interface { diff --git a/messaging/service/access_control.go b/messaging/service/access_control.go index af44884e8..bf30a4c7e 100644 --- a/messaging/service/access_control.go +++ b/messaging/service/access_control.go @@ -19,7 +19,6 @@ type ( Can([]uint64, permissions.Resource, permissions.Operation, ...permissions.CheckAccessFunc) bool Grant(context.Context, permissions.Whitelist, ...*permissions.Rule) error FindRulesByRoleID(roleID uint64) (rr permissions.RuleSet) - ResourceFilter([]uint64, permissions.Resource, permissions.Operation, permissions.Access) *permissions.ResourceFilter } ) @@ -220,22 +219,6 @@ func (svc accessControl) can(ctx context.Context, res permissions.Resource, op p return svc.permissions.Can(roles, res, op, ff...) } -func (svc accessControl) filter(ctx context.Context, res permissions.Resource, op permissions.Operation, a permissions.Access) *permissions.ResourceFilter { - var ( - u = auth.GetIdentityFromContext(ctx) - roles = u.Roles() - ) - - if auth.IsSuperUser(u) { - // Temp solution to allow migration from passing context to ResourceFilter - // and checking "superuser" privileges there to more sustainable solution - // (eg: creating super-role with allow-all) - return permissions.NewSuperuserFilter() - } - - return svc.permissions.ResourceFilter(roles, res, op, a) -} - func (svc accessControl) Grant(ctx context.Context, rr ...*permissions.Rule) error { if !svc.CanGrant(ctx) { return AccessControlErrNotAllowedToSetPermissions() diff --git a/pkg/permissions/filter.go b/pkg/permissions/filter.go deleted file mode 100644 index 72084fc46..000000000 --- a/pkg/permissions/filter.go +++ /dev/null @@ -1,128 +0,0 @@ -package permissions - -import ( - "fmt" - "github.com/Masterminds/squirrel" -) - -type ( - // ResourceFilter is Helper for *Filter structs - // - // It is used to provide filtering on db level and meant to be used - // mainly for checking for read operations. - // - // It creates a complex SQL syntax for permission checking depending on the - // permissions of the user: - // - // - if user is superuser no extra checks are made, simple TRUE sql is returned - // - if user is member of one or more roles a query is assembled that checks - // for allow & deny permissions for each resource - // - if one of the roles has wildcard ALLOW / DENY rule this is then the final check - // - we check everyone role rules for each resource - // - if everyone role has wildcard ALLOW / DENY rule this is then the final check - // - fallback access check is added at the end - // - // Resulting SQL check SHOULD reflect rules check ("overall flow" in the header of - // ruleset_checks.go file) - // - ResourceFilter struct { - dbTable string - pkColName string - - resource Resource - operation Operation - - chk interface { - Check(res Resource, op Operation, roles ...uint64) (v Access) - } - - fallback Access - - superuser bool - roles []uint64 - } -) - -func NewSuperuserFilter() *ResourceFilter { - return &ResourceFilter{superuser: true} -} - -func (rf *ResourceFilter) Build(pkColName string) *ResourceFilter { - rf.pkColName = pkColName - return rf -} - -func (rf ResourceFilter) ToSql() (sql string, args []interface{}, err error) { - if rf.superuser { - return "TRUE", nil, nil - } - - // selects first rule for res+op+role - // rules are ordered by access - denies first - // end query will return 1 row with 1 column - FALSE if user has at least one DENY rule - // and TRUE if there is at least one ALLOW - // - // Final query is then wrapped in simple CASE statement that casts NULL (no rules) - // to TRUE. So: no rule == inherit - base := squirrel. - Select(fmt.Sprintf("access = %d", Allow)). - From(rf.dbTable). - Where(squirrel.Eq{"operation": rf.operation}). - Where(squirrel.Expr(fmt.Sprintf("resource = CONCAT(?, %s)", rf.pkColName), rf.resource)). - OrderBy("access"). - Limit(1) - - var ( - checks = []squirrel.Sqlizer{} - - expTRUE = squirrel.Expr("TRUE") - expFALSE = squirrel.Expr("FALSE") - - check = func(rr ...uint64) squirrel.Sqlizer { - return squirrel.And{base.Where(squirrel.Eq{"rel_role": rr})} - } - - build = func(ss ...squirrel.Sqlizer) (sql string, args []interface{}, err error) { - return squirrel.Expr("FALSE").ToSql() - // @obsolete - //return rh.SquirrelFunction("COALESCE", append(checks, ss...)...).ToSql() - } - ) - - if len(rf.roles) > 0 { - // Add per-resource check for all roles - checks = append(checks, check(rf.roles...)) - - if rf.chk != nil { - switch rf.chk.Check(rf.resource.AppendWildcard(), rf.operation, rf.roles...) { - // Explicit deny/allow on wildcard: - // Add false/true to check-list and return it immediately - case Deny: - return build(expFALSE) - case Allow: - return build(expTRUE) - } - } - } - - // Add per-resource check for Everyone - checks = append(checks, check(EveryoneRoleID)) - - if rf.chk != nil { - switch rf.chk.Check(rf.resource.AppendWildcard(), rf.operation, rf.roles...) { - // Explicit deny/allow on wildcard: - // Add false/true to check-list and return it immediately - case Deny: - return build(expFALSE) - case Allow: - return build(expTRUE) - } - } - - // Fallback access - if rf.fallback == Deny { - return build(expFALSE) - } else { - return build(expTRUE) - } -} diff --git a/pkg/permissions/filter_test.go b/pkg/permissions/filter_test.go deleted file mode 100644 index 20da90b65..000000000 --- a/pkg/permissions/filter_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package permissions - -import ( - "testing" - - "github.com/Masterminds/squirrel" - "github.com/stretchr/testify/require" -) - -func TestResourceFilter_Build(t *testing.T) { - rf := ResourceFilter{ - dbTable: "ptbl", - pkColName: "pkcol", - resource: "res:", - operation: "read", - chk: nil, - } - - req := require.New(t) - - rf.fallback = Allow - req.Equal( - `COALESCE((SELECT access = 1 FROM ptbl WHERE operation = 'read' AND resource = CONCAT('res:', pkcol) AND rel_role IN ('1') ORDER BY access LIMIT 1), TRUE)`, - squirrel.DebugSqlizer(rf), - ) - - rf.roles = []uint64{123} - req.Equal( - `COALESCE((SELECT access = 1 FROM ptbl WHERE operation = 'read' AND resource = CONCAT('res:', pkcol) AND rel_role IN ('123') ORDER BY access LIMIT 1), (SELECT access = 1 FROM ptbl WHERE operation = 'read' AND resource = CONCAT('res:', pkcol) AND rel_role IN ('1') ORDER BY access LIMIT 1), TRUE)`, - squirrel.DebugSqlizer(rf), - ) - - rf.chk = &ServiceDenyAll{} - req.Equal( - `COALESCE((SELECT access = 1 FROM ptbl WHERE operation = 'read' AND resource = CONCAT('res:', pkcol) AND rel_role IN ('123') ORDER BY access LIMIT 1), FALSE)`, - squirrel.DebugSqlizer(rf), - ) - - rf.chk = &ServiceAllowAll{} - req.Equal( - `COALESCE((SELECT access = 1 FROM ptbl WHERE operation = 'read' AND resource = CONCAT('res:', pkcol) AND rel_role IN ('123') ORDER BY access LIMIT 1), TRUE)`, - squirrel.DebugSqlizer(rf), - ) - - rf.superuser = true - req.Equal( - `TRUE`, - squirrel.DebugSqlizer(rf), - ) - -} diff --git a/pkg/permissions/service.go b/pkg/permissions/service.go index b6c8a1555..d8f4a2490 100644 --- a/pkg/permissions/service.go +++ b/pkg/permissions/service.go @@ -171,23 +171,6 @@ func (svc *service) Reload(ctx context.Context) { } } -// ResourceFilter is store helper that we use to filter resources directly in the database -// -// See ResourceFilter struct documentation for details -// -// @deprecated -func (svc *service) ResourceFilter(roles []uint64, r Resource, op Operation, fallback Access) *ResourceFilter { - return &ResourceFilter{ - roles: roles, - resource: r, - operation: op, - dbTable: "rbac_rules", - chk: svc, - fallback: fallback, - pkColName: "id", - } -} - func (svc service) flush(ctx context.Context) (err error) { d, u := svc.rules.dirty() diff --git a/pkg/permissions/service_alt.go b/pkg/permissions/service_alt.go index 6a88f28a3..1a5185e34 100644 --- a/pkg/permissions/service_alt.go +++ b/pkg/permissions/service_alt.go @@ -33,10 +33,6 @@ func (ServiceAllowAll) FindRulesByRoleID(roleID uint64) (rr RuleSet) { return } -func (ServiceAllowAll) ResourceFilter([]uint64, Resource, Operation, Access) *ResourceFilter { - return &ResourceFilter{superuser: true} -} - func (ServiceDenyAll) Can([]uint64, Resource, Operation, ...CheckAccessFunc) bool { return false } diff --git a/system/service/access_control.go b/system/service/access_control.go index ac2c9aa5f..e6cf1f196 100644 --- a/system/service/access_control.go +++ b/system/service/access_control.go @@ -20,7 +20,6 @@ type ( Can([]uint64, permissions.Resource, permissions.Operation, ...permissions.CheckAccessFunc) bool Grant(context.Context, permissions.Whitelist, ...*permissions.Rule) error FindRulesByRoleID(roleID uint64) (rr permissions.RuleSet) - ResourceFilter([]uint64, permissions.Resource, permissions.Operation, permissions.Access) *permissions.ResourceFilter } permissionResource interface { @@ -85,10 +84,6 @@ func (svc accessControl) CanReadRole(ctx context.Context, rl *types.Role) bool { return svc.can(ctx, rl.PermissionResource(), "read", permissions.Allowed) } -func (svc accessControl) FilterReadableRoles(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.RolePermissionResource, "read", permissions.Allow) -} - func (svc accessControl) CanUpdateRole(ctx context.Context, rl *types.Role) bool { if rl.ID == permissions.EveryoneRoleID { return false @@ -116,10 +111,6 @@ func (svc accessControl) CanReadApplication(ctx context.Context, app *types.Appl return svc.can(ctx, app.PermissionResource(), "read", permissions.Allowed) } -func (svc accessControl) FilterReadableApplications(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.ApplicationPermissionResource, "read", permissions.Deny) -} - func (svc accessControl) CanUpdateApplication(ctx context.Context, app *types.Application) bool { return svc.can(ctx, app.PermissionResource(), "update") } @@ -128,18 +119,6 @@ func (svc accessControl) CanDeleteApplication(ctx context.Context, app *types.Ap return svc.can(ctx, app.PermissionResource(), "delete") } -func (svc accessControl) FilterReadableUsers(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.UserPermissionResource, "read", permissions.Deny) -} - -func (svc accessControl) FilterUsersWithUnmaskableEmail(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.UserPermissionResource, "unmask.email", permissions.Deny) -} - -func (svc accessControl) FilterUsersWithUnmaskableName(ctx context.Context) *permissions.ResourceFilter { - return svc.filter(ctx, types.UserPermissionResource, "unmask.name", permissions.Deny) -} - func (svc accessControl) CanReadUser(ctx context.Context, u *types.User) bool { return svc.can(ctx, u.PermissionResource(), "read") } @@ -198,22 +177,6 @@ func (svc accessControl) can(ctx context.Context, res permissions.Resource, op p return svc.permissions.Can(roles, res.PermissionResource(), op, ff...) } -func (svc accessControl) filter(ctx context.Context, res permissions.Resource, op permissions.Operation, a permissions.Access) *permissions.ResourceFilter { - var ( - u = internalAuth.GetIdentityFromContext(ctx) - roles = u.Roles() - ) - - if internalAuth.IsSuperUser(u) { - // Temp solution to allow migration from passing context to ResourceFilter - //and checking "superuser" privileges there - // to more sustainable solution (eg: creating super-role with allow-all) - return permissions.NewSuperuserFilter() - } - - return svc.permissions.ResourceFilter(roles, res, op, a) -} - func (svc accessControl) Grant(ctx context.Context, rr ...*permissions.Rule) error { if !svc.CanGrant(ctx) { return AccessControlErrNotAllowedToSetPermissions() diff --git a/system/service/application.go b/system/service/application.go index f3e5cc7cb..4fadb6c47 100644 --- a/system/service/application.go +++ b/system/service/application.go @@ -4,7 +4,6 @@ import ( "context" "github.com/cortezaproject/corteza-server/pkg/actionlog" "github.com/cortezaproject/corteza-server/pkg/filter" - "github.com/cortezaproject/corteza-server/pkg/permissions" "github.com/cortezaproject/corteza-server/store" "github.com/cortezaproject/corteza-server/system/service/event" "github.com/cortezaproject/corteza-server/system/types" @@ -24,8 +23,6 @@ type ( CanReadApplication(context.Context, *types.Application) bool CanUpdateApplication(context.Context, *types.Application) bool CanDeleteApplication(context.Context, *types.Application) bool - - FilterReadableApplications(ctx context.Context) *permissions.ResourceFilter } ) diff --git a/system/service/role.go b/system/service/role.go index db711be1f..ac04eaeb6 100644 --- a/system/service/role.go +++ b/system/service/role.go @@ -35,8 +35,6 @@ type ( CanUpdateRole(context.Context, *types.Role) bool CanDeleteRole(context.Context, *types.Role) bool CanManageRoleMembers(context.Context, *types.Role) bool - - FilterReadableRoles(ctx context.Context) *permissions.ResourceFilter } RoleService interface { diff --git a/system/service/user.go b/system/service/user.go index 421d42610..2d8b02103 100644 --- a/system/service/user.go +++ b/system/service/user.go @@ -9,7 +9,6 @@ import ( "github.com/cortezaproject/corteza-server/pkg/filter" "github.com/cortezaproject/corteza-server/pkg/handle" "github.com/cortezaproject/corteza-server/pkg/id" - "github.com/cortezaproject/corteza-server/pkg/permissions" "github.com/cortezaproject/corteza-server/store" "github.com/cortezaproject/corteza-server/system/service/event" "github.com/cortezaproject/corteza-server/system/types" @@ -61,10 +60,6 @@ type ( CanUnsuspendUser(context.Context, *types.User) bool CanUnmaskEmail(context.Context, *types.User) bool CanUnmaskName(context.Context, *types.User) bool - - FilterReadableUsers(ctx context.Context) *permissions.ResourceFilter - FilterUsersWithUnmaskableEmail(ctx context.Context) *permissions.ResourceFilter - FilterUsersWithUnmaskableName(ctx context.Context) *permissions.ResourceFilter } // Temp types to support user.Preloader