diff --git a/crm/service/permissions.go b/crm/service/permissions.go index e38de90b9..8097d1dc8 100644 --- a/crm/service/permissions.go +++ b/crm/service/permissions.go @@ -16,9 +16,6 @@ type ( prm systemService.PermissionsService } - // Fallback option - Fallback func() bool - PermissionsService interface { With(context.Context) PermissionsService @@ -46,19 +43,10 @@ func (p *permissions) CanAccessCompose() bool { return p.checkAccess("compose", "access") } -func (p *permissions) checkAccess(resource string, operation string, fbs ...Fallback) bool { - access := p.prm.Check(resource, operation) - switch access { - case internalRules.Allow: +func (p *permissions) checkAccess(resource string, operation string, fallbacks ...internalRules.CheckAccessFunc) bool { + access := p.prm.Check(resource, operation, fallbacks...) + if access == internalRules.Allow { return true - case internalRules.Deny: - return false - default: - for _, fb := range fbs { - if fb() == true { - return true - } - } - return false } + return false } diff --git a/internal/rules/interfaces.go b/internal/rules/interfaces.go index e9fba3a4e..1ef4add14 100644 --- a/internal/rules/interfaces.go +++ b/internal/rules/interfaces.go @@ -9,7 +9,7 @@ import ( type ResourcesInterface interface { With(ctx context.Context, db *factory.DB) ResourcesInterface - Check(resource string, operation string) Access + Check(resource string, operation string, fallbacks ...CheckAccessFunc) Access Grant(roleID uint64, rules []Rule) error Read(roleID uint64) ([]Rule, error) diff --git a/internal/rules/resources.go b/internal/rules/resources.go index 85c9d71a9..d3c081f1a 100644 --- a/internal/rules/resources.go +++ b/internal/rules/resources.go @@ -10,13 +10,20 @@ import ( ) const ( - delimiter = ":" + delimiter = ":" + everyoneRoleId uint64 = 1 + defaultAccess Access = Deny ) -type resources struct { - ctx context.Context - db *factory.DB -} +type ( + resources struct { + ctx context.Context + db *factory.DB + } + + // CheckAccessFunc function. + CheckAccessFunc func() Access +) func NewResources(ctx context.Context, db *factory.DB) ResourcesInterface { return (&resources{}).With(ctx, db) @@ -35,42 +42,48 @@ func (r *resources) identity() uint64 { // IsAllowed function checks granted permission for specific resource and operation. Permission checks on // global level are not allowed and will always return Deny. -func (r *resources) Check(resource string, operation string) Access { +func (r *resources) Check(resource string, operation string, fallbacks ...CheckAccessFunc) Access { parts := strings.Split(resource, delimiter) // Permission check on global level is not allowed. - if parts[len(parts)-1] == "*" { + if parts[len(parts)-1] == "*" || parts[len(parts)-1] == "" { return Deny } - access := r.checkAccess(resource, operation) - if access == Inherit { - // Create resource definition for global level. - parts[len(parts)-1] = "*" + // Create resource definition for global level. + parts[len(parts)-1] = "*" + globalResource := strings.Join(parts, delimiter) - resource = strings.Join(parts, delimiter) - - // If rule for specific resource is not set we check on global level. - return r.checkAccess(resource, operation) + // Access checks. + checks := []CheckAccessFunc{ + func() Access { return r.checkAccess(resource, operation) }, + func() Access { return r.checkAccessEveryone(resource, operation) }, + func() Access { return r.checkAccess(globalResource, operation) }, + func() Access { return r.checkAccessEveryone(globalResource, operation) }, } - return access + checks = append(checks, fallbacks...) + + for _, check := range checks { + if access := check(); access != Inherit { + return access + } + } + return defaultAccess } func (r *resources) checkAccess(resource string, operation string) Access { - const everyoneRoleId uint64 = 1 user := r.identity() result := []Access{} query := []string{ // select rules "select r.value from sys_rules r", // join members - "inner join sys_role_member m on ", - "((m.rel_role = r.rel_role and m.rel_user=?) or (r.rel_role = ?))", + "inner join sys_role_member m on (m.rel_role = r.rel_role and m.rel_user=?)", // add conditions "where r.resource=? and r.operation=?", } queryString := strings.Join(query, " ") - if err := r.db.Select(&result, queryString, user, everyoneRoleId, resource, operation); err != nil { + if err := r.db.Select(&result, queryString, user, resource, operation); err != nil { // @todo: log error return Deny } @@ -89,6 +102,26 @@ func (r *resources) checkAccess(resource string, operation string) Access { return Inherit } +func (r *resources) checkAccessEveryone(resource string, operation string) Access { + result := []Access{} + query := []string{ + // select rules + "select r.value from sys_rules r", + // add conditions + "where r.rel_role = ? and r.resource=? and r.operation=?", + } + queryString := strings.Join(query, " ") + if err := r.db.Select(&result, queryString, everyoneRoleId, resource, operation); err != nil { + // @todo: log error + return Deny + } + + if len(result) > 0 { + return result[0] + } + return Inherit +} + func (r *resources) Grant(roleID uint64, rules []Rule) error { return r.db.Transaction(func() error { var err error diff --git a/internal/rules/resources_mock_test.go b/internal/rules/resources_mock_test.go index ee4578718..bb7f2c8b3 100644 --- a/internal/rules/resources_mock_test.go +++ b/internal/rules/resources_mock_test.go @@ -47,15 +47,20 @@ func (mr *MockResourcesInterfaceMockRecorder) With(ctx, db interface{}) *gomock. } // Check mocks base method -func (m *MockResourcesInterface) Check(resource, operation string) Access { - ret := m.ctrl.Call(m, "Check", resource, operation) +func (m *MockResourcesInterface) Check(resource, operation string, fallbacks ...CheckAccessFunc) Access { + varargs := []interface{}{resource, operation} + for _, a := range fallbacks { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Check", varargs...) ret0, _ := ret[0].(Access) return ret0 } // Check indicates an expected call of Check -func (mr *MockResourcesInterfaceMockRecorder) Check(resource, operation interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Check", reflect.TypeOf((*MockResourcesInterface)(nil).Check), resource, operation) +func (mr *MockResourcesInterfaceMockRecorder) Check(resource, operation interface{}, fallbacks ...interface{}) *gomock.Call { + varargs := append([]interface{}{resource, operation}, fallbacks...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Check", reflect.TypeOf((*MockResourcesInterface)(nil).Check), varargs...) } // Grant mocks base method diff --git a/internal/rules/resources_test.go b/internal/rules/resources_test.go index cc8dca465..5614e592f 100644 --- a/internal/rules/resources_test.go +++ b/internal/rules/resources_test.go @@ -48,7 +48,7 @@ func TestRules(t *testing.T) { // default (unset=deny), forbidden check ...:* { - Expect(rules.Inherit, resources.Check("messaging:channel:1", "update"), "messaging:channel:1 update - Inherit") + Expect(rules.Deny, resources.Check("messaging:channel:1", "update"), "messaging:channel:1 update - Deny") Expect(rules.Deny, resources.Check("messaging:channel:*", "update"), "messaging:channel:* update - Deny") } @@ -61,7 +61,7 @@ func TestRules(t *testing.T) { err := resources.Grant(role.ID, list) NoError(t, err, "expect no error") - Expect(rules.Inherit, resources.Check("messaging:channel:1", "update"), "messaging:channel:1 update - Inherit") + Expect(rules.Deny, resources.Check("messaging:channel:1", "update"), "messaging:channel:1 update - Deny") Expect(rules.Allow, resources.Check("messaging:channel:2", "update"), "messaging:channel:2 update - Allow") Expect(rules.Deny, resources.Check("messaging:channel:*", "update"), "messaging:channel:* update - Deny") } @@ -103,8 +103,8 @@ func TestRules(t *testing.T) { err := resources.Grant(role.ID, list) NoError(t, err, "expect no error") - Expect(rules.Inherit, resources.Check("messaging:channel:1", "update"), "messaging:channel:1 update - Inherit") - Expect(rules.Inherit, resources.Check("messaging:channel:2", "update"), "messaging:channel:2 update - Inherit") + Expect(rules.Deny, resources.Check("messaging:channel:1", "update"), "messaging:channel:1 update - Deny") + Expect(rules.Deny, resources.Check("messaging:channel:2", "update"), "messaging:channel:2 update - Deny") } // [messaging:channel:*,update] - allow, [messaging:channel:1, deny] diff --git a/messaging/service/permissions.go b/messaging/service/permissions.go index a775ef63f..cf385d3eb 100644 --- a/messaging/service/permissions.go +++ b/messaging/service/permissions.go @@ -17,9 +17,6 @@ type ( prm systemService.PermissionsService } - // Fallback option - Fallback func() bool - PermissionsService interface { With(context.Context) PermissionsService @@ -140,19 +137,10 @@ func (p *permissions) CanReactMessage(ch *types.Channel) bool { return p.checkAccess(ch.Resource().String(), "message.react") } -func (p *permissions) checkAccess(resource string, operation string, fbs ...Fallback) bool { - access := p.prm.Check(resource, operation) - switch access { - case internalRules.Allow: +func (p *permissions) checkAccess(resource string, operation string, fallbacks ...internalRules.CheckAccessFunc) bool { + access := p.prm.Check(resource, operation, fallbacks...) + if access == internalRules.Allow { return true - case internalRules.Deny: - return false - default: - for _, fb := range fbs { - if fb() == true { - return true - } - } - return false } + return false } diff --git a/system/service/permissions.go b/system/service/permissions.go index f9caa237d..6bfd5c54e 100644 --- a/system/service/permissions.go +++ b/system/service/permissions.go @@ -28,7 +28,7 @@ type ( List() (interface{}, error) - Check(resource string, operation string) rules.Access + Check(resource string, operation string, fallbacks ...rules.CheckAccessFunc) rules.Access Read(roleID uint64) (interface{}, error) Update(roleID uint64, rules []rules.Rule) (interface{}, error) @@ -63,8 +63,8 @@ func (p *permissions) List() (interface{}, error) { return perms, nil } -func (p *permissions) Check(resource string, operation string) rules.Access { - return p.resources.Check(resource, operation) +func (p *permissions) Check(resource string, operation string, fallbacks ...rules.CheckAccessFunc) rules.Access { + return p.resources.Check(resource, operation, fallbacks...) } func (p *permissions) Read(roleID uint64) (interface{}, error) {