Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3805,27 +3805,27 @@ type ResourcePage[T types.ResourceWithLabels] struct {
// PaginatedResource returned from the rpc ListUnifiedResources.
func convertEnrichedResource(resource *proto.PaginatedResource) (*types.EnrichedResource, error) {
if r := resource.GetNode(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r, Logins: resource.Logins}, nil
return &types.EnrichedResource{ResourceWithLabels: r, Logins: resource.Logins, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetDatabaseServer(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetDatabaseService(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetAppServerOrSAMLIdPServiceProvider(); r != nil { //nolint:staticcheck // SA1019. TODO(sshah) DELETE IN 17.0
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetWindowsDesktop(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r, Logins: resource.Logins}, nil
return &types.EnrichedResource{ResourceWithLabels: r, Logins: resource.Logins, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetWindowsDesktopService(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetKubeCluster(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetKubernetesServer(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetUserGroup(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetAppServer(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else if r := resource.GetSAMLIdPServiceProvider(); r != nil {
return &types.EnrichedResource{ResourceWithLabels: r}, nil
return &types.EnrichedResource{ResourceWithLabels: r, RequiresRequest: resource.RequiresRequest}, nil
} else {
return nil, trace.BadParameter("received unsupported resource %T", resource.Resource)
}
Expand Down
1,785 changes: 937 additions & 848 deletions api/client/proto/authservice.pb.go

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions api/proto/teleport/legacy/client/proto/authservice.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1871,6 +1871,9 @@ message PaginatedResource {

// Logins allowed for the included resource. Only to be populated for SSH and Desktops.
repeated string Logins = 13 [(gogoproto.jsontag) = "logins,omitempty"];
// RequiresRequest indicates if a resource requires an access request to access. Only populated with requests
// that IncludeRequestable.
bool RequiresRequest = 14 [(gogoproto.jsontag) = "requires_request,omitempty"];

reserved 4;
reserved "KubeService";
Expand Down Expand Up @@ -1915,6 +1918,8 @@ message ListUnifiedResourcesRequest {
// IncludeLogins indicates that the response should include a users allowed logins
// for all returned resources.
bool IncludeLogins = 12 [(gogoproto.jsontag) = "include_logins,omitempty"];
// IncludeRequestable indicates that the response should include resources that the user must request access to.
bool IncludeRequestable = 14 [(gogoproto.jsontag) = "include_proto,omitempty"];
}

// ListUnifiedResourceResponse response of ListUnifiedResources.
Expand Down
4 changes: 4 additions & 0 deletions api/types/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ type EnrichedResource struct {
ResourceWithLabels
// Logins that the user is allowed to access the above resource with.
Logins []string
// RequiresRequest is true if a resource is being returned to the user but requires
// an access request to access. This is done during `ListUnifiedResources` when
// searchAsRoles is true
RequiresRequest bool
}

// ResourcesWithLabels is a list of labeled resources.
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/assist/assistv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func assembleSearchResponse(ctx context.Context, a *Service, documents []*ai.Doc
resources = append(resources, resource)
}

paginated, err := services.MakePaginatedResources(types.KindUnifiedResource, resources)
paginated, err := services.MakePaginatedResources(types.KindUnifiedResource, resources, nil /* requestable map */)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
106 changes: 72 additions & 34 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -1208,10 +1208,25 @@ func (a *ServerWithRoles) GetNode(ctx context.Context, namespace, name string) (
return node, nil
}

func (a *ServerWithRoles) checkUnifiedAccess(resource types.ResourceWithLabels, checker resourceAccessChecker, filter services.MatchResourceFilter, resourceAccessMap map[string]error) (bool, error) {
// resourceAccess is used for different rbac results when filtering items
type resourceAccess struct {
// accessChecker is used to check a user's access to resources. This can
// be extended to include SearchAsRoles
accessChecker resourceAccessChecker
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment to this field too?

// baseAuthChecker is set when a user's auth context is extended during a
// searchAsRoles request
baseAuthChecker resourceAccessChecker
// kindAccessMap is used to check errors for list/read verbs per kind
kindAccessMap map[string]error
// requestableMap is used to track if a resource matches a filter but is only
// available after an access request. This map is of Resource.GetName()
requestableMap map[string]struct{}
}

func (c *resourceAccess) checkAccess(resource types.ResourceWithLabels, filter services.MatchResourceFilter) (bool, error) {
resourceKind := resource.GetKind()

if canAccessErr := resourceAccessMap[resourceKind]; canAccessErr != nil {
if canAccessErr := c.kindAccessMap[resourceKind]; canAccessErr != nil {
// skip access denied error. It is expected that resources won't be available
// to some users and we want to keep iterating until we've reached the request limit
// of resources they have access to
Expand Down Expand Up @@ -1241,11 +1256,35 @@ func (a *ServerWithRoles) checkUnifiedAccess(resource types.ResourceWithLabels,
return true, nil
}

if err := checker.CanAccess(resource); err != nil {
if trace.IsAccessDenied(err) {
return false, nil
// check access normally if base checker doesnt exist
if c.baseAuthChecker == nil {
if err := c.accessChecker.CanAccess(resource); err != nil {
if trace.IsAccessDenied(err) {
return false, nil
}
return false, trace.Wrap(err)
}
return false, trace.Wrap(err)
return true, nil
}

// baseAuthChecker exists if the current auth context has been extended for a includeRequestable request.
// if true, we should check with the base auth checker first and if that returns false, check the extended context
// so we know if a resource is being matched because they have access to it currently, or only to be added
// to an access request
if err := c.baseAuthChecker.CanAccess(resource); err != nil {
if !trace.IsAccessDenied(err) {
return false, trace.Wrap(err)
}

// user does not have access with their base context
// check if they would have access with the extended context
if err := c.accessChecker.CanAccess(resource); err != nil {
if trace.IsAccessDenied(err) {
return false, nil
}
return false, trace.Wrap(err)
}
c.requestableMap[resource.GetName()] = struct{}{}
}

return true, nil
Expand All @@ -1267,10 +1306,16 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
filter.PredicateExpression = expression
}

// Populate resourceAccessMap with any access errors the user has for each possible
// resource kind. This allows the access check to occur a single time per resource
// kind instead of once per matching resource.
resourceAccessMap := make(map[string]error, len(services.UnifiedResourceKinds))
resourceAccess := &resourceAccess{
// Populate kindAccessMap with any access errors the user has for each possible
// resource kind. This allows the access check to occur a single time per resource
// kind instead of once per matching resource.
kindAccessMap: make(map[string]error, len(services.UnifiedResourceKinds)),
// requestableMap is populated with resources that are being returned but can only
// be accessed to the user via an access request
requestableMap: make(map[string]struct{}),
}

for _, kind := range services.UnifiedResourceKinds {
actionVerbs := []string{types.VerbList, types.VerbRead}
if kind == types.KindNode {
Expand All @@ -1282,7 +1327,7 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
actionVerbs = []string{types.VerbList}
}

resourceAccessMap[kind] = a.action(apidefaults.Namespace, kind, actionVerbs...)
resourceAccess.kindAccessMap[kind] = a.action(apidefaults.Namespace, kind, actionVerbs...)
}

// Before doing any listing, verify that the user is allowed to list
Expand All @@ -1294,7 +1339,7 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
}
var rbacErrors int
for _, kind := range requested {
if err, ok := resourceAccessMap[kind]; ok && err != nil {
if err, ok := resourceAccess.kindAccessMap[kind]; ok && err != nil {
rbacErrors++
}
}
Expand All @@ -1319,6 +1364,16 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
return nil, trace.Wrap(err)
}
baseContext := a.context
// If IncludeRequestable is true, we will create a baseChecker to
// use during RBAC to determine if a resource would be available only after extending
if req.IncludeRequestable {
baseChecker, err := a.newResourceAccessChecker(types.KindUnifiedResource)
if err != nil {
return nil, trace.Wrap(err)
}
resourceAccess.baseAuthChecker = baseChecker
}

a.context = *extendedContext
defer func() {
a.context = baseContext
Expand All @@ -1330,7 +1385,8 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
return nil, trace.Wrap(err)
}

// Fetch full list of resources in the backend.
resourceAccess.accessChecker = checker

var (
unifiedResources types.ResourcesWithLabels
nextKey string
Expand All @@ -1344,7 +1400,7 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
return &proto.ListUnifiedResourcesResponse{}, nil
}
unifiedResources, err = a.authServer.UnifiedResourceCache.GetUnifiedResourcesByIDs(ctx, prefs.ClusterPreferences.PinnedResources.GetResourceIds(), func(resource types.ResourceWithLabels) (bool, error) {
match, err := a.checkUnifiedAccess(resource, checker, filter, resourceAccessMap)
match, err := resourceAccess.checkAccess(resource, filter)
return match, trace.Wrap(err)
})
if err != nil {
Expand All @@ -1359,15 +1415,15 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
}
} else {
unifiedResources, nextKey, err = a.authServer.UnifiedResourceCache.IterateUnifiedResources(ctx, func(resource types.ResourceWithLabels) (bool, error) {
match, err := a.checkUnifiedAccess(resource, checker, filter, resourceAccessMap)
match, err := resourceAccess.checkAccess(resource, filter)
return match, trace.Wrap(err)
}, req)
if err != nil {
return nil, trace.Wrap(err)
}
}

paginatedResources, err := services.MakePaginatedResources(types.KindUnifiedResource, unifiedResources)
paginatedResources, err := services.MakePaginatedResources(types.KindUnifiedResource, unifiedResources, resourceAccess.requestableMap)
if err != nil {
return nil, trace.Wrap(err, "making paginated unified resources")
}
Expand Down Expand Up @@ -1467,24 +1523,6 @@ func (a *ServerWithRoles) authContextForSearch(ctx context.Context, req *proto.L
return nil, trace.Wrap(err)
}

// Only emit the event if the role list actually changed
if len(extendedContext.Checker.RoleNames()) != len(a.context.Checker.RoleNames()) {
if err := a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.AccessRequestResourceSearch{
Metadata: apievents.Metadata{
Type: events.AccessRequestResourceSearch,
Code: events.AccessRequestResourceSearchCode,
},
UserMetadata: authz.ClientUserMetadata(ctx),
SearchAsRoles: extendedContext.Checker.RoleNames(),
ResourceType: req.ResourceType,
Namespace: req.Namespace,
Labels: req.Labels,
PredicateExpression: req.PredicateExpression,
SearchKeywords: req.SearchKeywords,
}); err != nil {
return nil, trace.Wrap(err)
}
}
Comment thread
nklaassen marked this conversation as resolved.
return extendedContext, nil
}

Expand Down
Loading