Skip to content

Commit d95a5f4

Browse files
committed
feat: add agent cache in user package
- use cached agent in all middlewares - fix: race in casbin that gave permission denied error. - stop loading permissions into casbin on every `Enforce` function call instead cache user permissions in authz package and when permissions change only the load permission as policies atomically. - sort permissions in get-agents to make the permissions slice comparsion using slices.Equal work
1 parent 6981a07 commit d95a5f4

File tree

7 files changed

+121
-44
lines changed

7 files changed

+121
-44
lines changed

cmd/middlewares.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func tryAuth(handler fastglue.FastRequestHandler) fastglue.FastRequestHandler {
2424
}
2525

2626
// Try to get user.
27-
user, err := app.user.GetAgent(userSession.ID, "")
27+
user, err := app.user.GetAgentCachedOrLoad(userSession.ID)
2828
if err != nil {
2929
return handler(r)
3030
}
@@ -54,7 +54,7 @@ func auth(handler fastglue.FastRequestHandler) fastglue.FastRequestHandler {
5454
}
5555

5656
// Set user in the request context.
57-
user, err := app.user.GetAgent(userSession.ID, "")
57+
user, err := app.user.GetAgentCachedOrLoad(userSession.ID)
5858
if err != nil {
5959
return sendErrorEnvelope(r, err)
6060
}
@@ -92,8 +92,8 @@ func perm(handler fastglue.FastRequestHandler, perm string) fastglue.FastRequest
9292
return r.SendErrorEnvelope(http.StatusUnauthorized, app.i18n.T("auth.invalidOrExpiredSession"), nil, envelope.GeneralError)
9393
}
9494

95-
// Get user from DB.
96-
user, err := app.user.GetAgent(sessUser.ID, "")
95+
// Get agent user from cache or load it.
96+
user, err := app.user.GetAgentCachedOrLoad(sessUser.ID)
9797
if err != nil {
9898
return sendErrorEnvelope(r, err)
9999
}

cmd/users.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ func handleCreateAgent(r *fastglue.Request) error {
148148
var (
149149
app = r.Context.(*App)
150150
user = models.User{}
151+
err error
151152
)
152153
if err := r.Decode(&user, "json"); err != nil {
153154
return r.SendErrorEnvelope(fasthttp.StatusBadRequest, app.i18n.Ts("globals.messages.errorParsing", "name", "{globals.terms.request}"), nil, envelope.InputError)
@@ -165,8 +166,7 @@ func handleCreateAgent(r *fastglue.Request) error {
165166
return r.SendErrorEnvelope(fasthttp.StatusBadRequest, app.i18n.Ts("globals.messages.empty", "name", "`first_name`"), nil, envelope.InputError)
166167
}
167168

168-
// Right now, only agents can be created.
169-
if err := app.user.CreateAgent(&user); err != nil {
169+
if user, err = app.user.CreateAgent(&user); err != nil {
170170
return sendErrorEnvelope(r, err)
171171
}
172172

@@ -204,7 +204,7 @@ func handleCreateAgent(r *fastglue.Request) error {
204204
return r.SendEnvelope(true)
205205
}
206206
}
207-
return r.SendEnvelope(true)
207+
return r.SendEnvelope(user)
208208
}
209209

210210
// handleUpdateAgent updates an agent.
@@ -214,9 +214,9 @@ func handleUpdateAgent(r *fastglue.Request) error {
214214
user = models.User{}
215215
auser = r.RequestCtx.UserValue("user").(amodels.User)
216216
ip = realip.FromRequest(r.RequestCtx)
217+
id, _ = strconv.Atoi(r.RequestCtx.UserValue("id").(string))
217218
)
218-
id, err := strconv.Atoi(r.RequestCtx.UserValue("id").(string))
219-
if err != nil || id == 0 {
219+
if id == 0 {
220220
return r.SendErrorEnvelope(fasthttp.StatusBadRequest, app.i18n.Ts("globals.messages.empty", "name", "`id`"), nil, envelope.InputError)
221221
}
222222

@@ -247,6 +247,9 @@ func handleUpdateAgent(r *fastglue.Request) error {
247247
return sendErrorEnvelope(r, err)
248248
}
249249

250+
// Invalidate authz cache.
251+
app.authz.InvalidateUserCache(id)
252+
250253
// Create activity log if user availability status changed.
251254
if oldAvailabilityStatus != user.AvailabilityStatus {
252255
if err := app.activityLog.UserAvailability(auser.ID, auser.Email, user.AvailabilityStatus, ip, user.Email.String, id); err != nil {

frontend/src/features/admin/agents/AgentForm.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@
8585

8686
<FormField v-slot="{ componentField, handleChange }" name="teams">
8787
<FormItem v-auto-animate>
88-
<FormLabel>{{ $t('globals.terms.teams') }}</FormLabel>
88+
<FormLabel>{{ $t('globals.terms.team', 2) }}</FormLabel>
8989
<FormControl>
9090
<SelectTag
9191
:items="teamOptions"

internal/authz/authz.go

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"slices"
77
"strconv"
88
"strings"
9+
"sync"
910

1011
cmodels "github.com/abhinavxd/libredesk/internal/conversation/models"
1112
"github.com/abhinavxd/libredesk/internal/envelope"
@@ -19,8 +20,11 @@ import (
1920
// Enforcer is a wrapper around Casbin enforcer.
2021
type Enforcer struct {
2122
enforcer *casbin.SyncedEnforcer
22-
lo *logf.Logger
23-
i18n *i18n.I18n
23+
// User permissions cache to avoid loading policies into Casbin every time.
24+
permsCache map[int][]string
25+
permsCacheMu sync.RWMutex
26+
lo *logf.Logger
27+
i18n *i18n.I18n
2428
}
2529

2630
const casbinModel = `
@@ -48,36 +52,67 @@ func NewEnforcer(lo *logf.Logger, i18n *i18n.I18n) (*Enforcer, error) {
4852
return nil, fmt.Errorf("failed to create Casbin enforcer: %v", err)
4953
}
5054

51-
return &Enforcer{enforcer: e, lo: lo, i18n: i18n}, nil
55+
return &Enforcer{
56+
enforcer: e,
57+
permsCache: make(map[int][]string),
58+
lo: lo,
59+
i18n: i18n,
60+
}, nil
5261
}
5362

54-
// LoadPermissions syncs user permissions with Casbin enforcer by removing existing
55-
// policies and adding current permissions as new policies
63+
// LoadPermissions syncs user permissions with Casbin enforcer by removing existing policies and adding current permissions as new policies.
5664
func (e *Enforcer) LoadPermissions(user umodels.User) error {
57-
// Remove existing policies for the user
58-
_, err := e.enforcer.RemoveFilteredPolicy(0, strconv.Itoa(user.ID))
59-
if err != nil {
60-
return fmt.Errorf("failed to remove policies: %v", err)
65+
e.permsCacheMu.RLock()
66+
cached, exists := e.permsCache[user.ID]
67+
e.permsCacheMu.RUnlock()
68+
69+
if exists && slices.Equal(cached, user.Permissions) {
70+
return nil
6171
}
6272

63-
// Add each permission as a policy
73+
e.lo.Debug("loading user permissions in enforcer cache", "user_id", user.ID, "permissions", user.Permissions)
74+
75+
// Build all policies and add them to the enforcer.
76+
var policies [][]string
77+
userID := strconv.Itoa(user.ID)
6478
for _, perm := range user.Permissions {
6579
parts := strings.Split(perm, ":")
6680
if len(parts) != 2 {
6781
return fmt.Errorf("invalid permission format: %s", perm)
6882
}
83+
policies = append(policies, []string{userID, parts[0], parts[1]})
84+
}
85+
86+
_, err := e.enforcer.RemoveFilteredPolicy(0, userID)
87+
if err != nil {
88+
return fmt.Errorf("failed to remove policies: %v", err)
89+
}
6990

70-
userID, permObj, permAct := strconv.Itoa(user.ID), parts[0], parts[1]
71-
if _, err := e.enforcer.AddPolicy(userID, permObj, permAct); err != nil {
72-
return fmt.Errorf("failed to add casbin policy: %v", err)
91+
if len(policies) > 0 {
92+
_, err = e.enforcer.AddPolicies(policies)
93+
if err != nil {
94+
return fmt.Errorf("failed to add policies: %v", err)
7395
}
7496
}
97+
98+
// Update permsCache with the latest permissions
99+
e.permsCacheMu.Lock()
100+
e.permsCache[user.ID] = slices.Clone(user.Permissions)
101+
e.permsCacheMu.Unlock()
102+
75103
return nil
76104
}
77105

106+
// InvalidateUserCache removes user from permsCache to be called when user permissions change.
107+
func (e *Enforcer) InvalidateUserCache(userID int) {
108+
e.permsCacheMu.Lock()
109+
delete(e.permsCache, userID)
110+
e.permsCacheMu.Unlock()
111+
}
112+
78113
// Enforce checks if a user has permission to perform an action on an object.
79114
func (e *Enforcer) Enforce(user umodels.User, obj, act string) (bool, error) {
80-
// Load permissions before enforcing.
115+
// Load permissions before enforcing as user perjissions might have changed.
81116
err := e.LoadPermissions(user)
82117
if err != nil {
83118
e.lo.Error("error loading permissions", "user_id", user.ID, "object", obj, "action", act, "error", err)
@@ -93,12 +128,11 @@ func (e *Enforcer) Enforce(user umodels.User, obj, act string) (bool, error) {
93128
}
94129

95130
// EnforceConversationAccess determines if a user has access to a specific conversation based on their permissions.
96-
// Access can be granted under the following conditions:
131+
// Requires basic "read" permission AND one of the following conditions:
97132
// 1. User has the "read_all" permission, allowing access to all conversations.
98133
// 2. User has the "read_assigned" permission and is the assigned user.
99134
// 3. User has the "read_team_inbox" permission and is part of the assigned team, with the conversation NOT assigned to any user.
100135
// 4. User has the "read_unassigned" permission and the conversation is not assigned to any user or team.
101-
// 5. User has the "read" permission, allowing access to the conversation.
102136
// Returns true if access is granted, false otherwise. In case of an error while checking permissions returns false and the error.
103137
func (e *Enforcer) EnforceConversationAccess(user umodels.User, conversation cmodels.Conversation) (bool, error) {
104138
checkPermission := func(action string) (bool, error) {

internal/user/agent.go

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,44 @@ func (u *Manager) MonitorAgentAvailability(ctx context.Context) {
2929
}
3030
}
3131

32-
// GetAgent retrieves an agent by ID.
32+
// GetAgent retrieves an agent by ID and also caches it for future requests.
3333
func (u *Manager) GetAgent(id int, email string) (models.User, error) {
34-
return u.Get(id, email, models.UserTypeAgent)
34+
agent, err := u.Get(id, email, models.UserTypeAgent)
35+
if err != nil {
36+
return models.User{}, err
37+
}
38+
39+
u.agentCacheMu.Lock()
40+
u.agentCache[id] = agent
41+
u.agentCacheMu.Unlock()
42+
43+
return agent, nil
44+
}
45+
46+
// GetAgentFromCache retrieves an agent from the cache by ID.
47+
func (u *Manager) GetAgentFromCache(id int) (models.User, bool) {
48+
u.agentCacheMu.RLock()
49+
defer u.agentCacheMu.RUnlock()
50+
agent, exists := u.agentCache[id]
51+
if !exists {
52+
return models.User{}, false
53+
}
54+
return agent, true
55+
}
56+
57+
// GetAgentCachedOrLoad retrieves an agent from cache, falling back to DB if not cached.
58+
func (u *Manager) GetAgentCachedOrLoad(id int) (models.User, error) {
59+
if agent, exists := u.GetAgentFromCache(id); exists {
60+
return agent, nil
61+
}
62+
return u.GetAgent(id, "")
63+
}
64+
65+
// InvalidateAgentCache invalidates the agent cache for a specific agent ID.
66+
func (u *Manager) InvalidateAgentCache(id int) {
67+
u.agentCacheMu.Lock()
68+
defer u.agentCacheMu.Unlock()
69+
delete(u.agentCache, id)
3570
}
3671

3772
// GetAgentsCompact returns a compact list of users with limited fields.
@@ -47,22 +82,22 @@ func (u *Manager) GetAgentsCompact() ([]models.User, error) {
4782
return users, nil
4883
}
4984

50-
// CreateAgent creates a new agent user.
51-
func (u *Manager) CreateAgent(user *models.User) error {
85+
// CreateAgent creates a new agent user and returns the created user.
86+
func (u *Manager) CreateAgent(user *models.User) (models.User, error) {
5287
password, err := u.generatePassword()
5388
if err != nil {
5489
u.lo.Error("error generating password", "error", err)
55-
return envelope.NewError(envelope.GeneralError, u.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.user}"), nil)
90+
return models.User{}, envelope.NewError(envelope.GeneralError, u.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.user}"), nil)
5691
}
5792
user.Email = null.NewString(strings.TrimSpace(strings.ToLower(user.Email.String)), user.Email.Valid)
5893
if err := u.q.InsertAgent.QueryRow(user.Email, user.FirstName, user.LastName, password, user.AvatarURL, pq.Array(user.Roles)).Scan(&user.ID); err != nil {
5994
if dbutil.IsUniqueViolationError(err) {
60-
return envelope.NewError(envelope.GeneralError, u.i18n.T("user.sameEmailAlreadyExists"), nil)
95+
return models.User{}, envelope.NewError(envelope.GeneralError, u.i18n.T("user.sameEmailAlreadyExists"), nil)
6196
}
6297
u.lo.Error("error creating user", "error", err)
63-
return envelope.NewError(envelope.GeneralError, u.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.user}"), nil)
98+
return models.User{}, envelope.NewError(envelope.GeneralError, u.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.user}"), nil)
6499
}
65-
return nil
100+
return u.GetAgent(user.ID, "")
66101
}
67102

68103
// UpdateAgent updates an agent in the database, including their password if provided.
@@ -85,11 +120,12 @@ func (u *Manager) UpdateAgent(id int, user models.User) error {
85120
u.lo.Info("setting new password for user", "user_id", id)
86121
}
87122

88-
// Update user in the database.
123+
// Update user in the database and clear cache.
89124
if _, err := u.q.UpdateAgent.Exec(id, user.FirstName, user.LastName, user.Email, pq.Array(user.Roles), user.AvatarURL, hashedPassword, user.Enabled, user.AvailabilityStatus); err != nil {
90125
u.lo.Error("error updating user", "error", err)
91126
return envelope.NewError(envelope.GeneralError, u.i18n.Ts("globals.messages.errorUpdating", "name", "{globals.terms.user}"), nil)
92127
}
128+
u.InvalidateAgentCache(id)
93129
return nil
94130
}
95131

internal/user/queries.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ SELECT
5656
WHERE tm.user_id = u.id),
5757
'[]'
5858
) AS teams,
59-
array_agg(DISTINCT p) FILTER (WHERE p IS NOT NULL) AS permissions
59+
array_agg(DISTINCT p ORDER BY p) FILTER (WHERE p IS NOT NULL) AS permissions
6060
FROM users u
6161
LEFT JOIN user_roles ur ON ur.user_id = u.id
6262
LEFT JOIN roles r ON r.id = ur.role_id

internal/user/user.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"os"
1212
"regexp"
1313
"strings"
14+
"sync"
1415

1516
"log"
1617

@@ -43,10 +44,12 @@ var (
4344

4445
// Manager handles user-related operations.
4546
type Manager struct {
46-
lo *logf.Logger
47-
i18n *i18n.I18n
48-
q queries
49-
db *sqlx.DB
47+
lo *logf.Logger
48+
i18n *i18n.I18n
49+
q queries
50+
db *sqlx.DB
51+
agentCache map[int]models.User
52+
agentCacheMu sync.RWMutex
5053
}
5154

5255
// Opts contains options for initializing the Manager.
@@ -88,10 +91,11 @@ func New(i18n *i18n.I18n, opts Opts) (*Manager, error) {
8891
return nil, err
8992
}
9093
return &Manager{
91-
q: q,
92-
lo: opts.Lo,
93-
i18n: i18n,
94-
db: opts.DB,
94+
q: q,
95+
lo: opts.Lo,
96+
i18n: i18n,
97+
db: opts.DB,
98+
agentCache: make(map[int]models.User),
9599
}, nil
96100
}
97101

0 commit comments

Comments
 (0)