upd(system): add permission validation

Makefile

@@ -100,7 +100,7 @@ test.crm.db: $(GOTEST)
 	$(GO) tool cover -func=.cover.out | grep --color "^\|[^0-9]0.0%"
 
 test.system: $(GOTEST)
-	$(GOTEST) -covermode count -coverprofile .cover.out -v ./system/repository/...
+	$(GOTEST) -covermode count -coverprofile .cover.out -v ./system/repository/... ./system/service/...
 	$(GO) tool cover -func=.cover.out | grep --color "^\|[^0-9]0.0%"
 
 test.system.db: $(GOTEST)

internal/rules/main_test.go

@@ -41,5 +41,15 @@ func TestMain(m *testing.M) {
 		return
 	}
 
+	// clean up tables
+	{
+		for _, name := range []string{"sys_user", "sys_role", "sys_role_member", "sys_organisation", "sys_rules"} {
+			_, err := db.Exec("truncate " + name)
+			if err != nil {
+				panic("Error when clearing " + name + ": " + err.Error())
+			}
+		}
+	}
+
 	os.Exit(m.Run())
 }

internal/rules/resources.go

@@ -9,6 +9,10 @@ import (
 	"github.com/crusttech/crust/internal/auth"
 )
 
+const (
+	delimiter = ":"
+)
+
 type resources struct {
 	ctx context.Context
 	db  *factory.DB
@@ -29,43 +33,27 @@ func (r *resources) identity() uint64 {
 	return auth.GetIdentityFromContext(r.ctx).Identity()
 }
 
+// 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) IsAllowed(resource string, operation string) Access {
-	if strings.Contains(resource, "*") {
-		return r.checkAccessMulti(resource, operation)
-	}
-	return r.checkAccess(resource, operation)
-}
+	parts := strings.Split(resource, delimiter)
 
-func (r *resources) checkAccessMulti(resource string, operation string) Access {
-	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=?)",
-		// add conditions
-		"where r.resource LIKE ? and r.operation=?",
-	}
-	resource = strings.Replace(resource, "*", "%", -1)
-	queryString := strings.Join(query, " ")
-	if err := r.db.Select(&result, queryString, user, resource, operation); err != nil {
-		// @todo: log error
+	// Permission check on global level is not allowed.
+	if parts[len(parts)-1] == "*" {
 		return Deny
 	}
 
-	// order by deny, allow
-	for _, val := range result {
-		if val == Deny {
-			return Deny
-		}
+	access := r.checkAccess(resource, operation)
+	if access == Inherit {
+		// Create resource definition for global level.
+		parts[len(parts)-1] = "*"
+
+		resource = strings.Join(parts, delimiter)
+
+		// If rule for specific resource is not set we check on global level.
+		return r.checkAccess(resource, operation)
 	}
-	for _, val := range result {
-		if val == Allow {
-			return Allow
-		}
-	}
-	return Inherit
+	return access
 }
 
 func (r *resources) checkAccess(resource string, operation string) Access {

internal/rules/resources_test.go

@@ -20,14 +20,11 @@ func TestRules(t *testing.T) {
 
 	db := factory.Database.MustGet()
 
+	roleID := uint64(123456)
+
 	db.Insert("sys_user", user)
-	var i uint64 = 0
-	for i < 5 {
-		db.Insert("sys_role", types.Role{ID: i, Name: fmt.Sprintf("Role %d", i)})
-		i++
-	}
-	db.Insert("sys_role_member", types.RoleMember{RoleID: 1, UserID: user.ID})
-	db.Insert("sys_role_member", types.RoleMember{RoleID: 2, UserID: user.ID})
+	db.Insert("sys_role", types.Role{ID: roleID, Name: fmt.Sprintf("Role %d", roleID)})
+	db.Insert("sys_role_member", types.RoleMember{RoleID: roleID, UserID: user.ID})
 
 	Expect := func(expected rules.Access, actual rules.Access, format string, params ...interface{}) {
 		Assert(t, expected == actual, format, params...)
@@ -35,99 +32,104 @@ func TestRules(t *testing.T) {
 
 	resources := rules.NewResources(ctx, db)
 
-	// default (unset=deny)
+	// delete all for test roleID = 123456
 	{
-		Expect(rules.Inherit, resources.IsAllowed("channel:1", "update"), "expected inherit")
-		Expect(rules.Inherit, resources.IsAllowed("channel:*", "update"), "expected inherit")
+		err := resources.Delete(roleID)
+		NoError(t, err, "expected no error")
 	}
 
-	// allow channel:2 group:2 (default deny, multi=allow)
+	// default (unset=deny), forbidden check ...:*
+	{
+		Expect(rules.Inherit, resources.IsAllowed("messaging:channel:1", "update"), "messaging:channel:1 update - Inherit")
+		Expect(rules.Deny, resources.IsAllowed("messaging:channel:*", "update"), "messaging:channel:* update - Deny")
+	}
+
+	// allow messaging:channel:2 update,delete
 	{
 		list := []rules.Rule{
-			rules.Rule{Resource: "channel:2", Operation: "update", Value: rules.Allow},
-			rules.Rule{Resource: "channel:2", Operation: "delete", Value: rules.Allow},
+			rules.Rule{Resource: "messaging:channel:2", Operation: "update", Value: rules.Allow},
+			rules.Rule{Resource: "messaging:channel:2", Operation: "delete", Value: rules.Allow},
 		}
+		err := resources.Grant(roleID, list)
+		NoError(t, err, "expect no error")
 
-		resources.Grant(2, list)
-		Expect(rules.Inherit, resources.IsAllowed("channel:1", "update"), "expected error, got nil")
-		Expect(rules.Allow, resources.IsAllowed("channel:2", "update"), "channel:2 update, expected no error")
-		Expect(rules.Allow, resources.IsAllowed("channel:*", "update"), "channel:* update, expected no error")
+		Expect(rules.Inherit, resources.IsAllowed("messaging:channel:1", "update"), "messaging:channel:1 update - Inherit")
+		Expect(rules.Allow, resources.IsAllowed("messaging:channel:2", "update"), "messaging:channel:2 update - Allow")
+		Expect(rules.Deny, resources.IsAllowed("messaging:channel:*", "update"), "messaging:channel:* update - Deny")
 	}
 
-	// list grants for role 2
+	// list grants for test role
 	{
-		grants, err := resources.List(2)
+		grants, err := resources.List(roleID)
 		NoError(t, err, "expect no error")
 		Assert(t, len(grants) == 2, "expected 2 grants")
 
 		for _, grant := range grants {
-			Assert(t, grant.RoleID == 2, "expected RoleID == 2, got %v", grant.RoleID)
-			Assert(t, grant.Resource == "channel:2", "expected Resource == channel:2, got %s", grant.Resource)
-			// Assert(t, grant.Operation == "delete", "expected Operation == delete, got %s", grant.Operation)
+			Assert(t, grant.RoleID == roleID, "expected RoleID == 123456, got %v", grant.RoleID)
+			Assert(t, grant.Resource == "messaging:channel:2", "expected Resource == messaging:channel:2, got %s", grant.Resource)
 			Assert(t, grant.Value == rules.Allow, "expected Value == Allow, got %s", grant.Value)
 		}
 	}
 
-	// deny channel:1 group:1 (explicit deny, multi=deny)
+	// deny messaging:channel:1 update
 	{
 		list := []rules.Rule{
-			rules.Rule{Resource: "channel:1", Operation: "update", Value: rules.Deny},
+			rules.Rule{Resource: "messaging:channel:1", Operation: "update", Value: rules.Deny},
 		}
-		resources.Grant(1, list)
-		Expect(rules.Deny, resources.IsAllowed("channel:1", "update"), "expected error, got nil")
-		Expect(rules.Allow, resources.IsAllowed("channel:2", "update"), "channel:2 update, expected no error")
-		Expect(rules.Deny, resources.IsAllowed("channel:*", "update"), "expected error, got nil")
+		err := resources.Grant(roleID, list)
+		NoError(t, err, "expect no error")
+
+		Expect(rules.Deny, resources.IsAllowed("messaging:channel:1", "update"), "messaging:channel:1 update - Deny")
+		Expect(rules.Allow, resources.IsAllowed("messaging:channel:2", "update"), "messaging:channel:2 update - Allow")
+		Expect(rules.Deny, resources.IsAllowed("messaging:channel:*", "update"), "messaging:channel:* update - Deny")
 	}
 
-	// reset (unset=deny)
-	{
-		list1 := []rules.Rule{
-			rules.Rule{Resource: "channel:1", Operation: "update", Value: rules.Inherit},
-			rules.Rule{Resource: "channel:1", Operation: "delete", Value: rules.Inherit},
-		}
-		resources.Grant(1, list1)
-
-		list2 := []rules.Rule{
-			rules.Rule{Resource: "channel:2", Operation: "update", Value: rules.Inherit},
-			rules.Rule{Resource: "channel:2", Operation: "delete", Value: rules.Inherit},
-		}
-		resources.Grant(2, list2)
-
-		Expect(rules.Inherit, resources.IsAllowed("channel:1", "update"), "expected error, got nil")
-		Expect(rules.Inherit, resources.IsAllowed("channel:*", "update"), "expected error, got nil")
-	}
-
-	// Grant by roleID
+	// reset messaging:channel:1, messaging:channel:2
 	{
 		list := []rules.Rule{
-			rules.Rule{Resource: "channel:*", Operation: "update", Value: rules.Allow},
-			rules.Rule{Resource: "channel:1", Operation: "update", Value: rules.Deny},
-			rules.Rule{Resource: "channel:2", Operation: "update"},
+			rules.Rule{Resource: "messaging:channel:1", Operation: "update", Value: rules.Inherit},
+			rules.Rule{Resource: "messaging:channel:1", Operation: "delete", Value: rules.Inherit},
+			rules.Rule{Resource: "messaging:channel:2", Operation: "update", Value: rules.Inherit},
+			rules.Rule{Resource: "messaging:channel:2", Operation: "delete", Value: rules.Inherit},
+		}
+		err := resources.Grant(roleID, list)
+		NoError(t, err, "expect no error")
+
+		Expect(rules.Inherit, resources.IsAllowed("messaging:channel:1", "update"), "messaging:channel:1 update - Inherit")
+		Expect(rules.Inherit, resources.IsAllowed("messaging:channel:2", "update"), "messaging:channel:2 update - Inherit")
+	}
+
+	// [messaging:channel:*,update] - allow, [messaging:channel:1, deny]
+	{
+		list := []rules.Rule{
+			rules.Rule{Resource: "messaging:channel:*", Operation: "update", Value: rules.Allow},
+			rules.Rule{Resource: "messaging:channel:1", Operation: "update", Value: rules.Deny},
+			rules.Rule{Resource: "messaging:channel:2", Operation: "update"},
 			rules.Rule{Resource: "system", Operation: "organisation.create", Value: rules.Allow},
 		}
-		err := resources.Grant(2, list)
+		err := resources.Grant(roleID, list)
 		NoError(t, err, "expected no error")
+
+		Expect(rules.Deny, resources.IsAllowed("messaging:channel:1", "update"), "messaging:channel:1 update - Deny")
+		Expect(rules.Allow, resources.IsAllowed("messaging:channel:2", "update"), "messaging:channel:2 update - Allow")
 	}
 
 	// list all by roleID
 	{
-		grants, err := resources.List(2)
-
-		fmt.Println(grants)
-
+		grants, err := resources.List(roleID)
 		NoError(t, err, "expected no error")
 		Assert(t, len(grants) == 3, "expected grants == 3, got %v", len(grants))
 	}
 
 	// delete all by roleID
 	{
-		err := resources.Delete(2)
+		err := resources.Delete(roleID)
 		NoError(t, err, "expected no error")
 	}
 
 	// list all by roleID
 	{
-		grants, err := resources.List(2)
+		grants, err := resources.List(roleID)
 		NoError(t, err, "expected no error")
 		Assert(t, len(grants) == 0, "expected grants == 0, got %v", len(grants))
 	}

system/service/main_test.go

@@ -2,14 +2,64 @@ package service
 
 import (
 	"fmt"
+	"log"
+	"os"
 	"runtime"
 	"testing"
+
+	"github.com/joho/godotenv"
+	"github.com/namsral/flag"
+	"github.com/titpetric/factory"
+
+	systemMigrate "github.com/crusttech/crust/system/db"
 )
 
 type mockDB struct{}
 
 func (mockDB) Transaction(callback func() error) error { return callback() }
 
+func TestMain(m *testing.M) {
+	// @todo this is a very optimistic initialization, make it more robust
+	godotenv.Load("../../.env")
+
+	prefix := "system"
+	dsn := ""
+
+	p := func(s string) string {
+		return prefix + "-" + s
+	}
+
+	flag.StringVar(&dsn, p("db-dsn"), "crust:crust@tcp(db1:3306)/crust?collation=utf8mb4_general_ci", "DSN for database connection")
+	flag.Parse()
+
+	if testing.Short() {
+		return
+	}
+
+	factory.Database.Add("default", dsn)
+
+	db := factory.Database.MustGet()
+	db.Profiler = &factory.Database.ProfilerStdout
+
+	// migrate database schema
+	if err := systemMigrate.Migrate(db); err != nil {
+		log.Printf("Error running migrations: %+v\n", err)
+		return
+	}
+
+	// clean up tables
+	{
+		for _, name := range []string{"sys_user", "sys_role", "sys_role_member", "sys_organisation", "sys_rules"} {
+			_, err := db.Exec("truncate " + name)
+			if err != nil {
+				panic("Error when clearing " + name + ": " + err.Error())
+			}
+		}
+	}
+
+	os.Exit(m.Run())
+}
+
 func assert(t *testing.T, ok bool, format string, args ...interface{}) bool {
 	if !ok {
 		_, file, line, _ := runtime.Caller(1)

system/service/permission.go

@@ -43,6 +43,12 @@ func (p *permission) Get(roleID uint64) (interface{}, error) {
 }
 
 func (p *permission) Update(roleID uint64, rules []rules.Rule) (interface{}, error) {
+	for _, rule := range rules {
+		err := validatePermission(rule.Resource, rule.Operation)
+		if err != nil {
+			return nil, err
+		}
+	}
 	return nil, p.resources.Grant(roleID, rules)
 }
 

system/service/permission_test.go

@@ -13,7 +13,7 @@ import (
 	systemTypes "github.com/crusttech/crust/system/types"
 )
 
-func TestPermissions(t *testing.T) {
+func TestPermission(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping test in short mode.")
 		return
@@ -50,23 +50,47 @@ func TestPermissions(t *testing.T) {
 	// Create permission service.
 	permissionSvc := Permission().With(ctx)
 
-	// Setup rules for test role.
+	// Update rules for test role.
 	{
-		rules := []rules.Rule{
-			rules.Rule{Resource: "messaging:channel:*", Operation: "update", Value: rules.Allow},
-			rules.Rule{Resource: "messaging:channel:1", Operation: "update", Value: rules.Deny},
-			rules.Rule{Resource: "messaging:channel:2", Operation: "update"},
+		list := []rules.Rule{
+			rules.Rule{Resource: "messaging:channel:*", Operation: "message.update.all", Value: rules.Allow},
+			rules.Rule{Resource: "messaging:channel:1", Operation: "message.update.all", Value: rules.Deny},
+			rules.Rule{Resource: "messaging:channel:2", Operation: "message.update.all"},
 			rules.Rule{Resource: "system", Operation: "organisation.create", Value: rules.Allow},
-			rules.Rule{Resource: "system:organisation:*", Operation: "update", Value: rules.Allow},
-			rules.Rule{Resource: "system:organisation:*", Operation: "delete", Value: rules.Allow},
-			rules.Rule{Resource: "messaging:channel:*", Operation: "update.name", Value: rules.Allow},
-			rules.Rule{Resource: "messaging:channel:*", Operation: "update.topic", Value: rules.Allow},
-			rules.Rule{Resource: "messaging:channel:*", Operation: "members.manage", Value: rules.Allow},
+			rules.Rule{Resource: "system:organisation:*", Operation: "access", Value: rules.Allow},
+			rules.Rule{Resource: "messaging:channel", Operation: "message.update.all", Value: rules.Allow},
 		}
-		_, err := permissionSvc.Update(role.ID, rules)
+		_, err := permissionSvc.Update(role.ID, list)
 		NoError(t, err, "expected no error, setting rules")
 	}
 
+	// Update with invalid roles
+	{
+		list := []rules.Rule{
+			rules.Rule{Resource: "nosystem:channel:*", Operation: "message.update.all", Value: rules.Allow},
+		}
+		_, err := permissionSvc.Update(role.ID, list)
+		Error(t, err, "expected error")
+
+		list = []rules.Rule{
+			rules.Rule{Resource: "messaging:noresource:1", Operation: "message.update.all", Value: rules.Deny},
+		}
+		_, err = permissionSvc.Update(role.ID, list)
+		Error(t, err, "expected error")
+
+		list = []rules.Rule{
+			rules.Rule{Resource: "messaging:channel:", Operation: "message.update.all"},
+		}
+		_, err = permissionSvc.Update(role.ID, list)
+		Error(t, err, "expected error")
+
+		list = []rules.Rule{
+			rules.Rule{Resource: "system:organisation:*", Operation: "invalid", Value: rules.Allow},
+		}
+		_, err = permissionSvc.Update(role.ID, list)
+		Error(t, err, "expected error")
+	}
+
 	// List rules for test role.
 	{
 		ret, err := permissionSvc.Get(role.ID)
@@ -74,7 +98,7 @@ func TestPermissions(t *testing.T) {
 
 		rules := ret.([]rules.Rule)
 
-		Assert(t, len(rules) == 8, "expected len(rules) == 8, got %v", len(rules))
+		Assert(t, len(rules) == 5, "expected len(rules) == 5, got %v", len(rules))
 	}
 
 	// Delete rules for test role.

system/service/permissions.go

@@ -1,41 +0,0 @@
-package service
-
-import (
-	"strings"
-
-	"github.com/pkg/errors"
-)
-
-func validatePermission(resource string, operation string) error {
-	var services = map[string]map[string]bool{
-		"messaging:channel": map[string]bool{
-			"manage.webhooks":    false,
-			"message.send":       true,
-			"message.embed":      true,
-			"message.attach":     true,
-			"message.update_own": true,
-			"message.update_all": true,
-			"message.react":      true,
-		},
-	}
-
-	delimiter := ":"
-	resourceParts := strings.Split(resource, delimiter)
-	if len(resourceParts) < 2 {
-		return errors.Errorf("Invalid resource format, expected >= 2, got %d", len(resourceParts))
-	}
-
-	resourceName := resourceParts[0] + delimiter + resourceParts[1]
-	if service, ok := services[resourceName]; ok {
-		if op := service[operation]; op {
-			if len(resourceParts) == 3 {
-				if val := resourceParts[2]; val != "" {
-					return nil
-				}
-			}
-			return errors.Errorf("Invalid resource format, missing resource ID")
-		}
-		return errors.Errorf("Unknown operation: '%s'", operation)
-	}
-	return errors.Errorf("Unknown resource name: '%s'", resourceName)
-}

system/service/validation.go

@@ -0,0 +1,110 @@
+package service
+
+import (
+	"strings"
+
+	"github.com/pkg/errors"
+)
+
+func validatePermission(resource string, operation string) error {
+	var services = map[string]map[string]bool{
+		"system": map[string]bool{
+			"access":              true,
+			"organisation.create": true,
+			"role.create":         true,
+		},
+		"system:organisation": map[string]bool{
+			"access": true,
+		},
+		"system:role": map[string]bool{
+			"read":           true,
+			"update":         true,
+			"delete":         true,
+			"members.manage": true,
+		},
+		"messaging": map[string]bool{
+			"access":                 true,
+			"channel.public.create":  true,
+			"channel.private.create": true,
+			"channel.direct.create":  true,
+		},
+		"messaging:channel": map[string]bool{
+			"update":             true,
+			"read":               true,
+			"join":               true,
+			"leave":              true,
+			"members.manage":     true,
+			"webhooks.manage":    true,
+			"attachments.manage": true,
+			"message.send":       true,
+			"message.reply":      true,
+			"message.embed":      true,
+			"message.attach":     true,
+			"message.update.own": true,
+			"message.update.all": true,
+			"message.react":      true,
+		},
+		"compose": map[string]bool{
+			"access":           true,
+			"namespace.create": true,
+		},
+		"compose:namespace": map[string]bool{
+			"read":           true,
+			"update":         true,
+			"delete":         true,
+			"module.create":  true,
+			"chart.create":   true,
+			"trigger.create": true,
+			"page.create":    true,
+		},
+		"compose:module": map[string]bool{
+			"read":          true,
+			"update":        true,
+			"delete":        true,
+			"record.create": true,
+			"record.read":   true,
+			"record.update": true,
+			"record.delete": true,
+		},
+		"compose:chart": map[string]bool{
+			"read":   true,
+			"update": true,
+			"delete": true,
+		},
+		"compose:trigger": map[string]bool{
+			"read":   true,
+			"update": true,
+			"delete": true,
+		},
+		"compose:page": map[string]bool{
+			"read":   true,
+			"update": true,
+			"delete": true,
+		},
+	}
+
+	delimiter := ":"
+	resourceParts := strings.Split(resource, delimiter)
+	if len(resourceParts) < 1 {
+		return errors.Errorf("Invalid resource format, expected >= 1, got %d", len(resourceParts))
+	}
+
+	resourceName := resourceParts[0]
+	if len(resourceParts) > 1 {
+		resourceName = resourceParts[0] + delimiter + resourceParts[1]
+	}
+
+	if service, ok := services[resourceName]; ok {
+		if op := service[operation]; op {
+			if len(resourceParts) == 3 {
+				if val := resourceParts[2]; val != "" {
+					return nil
+				}
+				return errors.Errorf("Invalid resource format, missing resource ID")
+			}
+			return nil
+		}
+		return errors.Errorf("Unknown operation: '%s'", operation)
+	}
+	return errors.Errorf("Unknown resource name: '%s'", resourceName)
+}