Skip to content

Commit

Permalink
Better describe AccessCapabilities response when role filtering is ap…
Browse files Browse the repository at this point in the history 
…plied
  • Loading branch information
@kopiczko
kopiczko committed Nov 15, 2024
1 parent 021d2a0 commit 2e4a6f2
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 43 deletions.
12 changes: 10 additions & 2 deletions api/proto/teleport/legacy/types/types.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down
12 changes: 10 additions & 2 deletions api/types/types.pb.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 5 additions & 6 deletions lib/auth/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{}
Expand Down Expand Up @@ -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() {
Expand Down
85 changes: 52 additions & 33 deletions lib/services/access_request_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
{
Expand Down Expand Up @@ -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",
Expand All @@ -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{},
},
}

Expand All @@ -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")
})
}
}
Expand Down

0 comments on commit 2e4a6f2

Please sign in to comment.