Skip to content

Commit 39f2145

Browse files
recharterecharte
committed
EVEREST-1799 get groups claim and validate permissions
1 parent e7ff34e commit 39f2145

12 files changed

+341
-78
lines changed

‎internal/server/handlers/rbac/backup_storage_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func TestRBAC_BackupStorage(t *testing.T) {
142142
},
143143
}
144144

145-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
145+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
146146
for _, tc := range testCases {
147147
t.Run(tc.desc, func(t *testing.T) {
148148
t.Parallel()
@@ -228,7 +228,7 @@ func TestRBAC_BackupStorage(t *testing.T) {
228228
},
229229
}
230230

231-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
231+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
232232
for _, tc := range testCases {
233233
t.Run(tc.desc, func(t *testing.T) {
234234
t.Parallel()
@@ -395,7 +395,7 @@ func TestRBAC_BackupStorage(t *testing.T) {
395395
},
396396
}
397397

398-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
398+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
399399
for _, tc := range testCases {
400400
t.Run(tc.desc, func(t *testing.T) {
401401
t.Parallel()
@@ -563,7 +563,7 @@ func TestRBAC_BackupStorage(t *testing.T) {
563563
},
564564
}
565565

566-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
566+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
567567
for _, tc := range testCases {
568568
t.Run(tc.desc, func(t *testing.T) {
569569
t.Parallel()
@@ -727,7 +727,7 @@ func TestRBAC_BackupStorage(t *testing.T) {
727727
},
728728
}
729729

730-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
730+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
731731
for _, tc := range testCases {
732732
t.Run(tc.desc, func(t *testing.T) {
733733
t.Parallel()
@@ -768,10 +768,10 @@ func newConfigMapPolicy(policy string) *corev1.ConfigMap {
768768
}
769769
}
770770

771-
func testUserGetter(ctx context.Context) (string, error) {
772-
user, ok := ctx.Value(common.UserCtxKey).(string)
771+
func testUserGetter(ctx context.Context) (rbac.User, error) {
772+
user, ok := ctx.Value(common.UserCtxKey).(rbac.User)
773773
if !ok {
774-
return "", errors.New("user not found in context")
774+
return rbac.User{}, errors.New("user not found in context")
775775
}
776776
return user, nil
777777
}

‎internal/server/handlers/rbac/database_cluster_backup_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func TestRBAC_DatabaseClusterBackup(t *testing.T) {
138138
},
139139
}
140140

141-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
141+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
142142
for _, tc := range testCases {
143143
t.Run(tc.desc, func(t *testing.T) {
144144
t.Parallel()
@@ -216,7 +216,7 @@ func TestRBAC_DatabaseClusterBackup(t *testing.T) {
216216
},
217217
}
218218

219-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
219+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
220220
for _, tc := range testCases {
221221
t.Run(tc.desc, func(t *testing.T) {
222222
t.Parallel()
@@ -282,7 +282,7 @@ func TestRBAC_DatabaseClusterBackup(t *testing.T) {
282282
},
283283
}
284284

285-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
285+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
286286
for _, tc := range testCases {
287287
t.Run(tc.desc, func(t *testing.T) {
288288
t.Parallel()
@@ -355,7 +355,7 @@ func TestRBAC_DatabaseClusterBackup(t *testing.T) {
355355
},
356356
}
357357

358-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
358+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
359359
for _, tc := range testCases {
360360
t.Run(tc.desc, func(t *testing.T) {
361361
t.Parallel()

‎internal/server/handlers/rbac/database_cluster_restore_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func TestRBAC_DatabaseClusterRestore(t *testing.T) {
9191
},
9292
}
9393

94-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
94+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
9595
for _, tc := range testCases {
9696
t.Run(tc.desc, func(t *testing.T) {
9797
t.Parallel()
@@ -156,7 +156,7 @@ func TestRBAC_DatabaseClusterRestore(t *testing.T) {
156156
},
157157
}
158158

159-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
159+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
160160
for _, tc := range testCases {
161161
t.Run(tc.desc, func(t *testing.T) {
162162
t.Parallel()
@@ -248,7 +248,7 @@ func TestRBAC_DatabaseClusterRestore(t *testing.T) {
248248
},
249249
}
250250

251-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
251+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
252252
for _, tc := range testCases {
253253
t.Run(tc.desc, func(t *testing.T) {
254254
t.Parallel()
@@ -359,7 +359,7 @@ func TestRBAC_DatabaseClusterRestore(t *testing.T) {
359359
},
360360
}
361361

362-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
362+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
363363
for _, tc := range testCases {
364364
t.Run(tc.desc, func(t *testing.T) {
365365
t.Parallel()
@@ -430,7 +430,7 @@ func TestRBAC_DatabaseClusterRestore(t *testing.T) {
430430
},
431431
}
432432

433-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
433+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
434434
for _, tc := range testCases {
435435
t.Run(tc.desc, func(t *testing.T) {
436436
t.Parallel()

‎internal/server/handlers/rbac/database_cluster_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
197197
},
198198
}
199199

200-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
200+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
201201
for _, tc := range testCases {
202202
t.Run(tc.desc, func(t *testing.T) {
203203
t.Parallel()
@@ -306,7 +306,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
306306
},
307307
}
308308

309-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
309+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
310310
for _, tc := range testCases {
311311
t.Run(tc.desc, func(t *testing.T) {
312312
t.Parallel()
@@ -434,7 +434,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
434434
},
435435
}
436436

437-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
437+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
438438
for _, tc := range testCases {
439439
t.Run(tc.desc, func(t *testing.T) {
440440
t.Parallel()
@@ -812,7 +812,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
812812
},
813813
}
814814

815-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
815+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
816816
for _, tc := range testCases {
817817
t.Run(tc.desc, func(t *testing.T) {
818818
t.Parallel()
@@ -892,7 +892,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
892892
)
893893
return h
894894
}
895-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
895+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
896896
for _, tc := range testCases {
897897
t.Run(tc.desc, func(t *testing.T) {
898898
t.Parallel()
@@ -956,7 +956,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
956956
&api.DatabaseClusterCredential{}, nil)
957957
return h
958958
}
959-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
959+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
960960
for _, tc := range testCases {
961961
t.Run(tc.desc, func(t *testing.T) {
962962
t.Parallel()
@@ -1011,7 +1011,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
10111011
[]api.DatabaseClusterComponent{}, nil)
10121012
return h
10131013
}
1014-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
1014+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
10151015
for _, tc := range testCases {
10161016
t.Run(tc.desc, func(t *testing.T) {
10171017
t.Parallel()
@@ -1066,7 +1066,7 @@ func TestRBAC_DatabaseCluster(t *testing.T) {
10661066
&api.DatabaseClusterPitr{}, nil)
10671067
return h
10681068
}
1069-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "test-user")
1069+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "test-user"})
10701070
for _, tc := range testCases {
10711071
t.Run(tc.desc, func(t *testing.T) {
10721072
t.Parallel()

‎internal/server/handlers/rbac/database_engine_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func TestRBAC_DatabaseEngines(t *testing.T) {
198198
},
199199
},
200200
}
201-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
201+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
202202
for _, tc := range testCases {
203203
t.Run(tc.desc, func(t *testing.T) {
204204
t.Parallel()
@@ -257,7 +257,7 @@ func TestRBAC_DatabaseEngines(t *testing.T) {
257257
return &h
258258
}
259259

260-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
260+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
261261
for _, tc := range testCases {
262262
t.Run(tc.desc, func(t *testing.T) {
263263
t.Parallel()
@@ -313,7 +313,7 @@ func TestRBAC_DatabaseEngines(t *testing.T) {
313313
return &h
314314
}
315315

316-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
316+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
317317
for _, tc := range testCases {
318318
t.Run(tc.desc, func(t *testing.T) {
319319
t.Parallel()
@@ -448,7 +448,7 @@ func TestRBAC_DatabaseEngines(t *testing.T) {
448448
},
449449
}
450450

451-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
451+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
452452
for _, tc := range testCases {
453453
t.Run(tc.desc, func(t *testing.T) {
454454
t.Parallel()
@@ -567,7 +567,7 @@ func TestRBAC_DatabaseEngines(t *testing.T) {
567567
},
568568
}
569569

570-
ctx := context.WithValue(context.Background(), common.UserCtxKey, "bob")
570+
ctx := context.WithValue(context.Background(), common.UserCtxKey, rbac.User{Subject: "bob"})
571571
for _, tc := range testCases {
572572
t.Run(tc.desc, func(t *testing.T) {
573573
t.Parallel()

‎internal/server/handlers/rbac/handler.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type rbacHandler struct {
2121
enforcer casbin.IEnforcer
2222
log *zap.SugaredLogger
2323
next handlers.Handler
24-
userGetter func(ctx context.Context) (string, error)
24+
userGetter func(ctx context.Context) (rbac.User, error)
2525
}
2626

2727
// New returns a new RBAC handler.
@@ -55,17 +55,23 @@ func (h *rbacHandler) enforce(
5555
action,
5656
object string,
5757
) error {
58-
subject, err := h.userGetter(ctx)
58+
user, err := h.userGetter(ctx)
5959
if err != nil {
6060
return err
6161
}
62-
ok, err := h.enforcer.Enforce(subject, resource, action, object)
63-
if err != nil {
64-
return fmt.Errorf("enforce error: %w", err)
65-
}
66-
if !ok {
67-
h.log.Warnf("Permission denied: [%s %s %s %s]", subject, resource, action, object)
68-
return ErrInsufficientPermissions
62+
63+
// User is allowed to perform the operation if the user's subject or any
64+
// of its groups have the required permission.
65+
for _, sub := range append([]string{user.Subject}, user.Groups...) {
66+
ok, err := h.enforcer.Enforce(sub, resource, action, object)
67+
if err != nil {
68+
return fmt.Errorf("enforce error: %w", err)
69+
}
70+
if ok {
71+
return nil
72+
}
6973
}
70-
return nil
74+
75+
h.log.Warnf("Permission denied: [%s %s %s %s]", user.Subject, resource, action, object)
76+
return ErrInsufficientPermissions
7177
}

‎internal/server/handlers/rbac/kubernetes.go

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package rbac
33

44
import (
55
"context"
6-
"errors"
76
"fmt"
87

98
"github.com/AlekSi/pointer"
@@ -28,15 +27,39 @@ func (h *rbacHandler) GetUserPermissions(ctx context.Context) (*api.UserPermissi
2827
if err != nil {
2928
return nil, err
3029
}
31-
perms, err := h.enforcer.GetImplicitPermissionsForUser(user)
32-
if err != nil {
33-
return nil, fmt.Errorf("failed to GetImplicitPermissionsForUser: %w", err)
30+
31+
// Let's use a map to deduplicate the permissions after resolving all roles
32+
permsMap := make(map[[4]string]struct{})
33+
34+
// Get permissions for the user and the groups it belongs to
35+
for _, sub := range append([]string{user.Subject}, user.Groups...) {
36+
perms, err := h.enforcer.GetImplicitPermissionsForUser(sub)
37+
if err != nil {
38+
return nil, fmt.Errorf("failed to GetImplicitPermissionsForUser: %w", err)
39+
}
40+
41+
// GetImplicitPermissionsForUser returns all policies assigned to the
42+
// user/group directly as well as the policies assigned to the roles
43+
// the user/group has. We need to resolve all roles for the user.
44+
for _, perm := range perms {
45+
if len(perm) != 4 {
46+
// This should never happen, but let's be safe
47+
return nil, fmt.Errorf("invalid permission")
48+
}
49+
50+
// We don't want to expose the groups or roles in the permissions
51+
// so we replace them with the user
52+
permsMap[[4]string{user.Subject, perm[1], perm[2], perm[3]}] = struct{}{}
53+
}
3454
}
3555

36-
if err := h.resolveRoles(user, perms); err != nil {
37-
return nil, err
56+
// Convert the map back to a slice
57+
result := make([][]string, len(permsMap))
58+
i := 0
59+
for k := range permsMap {
60+
result[i] = []string(k[:])
61+
i++
3862
}
39-
result := pointer.To(perms)
4063

4164
nextRes, err := h.next.GetUserPermissions(ctx)
4265
if err != nil {
@@ -49,25 +72,8 @@ func (h *rbacHandler) GetUserPermissions(ctx context.Context) (*api.UserPermissi
4972
}
5073

5174
res := &api.UserPermissions{
52-
Permissions: result,
75+
Permissions: pointer.To(result),
5376
Enabled: enabled,
5477
}
5578
return res, nil
5679
}
57-
58-
// For a given set of `permissions` for a `user`, this function
59-
// will resolve all roles for the user.
60-
func (h *rbacHandler) resolveRoles(user string, permissions [][]string) error {
61-
userRoles, err := h.enforcer.GetRolesForUser(user)
62-
if err != nil {
63-
return errors.Join(err, errors.New("cannot get user roles"))
64-
}
65-
for _, role := range userRoles {
66-
for i, perm := range permissions {
67-
if perm[0] == role {
68-
permissions[i][0] = user
69-
}
70-
}
71-
}
72-
return nil
73-
}

0 commit comments

Comments
 (0)