diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 6c09f22fa2e..0e147bb53ae 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -1590,6 +1590,56 @@ const ( kubeService = "kube_service" ) +// authContextForSearch returns an extended authz.Context which should be used +// when searching for resources that a user may be able to request access to, +// but does not already have access to. +// Extra roles are determined from the user's search_as_roles and +// preview_as_roles if [req] requested that each be used. +func (a *ServerWithRoles) authContextForSearch(ctx context.Context, req *proto.ListResourcesRequest) (*authz.Context, error) { + var extraRoles []string + if req.UseSearchAsRoles { + extraRoles = append(extraRoles, a.context.Checker.GetAllowedSearchAsRoles()...) + } + if req.UsePreviewAsRoles { + extraRoles = append(extraRoles, a.context.Checker.GetAllowedPreviewAsRoles()...) + } + if len(extraRoles) == 0 { + // Return the current auth context unmodified. + return &a.context, nil + } + + clusterName, err := a.authServer.GetClusterName() + if err != nil { + return nil, trace.Wrap(err) + } + + // Get a new auth context with the additional roles + extendedContext, err := a.context.WithExtraRoles(a.authServer, clusterName.GetClusterName(), extraRoles) + if err != nil { + 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) + } + } + return extendedContext, nil +} + // ListResources returns a paginated list of resources filtered by user access. func (a *ServerWithRoles) ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error) { // kubeService is a special resource type that is used to keep compatibility @@ -1611,34 +1661,18 @@ func (a *ServerWithRoles) ListResources(ctx context.Context, req proto.ListResou return nil, trace.Wrap(err) } + // Apply any requested additional search_as_roles and/or preview_as_roles + // for the duration of the search. if req.UseSearchAsRoles || req.UsePreviewAsRoles { - var extraRoles []string - if req.UseSearchAsRoles { - extraRoles = append(extraRoles, a.context.Checker.GetAllowedSearchAsRoles()...) - } - if req.UsePreviewAsRoles { - extraRoles = append(extraRoles, a.context.Checker.GetAllowedPreviewAsRoles()...) - } - clusterName, err := a.authServer.GetClusterName() + extendedContext, err := a.authContextForSearch(ctx, &req) if err != nil { return nil, trace.Wrap(err) } - if err := a.context.UseExtraRoles(a.authServer, clusterName.GetClusterName(), extraRoles); err != nil { - return nil, trace.Wrap(err) - } - a.authServer.emitter.EmitAuditEvent(a.authServer.closeCtx, &apievents.AccessRequestResourceSearch{ - Metadata: apievents.Metadata{ - Type: events.AccessRequestResourceSearch, - Code: events.AccessRequestResourceSearchCode, - }, - UserMetadata: authz.ClientUserMetadata(ctx), - SearchAsRoles: a.context.Checker.RoleNames(), - ResourceType: req.ResourceType, - Namespace: req.Namespace, - Labels: req.Labels, - PredicateExpression: req.PredicateExpression, - SearchKeywords: req.SearchKeywords, - }) + baseContext := a.context + a.context = *extendedContext + defer func() { + a.context = baseContext + }() } // ListResources request coming through this auth layer gets request filters diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index 2ab709948eb..768e7090df4 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/lib/auth/auth_with_roles_test.go @@ -2844,45 +2844,56 @@ func TestListResources_SearchAsRoles(t *testing.T) { require.Len(t, testNodes, numTestNodes) // create user and client - user, role, err := CreateUserAndRole(srv.Auth(), "user", []string{"user"}, nil) + requester, role, err := CreateUserAndRole(srv.Auth(), "requester", []string{"requester"}, nil) require.NoError(t, err) // only allow user to see first node role.SetNodeLabels(types.Allow, types.Labels{"name": {testNodes[0].GetName()}}) // create a new role which can see second node - searchAsRole := services.RoleForUser(user) + searchAsRole := services.RoleForUser(requester) searchAsRole.SetName("test_search_role") searchAsRole.SetNodeLabels(types.Allow, types.Labels{"name": {testNodes[1].GetName()}}) - searchAsRole.SetLogins(types.Allow, []string{"user"}) + searchAsRole.SetLogins(types.Allow, []string{"requester"}) require.NoError(t, srv.Auth().UpsertRole(ctx, searchAsRole)) // create a third role which can see the third node - previewAsRole := services.RoleForUser(user) + previewAsRole := services.RoleForUser(requester) previewAsRole.SetName("test_preview_role") previewAsRole.SetNodeLabels(types.Allow, types.Labels{"name": {testNodes[2].GetName()}}) - previewAsRole.SetLogins(types.Allow, []string{"user"}) + previewAsRole.SetLogins(types.Allow, []string{"requester"}) require.NoError(t, srv.Auth().UpsertRole(ctx, previewAsRole)) role.SetSearchAsRoles(types.Allow, []string{searchAsRole.GetName()}) role.SetPreviewAsRoles(types.Allow, []string{previewAsRole.GetName()}) require.NoError(t, srv.Auth().UpsertRole(ctx, role)) - clt, err := srv.NewClient(TestUser(user.GetName())) + requesterClt, err := srv.NewClient(TestUser(requester.GetName())) + require.NoError(t, err) + + // create another user that can see all nodes but has no search_as_roles or + // preview_as_roles + admin, _, err := CreateUserAndRole(srv.Auth(), "admin", []string{"admin"}, nil) + require.NoError(t, err) + adminClt, err := srv.NewClient(TestUser(admin.GetName())) require.NoError(t, err) for _, tc := range []struct { desc string + clt *Client requestOpt func(*proto.ListResourcesRequest) expectNodes []string + expectSearchEvent bool expectSearchEventRoles []string }{ { - desc: "basic", + desc: "no search", + clt: requesterClt, expectNodes: []string{testNodes[0].GetName()}, }, { desc: "search as roles", + clt: requesterClt, requestOpt: func(req *proto.ListResourcesRequest) { req.UseSearchAsRoles = true }, @@ -2891,6 +2902,7 @@ func TestListResources_SearchAsRoles(t *testing.T) { }, { desc: "preview as roles", + clt: requesterClt, requestOpt: func(req *proto.ListResourcesRequest) { req.UsePreviewAsRoles = true }, @@ -2899,6 +2911,7 @@ func TestListResources_SearchAsRoles(t *testing.T) { }, { desc: "both", + clt: requesterClt, requestOpt: func(req *proto.ListResourcesRequest) { req.UseSearchAsRoles = true req.UsePreviewAsRoles = true @@ -2906,8 +2919,25 @@ func TestListResources_SearchAsRoles(t *testing.T) { expectNodes: []string{testNodes[0].GetName(), testNodes[1].GetName(), testNodes[2].GetName()}, expectSearchEventRoles: []string{role.GetName(), searchAsRole.GetName(), previewAsRole.GetName()}, }, + { + // this tests the case where the request includes UseSearchAsRoles + // and UsePreviewAsRoles, but the user has none, so there should be + // no audit event. + desc: "no extra roles", + clt: adminClt, + requestOpt: func(req *proto.ListResourcesRequest) { + req.UseSearchAsRoles = true + req.UsePreviewAsRoles = true + }, + expectNodes: []string{testNodes[0].GetName(), testNodes[1].GetName(), testNodes[2].GetName()}, + }, } { t.Run(tc.desc, func(t *testing.T) { + // Overwrite the auth server emitter to capture all events emitted + // during this test case. + emitter := eventstest.NewChannelEmitter(1) + srv.AuthServer.AuthServer.emitter = emitter + req := proto.ListResourcesRequest{ ResourceType: types.KindNode, Limit: int32(len(testNodes)), @@ -2915,7 +2945,7 @@ func TestListResources_SearchAsRoles(t *testing.T) { if tc.requestOpt != nil { tc.requestOpt(&req) } - resp, err := clt.ListResources(ctx, req) + resp, err := tc.clt.ListResources(ctx, req) require.NoError(t, err) require.Len(t, resp.Resources, len(tc.expectNodes)) var gotNodes []string @@ -2925,29 +2955,11 @@ func TestListResources_SearchAsRoles(t *testing.T) { require.ElementsMatch(t, tc.expectNodes, gotNodes) if len(tc.expectSearchEventRoles) > 0 { - require.Eventually(t, func() bool { - // make sure an audit event is logged for the search - auditEvents, _, err := srv.AuthServer.AuditLog.SearchEvents(ctx, events.SearchEventsRequest{ - From: time.Time{}, - To: time.Now(), - EventTypes: []string{events.AccessRequestResourceSearch}, - Limit: 10, - Order: types.EventOrderAscending, - }) - require.NoError(t, err) - if len(auditEvents) == 0 { - t.Log("no search audit events found") - return false - } - lastEvent := auditEvents[len(auditEvents)-1].(*apievents.AccessRequestResourceSearch) - diff := cmp.Diff(tc.expectSearchEventRoles, lastEvent.SearchAsRoles) - if diff == "" { - // Found the event we're looking for. - return true - } - t.Logf("most recent search event does not have the expected roles, diff: %s", diff) - return false - }, 10*time.Second, 250*time.Millisecond, "did not find expected search event") + searchEvent := <-emitter.C() + require.ElementsMatch(t, tc.expectSearchEventRoles, searchEvent.(*apievents.AccessRequestResourceSearch).SearchAsRoles) + } else { + // expect no event to have been emitted + require.Empty(t, emitter.C()) } }) } diff --git a/lib/authz/permissions.go b/lib/authz/permissions.go index 08d0d8e72f9..2c398dafd73 100644 --- a/lib/authz/permissions.go +++ b/lib/authz/permissions.go @@ -208,23 +208,32 @@ Loop: return lockTargets } -// UseExtraRoles extends the roles of the Checker on the current Context with -// the given extra roles. -func (c *Context) UseExtraRoles(access services.RoleGetter, clusterName string, roles []string) error { +// WithExtraRoles returns a shallow copy of [c], where the users roles have been +// extended with [roles]. It may return [c] unmodified. +func (c *Context) WithExtraRoles(access services.RoleGetter, clusterName string, roles []string) (*Context, error) { var newRoleNames []string newRoleNames = append(newRoleNames, c.Checker.RoleNames()...) newRoleNames = append(newRoleNames, roles...) newRoleNames = utils.Deduplicate(newRoleNames) - // set new roles on the context user and create a new access checker - c.User.SetRoles(newRoleNames) - accessInfo := services.AccessInfoFromUser(c.User) + // Return early if there are no extra roles. + if len(newRoleNames) == len(c.Checker.RoleNames()) { + return c, nil + } + + accessInfo := &services.AccessInfo{ + Roles: newRoleNames, + Traits: c.User.GetTraits(), + AllowedResourceIDs: c.Checker.GetAllowedResourceIDs(), + } checker, err := services.NewAccessChecker(accessInfo, clusterName, access) if err != nil { - return trace.Wrap(err) + return nil, trace.Wrap(err) } - c.Checker = checker - return nil + + newContext := *c + newContext.Checker = checker + return &newContext, nil } // GetAccessState returns the AccessState based on the underlying diff --git a/lib/kube/grpc/grpc.go b/lib/kube/grpc/grpc.go index 4f31bacfb67..580b02d4475 100644 --- a/lib/kube/grpc/grpc.go +++ b/lib/kube/grpc/grpc.go @@ -135,12 +135,16 @@ func (s *Server) ListKubernetesResources(ctx context.Context, req *proto.ListKub extraRoles = append(extraRoles, userContext.Checker.GetAllowedPreviewAsRoles()...) } - if err := userContext.UseExtraRoles(s.cfg.AccessPoint, s.cfg.ClusterName, extraRoles); err != nil { + extendedContext, err := userContext.WithExtraRoles(s.cfg.AccessPoint, s.cfg.ClusterName, extraRoles) + if err != nil { return nil, trail.ToGRPC(err) } - if err := s.emitAuditEvent(ctx, userContext, req); err != nil { - return nil, trail.ToGRPC(err) + if len(extendedContext.Checker.RoleNames()) != len(userContext.Checker.RoleNames()) { + if err := s.emitAuditEvent(ctx, userContext, req); err != nil { + return nil, trail.ToGRPC(err) + } } + userContext = extendedContext } // We use the unmapped identity here because Kube Proxy will handle // the forwarding of the request to the correct leaf cluster if that's the case diff --git a/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap b/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap index b5c8f58d589..1264b86a1e0 100644 --- a/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap +++ b/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap @@ -2988,7 +2988,7 @@ exports[`list of all events 1`] = ` class="c14 c20 icon icon-info_outline c14 c20" font-size="3" /> - Resource Access Request Search + Resource Access Search `User [${user}] searched for resource type [${resource_type}] with role(s) [${search_as_roles}]`, },