diff --git a/api/proto/teleport/legacy/types/types.proto b/api/proto/teleport/legacy/types/types.proto index 84e13ec12c6bd..dcd936e723983 100644 --- a/api/proto/teleport/legacy/types/types.proto +++ b/api/proto/teleport/legacy/types/types.proto @@ -2641,11 +2641,19 @@ message AccessRequestFilter { // is granted via their dynamic access privileges which may not be // calculable by directly examining the user's own static roles. message AccessCapabilities { - // RequestableRoles is a list of existent roles which the user is allowed to request. + // RequestableRoles is a list of existent roles which the user is allowed to request. It's only + // set when [[AccessCapabilitiesRequest]].RequestableRoles flag is set to true. + // + // If [[AccessCapabilitiesRequest]].RequestIDs is not empty and + // [[AccessCapabilitiesRequest]].FilterRequestableRolesByResource is set to true, + // ApplicableRolesForResources will be empty and this list will be trimmed down to roles which + // allow access to the provided resources. repeated string RequestableRoles = 1 [(gogoproto.jsontag) = "requestable_roles,omitempty"]; // SuggestedReviewers is a list of all reviewers which are suggested by the user's roles. repeated string SuggestedReviewers = 2 [(gogoproto.jsontag) = "suggested_reviewers,omitempty"]; - // ApplicableRolesForResources is a list of the roles applicable for access to a given set of resources. + // ApplicableRolesForResources is a list of the roles applicable for access to a given set of + // resources. It's calculated only when the [[AccessCapabilitiesRequest]].RequestIDs is not empty + // [[AccessCapabilitiesRequest]].FilterRequestableRolesByResource is false. repeated string ApplicableRolesForResources = 3 [(gogoproto.jsontag) = "applicable_roles,omitempty"]; // RequestPrompt is an optional message which tells users what they aught to request. string RequestPrompt = 4 [(gogoproto.jsontag) = "request_prompt,omitempty"]; diff --git a/api/types/types.pb.go b/api/types/types.pb.go index 8584c4bb8431b..0fce7d6ec2c72 100644 --- a/api/types/types.pb.go +++ b/api/types/types.pb.go @@ -7355,11 +7355,19 @@ var xxx_messageInfo_AccessRequestFilter proto.InternalMessageInfo // is granted via their dynamic access privileges which may not be // calculable by directly examining the user's own static roles. type AccessCapabilities struct { - // RequestableRoles is a list of existent roles which the user is allowed to request. + // RequestableRoles is a list of existent roles which the user is allowed to request. It's only + // set when [[AccessCapabilitiesRequest]].RequestableRoles flag is set to true. + // + // If [[AccessCapabilitiesRequest]].RequestIDs is not empty and + // [[AccessCapabilitiesRequest]].FilterRequestableRolesByResource is set to true, + // ApplicableRolesForResources will be empty and this list will be trimmed down to roles which + // allow access to the provided resources. RequestableRoles []string `protobuf:"bytes,1,rep,name=RequestableRoles,proto3" json:"requestable_roles,omitempty"` // SuggestedReviewers is a list of all reviewers which are suggested by the user's roles. SuggestedReviewers []string `protobuf:"bytes,2,rep,name=SuggestedReviewers,proto3" json:"suggested_reviewers,omitempty"` - // ApplicableRolesForResources is a list of the roles applicable for access to a given set of resources. + // ApplicableRolesForResources is a list of the roles applicable for access to a given set of + // resources. It's calculated only when the [[AccessCapabilitiesRequest]].RequestIDs is not empty + // [[AccessCapabilitiesRequest]].FilterRequestableRolesByResource is false. ApplicableRolesForResources []string `protobuf:"bytes,3,rep,name=ApplicableRolesForResources,proto3" json:"applicable_roles,omitempty"` // RequestPrompt is an optional message which tells users what they aught to request. RequestPrompt string `protobuf:"bytes,4,opt,name=RequestPrompt,proto3" json:"request_prompt,omitempty"` diff --git a/lib/auth/auth.go b/lib/auth/auth.go index a9a33d86c0ed1..e0377a8117157 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -5129,20 +5129,19 @@ func (a *Server) CreateAccessRequestV2(ctx context.Context, req types.AccessRequ req.SetCreationTime(now) - // Always perform variable expansion on creation only; this ensures the - // access request that is reviewed is the same that is approved. + // Access request has to be expanded during creation. See [[services.ExpandVars]] godoc to + // see why. expandOpts := services.ExpandVars(true) if err := services.ValidateAccessRequestForUser(ctx, a.clock, a, req, identity, expandOpts); err != nil { return nil, trace.Wrap(err) } // Look for user groups and associated applications to the request. - requestedResourceIDs := req.GetRequestedResourceIDs() var additionalResources []types.ResourceID var userGroups []types.ResourceID existingApps := map[string]struct{}{} - for _, resource := range requestedResourceIDs { + for _, resource := range req.GetRequestedResourceIDs() { switch resource.Kind { case types.KindApp: existingApps[resource.Name] = struct{}{} @@ -5175,8 +5174,8 @@ func (a *Server) CreateAccessRequestV2(ctx context.Context, req types.AccessRequ } if len(additionalResources) > 0 { - requestedResourceIDs = append(requestedResourceIDs, additionalResources...) - req.SetRequestedResourceIDs(requestedResourceIDs) + ids := append(req.GetRequestedResourceIDs(), additionalResources...) + req.SetRequestedResourceIDs(ids) } if req.GetDryRun() { diff --git a/lib/services/access_request_test.go b/lib/services/access_request_test.go index d4fdf79bc4de6..c30ada6b586fc 100644 --- a/lib/services/access_request_test.go +++ b/lib/services/access_request_test.go @@ -1512,7 +1512,8 @@ func TestPruneRequestRoles(t *testing.T) { }, }, loginHint: "responder", - // With "responder" login hint, only request node-access. + // With "responder" login hint, only request node-access (it has fewer + // Logins). expectRoles: []string{"node-access"}, }, { @@ -1775,30 +1776,34 @@ func TestGetRequestableRoles(t *testing.T) { user := g.user(t) tests := []struct { - name string - userRole string - requestedResources []types.ResourceID - disableFilter bool - allowedResourceIDs []types.ResourceID - expectedRoles []string + name string + userRole string + requestedResources []types.ResourceID + disableFilter bool + allowedResourceIDs []types.ResourceID + expectedRoles []string + expectedApplicableRolesForResources []string }{ { - name: "no resources to filter by", - userRole: "full-search", - expectedRoles: []string{"partial-access", "full-access"}, + name: "no resources to filter by", + userRole: "full-search", + expectedRoles: []string{"partial-access", "full-access"}, + expectedApplicableRolesForResources: []string{}, }, { - name: "filtering disabled", - userRole: "full-search", - requestedResources: []types.ResourceID{getResourceID(9)}, - disableFilter: true, - expectedRoles: []string{"partial-access", "full-access"}, + name: "filter by resources", + userRole: "full-search", + requestedResources: []types.ResourceID{getResourceID(9)}, + expectedRoles: []string{"full-access"}, + expectedApplicableRolesForResources: []string{}, }, { - name: "filter by resources", - userRole: "full-search", - requestedResources: []types.ResourceID{getResourceID(9)}, - expectedRoles: []string{"full-access"}, + name: "filtering disabled", + userRole: "full-search", + requestedResources: []types.ResourceID{getResourceID(9)}, + disableFilter: true, + expectedRoles: []string{"partial-access", "full-access"}, + expectedApplicableRolesForResources: []string{"full-access"}, }, { name: "resource in another cluster", @@ -1811,25 +1816,38 @@ func TestGetRequestableRoles(t *testing.T) { Name: "node-9", }, }, - expectedRoles: []string{"partial-access", "full-access"}, + expectedRoles: []string{"partial-access", "full-access"}, + expectedApplicableRolesForResources: []string{}, + }, + { + name: "resource user shouldn't know about", + userRole: "partial-search", + requestedResources: []types.ResourceID{getResourceID(9)}, + expectedRoles: []string{"partial-access", "full-access"}, + expectedApplicableRolesForResources: []string{}, }, { - name: "resource user shouldn't know about", - userRole: "partial-search", - requestedResources: []types.ResourceID{getResourceID(9)}, - expectedRoles: []string{"partial-access", "full-access"}, + name: "can request resource but not assume role", + userRole: "partial-roles", + requestedResources: []types.ResourceID{getResourceID(9)}, + expectedRoles: []string{}, + expectedApplicableRolesForResources: []string{}, }, { - name: "can view resource but not assume role", - userRole: "partial-roles", - requestedResources: []types.ResourceID{getResourceID(9)}, + name: "confirm can request resource when filtering disabled", + userRole: "partial-roles", + requestedResources: []types.ResourceID{getResourceID(9)}, + disableFilter: true, + expectedRoles: []string{"partial-access"}, + expectedApplicableRolesForResources: []string{"full-access"}, }, { - name: "prevent transitive access", - userRole: "partial-access", - requestedResources: []types.ResourceID{getResourceID(9)}, - allowedResourceIDs: []types.ResourceID{getResourceID(0), getResourceID(1), getResourceID(2), getResourceID(3), getResourceID(4)}, - expectedRoles: []string{"full-access", "full-search"}, + name: "prevent transitive access", + userRole: "partial-access", + requestedResources: []types.ResourceID{getResourceID(9)}, + allowedResourceIDs: []types.ResourceID{getResourceID(0), getResourceID(1), getResourceID(2), getResourceID(3), getResourceID(4)}, + expectedRoles: []string{"full-access", "full-search"}, + expectedApplicableRolesForResources: []string{}, }, } @@ -1847,7 +1865,8 @@ func TestGetRequestableRoles(t *testing.T) { FilterRequestableRolesByResource: !tc.disableFilter, }) require.NoError(t, err) - require.ElementsMatch(t, tc.expectedRoles, accessCaps.RequestableRoles) + require.ElementsMatch(t, tc.expectedRoles, accessCaps.RequestableRoles, "RequestableRoles") + require.ElementsMatch(t, tc.expectedApplicableRolesForResources, accessCaps.ApplicableRolesForResources, "ApplicableRolesForResources") }) } }