v1.3.0: fix some known issues

README.md

@@ -8,23 +8,23 @@ First of all, you need to determine `who` is trying to do `what` and `with whom`
 
 ### Who? - Entity
 
-Entity is a subject wich will be authorized to perform actions on a specific resources.
+Entity is a subject which will be authorized to perform actions on specific resources.
 
 In most cases you will have only 1 entity in your application - user.
 
 ### What? - Action
 
 Action just represents name of this action (under the hood is just a type definition based on string). Each action is bound to specific entity.
 
-Action can be create via `.NewAction(<name>, <required permissions>)` method of some entity.
+Action can be created via `.NewAction(<name>, <required permissions>)` method of some entity.
 
 ### With whom? - Resource
 
-Resource is a thing on wich action is supposed to be performed.
+Resource is a thing on which action is supposed to be performed.
 
 ### Context
 
-Pretty simple, isn't it? All this 3 things combined together represents an `Authorization Context`, since they are used to just show what actually happens during authorization. But this still won't be enough, cuz we also need to know what this specific entity is allowed to do, in other words - it's `Permissions`.
+Pretty simple, isn't it? All these 3 things combined together represent an `Authorization Context`, since they are used to show what actually happens during authorization. But this still won't be enough, because we also need to know what this specific entity is allowed to do, in other words - it's `Permissions`.
 
 ### PERMISSIONS
 
@@ -46,13 +46,13 @@ Permissions are represented via bitmask, so they are very fast to work with. The
 
 -   Self Delete
 
-I assume that there are no need to describe what each one of them should permit to do.
+I assume that there is no need to describe what each one of them should permit to do.
 
-But how to store all this permissions? Of course we could just left it as a bitmask (which is just a number), but it will be really hard to maintain. So we need more convenient way to represent all this permissions and for that we will use `Roles`.
+But how to store all these permissions? Of course we could just leave it as a bitmask (which is just a number), but it will be really hard to maintain. So we need a more convenient way to represent all these permissions and for that we will use `Roles`.
 
 ### ROLES
 
-Each role consist of 2 parts: the name and permissions for this role. You can think of them as a named set of permissions.
+Each role consists of 2 parts: the name and permissions for this role. You can think of them as a named set of permissions.
 
 ### Authorization
 

action.go

@@ -55,29 +55,29 @@
 }
 
 // Validates that required fields are non-zero.
-func (r *ActionGateRule) Validate() *Error {
+func (r *ActionGateRule) Validate() error {
 	if err := r.Effect.Validate(); err != nil {
-		return NewError("Invalid Action Gate Rule: " + err.Error())
+		return errors.New("invalid action gate rule: " + err.Error())
 	}
 	if r.Roles == nil || len(r.Roles) == 0 {
-		return NewError("Invalid Action Gate Rule: Roles are missing")
+		return errors.New("invalid action gate rule: roles are missing")
 	}
 	if r.Entity.name == "" {
-		return NewError("Invalid Action Gate Rule: Entity name is missing")
+		return errors.New("invalid action gate rule: entity name is missing")
 	}
 	if r.Action == "" {
-		return NewError("Invalid Action Gate Rule: Action is missing")
+		return errors.New("invalid action gate rule: action is missing")
 	}
 	var zeroResource Resource
 	if r.Resource == zeroResource {
-		return NewError("Invalid Action Gate Rule: Resource is missing")
+		return errors.New("invalid action gate rule: resource is missing")
 	}
 	return nil
 }
 
 // Applies this rule for the given action with roles.
 // Returns true if default authorization must be skipped.
-func (r *ActionGateRule) Apply(act Action, roles []Role) (bypassAuthz bool, err *Error) {
+func (r *ActionGateRule) Apply(act Action, roles []Role) (bypassAuthz bool, err error) {
 	if r.Roles != nil && r.Action != act {
 		return false, nil
 	}
@@ -96,11 +96,11 @@
 	switch r.Effect {
 	case DenyActionGateEffect:
 		if matchRuleRoles {
-			return false, ActionDeniedByAGP
+			return false, ErrActionDeniedByAGP
 		}
 	case RequireActionGateEffect:
 		if !matchRuleRoles {
-			return false, ActionDeniedByAGP
+			return false, ErrActionDeniedByAGP
 		}
 	case AllowActionGateEffect:
 		if matchRuleRoles {
@@ -134,15 +134,15 @@
 
 // Adds new rule in police, will return error if rule
 // is either invalid, either already exist in policy
-func (agp ActionGatePolicy) AddRule(rule *ActionGateRule) *Error {
+func (agp ActionGatePolicy) AddRule(rule *ActionGateRule) error {
 	if err := rule.Validate(); err != nil {
 		return err
 	}
 
 	key := agp.keyFrom(&rule.Entity, rule.Action, &rule.Resource)
 
 	if _, ok := agp.rules[key]; ok {
-		return NewError("Rule " + key + " already exist in Action Gate Policy")
+		return errors.New("rule " + key + " already exists in action gate policy")
 	}
 
 	agp.rules[key] = rule

action_gate_policy_test.go

@@ -30,7 +30,7 @@ func TestActionGatePolicy(t *testing.T) {
 
 	// Test rule application
 	bypass, err := rule.Apply(readAction, []Role{adminRole})
-	if err != ActionDeniedByAGP {
+	if err != ErrActionDeniedByAGP {
 		t.Errorf("Expected ActionDeniedByAGP, got %v", err)
 	}
 	if bypass {

authorization.go

@@ -1,35 +1,51 @@
 package rbac
 
-type AuthzFunc func(Permissions, Permissions) *Error
+// AuthzFunc checks user's permissions.
+type AuthzFunc func(Permissions, Permissions) error
 
-var authorize AuthzFunc = AuthorizeCRUDFunc
+// RuleProvider can lookup ActionGate rules for provided context.
+type RuleProvider interface {
+	GetRule(ctx *AuthorizationContext) (*ActionGateRule, bool)
+}
 
-// AuthorizationFunc checks user's permissions.
-//
-// This function is incapsulated in rbac package, so it can't be called directly,
-// instead use "Authorize" function.
-//
-// AuthorizationFunc can be overridden via this function, to implement custom authorization logic.
-// By default it uses the AuthorizeCRUDFunc function.
+// Authorizer encapsulates authorization behavior.
+type Authorizer struct {
+	authzFunc AuthzFunc
+}
+
+// NewAuthorizer creates authorizer with default authorization function.
+func NewAuthorizer() *Authorizer {
+	return &Authorizer{
+		authzFunc: AuthorizeCRUDFunc,
+	}
+}
+
+var defaultAuthorizer = NewAuthorizer()
+
+// SetAuthzFunc overrides default authorization function globally.
 func SetAuthzFunc(fn AuthzFunc) {
+	defaultAuthorizer.SetAuthzFunc(fn)
+}
+
+// SetAuthzFunc overrides authorization function for this authorizer.
+func (a *Authorizer) SetAuthzFunc(fn AuthzFunc) {
 	if fn == nil {
 		panic("authorization function can't be nil")
 	}
-
-	authorize = fn
+	a.authzFunc = fn
 }
 
 // Checks if the "permitted" permissions are sufficient to satisfy the "required" CRUD permissions.
 //
 // It returns an "InsufficientPermissions" error if any of the "required" permissions are not covered by the "permitted" permissions.
-func AuthorizeCRUDFunc(required Permissions, permitted Permissions) *Error {
+func AuthorizeCRUDFunc(required Permissions, permitted Permissions) error {
 	// To verify that 'permitted' satisfies 'required' need to check
 	// if all 1 bits in 'required' are set in 'permitted',
 	// For that need to perform a bitwise AND between 'required' and 'permitted',
 	// then verify if the result equals 'required'.
 	// If (required & permitted) == required, all ones in required are present in permitted.
 	if required&permitted != required {
-		return InsufficientPermissions
+		return ErrInsufficientPermissions
 	}
 
 	return nil
@@ -38,13 +54,18 @@ func AuthorizeCRUDFunc(required Permissions, permitted Permissions) *Error {
 // Checks if the user has sufficient permissions to perform an action on this resource.
 //
 // Returns an error if any of the required permissions for the action are not covered by given roles.
-func Authorize(ctx *AuthorizationContext, roles []Role, AGP *ActionGatePolicy) *Error {
+func Authorize(ctx *AuthorizationContext, roles []Role, provider RuleProvider) error {
+	return defaultAuthorizer.Authorize(ctx, roles, provider)
+}
+
+// Authorize checks authorization using provided rule provider.
+func (a *Authorizer) Authorize(ctx *AuthorizationContext, roles []Role, provider RuleProvider) error {
 	if !ctx.Entity.HasAction(ctx.Action) {
-		return EntityDoesNotHaveSuchAction
+		return ErrEntityDoesNotHaveSuchAction
 	}
 
-	if AGP != nil {
-		if rule, ok := AGP.GetRule(ctx); ok {
+	if provider != nil {
+		if rule, ok := provider.GetRule(ctx); ok {
 			bypass, err := rule.Apply(ctx.Action, roles)
 			if err != nil {
 				return err
@@ -62,9 +83,9 @@ func Authorize(ctx *AuthorizationContext, roles []Role, AGP *ActionGatePolicy) *
 		mergredPermissions |= role.Permissions
 	}
 
-	if err := authorize(requiredPermissions, mergredPermissions); err == nil {
-		return nil
+	if err := a.authzFunc(requiredPermissions, mergredPermissions); err != nil {
+		return err
 	}
 
-	return InsufficientPermissions
+	return nil
 }

authorization_test.go

@@ -1,6 +1,7 @@
 package rbac
 
 import (
+	"errors"
 	"testing"
 )
 
@@ -32,17 +33,25 @@ func TestAuthorizeCRUDFunc(t *testing.T) {
 
 func TestSetAuthzFunc(t *testing.T) {
 	// Test custom function
-	customFunc := func(required, permitted Permissions) *Error {
+	customFunc := func(required, permitted Permissions) error {
 		if required == 0 {
-			return NewError("custom error")
+			return errors.New("custom error")
 		}
 		return nil
 	}
 
 	SetAuthzFunc(customFunc)
-	err := authorize(0, ReadPermission)
+
+	user := NewEntity("user")
+	action, _ := user.NewAction("test", 0)
+	resource := NewResource("res")
+	ctx := NewAuthorizationContext(&user, action, resource)
+
+	err := Authorize(&ctx, nil, nil)
 	if err == nil {
 		t.Error("Expected custom error, got nil")
+	} else if err.Error() != "custom error" {
+		t.Errorf("Expected 'custom error', got %s", err.Error())
 	}
 
 	// Reset and test panic
@@ -107,13 +116,13 @@ func TestAuthorizeWithActionGatePolicy(t *testing.T) {
 
 	// Test deny effect
 	err := Authorize(&ctx, []Role{adminRole}, &agp)
-	if err != ActionDeniedByAGP {
+	if err != ErrActionDeniedByAGP {
 		t.Errorf("Expected ActionDeniedByAGP, got %v", err)
 	}
 
 	// Test no rule applies
 	err = Authorize(&ctx, []Role{userRole}, &agp)
-	if err != InsufficientPermissions {
+	if err != ErrInsufficientPermissions {
 		t.Errorf("Expected InsufficientPermissions, got %v", err)
 	}
 }

cmd/main.go

@@ -80,10 +80,10 @@ func example() {
 	}
 
 	e := rbac.Authorize(&ctx, roles1, &agp)
-	// Error: Action has been denied by Action Gate Policy
+	// <nil>
 	fmt.Println(e)
 
 	e = rbac.Authorize(&ctx, roles2, &agp)
-	// Error: Insufficient permissions to perform this action
+	// Error: Action has been denied by Action Gate Policy
 	fmt.Println(e)
 }

common.go

@@ -27,9 +27,8 @@ func load[T any, R loadable[T]](path string, postLoad func(*R)) (T, error) {
 		return zero, err
 	}
 	defer func() {
-		if err = file.Close(); err != nil {
-			Debug.Log(err.Error())
-			os.Exit(1)
+		if closeErr := file.Close(); closeErr != nil {
+			Debug.Log(closeErr.Error())
 		}
 	}()
 

error.go

@@ -1,19 +1,9 @@
 package rbac
 
-type Error struct {
-	message string
-}
-
-func (e *Error) Error() string {
-	return e.message
-}
-
-func NewError(message string) *Error {
-	return &Error{message}
-}
+import "errors"
 
 var (
-	InsufficientPermissions     = NewError("Insufficient permissions to perform this action")
-	EntityDoesNotHaveSuchAction = NewError("Entity doesn't have such action")
-	ActionDeniedByAGP           = NewError("Action has been denied by Action Gate Policy")
+	ErrInsufficientPermissions     = errors.New("insufficient permissions to perform this action")
+	ErrEntityDoesNotHaveSuchAction = errors.New("entity doesn't have such action")
+	ErrActionDeniedByAGP           = errors.New("action has been denied by action gate policy")
 )

error_test.go

@@ -5,34 +5,35 @@ import (
 )
 
 func TestError(t *testing.T) {
-	// Test custom error creation
-	err := NewError("test error")
-	if err.Error() != "test error" {
-		t.Errorf("Expected 'test error', got %s", err.Error())
-	}
-
 	// Test error interface implementation
-	var errorInterface error = err
+	var errorInterface error = ErrInsufficientPermissions
 	if errorInterface == nil {
 		t.Error("Error should implement error interface")
 	}
 
 	// Test predefined errors
-	if InsufficientPermissions.Error() != "Insufficient permissions to perform this action" {
-		t.Error("InsufficientPermissions message incorrect")
+	if ErrInsufficientPermissions.Error() != "insufficient permissions to perform this action" {
+		t.Error("ErrInsufficientPermissions message incorrect")
+	}
+
+	if ErrEntityDoesNotHaveSuchAction.Error() != "entity doesn't have such action" {
+		t.Error("ErrEntityDoesNotHaveSuchAction message incorrect")
 	}
 
-	if EntityDoesNotHaveSuchAction.Error() != "Entity doesn't have such action" {
-		t.Error("EntityDoesNotHaveSuchAction message incorrect")
+	if ErrActionDeniedByAGP.Error() != "action has been denied by action gate policy" {
+		t.Error("ErrActionDeniedByAGP message incorrect")
 	}
 
-	if ActionDeniedByAGP.Error() != "Action has been denied by Action Gate Policy" {
-		t.Error("ActionDeniedByAGP message incorrect")
+	// Test legacy support
+	if ErrInsufficientPermissions.Error() != "insufficient permissions to perform this action" {
+		t.Error("Legacy InsufficientPermissions message incorrect")
 	}
 
-	// Test error comparison
-	_ = NewError("same message")
-	_ = NewError("same message")
-	// Note: Different error instances will never be equal
-	// This test documents the current behavior
+	if ErrEntityDoesNotHaveSuchAction.Error() != "entity doesn't have such action" {
+		t.Error("Legacy EntityDoesNotHaveSuchAction message incorrect")
+	}
+
+	if ErrActionDeniedByAGP.Error() != "action has been denied by action gate policy" {
+		t.Error("Legacy ActionDeniedByAGP message incorrect")
+	}
 }