diff --git a/lib/services/access_request.go b/lib/services/access_request.go index d9343e20168f..2c78aacad0cb 100644 --- a/lib/services/access_request.go +++ b/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 { diff --git a/lib/services/access_request_test.go b/lib/services/access_request_test.go index 7268ec3223a0..1d1712bb6f72 100644 --- a/lib/services/access_request_test.go +++ b/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) {