Skip to content
Open
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
17 changes: 14 additions & 3 deletions api/types/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,22 @@ type Server interface {
GetImmutableLabels() map[string]string
}

type serverOpt func(*ServerV2)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
type serverOpt func(*ServerV2)
type ServerOpt func(*ServerV2)


func ServerWithScope(scope string) serverOpt {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func ServerWithScope(scope string) serverOpt {
func WithScope(scope string) serverOpt {

return func(s *ServerV2) {
s.Scope = scope
}
}

// NewServer creates an instance of Server.
func NewServer(name, kind string, spec ServerSpecV2) (Server, error) {
return NewServerWithLabels(name, kind, spec, map[string]string{})
func NewServer(name, kind string, spec ServerSpecV2, opts ...serverOpt) (Server, error) {

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.

We could also consider scope ...string instead of serverOpt here if the intent is to limit the blast radius in this PR, then just follow up and update all call sites to either pass in the scope or empty string later.

return NewServerWithLabels(name, kind, spec, map[string]string{}, opts...)
}

// NewServerWithLabels is a convenience method to create
// ServerV2 with a specific map of labels.
func NewServerWithLabels(name, kind string, spec ServerSpecV2, labels map[string]string) (Server, error) {
func NewServerWithLabels(name, kind string, spec ServerSpecV2, labels map[string]string, opts ...serverOpt) (Server, error) {
server := &ServerV2{
Kind: kind,
Metadata: Metadata{
Expand All @@ -140,6 +148,9 @@ func NewServerWithLabels(name, kind string, spec ServerSpecV2, labels map[string
},
Spec: spec,
}
for _, opt := range opts {
opt(server)
}
if err := server.CheckAndSetDefaults(); err != nil {
return nil, trace.Wrap(err)
}
Expand Down
122 changes: 83 additions & 39 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -1961,8 +1961,6 @@ func (a *ScopedServerWithRoles) ListUnifiedResources(ctx context.Context, req *p
return nil, trace.AccessDenied("include_requestable is not supported for scoped identities")
case req.PinnedOnly:
return nil, trace.AccessDenied("pinned_only is not supported for scoped identities")
case req.IncludeLogins:
return nil, trace.AccessDenied("include_logins is not supported for scoped identities")
}

if len(req.Kinds) != 1 || req.Kinds[0] != types.KindNode {
Expand All @@ -1989,49 +1987,20 @@ func (a *ScopedServerWithRoles) ListUnifiedResources(ctx context.Context, req *p
return nil, trace.Wrap(err)
}

resourceLister := &unifiedResourceLister{}
resourceLister.accessChecker = &scopedResourceChecker{
ctx: ctx,
scopedContext: *a.scopedContext,
}
Comment on lines +1990 to +1994

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
resourceLister := &unifiedResourceLister{}
resourceLister.accessChecker = &scopedResourceChecker{
ctx: ctx,
scopedContext: *a.scopedContext,
}
resourceLister := &unifiedResourceLister{
accessChecker: &scopedResourceChecker{
ctx: ctx,
scopedContext: *a.scopedContext,
},
}


unifiedResources, nextKey, err := a.authServer.UnifiedResourceCache.IterateUnifiedResources(ctx, func(resource types.ResourceWithLabels) (bool, error) {
// currently only nodes are supported
if resource.GetKind() != types.KindNode {
return false, nil
}

// Filter first and only check RBAC if there is a match to improve perf.
match, err := services.MatchResourceByFilters(resource, userFilter, nil)
if err != nil {
logger.WarnContext(ctx, "Unable to determine access to resource, matching with filter failed",
"resource_name", resource.GetName(),
"resource_kind", resource.GetKind(),
"error", err,
)
return false, nil
}
if !match {
return false, nil
}

server, ok := resource.(*types.ServerV2)
if !ok {
logger.WarnContext(ctx, "Unable to cast unified resource to server",
"resource_name", resource.GetName(),
"resource_kind", resource.GetKind(),
)
return false, nil
}

serverScope := scopes.Root
if server.Scope != "" {
serverScope = server.Scope
}

if err := a.scopedContext.CheckerContext.Decision(ctx, serverScope, func(checker *services.ScopedAccessChecker) error {
return checker.SSH().CanAccessSSHServer(server)
}); err == nil {
return true, nil
} else if !trace.IsAccessDenied(err) {
return false, trace.Wrap(err)
}

return false, nil
match, err := resourceLister.canList(resource, userFilter)
return match, trace.Wrap(err)
}, req)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -2042,6 +2011,22 @@ func (a *ScopedServerWithRoles) ListUnifiedResources(ctx context.Context, req *p
return nil, trace.Wrap(err, "making paginated unified resources")
}

if req.IncludeLogins {
for _, r := range paginatedResources {
if n := r.GetNode(); n != nil {
logins, err := resourceLister.getAllowedLogins(n)
if err != nil {
a.authServer.logger.WarnContext(ctx, "Unable to determine logins for node",
"error", err,
"resource", n.GetName(),
)
continue
}
r.Logins = logins
}
}
}

return &proto.ListUnifiedResourcesResponse{
NextKey: nextKey,
Resources: paginatedResources,
Expand Down Expand Up @@ -2698,6 +2683,65 @@ func newResourceAccessChecker(authCtx authz.Context, resource string) (*resource
}
}

type scopedResourceChecker struct {
ctx context.Context
scopedContext authz.ScopedContext
}

func (c *scopedResourceChecker) CanAccess(resource types.ResourceWithLabels) error {
server, ok := resource.(*types.ServerV2)
if !ok {
logger.WarnContext(c.ctx, "Unable to cast unified resource to server",
"resource_name", resource.GetName(),
"resource_kind", resource.GetKind(),
)
return trace.AccessDenied("scoped resource checker only supports servers")
}

serverScope := scopes.Root
if server.Scope != "" {
serverScope = server.Scope
}
Comment on lines +2701 to +2704

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Here and below:

Suggested change
serverScope := scopes.Root
if server.Scope != "" {
serverScope = server.Scope
}
serverScope := cmp.Or(server.Scope, scopes.Root)


err := c.scopedContext.CheckerContext.Decision(c.ctx, serverScope,
func(checker *services.ScopedAccessChecker) error {
return checker.SSH().CanAccessSSHServer(server)
})
return trace.Wrap(err)
}

func (c *scopedResourceChecker) GetAllowedLoginsForResource(
resource services.AccessCheckable,
) ([]string, error) {
server, ok := resource.(*types.ServerV2)
if !ok {
logger.WarnContext(c.ctx, "Unable to cast unified resource to server",
"resource_name", resource.GetName(),
"resource_kind", resource.GetKind(),
)
return nil, trace.AccessDenied("scoped resource checker only supports servers")
}

serverScope := scopes.Root
if server.Scope != "" {
serverScope = server.Scope
}

var logins []string
for checker, err := range c.scopedContext.CheckerContext.CheckersForResourceScope(c.ctx, serverScope) {
if err != nil {
return nil, trace.Wrap(err)
}
serverLogins, err := checker.SSH().GetAllowedLoginsForServer(server)
if err != nil {
return nil, trace.Wrap(err)
}
logins = append(logins, serverLogins...)
}

return logins, nil
}

// createOktaRequestableResourceChecker creates [oktaRequestableResoruceChecker].
func createOktaRequestableResourceChecker(ctx context.Context, plugins services.Plugins, underlying resourceCheckerI) (*oktaRequestableResoruceChecker, error) {
bidirectionalSync, err := okta.BidirectionalSyncEnabled(ctx, plugins)
Expand Down
163 changes: 163 additions & 0 deletions lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8989,6 +8989,169 @@ func TestListUnifiedResources_WithPredicate(t *testing.T) {
require.Error(t, err)
}

func TestListUnifiedResources_ScopedNodes(t *testing.T) {
t.Parallel()
srv := newTestTLSServer(t, withScopesFeatures(scopes.Features{Enabled: true, AgentPinEnabled: true}))

const scope = "/test"
const childScope = "/test/child"
const otherScope = "/other"

createNode := func(t *testing.T, name, scope string, labels map[string]string) {
t.Helper()
node, err := types.NewServerWithLabels(
name,
types.KindNode,
types.ServerSpecV2{
Hostname: name + "-host",
},
labels,
types.ServerWithScope(scope),
)
require.NoError(t, err)
_, err = srv.Auth().UpsertNode(t.Context(), node)
require.NoError(t, err)
}

// Create nodes in various scopes with differing labels.
createNode(t, "prod-node", scope, map[string]string{"env": "prod"})
createNode(t, "dev-node", scope, map[string]string{"env": "dev"})
createNode(t, "dev-child-node", childScope, map[string]string{"env": "dev"})
createNode(t, "other-scope-node", otherScope, map[string]string{"env": "prod"})
createNode(t, "unscoped-node", "", map[string]string{"env": "prod"})

sshLabels := func(env string) *scopedaccessv1.ScopedRoleSSH {
return scopedaccessv1.ScopedRoleSSH_builder{
Logins: []string{"root"},
Labels: []*labelv1.Label{
labelv1.Label_builder{
Name: "env",
Values: []string{env},
}.Build(),
},
}.Build()
}
sshLabelExpression := func(expr string) *scopedaccessv1.ScopedRoleSSH {
return scopedaccessv1.ScopedRoleSSH_builder{
Logins: []string{"root"},
LabelExpression: expr,
}.Build()
}

// scopedSSHUser creates a scope-pinned user whose scoped role grants the given ssh block.
scopedSSHUser := func(username, scope string, ssh *scopedaccessv1.ScopedRoleSSH) *auth.ScopedServerWithRoles {
return newScopePinnedTestServerWithScopedUser(t, srv.AuthServer, username, scope,
scopedaccessv1.ScopedRoleSpec_builder{
AssignableScopes: []string{scope},
Ssh: ssh,
}.Build())
}

cases := []struct {
name string
server *auth.ScopedServerWithRoles
nodeNamesExpected []string
loginsExpected []string
req *proto.ListUnifiedResourcesRequest
}{
{
name: "prod labels in scope " + scope,
server: scopedSSHUser("node-test-prod-label", scope, sshLabels("prod")),
nodeNamesExpected: []string{"prod-node"},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
},
},
{
name: "dev label in " + scope,
server: scopedSSHUser("node-test-dev-label", scope, sshLabels("dev")),
nodeNamesExpected: []string{"dev-node", "dev-child-node"},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
},
},
{
name: "dev label expression in " + scope,
server: scopedSSHUser("node-test-dev-expression", scope, sshLabelExpression(`contains(labels["env"], "dev")`)),
nodeNamesExpected: []string{"dev-node", "dev-child-node"},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
},
},
{
name: "prod label expression in " + scope,
server: scopedSSHUser("node-test-prod-expression", scope, sshLabelExpression(`contains(labels["env"], "prod")`)),
nodeNamesExpected: []string{"prod-node"},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
},
},
{
name: "prod label expression in " + childScope,
server: scopedSSHUser("node-test-child-prod-expr", childScope, sshLabelExpression(`contains(labels["env"], "prod")`)),
nodeNamesExpected: []string{},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
},
},
{
name: "prod label expression in " + otherScope,
server: scopedSSHUser("node-other-prod-expression", otherScope, sshLabelExpression(`contains(labels["env"], "prod")`)),
nodeNamesExpected: []string{"other-scope-node"},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
},
},
{
name: "sorting",
server: scopedSSHUser("node-test-sorting", scope, sshLabels("*")),
nodeNamesExpected: []string{"dev-node", "dev-child-node", "prod-node"},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
SortBy: types.SortBy{Field: types.ResourceMetadataName},
},
},
{
name: "include logins",
server: scopedSSHUser("node-test-include-logins", scope, sshLabels("prod")),
nodeNamesExpected: []string{"prod-node"},
req: &proto.ListUnifiedResourcesRequest{
Kinds: []string{types.KindNode},
Limit: 10,
IncludeLogins: true,
},
loginsExpected: []string{"root"},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
res, err := tc.server.ListUnifiedResources(t.Context(), tc.req)
require.NoError(t, err)

names := make([]string, 0, len(res.Resources))
for _, r := range res.Resources {
names = append(names, r.GetNode().GetName())
assert.NotEqual(t, "unscoped-node", r.GetNode().GetName())
assert.Equal(t, tc.loginsExpected, r.Logins)
}

assert.ElementsMatch(t, tc.nodeNamesExpected, names)

if tc.req.SortBy.Field != "" {
assert.True(t, slices.IsSorted(names))
}
})
}
}

func withAccountAssignment(condition types.RoleConditionType, accountID, permissionSet string) authtest.CreateUserAndRoleOption {
return authtest.WithRoleMutator(func(role types.Role) {
r := role.(*types.RoleV6)
Expand Down
7 changes: 7 additions & 0 deletions lib/services/ssh_access_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,13 @@ func (c *SSHAccessChecker) MaxSessions() int64 {
return c.checker.role.GetSpec().GetSsh().GetMaxSessions()
}

func (c *SSHAccessChecker) GetAllowedLoginsForServer(server types.Server) ([]string, error) {
if !c.checker.isScoped() {
return c.checker.unscopedChecker.GetAllowedLoginsForResource(server)
}
return c.checker.scopedCompatChecker.GetAllowedLoginsForResource(server)
}

// getScopedLogins returns the OS logins permitted by this scoped role. Returns nil for unscoped
// identities, which aggregate logins differently via [CertificateParameterContext.GetSSHLoginsForTTL].
// This method is intentionally unexported to prevent accidental use outside cert-param aggregation.
Expand Down
2 changes: 1 addition & 1 deletion lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -3619,7 +3619,7 @@ func makeUnifiedResourceRequest(r *http.Request, scopePin *scopesv1.Pin) (*proto
PredicateExpression: values.Get("query"),
SearchKeywords: client.ParseSearchKeywords(values.Get("search"), ' '),
UseSearchAsRoles: useSearchAsRoles,
IncludeLogins: !scopedIdentity,
IncludeLogins: true,
IncludeRequestable: includeRequestable,
}, nil
}
Expand Down
Loading