Skip to content

Fix concurrent map access in rbac #1844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timbastin merged 4 commits into from
Apr 2, 2026
Merged

Fix concurrent map access in rbac #1844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
851833b
added mutex guards around each call of the casbin context enforcer
Hubtrick-Git Apr 2, 2026
8044ac5
remove lock scopes
Hubtrick-Git Apr 2, 2026
93c6899
added test cases
Hubtrick-Git Apr 2, 2026
a99ddd8
Apply suggestion from @Copilot
timbastin Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 63 additions & 2 deletions accesscontrol/casbin_rbac.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"log/slog"
"os"
"strings"
"sync"

"github.com/casbin/casbin/v3"
gormadapter "github.com/casbin/gorm-adapter/v3"
Expand All @@ -34,6 +35,10 @@ import (
var _ shared.AccessControl = &casbinRBAC{}
var casbinEnforcer *casbin.ContextEnforcer

// protect against concurrent access on shared rbac structures like maps
// in practical terms this means that whenever we call a function of the casbin context enforcer, we wrap the call inside a mutex lock and unlock
Comment thread
Hubtrick-Git marked this conversation as resolved.
var concurrencyMutex sync.RWMutex

type casbinRBAC struct {
domain string // scopes this to a specific domain - or organization
enforcer *casbin.ContextEnforcer
Expand All @@ -55,7 +60,9 @@ func (c *casbinRBAC) GetExternalEntityProviderID() *string {
}

func (c *casbinRBAC) GetOwnerOfOrganization() (string, error) {
concurrencyMutex.Lock()
listOfUsers := c.enforcer.GetUsersForRoleInDomain("role::owner", "domain::"+c.domain)
concurrencyMutex.Unlock()
if len(listOfUsers) == 0 {
return "", fmt.Errorf("no owner found for organization")
}
Expand All @@ -66,7 +73,9 @@ func (c *casbinRBAC) GetOwnerOfOrganization() (string, error) {
}

func (c *casbinRBAC) GetAllMembersOfOrganization() ([]string, error) {
concurrencyMutex.Lock()
users, err := c.enforcer.GetAllUsersByDomain("domain::" + c.domain)
concurrencyMutex.Unlock()
if err != nil {
return nil, err
}
Expand All @@ -78,7 +87,9 @@ func (c *casbinRBAC) GetAllMembersOfOrganization() ([]string, error) {
}

func (c *casbinRBAC) GetAllMembersOfProject(projectID string) ([]string, error) {
concurrencyMutex.Lock()
users, err := c.enforcer.GetImplicitUsersForRole("project::"+projectID+"|role::member", "domain::"+c.domain)
concurrencyMutex.Unlock()
if err != nil {
return nil, err
}
Expand All @@ -90,7 +101,9 @@ func (c *casbinRBAC) GetAllMembersOfProject(projectID string) ([]string, error)
}

func (c *casbinRBAC) GetAllMembersOfAsset(assetID string) ([]string, error) {
concurrencyMutex.Lock()
users, err := c.enforcer.GetImplicitUsersForRole("asset::"+assetID+"|role::member", "domain::"+c.domain)
concurrencyMutex.Unlock()
if err != nil {
return nil, err
}
Expand All @@ -102,13 +115,17 @@ func (c *casbinRBAC) GetAllMembersOfAsset(assetID string) ([]string, error) {
}

func (c *casbinRBAC) HasAccess(ctx context.Context, user string) (bool, error) {
concurrencyMutex.Lock()
roles := c.enforcer.GetRolesForUserInDomain("user::"+user, "domain::"+c.domain)
concurrencyMutex.Unlock()
return len(roles) > 0, nil
}

func (c *casbinRBAC) GetAllProjectsForUser(user string) ([]string, error) {
projectIDs := []string{}
concurrencyMutex.Lock()
roles, _ := c.enforcer.GetImplicitRolesForUser("user::"+user, "domain::"+c.domain)
concurrencyMutex.Unlock()
for _, role := range roles {
if !strings.HasPrefix(role, "project::") || !strings.Contains(role, "role::") {
continue
Expand All @@ -120,7 +137,9 @@ func (c *casbinRBAC) GetAllProjectsForUser(user string) ([]string, error) {

func (c *casbinRBAC) GetAllAssetsForUser(user string) ([]string, error) {
assetIDs := []string{}
concurrencyMutex.Lock()
roles, _ := c.enforcer.GetImplicitRolesForUser("user::"+user, "domain::"+c.domain)
concurrencyMutex.Unlock()
for _, role := range roles {
if !strings.HasPrefix(role, "asset::") || !strings.Contains(role, "role::") {
continue
Expand All @@ -131,7 +150,9 @@ func (c *casbinRBAC) GetAllAssetsForUser(user string) ([]string, error) {
}

func (c *casbinRBAC) GetAllRoles(user string) []string {
concurrencyMutex.Lock()
roles, err := c.enforcer.GetImplicitRolesForUser("user::"+user, "domain::"+c.domain)
concurrencyMutex.Unlock()
if err != nil {
slog.Error("GetAllRoles failed", "err", err)
return []string{}
Expand Down Expand Up @@ -204,32 +225,44 @@ func (c *casbinRBAC) getAssetRoleName(role shared.Role, asset string) string {
}

func (c *casbinRBAC) GrantRole(ctx context.Context, user string, role shared.Role) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, "user::"+user, "role::"+string(role), "domain::"+c.domain)
return err
Comment thread
Hubtrick-Git marked this conversation as resolved.
}

func (c *casbinRBAC) RevokeRole(ctx context.Context, user string, role shared.Role) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.DeleteRoleForUserInDomainCtx(ctx, "user::"+user, "role::"+string(role), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) GrantRoleInProject(ctx context.Context, user string, role shared.Role, project string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, "user::"+user, "project::"+project+"|role::"+string(role), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) GrantRoleInAsset(ctx context.Context, user string, role shared.Role, asset string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, "user::"+user, "asset::"+asset+"|role::"+string(role), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) RevokeRoleInProject(ctx context.Context, user string, role shared.Role, project string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.DeleteRoleForUserInDomainCtx(ctx, "user::"+user, "project::"+project+"|role::"+string(role), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) RevokeRoleInAsset(ctx context.Context, user string, role shared.Role, project string) error {
_, err := c.enforcer.DeleteRoleForUserInDomainCtx(ctx, "user::"+user, "asset::"+project+"|role::"+string(role), "domain::"+c.domain)
func (c *casbinRBAC) RevokeRoleInAsset(ctx context.Context, user string, role shared.Role, asset string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.DeleteRoleForUserInDomainCtx(ctx, "user::"+user, "asset::"+asset+"|role::"+string(role), "domain::"+c.domain)
return err
}

Expand All @@ -252,31 +285,43 @@ func (c *casbinRBAC) RevokeAllRolesInAssetForUser(ctx context.Context, user stri
}

func (c *casbinRBAC) InheritRole(ctx context.Context, roleWhichGetsPermissions, roleWhichProvidesPermissions shared.Role) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, "role::"+string(roleWhichGetsPermissions), "role::"+string(roleWhichProvidesPermissions), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) InheritProjectRole(ctx context.Context, roleWhichGetsPermissions, roleWhichProvidesPermissions shared.Role, project string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, c.getProjectRoleName(roleWhichGetsPermissions, project), c.getProjectRoleName(roleWhichProvidesPermissions, project), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) InheritAssetRole(ctx context.Context, roleWhichGetsPermissions, roleWhichProvidesPermissions shared.Role, asset string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, c.getAssetRoleName(roleWhichGetsPermissions, asset), c.getAssetRoleName(roleWhichProvidesPermissions, asset), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) InheritProjectRolesAcrossProjects(ctx context.Context, roleWhichGetsPermissions, roleWhichProvidesPermissions shared.ProjectRole) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, c.getProjectRoleName(roleWhichGetsPermissions.Role, roleWhichGetsPermissions.Project), c.getProjectRoleName(roleWhichProvidesPermissions.Role, roleWhichProvidesPermissions.Project), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) LinkDomainAndProjectRole(ctx context.Context, domainRoleWhichGetsPermission, projectRoleWhichProvidesPermissions shared.Role, project string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, "role::"+string(domainRoleWhichGetsPermission), c.getProjectRoleName(projectRoleWhichProvidesPermissions, project), "domain::"+c.domain)
return err
}

func (c *casbinRBAC) LinkProjectAndAssetRole(ctx context.Context, projectRoleWhichGetsPermission, assetRoleWhichProvidesPermissions shared.Role, project string, asset string) error {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddRoleForUserInDomainCtx(ctx, c.getProjectRoleName(projectRoleWhichGetsPermission, project), c.getAssetRoleName(assetRoleWhichProvidesPermissions, asset), "domain::"+c.domain)
return err
}
Expand All @@ -286,6 +331,8 @@ func (c *casbinRBAC) AllowRole(ctx context.Context, role shared.Role, object sha
for i, ac := range action {
policies[i] = []string{"role::" + string(role), "domain::" + c.domain, "obj::" + string(object), "act::" + string(ac)}
}
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddPoliciesCtx(ctx, policies)
return err
}
Expand All @@ -295,6 +342,8 @@ func (c *casbinRBAC) AllowRoleInProject(ctx context.Context, project string, rol
for i, ac := range action {
policies[i] = []string{"project::" + project + "|role::" + string(role), "domain::" + c.domain, "project::" + project + "|obj::" + string(object), "act::" + string(ac)}
}
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddPoliciesCtx(ctx, policies)
return err
}
Expand All @@ -304,12 +353,16 @@ func (c *casbinRBAC) AllowRoleInAsset(ctx context.Context, asset string, role sh
for i, ac := range action {
policies[i] = []string{"asset::" + asset + "|role::" + string(role), "domain::" + c.domain, "asset::" + asset + "|obj::" + string(object), "act::" + string(ac)}
}
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
_, err := c.enforcer.AddPoliciesCtx(ctx, policies)
return err
}

func (c *casbinRBAC) IsAllowed(ctx context.Context, user string, object shared.Object, action shared.Action) (bool, error) {
concurrencyMutex.Lock()
permissions, err := c.enforcer.GetImplicitPermissionsForUser("user::"+user, "domain::"+c.domain)
concurrencyMutex.Unlock()
if err != nil {
return false, err
}
Expand All @@ -322,7 +375,9 @@ func (c *casbinRBAC) IsAllowed(ctx context.Context, user string, object shared.O
}

func (c *casbinRBAC) IsAllowedInProject(ctx context.Context, project *models.Project, user string, object shared.Object, action shared.Action) (bool, error) {
concurrencyMutex.Lock()
permissions, err := c.enforcer.GetImplicitPermissionsForUser("user::"+user, "domain::"+c.domain)
concurrencyMutex.Unlock()
if err != nil {
return false, err
}
Expand All @@ -336,7 +391,9 @@ func (c *casbinRBAC) IsAllowedInProject(ctx context.Context, project *models.Pro
}

func (c *casbinRBAC) IsAllowedInAsset(ctx context.Context, asset *models.Asset, user string, object shared.Object, action shared.Action) (bool, error) {
concurrencyMutex.Lock()
permissions, err := c.enforcer.GetImplicitPermissionsForUser("user::"+user, "domain::"+c.domain)
concurrencyMutex.Unlock()
if err != nil {
return false, err
}
Expand All @@ -350,7 +407,9 @@ func (c *casbinRBAC) IsAllowedInAsset(ctx context.Context, asset *models.Asset,
}

func (c casbinRBACProvider) DomainsOfUser(user string) ([]string, error) {
concurrencyMutex.Lock()
domains, err := c.enforcer.GetDomainsForUser("user::" + user)
concurrencyMutex.Unlock()
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -396,6 +455,8 @@ func buildEnforcer(db *gorm.DB, broker shared.PubSubBroker) (*casbin.ContextEnfo
return nil, fmt.Errorf("could not set watcher: %w", err)
}
err = watcher.SetUpdateCallback(func(string) {
concurrencyMutex.Lock()
defer concurrencyMutex.Unlock()
err := e.LoadPolicy()
if err != nil {
slog.Error("error while loading policy after update", "err", err)
Expand Down
Loading
Loading