Skip to content

Commit b194048

Browse files
akuitybotfuskovic
akuitybot
and
fuskovic
authored
chore(backport release-1.8): fix: new OIDC claims rendering in UI (#5296)
Signed-off-by: fuskovic <[email protected]> Co-authored-by: Faris Huskovic <[email protected]>
1 parent 77f1c14 commit b194048

File tree

2 files changed

+173
-22
lines changed

2 files changed

+173
-22
lines changed

‎pkg/server/rbac/roles.go‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -682,16 +682,18 @@ func ResourcesToRole(
682682
kargoRole.KargoManaged = true
683683
}
684684

685-
for annotationKey, annotationValue := range sa.Annotations {
686-
if strings.HasPrefix(annotationKey, rbacapi.AnnotationKeyOIDCClaimNamePrefix) {
687-
kargoRole.Claims = append(
688-
kargoRole.Claims,
689-
rbacapi.Claim{
690-
Name: strings.ReplaceAll(annotationKey, rbacapi.AnnotationKeyOIDCClaimNamePrefix, ""),
691-
Values: strings.Split(annotationValue, ","),
692-
},
693-
)
694-
}
685+
claims, err := rbacapi.OIDCClaimsFromAnnotationValues(sa.Annotations)
686+
if err != nil {
687+
return nil, fmt.Errorf("failed to parse OIDC claims from annotation values: %w", err)
688+
}
689+
690+
for name, values := range claims {
691+
kargoRole.Claims = append(kargoRole.Claims,
692+
rbacapi.Claim{
693+
Name: name,
694+
Values: values,
695+
},
696+
)
695697
}
696698

697699
slices.SortFunc(kargoRole.Claims, func(lhs, rhs rbacapi.Claim) int {

‎pkg/server/rbac/roles_test.go‎

Lines changed: 161 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -267,15 +267,15 @@ func TestGet(t *testing.T) {
267267
Claims: []rbacapi.Claim{
268268
{
269269
Name: "email",
270-
Values: []string{"foo-email", "bar-email"},
270+
Values: []string{"bar-email", "foo-email"},
271271
},
272272
{
273273
Name: "groups",
274-
Values: []string{"foo-group", "bar-group"},
274+
Values: []string{"bar-group", "foo-group"},
275275
},
276276
{
277277
Name: "sub",
278-
Values: []string{"foo-sub", "bar-sub"},
278+
Values: []string{"bar-sub", "foo-sub"},
279279
},
280280
},
281281
// There should have been no attempt to normalize these rules
@@ -328,15 +328,15 @@ func TestGet(t *testing.T) {
328328
Claims: []rbacapi.Claim{
329329
{
330330
Name: "email",
331-
Values: []string{"foo-email", "bar-email"},
331+
Values: []string{"bar-email", "foo-email"},
332332
},
333333
{
334334
Name: "groups",
335-
Values: []string{"foo-group", "bar-group"},
335+
Values: []string{"bar-group", "foo-group"},
336336
},
337337
{
338338
Name: "sub",
339-
Values: []string{"foo-sub", "bar-sub"},
339+
Values: []string{"bar-sub", "foo-sub"},
340340
},
341341
},
342342
Rules: []rbacv1.PolicyRule{
@@ -654,15 +654,15 @@ func TestList(t *testing.T) {
654654
Claims: []rbacapi.Claim{
655655
{
656656
Name: "email",
657-
Values: []string{"foo-email", "bar-email"},
657+
Values: []string{"bar-email", "foo-email"},
658658
},
659659
{
660660
Name: "groups",
661-
Values: []string{"foo-group", "bar-group"},
661+
Values: []string{"bar-group", "foo-group"},
662662
},
663663
{
664664
Name: "sub",
665-
Values: []string{"foo-sub", "bar-sub"},
665+
Values: []string{"bar-sub", "foo-sub"},
666666
},
667667
},
668668
Rules: []rbacv1.PolicyRule{
@@ -725,15 +725,15 @@ func TestList(t *testing.T) {
725725
Claims: []rbacapi.Claim{
726726
{
727727
Name: "email",
728-
Values: []string{"foo-email", "bar-email"},
728+
Values: []string{"bar-email", "foo-email"},
729729
},
730730
{
731731
Name: "groups",
732-
Values: []string{"foo-group", "bar-group"},
732+
Values: []string{"bar-group", "foo-group"},
733733
},
734734
{
735735
Name: "sub",
736-
Values: []string{"foo-sub", "bar-sub"},
736+
Values: []string{"bar-sub", "foo-sub"},
737737
},
738738
},
739739
// There should have been no attempt to normalize these rules
@@ -1371,3 +1371,152 @@ func managedObjectMeta(annotations map[string]string) metav1.ObjectMeta {
13711371
Annotations: annotations,
13721372
}
13731373
}
1374+
1375+
func TestResourcesToRole(t *testing.T) {
1376+
testCases := []struct {
1377+
name string
1378+
sa *corev1.ServiceAccount
1379+
roles []rbacv1.Role
1380+
expectedClaims []rbacapi.Claim
1381+
assertions func(t *testing.T, role *rbacapi.Role, err error)
1382+
}{
1383+
{
1384+
name: "nil service account",
1385+
sa: nil,
1386+
assertions: func(t *testing.T, role *rbacapi.Role, err error) {
1387+
require.Nil(t, role)
1388+
require.Nil(t, err)
1389+
},
1390+
},
1391+
{
1392+
name: "no resources",
1393+
sa: new(corev1.ServiceAccount),
1394+
assertions: func(t *testing.T, role *rbacapi.Role, err error) {
1395+
require.NoError(t, err)
1396+
require.Empty(t, role.Claims)
1397+
require.Empty(t, role.Rules)
1398+
},
1399+
},
1400+
{
1401+
name: "kargo-managed",
1402+
sa: &corev1.ServiceAccount{
1403+
ObjectMeta: metav1.ObjectMeta{
1404+
Annotations: map[string]string{
1405+
rbacapi.AnnotationKeyManaged: rbacapi.AnnotationValueTrue,
1406+
},
1407+
},
1408+
},
1409+
assertions: func(t *testing.T, role *rbacapi.Role, err error) {
1410+
require.NoError(t, err)
1411+
require.True(t, role.KargoManaged)
1412+
require.Empty(t, role.Claims)
1413+
require.Empty(t, role.Rules)
1414+
},
1415+
},
1416+
{
1417+
name: "with old annotations",
1418+
sa: &corev1.ServiceAccount{
1419+
ObjectMeta: metav1.ObjectMeta{
1420+
Annotations: map[string]string{
1421+
rbacapi.AnnotationKeyOIDCClaim("groups"): "foo:bar",
1422+
},
1423+
},
1424+
},
1425+
assertions: func(t *testing.T, role *rbacapi.Role, err error) {
1426+
require.NoError(t, err)
1427+
require.Empty(t, role.Rules)
1428+
require.Len(t, role.Claims, 1)
1429+
require.Len(t, role.Claims[0].Values, 1)
1430+
require.Equal(t, "groups", role.Claims[0].Name)
1431+
require.Equal(t, "foo:bar", role.Claims[0].Values[0])
1432+
},
1433+
},
1434+
{
1435+
name: "with new annotations",
1436+
sa: &corev1.ServiceAccount{
1437+
ObjectMeta: metav1.ObjectMeta{
1438+
Annotations: map[string]string{
1439+
rbacapi.AnnotationKeyOIDCClaims: `{"groups":["foo:bar"]}`,
1440+
},
1441+
},
1442+
},
1443+
assertions: func(t *testing.T, role *rbacapi.Role, err error) {
1444+
require.NoError(t, err)
1445+
require.Empty(t, role.Rules)
1446+
require.Len(t, role.Claims, 1)
1447+
require.Len(t, role.Claims[0].Values, 1)
1448+
require.Equal(t, "groups", role.Claims[0].Name)
1449+
require.Equal(t, "foo:bar", role.Claims[0].Values[0])
1450+
},
1451+
},
1452+
{
1453+
name: "with both old and new annotations",
1454+
sa: &corev1.ServiceAccount{
1455+
ObjectMeta: metav1.ObjectMeta{
1456+
Annotations: map[string]string{
1457+
rbacapi.AnnotationKeyOIDCClaim("sub"): "foo-sub",
1458+
rbacapi.AnnotationKeyOIDCClaims: `
1459+
{
1460+
"email":["[email protected]"],
1461+
"sub":["another-sub"],
1462+
"groups":["another-group"]
1463+
}`,
1464+
rbacapi.AnnotationKeyOIDCClaim("groups"): "foo:bar",
1465+
},
1466+
},
1467+
},
1468+
assertions: func(t *testing.T, role *rbacapi.Role, err error) {
1469+
require.NoError(t, err)
1470+
require.Empty(t, role.Rules)
1471+
require.Len(t, role.Claims, 3)
1472+
1473+
claimsMap := map[string]rbacapi.Claim{}
1474+
for _, claim := range role.Claims {
1475+
claimsMap[claim.Name] = claim
1476+
}
1477+
1478+
emailClaim, ok := claimsMap["email"]
1479+
require.True(t, ok)
1480+
require.Len(t, emailClaim.Values, 1)
1481+
require.Equal(t, "[email protected]", emailClaim.Values[0])
1482+
1483+
groupsClaim, ok := claimsMap["groups"]
1484+
require.True(t, ok)
1485+
require.Len(t, groupsClaim.Values, 2)
1486+
require.Equal(t, "another-group", groupsClaim.Values[0])
1487+
require.Equal(t, "foo:bar", groupsClaim.Values[1])
1488+
1489+
subClaim, ok := claimsMap["sub"]
1490+
require.True(t, ok)
1491+
require.Len(t, subClaim.Values, 2)
1492+
require.Equal(t, "another-sub", subClaim.Values[0])
1493+
require.Equal(t, "foo-sub", subClaim.Values[1])
1494+
},
1495+
},
1496+
{
1497+
name: "policy rules",
1498+
sa: new(corev1.ServiceAccount),
1499+
roles: []rbacv1.Role{
1500+
*managedRole([]rbacv1.PolicyRule{{
1501+
APIGroups: []string{kargoapi.GroupVersion.Group},
1502+
Resources: []string{"stages", "promotions"},
1503+
Verbs: []string{"list", "get"},
1504+
}}),
1505+
},
1506+
assertions: func(t *testing.T, role *rbacapi.Role, err error) {
1507+
require.NoError(t, err)
1508+
require.Empty(t, role.Claims)
1509+
require.Len(t, role.Rules, 1)
1510+
require.Equal(t, []string{kargoapi.GroupVersion.Group}, role.Rules[0].APIGroups)
1511+
require.Equal(t, []string{"stages", "promotions"}, role.Rules[0].Resources)
1512+
require.Equal(t, []string{"list", "get"}, role.Rules[0].Verbs)
1513+
},
1514+
},
1515+
}
1516+
for _, tc := range testCases {
1517+
t.Run(tc.name, func(t *testing.T) {
1518+
role, err := ResourcesToRole(tc.sa, tc.roles, nil)
1519+
tc.assertions(t, role, err)
1520+
})
1521+
}
1522+
}

0 commit comments

Comments
 (0)