From 715530a13bd54e8bccb4adfd091cbdbd8458797c Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Wed, 6 Feb 2019 14:50:49 +0100 Subject: [PATCH] upd(all): rules refactor - allow empty response for empty rules.Resource, - rely more on rules.Access instead of bool, - update resource test --- internal/rules/interfaces.go | 3 +- internal/rules/resource.go | 5 +++- internal/rules/resource_test.go | 5 ++++ internal/rules/resources.go | 30 ++++++++++++------- messaging/types/channel.go | 9 ++++-- .../types/permissions/1-organisation.json | 3 +- system/types/organisation.go | 9 ++++-- system/types/team.go | 9 ++++-- 8 files changed, 49 insertions(+), 24 deletions(-) diff --git a/internal/rules/interfaces.go b/internal/rules/interfaces.go index 39c8a5d79..13efb9d7f 100644 --- a/internal/rules/interfaces.go +++ b/internal/rules/interfaces.go @@ -9,8 +9,7 @@ import ( type ResourcesInterface interface { With(ctx context.Context, db *factory.DB) ResourcesInterface - CheckAccessMulti(resource string, operation string) error - CheckAccess(resource string, operation string) error + IsAllowed(resource string, operation string) Access Grant(resource string, teamID uint64, operations []string, value Access) error ListGrants(resource string, teamID uint64) ([]Rules, error) diff --git a/internal/rules/resource.go b/internal/rules/resource.go index 852f7866f..28694d7ea 100644 --- a/internal/rules/resource.go +++ b/internal/rules/resource.go @@ -20,7 +20,10 @@ type ResourceJSON struct { } func (r Resource) String() string { - return fmt.Sprintf("%s:%d", r.Scope, r.ID) + if r.ID > 0 { + return fmt.Sprintf("%s:%d", r.Scope, r.ID) + } + return "" } func (r Resource) All() string { diff --git a/internal/rules/resource_test.go b/internal/rules/resource_test.go index 6e167f889..fb4809018 100644 --- a/internal/rules/resource_test.go +++ b/internal/rules/resource_test.go @@ -30,4 +30,9 @@ func TestResource(t *testing.T) { json.Unmarshal(b, &r) assert(t, r.String() == "team:123", "Decoded full-json resource ID doesn't match, team:123 != '%s'", r.String()) } + + { + r.ID = 0 + assert(t, r.String() == "", "Empty resource should return empty string, got '%s'", r.String()) + } } diff --git a/internal/rules/resources.go b/internal/rules/resources.go index 441be1ffd..231da287a 100644 --- a/internal/rules/resources.go +++ b/internal/rules/resources.go @@ -4,7 +4,6 @@ import ( "context" "strings" - "github.com/pkg/errors" "github.com/titpetric/factory" "github.com/crusttech/crust/internal/auth" @@ -30,7 +29,14 @@ func (r *resources) identity() uint64 { return auth.GetIdentityFromContext(r.ctx).Identity() } -func (r *resources) CheckAccessMulti(resource string, operation string) error { +func (r *resources) IsAllowed(resource string, operation string) Access { + if strings.Contains(resource, "*") { + return r.checkAccessMulti(resource, operation) + } + return r.checkAccess(resource, operation) +} + +func (r *resources) checkAccessMulti(resource string, operation string) Access { user := r.identity() result := []Access{} query := []string{ @@ -44,24 +50,25 @@ func (r *resources) CheckAccessMulti(resource string, operation string) error { resource = strings.Replace(resource, "*", "%", -1) queryString := strings.Join(query, " ") if err := r.db.Select(&result, queryString, user, resource, operation); err != nil { - return err + // @todo: log error + return Deny } // order by deny, allow for _, val := range result { if val == Deny { - return errors.New("Access not allowed") + return Deny } } for _, val := range result { if val == Allow { - return nil + return Allow } } - return errors.New("Access not allowed") + return Inherit } -func (r *resources) CheckAccess(resource string, operation string) error { +func (r *resources) checkAccess(resource string, operation string) Access { user := r.identity() result := []Access{} query := []string{ @@ -74,21 +81,22 @@ func (r *resources) CheckAccess(resource string, operation string) error { } queryString := strings.Join(query, " ") if err := r.db.Select(&result, queryString, user, resource, operation); err != nil { - return err + // @todo: log error + return Deny } // order by deny, allow for _, val := range result { if val == Deny { - return errors.New("Access not allowed") + return Deny } } for _, val := range result { if val == Allow { - return nil + return Allow } } - return errors.New("Access not allowed") + return Inherit } func (r *resources) Grant(resource string, teamID uint64, operations []string, value Access) error { diff --git a/messaging/types/channel.go b/messaging/types/channel.go index 3c3c2e367..a21ec8509 100644 --- a/messaging/types/channel.go +++ b/messaging/types/channel.go @@ -57,11 +57,14 @@ type ( // Resource returns a system resource ID for this type func (r *Channel) Resource() rules.Resource { - return rules.Resource{ - ID: r.ID, - Name: r.Name, + resource := rules.Resource{ Scope: "channel", } + if r != nil { + resource.ID = r.ID + resource.Name = r.Name + } + return resource } func (c *Channel) IsValid() bool { diff --git a/messaging/types/permissions/1-organisation.json b/messaging/types/permissions/1-organisation.json index 24ed4bcd7..3a1ba1c62 100644 --- a/messaging/types/permissions/1-organisation.json +++ b/messaging/types/permissions/1-organisation.json @@ -71,7 +71,8 @@ "key": "text.edit_own", "title": "Manage own messages", "subtitle": "Members with this permission can edit/delete their own messages inside channels", - "enabled": true + "enabled": true, + "default": "allow" }, { "key": "text.edit_all", diff --git a/system/types/organisation.go b/system/types/organisation.go index 15fd01561..68fb973d3 100644 --- a/system/types/organisation.go +++ b/system/types/organisation.go @@ -25,9 +25,12 @@ type ( // Resource returns a system resource ID for this type func (r *Organisation) Resource() rules.Resource { - return rules.Resource{ - ID: r.ID, - Name: r.Name, + resource := rules.Resource{ Scope: "organisation", } + if r != nil { + resource.ID = r.ID + resource.Name = r.Name + } + return resource } diff --git a/system/types/team.go b/system/types/team.go index fdb530d47..2f8698c5c 100644 --- a/system/types/team.go +++ b/system/types/team.go @@ -25,9 +25,12 @@ type ( // Resource returns a system resource ID for this type func (r *Team) Resource() rules.Resource { - return rules.Resource{ - ID: r.ID, - Name: r.Name, + resource := rules.Resource{ Scope: "team", } + if r != nil { + resource.ID = r.ID + resource.Name = r.Name + } + return resource }