Only run deduplication when the request is being created

lib/services/access_request.go

@@ -1110,19 +1110,6 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest
 		return trace.BadParameter("only promoted requests can set the promoted access list title")
 	}
 
-	// deduplicate requested resource IDs
-	var deduplicated []types.ResourceID
-	seen := make(map[string]struct{})
-	for _, resource := range req.GetRequestedResourceIDs() {
-		id := types.ResourceIDToString(resource)
-		if _, isDuplicate := seen[id]; isDuplicate {
-			continue
-		}
-		seen[id] = struct{}{}
-		deduplicated = append(deduplicated, resource)
-	}
-	req.SetRequestedResourceIDs(deduplicated)
-
 	// check for "wildcard request" (`roles=*`).  wildcard requests
 	// need to be expanded into a list consisting of all existing roles
 	// that the user does not hold and is allowed to request.
@@ -1168,6 +1155,19 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest
 	}
 
 	if m.opts.expandVars {
+		// deduplicate requested resource IDs
+		var deduplicated []types.ResourceID
+		seen := make(map[string]struct{})
+		for _, resource := range req.GetRequestedResourceIDs() {
+			id := types.ResourceIDToString(resource)
+			if _, isDuplicate := seen[id]; isDuplicate {
+				continue
+			}
+			seen[id] = struct{}{}
+			deduplicated = append(deduplicated, resource)
+		}
+		req.SetRequestedResourceIDs(deduplicated)
+
 		// determine the roles which should be requested for a resource access
 		// request, and write them to the request
 		if err := m.setRolesForResourceRequest(ctx, req); err != nil {

lib/services/access_request_test.go

@@ -21,7 +21,6 @@ package services
 import (
 	"context"
 	"fmt"
-	"slices"
 	"strconv"
 	"strings"
 	"testing"
@@ -795,13 +794,6 @@ func TestMaxLength(t *testing.T) {
 
 	req.SetRequestReason(strings.Repeat("a", maxAccessRequestReasonSize))
 	require.Error(t, ValidateAccessRequest(req))
-
-	resourceIDs, err := types.ResourceIDsFromString(`["/cluster/node/some-node", "/cluster/node/some-other-node" ]`)
-	require.NoError(t, err)
-
-	req.SetRequestReason("")
-	req.SetRequestedResourceIDs(slices.Repeat(resourceIDs, maxResourcesPerRequest/2+1))
-	require.Error(t, ValidateAccessRequest(req))
 }
 
 // TestThresholdReviewFilter verifies basic filter syntax.
@@ -2170,9 +2162,37 @@ func TestValidateDuplicateRequestedResources(t *testing.T) {
 		desktops:    make(map[string]types.WindowsDesktop),
 		clusterName: "someCluster",
 	}
-	testRole, err := types.NewRole("testRole", types.RoleSpecV6{})
+
+	for i := 1; i < 3; i++ {
+		node, err := types.NewServerWithLabels(
+			fmt.Sprintf("resource%d", i),
+			types.KindNode,
+			types.ServerSpecV2{},
+			map[string]string{"foo": "bar"},
+		)
+		require.NoError(t, err)
+		g.nodes[node.GetName()] = node
+	}
+
+	searchAsRole, err := types.NewRole("searchAs", types.RoleSpecV6{
+		Allow: types.RoleConditions{
+			Logins:     []string{"root"},
+			NodeLabels: types.Labels{"*": []string{"*"}},
+		},
+	})
+	require.NoError(t, err)
+	g.roles[searchAsRole.GetName()] = searchAsRole
+
+	testRole, err := types.NewRole("testRole", types.RoleSpecV6{
+		Allow: types.RoleConditions{
+			Request: &types.AccessRequestConditions{
+				SearchAsRoles: []string{searchAsRole.GetName()},
+			},
+		},
+	})
 	require.NoError(t, err)
 	g.roles[testRole.GetName()] = testRole
+
 	user := g.user(t, testRole.GetName())
 
 	clock := clockwork.NewFakeClock()
@@ -2183,15 +2203,15 @@ func TestValidateDuplicateRequestedResources(t *testing.T) {
 	req, err := types.NewAccessRequestWithResources("name", user, nil, /* roles */
 		[]types.ResourceID{
 			{ClusterName: "someCluster", Kind: "node", Name: "resource1"},
-			{ClusterName: "someCluster", Kind: "node", Name: "resource1"}, // a true duplicate
-			{ClusterName: "someCluster", Kind: "app", Name: "resource1"},  // not a duplicate
+			{ClusterName: "someCluster", Kind: "node", Name: "resource1"}, // a  duplicate
+			{ClusterName: "someCluster", Kind: "node", Name: "resource2"}, // not a duplicate
 		})
 	require.NoError(t, err)
 
-	require.NoError(t, ValidateAccessRequestForUser(context.Background(), clock, g, req, identity))
+	require.NoError(t, ValidateAccessRequestForUser(context.Background(), clock, g, req, identity, ExpandVars(true)))
 	require.Len(t, req.GetRequestedResourceIDs(), 2)
 	require.Equal(t, "/someCluster/node/resource1", types.ResourceIDToString(req.GetRequestedResourceIDs()[0]))
-	require.Equal(t, "/someCluster/app/resource1", types.ResourceIDToString(req.GetRequestedResourceIDs()[1]))
+	require.Equal(t, "/someCluster/node/resource2", types.ResourceIDToString(req.GetRequestedResourceIDs()[1]))
 }
 
 func TestValidateAccessRequestClusterNames(t *testing.T) {