[v17] Filter IC Permission Sets based on access

Backports #49936 If a user is granted access to an IC Permission Set via an access request, the UI treats the referenced account as if all Permission Sets are granted. This hides the requestable Permission Sets from the user, making it impossible for a user to requests more Permission Sets on that account. This patch filters the Permission Sets delivered to the UI based on whether the user already has access the Permission Sets and marks any requestable Permission Sets on the account. This provides the UI with enough information to decide how to display the Permission Set list in order to get around the issue. --------- Co-authored-by: Marek Smoliński <[email protected]>
gravitational · Dec 13, 2024 · a53b42c · a53b42c
1 parent 70ab82c
commit a53b42c
Show file tree

Hide file tree

Showing 5 changed files with 232 additions and 31 deletions.
diff --git a/api/types/app.go b/api/types/app.go
@@ -421,7 +421,7 @@ func (a *AppV3) GetDisplayName() string {
 	if a.Spec.IdentityCenter == nil {
 		return ""
 	}
-	return a.GetName()
+	return a.Metadata.Description
 }
 
 // IsEqual determines if two application resources are equivalent to one another.

diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go
@@ -1301,7 +1301,7 @@ func (c *resourceAccess) checkAccess(resource types.ResourceWithLabels, filter s
 		return true, nil
 	}
 
-	// check access normally if base checker doesnt exist
+	// check access normally if base checker doesn't exist
 	if c.baseAuthChecker == nil {
 		if err := c.accessChecker.CanAccess(resource); err != nil {
 			if trace.IsAccessDenied(err) {

diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go
@@ -9513,3 +9513,206 @@ func TestRoleRequestReasonModeValidation(t *testing.T) {
 		})
 	}
 }
+
+func testUserName(testName string) string {
+	return strings.ReplaceAll(testName, " ", "_")
+}
+
+func TestFilterIdentityCenterPermissionSets(t *testing.T) {
+	const (
+		allAccessRoleName      = "all-access"
+		accountID              = "1234567890"
+		permissionSetArnPrefix = "aws:awn:test:permission:set:"
+	)
+
+	// GIVEN a test cluster...
+	ctx := context.Background()
+	srv := newTestTLSServer(t)
+	s := newTestServerWithRoles(t, srv.AuthServer, types.RoleAdmin)
+
+	// GIVEN an Identity Center Account with some associated Permission Set
+	// resources
+	permissionSets := []*identitycenterv1.PermissionSetInfo{
+		{
+			Name:         "PS One",
+			Arn:          permissionSetArnPrefix + "one",
+			AssignmentId: accountID + "-" + "ps_one",
+		},
+		{
+			Name:         "PS Two",
+			Arn:          permissionSetArnPrefix + "two",
+			AssignmentId: accountID + "-" + "ps_two",
+		},
+		{
+			Name:         "PS Three",
+			Arn:          permissionSetArnPrefix + "ps_three",
+			AssignmentId: accountID + "-" + "ps_three",
+		},
+	}
+
+	_, err := s.authServer.CreateIdentityCenterAccount(ctx,
+		services.IdentityCenterAccount{
+			Account: &identitycenterv1.Account{
+				Kind:    types.KindIdentityCenterAccount,
+				Version: types.V1,
+				Metadata: &headerv1.Metadata{
+					Name: accountID,
+					Labels: map[string]string{
+						types.OriginLabel: apicommon.OriginAWSIdentityCenter,
+					},
+				},
+				Spec: &identitycenterv1.AccountSpec{
+					Id:                accountID,
+					Arn:               "aws:arn:test:account",
+					Name:              "Test Account",
+					Description:       "An account for testing",
+					PermissionSetInfo: permissionSets,
+				},
+			},
+		})
+	require.NoError(t, err)
+
+	// GIVEN a role that allows access to all permission sets on the target
+	// Identity Center account
+	roleAccessAll, err := types.NewRole(allAccessRoleName, types.RoleSpecV6{
+		Allow: types.RoleConditions{
+			AccountAssignments: []types.IdentityCenterAccountAssignment{
+				{
+					Account:       accountID,
+					PermissionSet: types.Wildcard,
+				},
+			},
+		},
+	})
+	require.NoError(t, err, "Constructing role should succeed")
+	_, err = srv.Auth().CreateRole(ctx, roleAccessAll)
+	require.NoError(t, err, "Cretaing role should succeed")
+
+	withRequesterRole := WithRoleMutator(func(role types.Role) {
+		r := role.(*types.RoleV6)
+		r.Spec.Allow.Request = &types.AccessRequestConditions{
+			SearchAsRoles: []string{allAccessRoleName},
+		}
+	})
+
+	// EXPECT that the IC Account has made it to the cache
+	inlineEventually(t,
+		func() bool {
+			testAssignments, _, err := srv.Auth().ListIdentityCenterAccounts(
+				ctx, 100, &pagination.PageRequestToken{})
+			require.NoError(t, err)
+			return len(testAssignments) == 1
+		},
+		5*time.Second, 200*time.Millisecond,
+		"Target resource missing from cache")
+
+	testCases := []struct {
+		name                   string
+		roleModifiers          []CreateUserAndRoleOption
+		includeRequestable     bool
+		expectedPSs            []*types.IdentityCenterPermissionSet
+		expectedRequireRequest require.BoolAssertionFunc
+	}{
+		{
+			name: "basic access",
+			roleModifiers: []CreateUserAndRoleOption{
+				withAccountAssignment(types.Allow, accountID, permissionSets[0].Arn),
+				withAccountAssignment(types.Allow, accountID, permissionSets[1].Arn),
+			},
+			expectedPSs: []*types.IdentityCenterPermissionSet{
+				paginatedAppPermissionSet(permissionSets[0]),
+				paginatedAppPermissionSet(permissionSets[1]),
+			},
+			expectedRequireRequest: require.False,
+		},
+		{
+			name: "ignore search as roles when disabled",
+			roleModifiers: []CreateUserAndRoleOption{
+				withAccountAssignment(types.Allow, accountID, permissionSets[1].Arn),
+				withRequesterRole,
+			},
+			includeRequestable: false,
+			expectedPSs: []*types.IdentityCenterPermissionSet{
+				paginatedAppPermissionSet(permissionSets[1]),
+			},
+			expectedRequireRequest: require.False,
+		},
+		{
+			name: "requestable access",
+			roleModifiers: []CreateUserAndRoleOption{
+				withAccountAssignment(types.Allow, accountID, permissionSets[1].Arn),
+				withRequesterRole,
+			},
+			includeRequestable: true,
+			expectedPSs: []*types.IdentityCenterPermissionSet{
+				paginatedAppPermissionSet(permissionSets[0]),
+				paginatedAppPermissionSet(permissionSets[1]),
+				paginatedAppPermissionSet(permissionSets[2]),
+			},
+			expectedRequireRequest: require.True,
+		},
+		{
+			name: "no access",
+			roleModifiers: []CreateUserAndRoleOption{
+				withAccountAssignment(types.Allow, accountID, "some-non-existent-ps"),
+			},
+			expectedRequireRequest: require.False,
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			// GIVEN a user who has a role that allows a test-defined level of
+			// Identity Center access
+			user, _, err := CreateUserAndRole(srv.Auth(), testUserName(test.name),
+				nil, nil, test.roleModifiers...)
+			require.NoError(t, err)
+
+			// GIVEN an auth client using the above user
+			identity := TestUser(user.GetName())
+			clt, err := srv.NewClient(identity)
+			require.NoError(t, err)
+			t.Cleanup(func() { clt.Close() })
+
+			// WHEN I list the unified resources, with a filter specifically for
+			// the account resource defined above...
+			resp, err := clt.ListUnifiedResources(ctx, &proto.ListUnifiedResourcesRequest{
+				Kinds: []string{types.KindApp},
+				Labels: map[string]string{
+					types.OriginLabel: apicommon.OriginAWSIdentityCenter,
+				},
+				UseSearchAsRoles:   test.includeRequestable,
+				IncludeRequestable: test.includeRequestable,
+				IncludeLogins:      true,
+				SortBy:             types.SortBy{IsDesc: true, Field: types.ResourceMetadataName},
+			})
+
+			// EXPECT that the listing succeeds and returns a single resource
+			require.NoError(t, err)
+			require.Len(t, resp.Resources, 1, "Must return exactly one resource")
+
+			// EXPECT that the contained resource has the test-defined value for
+			// the RequiresRequest flag
+			resource := resp.Resources[0]
+			test.expectedRequireRequest(t, resource.RequiresRequest)
+
+			// EXPECT that the returned resource is an App
+			appServer := resp.Resources[0].GetAppServer()
+			require.NotNil(t, appServer, "Expected resource to be an app")
+			app := appServer.GetApp()
+
+			// EXPECT that the app PermissionSets are filtered to the test-defined
+			// list
+			require.ElementsMatch(t,
+				test.expectedPSs, app.GetIdentityCenter().PermissionSets)
+		})
+	}
+}
+
+func paginatedAppPermissionSet(src *identitycenterv1.PermissionSetInfo) *types.IdentityCenterPermissionSet {
+	return &types.IdentityCenterPermissionSet{
+		ARN:          src.Arn,
+		Name:         src.Name,
+		AssignmentID: src.AssignmentId,
+	}
+}
diff --git a/lib/services/local/identitycenter_events.go b/lib/services/local/identitycenter_events.go
@@ -65,7 +65,7 @@ func (p *identityCenterAccountParser) parse(event backend.Event) (types.Resource
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
-		return types.Resource153ToLegacy(services.IdentityCenterAccount{Account: r}), nil
+		return types.Resource153ToResourceWithLabels(services.IdentityCenterAccount{Account: r}), nil
 	default:
 		return nil, trace.BadParameter("event %v is not supported", event.Type)
 	}
@@ -109,7 +109,7 @@ func (p *identityCenterPrincipalAssignmentParser) parse(event backend.Event) (ty
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
-		return types.Resource153ToLegacy(r), nil
+		return types.Resource153ToResourceWithLabels(r), nil
 
 	default:
 		return nil, trace.BadParameter("event %v is not supported", event.Type)

diff --git a/lib/services/unified_resource.go b/lib/services/unified_resource.go
@@ -20,7 +20,6 @@ package services
 
 import (
 	"context"
-	"maps"
 	"strings"
 	"sync"
 	"time"
@@ -1015,37 +1014,36 @@ func makePaginatedIdentityCenterAccount(resourceKind string, resource types.Reso
 		}
 	}
 
-	protoResource := &proto.PaginatedResource{
-		Resource: &proto.PaginatedResource_AppServer{
-			AppServer: &types.AppServerV3{
-				Kind:     types.KindAppServer,
+	appServer := &types.AppServerV3{
+		Kind:     types.KindAppServer,
+		Version:  types.V3,
+		Metadata: resource.GetMetadata(),
+		Spec: types.AppServerSpecV3{
+			App: &types.AppV3{
+				Kind:     types.KindApp,
+				SubKind:  types.KindIdentityCenterAccount,
 				Version:  types.V3,
-				Metadata: resource.GetMetadata(),
-				Spec: types.AppServerSpecV3{
-					App: &types.AppV3{
-						Kind:    types.KindApp,
-						SubKind: types.KindIdentityCenterAccount,
-						Version: types.V3,
-						Metadata: types.Metadata{
-							Name:        acct.Spec.Name,
-							Description: acct.Spec.Description,
-							Labels:      maps.Clone(acct.Metadata.Labels),
-						},
-						Spec: types.AppSpecV3{
-							URI:        acct.Spec.StartUrl,
-							PublicAddr: acct.Spec.StartUrl,
-							AWS: &types.AppAWS{
-								ExternalID: acct.Spec.Id,
-							},
-							IdentityCenter: &types.AppIdentityCenter{
-								AccountID:      acct.Spec.Id,
-								PermissionSets: pss,
-							},
-						},
+				Metadata: types.Metadata153ToLegacy(acct.Metadata),
+				Spec: types.AppSpecV3{
+					URI:        acct.Spec.StartUrl,
+					PublicAddr: acct.Spec.StartUrl,
+					AWS: &types.AppAWS{
+						ExternalID: acct.Spec.Id,
+					},
+					IdentityCenter: &types.AppIdentityCenter{
+						AccountID:      acct.Spec.Id,
+						PermissionSets: pss,
 					},
 				},
 			},
 		},
+	}
+	appServer.Metadata.Description = acct.Spec.Name
+
+	protoResource := &proto.PaginatedResource{
+		Resource: &proto.PaginatedResource_AppServer{
+			AppServer: appServer,
+		},
 		RequiresRequest: requiresRequest,
 	}