diff --git a/api/accessrequest/access_request.go b/api/accessrequest/access_request.go new file mode 100644 index 0000000000000..2823aefbdc7d5 --- /dev/null +++ b/api/accessrequest/access_request.go @@ -0,0 +1,181 @@ +/* +Copyright 2023 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package accessrequest + +import ( + "context" + "fmt" + "strings" + + "github.com/gravitational/trace" + + "github.com/gravitational/teleport/api/client" + "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/types" +) + +type ListResourcesRequestOption func(*proto.ListResourcesRequest) + +// GetResourcesByKind is an alternative to client.GetResourcesWithFilters +// that searches with the resource kinds used in access requests instead of the +// resource types expected by ListResources. +// +// The ResourceType field in the request should not be set by the caller, as +// it will be overridden. +func GetResourcesByKind(ctx context.Context, clt client.ListResourcesClient, req proto.ListResourcesRequest, kind string) ([]types.ResourceWithLabels, error) { + req.ResourceType = mapResourceKindToListResourcesType(kind) + results, err := client.GetResourcesWithFilters(ctx, clt, req) + if err != nil { + return nil, trace.Wrap(err) + } + resources := make([]types.ResourceWithLabels, 0, len(results)) + for _, result := range results { + leafResource, err := mapListResourcesResultToLeafResource(result, kind) + if err != nil { + return nil, trace.Wrap(err) + } + resources = append(resources, leafResource) + } + return resources, nil +} + +// GetResourceDetails gets extra details for a list of resources in a given cluster. +func GetResourceDetails(ctx context.Context, clusterName string, lister client.ListResourcesClient, ids []types.ResourceID) (map[string]types.ResourceDetails, error) { + var resourceIDs []types.ResourceID + for _, resourceID := range ids { + // We're interested in hostname or friendly name details. These apply to + // nodes, app servers, and user groups. + switch resourceID.Kind { + case types.KindNode, types.KindApp, types.KindUserGroup: + resourceIDs = append(resourceIDs, resourceID) + } + } + + withExtraRoles := func(req *proto.ListResourcesRequest) { + req.UseSearchAsRoles = true + req.UsePreviewAsRoles = true + } + + resources, err := GetResourcesByResourceIDs(ctx, lister, resourceIDs, withExtraRoles) + if err != nil { + return nil, trace.Wrap(err) + } + + result := make(map[string]types.ResourceDetails) + for _, resource := range resources { + friendlyName := types.FriendlyName(resource) + + // No friendly name was found, so skip to the next resource. + if friendlyName == "" { + continue + } + + id := types.ResourceID{ + ClusterName: clusterName, + Kind: resource.GetKind(), + Name: resource.GetName(), + } + result[types.ResourceIDToString(id)] = types.ResourceDetails{ + FriendlyName: friendlyName, + } + } + + return result, nil +} + +// GetResourceIDsByCluster will return resource IDs grouped by cluster. +func GetResourceIDsByCluster(r types.AccessRequest) map[string][]types.ResourceID { + resourceIDsByCluster := make(map[string][]types.ResourceID) + for _, resourceID := range r.GetRequestedResourceIDs() { + resourceIDsByCluster[resourceID.ClusterName] = append(resourceIDsByCluster[resourceID.ClusterName], resourceID) + } + return resourceIDsByCluster +} + +// GetResourcesByResourceID gets a list of resources by their resource IDs. +func GetResourcesByResourceIDs(ctx context.Context, lister client.ListResourcesClient, resourceIDs []types.ResourceID, opts ...ListResourcesRequestOption) ([]types.ResourceWithLabels, error) { + resourceNamesByKind := make(map[string][]string) + for _, resourceID := range resourceIDs { + resourceNamesByKind[resourceID.Kind] = append(resourceNamesByKind[resourceID.Kind], resourceID.Name) + } + var resources []types.ResourceWithLabels + for kind, resourceNames := range resourceNamesByKind { + req := proto.ListResourcesRequest{ + PredicateExpression: anyNameMatcher(resourceNames), + Limit: int32(len(resourceNames)), + } + for _, opt := range opts { + opt(&req) + } + resp, err := GetResourcesByKind(ctx, lister, req, kind) + if err != nil { + return nil, trace.Wrap(err) + } + resources = append(resources, resp...) + } + return resources, nil +} + +// anyNameMatcher returns a PredicateExpression which matches any of a given list +// of names. Given names will be escaped and quoted when building the expression. +func anyNameMatcher(names []string) string { + matchers := make([]string, len(names)) + for i := range names { + matchers[i] = fmt.Sprintf(`resource.metadata.name == %q`, names[i]) + } + return strings.Join(matchers, " || ") +} + +// mapResourceKindToListResourcesType returns the value to use for ResourceType in a +// ListResourcesRequest based on the kind of resource you're searching for. +// Necessary because some resource kinds don't support ListResources directly, +// so you have to list the parent kind. Use MapListResourcesResultToLeafResource to map back +// to the given kind. +func mapResourceKindToListResourcesType(kind string) string { + switch kind { + case types.KindApp: + return types.KindAppServer + case types.KindDatabase: + return types.KindDatabaseServer + case types.KindKubernetesCluster: + return types.KindKubeServer + default: + return kind + } +} + +// mapListResourcesResultToLeafResource is the inverse of +// MapResourceKindToListResourcesType, after the ListResources call it maps the +// result back to the kind we really want. `hint` should be the name of the +// desired resource kind, used to disambiguate normal SSH nodes and kubernetes +// services which are both returned as `types.Server`. +func mapListResourcesResultToLeafResource(resource types.ResourceWithLabels, hint string) (types.ResourceWithLabels, error) { + switch r := resource.(type) { + case types.AppServer: + return r.GetApp(), nil + case types.KubeServer: + return r.GetCluster(), nil + case types.DatabaseServer: + return r.GetDatabase(), nil + case types.Server: + if hint == types.KindKubernetesCluster { + return nil, trace.BadParameter("expected kubernetes server, got server") + } + default: + } + return resource, nil +} diff --git a/api/accessrequest/access_request_test.go b/api/accessrequest/access_request_test.go new file mode 100644 index 0000000000000..b9c1262a4b42e --- /dev/null +++ b/api/accessrequest/access_request_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2023 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package accessrequest + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/types" +) + +func newNode(t *testing.T, name, hostname string) types.Server { + t.Helper() + node, err := types.NewServer(name, types.KindNode, + types.ServerSpecV2{ + Hostname: hostname, + }) + require.NoError(t, err) + return node +} + +func newApp(t *testing.T, name, description, origin string) types.Application { + t.Helper() + app, err := types.NewAppV3(types.Metadata{ + Name: name, + Description: description, + Labels: map[string]string{ + types.OriginLabel: origin, + }, + }, + types.AppSpecV3{ + URI: "https://some-addr.com", + PublicAddr: "https://some-addr.com", + }) + require.NoError(t, err) + return app +} + +func newUserGroup(t *testing.T, name, description, origin string) types.UserGroup { + t.Helper() + userGroup, err := types.NewUserGroup(types.Metadata{ + Name: name, + Description: description, + Labels: map[string]string{ + types.OriginLabel: origin, + }, + }, types.UserGroupSpecV1{}) + require.NoError(t, err) + return userGroup +} + +func newResourceID(clusterName, kind, name string) types.ResourceID { + return types.ResourceID{ + ClusterName: clusterName, + Kind: kind, + Name: name, + } +} + +type mockResourceLister struct { + resources []types.ResourceWithLabels +} + +func (m *mockResourceLister) ListResources(ctx context.Context, _ proto.ListResourcesRequest) (*types.ListResourcesResponse, error) { + return &types.ListResourcesResponse{ + Resources: m.resources, + }, nil +} + +func TestGetResourceDetails(t *testing.T) { + clusterName := "cluster" + + presence := &mockResourceLister{ + resources: []types.ResourceWithLabels{ + newNode(t, "node1", "hostname 1"), + newApp(t, "app1", "friendly app 1", types.OriginDynamic), + newApp(t, "app2", "friendly app 2", types.OriginDynamic), + newApp(t, "app3", "friendly app 3", types.OriginOkta), + newUserGroup(t, "group1", "friendly group 1", types.OriginOkta), + }, + } + resourceIDs := []types.ResourceID{ + newResourceID(clusterName, types.KindNode, "node1"), + newResourceID(clusterName, types.KindApp, "app1"), + newResourceID(clusterName, types.KindApp, "app2"), + newResourceID(clusterName, types.KindApp, "app3"), + newResourceID(clusterName, types.KindUserGroup, "group1"), + } + + ctx := context.Background() + + details, err := GetResourceDetails(ctx, clusterName, presence, resourceIDs) + require.NoError(t, err) + + // Check the resource details to see if friendly names properly propagated. + + // Node should be named for its hostname. + require.Equal(t, "hostname 1", details[types.ResourceIDToString(resourceIDs[0])].FriendlyName) + + // app1 and app2 are expected to be empty because they're not Okta sourced resources. + require.Empty(t, details[types.ResourceIDToString(resourceIDs[1])].FriendlyName) + + require.Empty(t, details[types.ResourceIDToString(resourceIDs[2])].FriendlyName) + + // This Okta sourced app should have a friendly name. + require.Equal(t, "friendly app 3", details[types.ResourceIDToString(resourceIDs[3])].FriendlyName) + + // This Okta sourced user group should have a friendly name. + require.Equal(t, "friendly group 1", details[types.ResourceIDToString(resourceIDs[4])].FriendlyName) +} diff --git a/api/types/resource.go b/api/types/resource.go index 44e77bb5bf910..3cb719d512851 100644 --- a/api/types/resource.go +++ b/api/types/resource.go @@ -654,3 +654,18 @@ func ValidateResourceName(validationRegex *regexp.Regexp, name string) error { name, validationRegex.String(), ) } + +// FriendlyName will return the friendly name for a resource if it has one. Otherwise, it +// will return an empty string. +func FriendlyName(resource ResourceWithLabels) string { + // Right now, only resources sourced from Okta and nodes have friendly names. + if resource.Origin() == OriginOkta { + return resource.GetMetadata().Description + } + + if hn, ok := resource.(interface{ GetHostname() string }); ok { + return hn.GetHostname() + } + + return "" +} diff --git a/api/types/resource_test.go b/api/types/resource_test.go index 0b6066fe30a77..fbd6309724700 100644 --- a/api/types/resource_test.go +++ b/api/types/resource_test.go @@ -501,3 +501,59 @@ func TestValidLabelKey(t *testing.T) { require.Equal(t, tc.valid, isValid) } } + +func TestFriendlyName(t *testing.T) { + appNoFriendly, err := NewAppV3(Metadata{ + Name: "no friendly", + }, AppSpecV3{ + URI: "https://some-uri.com", + }, + ) + require.NoError(t, err) + + appFriendly, err := NewAppV3(Metadata{ + Name: "no friendly", + Description: "friendly name", + Labels: map[string]string{ + OriginLabel: OriginOkta, + }, + }, AppSpecV3{ + URI: "https://some-uri.com", + }, + ) + require.NoError(t, err) + + node, err := NewServer("node", KindNode, ServerSpecV2{ + Hostname: "friendly hostname", + }) + require.NoError(t, err) + + tests := []struct { + name string + resource ResourceWithLabels + expected string + }{ + { + name: "no friendly name", + resource: appNoFriendly, + expected: "", + }, + { + name: "friendly app name", + resource: appFriendly, + expected: "friendly name", + }, + { + name: "friendly node name", + resource: node, + expected: "friendly hostname", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.expected, FriendlyName(test.resource)) + }) + } +} diff --git a/integration/helpers/helpers.go b/integration/helpers/helpers.go index 26fb723498415..e519acc593748 100644 --- a/integration/helpers/helpers.go +++ b/integration/helpers/helpers.go @@ -35,6 +35,7 @@ import ( "golang.org/x/crypto/ssh/agent" "github.com/gravitational/teleport/api/breaker" + apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" @@ -473,7 +474,7 @@ func MustCreateListener(t *testing.T) net.Listener { return listener } -func FindNodeWithLabel(t *testing.T, ctx context.Context, cl services.ResourceLister, key, value string) func() bool { +func FindNodeWithLabel(t *testing.T, ctx context.Context, cl apiclient.ListResourcesClient, key, value string) func() bool { t.Helper() return func() bool { servers, err := cl.ListResources(ctx, proto.ListResourcesRequest{ diff --git a/integrations/access/common/app.go b/integrations/access/common/app.go index c73d52695803e..74c46b4eab5e1 100644 --- a/integrations/access/common/app.go +++ b/integrations/access/common/app.go @@ -18,10 +18,12 @@ package common import ( "context" + "fmt" "time" "github.com/gravitational/trace" + "github.com/gravitational/teleport/api/accessrequest" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/integrations/access/common/teleport" @@ -242,14 +244,21 @@ func (a *BaseApp) onPendingRequest(ctx context.Context, req types.AccessRequest) log := logger.Get(ctx) reqID := req.GetName() + + resourceNames, err := a.getResourceNames(ctx, req) + if err != nil { + return trace.Wrap(err) + } + reqData := pd.AccessRequestData{ User: req.GetUser(), Roles: req.GetRoles(), RequestReason: req.GetRequestReason(), SystemAnnotations: req.GetSystemAnnotations(), + Resources: resourceNames, } - _, err := a.pluginData.Create(ctx, reqID, GenericPluginData{AccessRequestData: reqData}) + _, err = a.pluginData.Create(ctx, reqID, GenericPluginData{AccessRequestData: reqData}) switch { case err == nil: // This is a new access-request, we have to broadcast it first. @@ -479,3 +488,24 @@ func (a *BaseApp) updateMessages(ctx context.Context, reqID string, tag pd.Resol return nil } + +func (a *BaseApp) getResourceNames(ctx context.Context, req types.AccessRequest) ([]string, error) { + resourceNames := make([]string, 0, len(req.GetRequestedResourceIDs())) + resourcesByCluster := accessrequest.GetResourceIDsByCluster(req) + + for cluster, resources := range resourcesByCluster { + resourceDetails, err := accessrequest.GetResourceDetails(ctx, cluster, a.apiClient, resources) + if err != nil { + return nil, trace.Wrap(err) + } + + for _, resource := range resources { + resourceName := types.ResourceIDToString(resource) + if details, ok := resourceDetails[resourceName]; ok && details.FriendlyName != "" { + resourceName = fmt.Sprintf("%s/%s", resource.Kind, details.FriendlyName) + } + resourceNames = append(resourceNames, resourceName) + } + } + return resourceNames, nil +} diff --git a/integrations/access/common/message.go b/integrations/access/common/message.go index 6b0bdf6bb0dd6..affa3a3143628 100644 --- a/integrations/access/common/message.go +++ b/integrations/access/common/message.go @@ -78,9 +78,12 @@ func MsgFields(reqID string, reqData pd.AccessRequestData, clusterName string, w if len(reqData.User) > 0 { msgFieldToBuilder(&builder, "User", reqData.User) } - if reqData.Roles != nil { + if len(reqData.Roles) > 0 { msgFieldToBuilder(&builder, "Role(s)", strings.Join(reqData.Roles, ",")) } + if len(reqData.Resources) > 0 { + msgFieldToBuilder(&builder, "Resource(s)", strings.Join(reqData.Resources, ",")) + } if reqData.RequestReason != "" { msgFieldToBuilder(&builder, "Reason", lib.MarkdownEscape(reqData.RequestReason, requestReasonLimit)) } diff --git a/integrations/access/common/plugindata_test.go b/integrations/access/common/plugindata_test.go index 4ca6fa5be3268..6ec603a485961 100644 --- a/integrations/access/common/plugindata_test.go +++ b/integrations/access/common/plugindata_test.go @@ -26,6 +26,7 @@ var samplePluginData = GenericPluginData{ AccessRequestData: plugindata.AccessRequestData{ User: "user-foo", Roles: []string{"role-foo", "role-bar"}, + Resources: []string{"cluster-a/node/foo", "cluster-a/node/bar"}, RequestReason: "foo reason", ReviewsCount: 3, ResolutionTag: plugindata.ResolvedApproved, @@ -40,9 +41,10 @@ var samplePluginData = GenericPluginData{ func TestEncodePluginData(t *testing.T) { dataMap, err := EncodePluginData(samplePluginData) assert.NoError(t, err) - assert.Len(t, dataMap, 7) + assert.Len(t, dataMap, 8) assert.Equal(t, "user-foo", dataMap["user"]) assert.Equal(t, "role-foo,role-bar", dataMap["roles"]) + assert.Equal(t, `["cluster-a/node/foo","cluster-a/node/bar"]`, dataMap["resources"]) assert.Equal(t, "foo reason", dataMap["request_reason"]) assert.Equal(t, "3", dataMap["reviews_count"]) assert.Equal(t, "APPROVED", dataMap["resolution"]) @@ -54,6 +56,7 @@ func TestDecodePluginData(t *testing.T) { pluginData, err := DecodePluginData(map[string]string{ "user": "user-foo", "roles": "role-foo,role-bar", + "resources": `["cluster-a/node/foo","cluster-a/node/bar"]`, "request_reason": "foo reason", "reviews_count": "3", "resolution": "APPROVED", @@ -67,7 +70,7 @@ func TestDecodePluginData(t *testing.T) { func TestEncodeEmptyPluginData(t *testing.T) { dataMap, err := EncodePluginData(GenericPluginData{}) assert.NoError(t, err) - assert.Len(t, dataMap, 7) + assert.Len(t, dataMap, 8) for key, value := range dataMap { assert.Emptyf(t, value, "value at key %q must be empty", key) } diff --git a/integrations/access/common/teleport/client.go b/integrations/access/common/teleport/client.go index 5cefc66b1d530..aff3b17dc022a 100644 --- a/integrations/access/common/teleport/client.go +++ b/integrations/access/common/teleport/client.go @@ -32,4 +32,5 @@ type Client interface { GetAccessRequests(ctx context.Context, filter types.AccessRequestFilter) ([]types.AccessRequest, error) SubmitAccessReview(ctx context.Context, params types.AccessReviewSubmission) (types.AccessRequest, error) SetAccessRequestState(ctx context.Context, params types.AccessRequestUpdate) error + ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error) } diff --git a/integrations/lib/plugindata/access_request.go b/integrations/lib/plugindata/access_request.go index f16ee0af68ce9..2af356b804158 100644 --- a/integrations/lib/plugindata/access_request.go +++ b/integrations/lib/plugindata/access_request.go @@ -42,6 +42,7 @@ type AccessRequestData struct { ResolutionTag ResolutionTag ResolutionReason string SystemAnnotations map[string][]string + Resources []string } // DecodeAccessRequestData deserializes a string map to PluginData struct. @@ -57,8 +58,16 @@ func DecodeAccessRequestData(dataMap map[string]string) (data AccessRequestData, data.ResolutionTag = ResolutionTag(dataMap["resolution"]) data.ResolutionReason = dataMap["resolve_reason"] - if _, ok := dataMap["system_annotations"]; ok { - err = json.Unmarshal([]byte(dataMap["system_annotations"]), &data.SystemAnnotations) + if str, ok := dataMap["resources"]; ok { + err = json.Unmarshal([]byte(str), &data.Resources) + if err != nil { + err = trace.Wrap(err) + return + } + } + + if str, ok := dataMap["system_annotations"]; ok { + err = json.Unmarshal([]byte(str), &data.SystemAnnotations) if err != nil { err = trace.Wrap(err) return @@ -76,8 +85,17 @@ func EncodeAccessRequestData(data AccessRequestData) (map[string]string, error) result["user"] = data.User result["roles"] = strings.Join(data.Roles, ",") + result["resources"] = strings.Join(data.Resources, ",") result["request_reason"] = data.RequestReason + if len(data.Resources) != 0 { + resources, err := json.Marshal(data.Resources) + if err != nil { + return nil, trace.Wrap(err) + } + result["resources"] = string(resources) + } + var reviewsCountStr string if data.ReviewsCount > 0 { reviewsCountStr = fmt.Sprintf("%d", data.ReviewsCount) diff --git a/integrations/lib/plugindata/access_request_test.go b/integrations/lib/plugindata/access_request_test.go index a121a33db9deb..935802aafacc0 100644 --- a/integrations/lib/plugindata/access_request_test.go +++ b/integrations/lib/plugindata/access_request_test.go @@ -23,6 +23,7 @@ import ( var sampleAccessRequestData = AccessRequestData{ User: "user-foo", Roles: []string{"role-foo", "role-bar"}, + Resources: []string{"cluster/node/foo", "cluster/node/bar"}, RequestReason: "foo reason", ReviewsCount: 3, ResolutionTag: ResolvedApproved, @@ -32,9 +33,10 @@ var sampleAccessRequestData = AccessRequestData{ func TestEncodeAccessRequestData(t *testing.T) { dataMap, err := EncodeAccessRequestData(sampleAccessRequestData) assert.Nil(t, err) - assert.Len(t, dataMap, 6) + assert.Len(t, dataMap, 7) assert.Equal(t, "user-foo", dataMap["user"]) assert.Equal(t, "role-foo,role-bar", dataMap["roles"]) + assert.Equal(t, `["cluster/node/foo","cluster/node/bar"]`, dataMap["resources"]) assert.Equal(t, "foo reason", dataMap["request_reason"]) assert.Equal(t, "3", dataMap["reviews_count"]) assert.Equal(t, "APPROVED", dataMap["resolution"]) @@ -45,6 +47,7 @@ func TestDecodeAccessRequestData(t *testing.T) { pluginData, err := DecodeAccessRequestData(map[string]string{ "user": "user-foo", "roles": "role-foo,role-bar", + "resources": `["cluster/node/foo", "cluster/node/bar"]`, "request_reason": "foo reason", "reviews_count": "3", "resolution": "APPROVED", @@ -57,7 +60,7 @@ func TestDecodeAccessRequestData(t *testing.T) { func TestEncodeEmptyAccessRequestData(t *testing.T) { dataMap, err := EncodeAccessRequestData(AccessRequestData{}) assert.Nil(t, err) - assert.Len(t, dataMap, 6) + assert.Len(t, dataMap, 7) for key, value := range dataMap { assert.Emptyf(t, value, "value at key %q must be empty", key) } diff --git a/lib/services/access_request.go b/lib/services/access_request.go index c333794f8b809..0e790d9453cc7 100644 --- a/lib/services/access_request.go +++ b/lib/services/access_request.go @@ -18,7 +18,6 @@ package services import ( "context" - "fmt" "sort" "strings" "time" @@ -30,7 +29,8 @@ import ( "github.com/vulcand/predicate" "golang.org/x/exp/slices" - "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/accessrequest" + "github.com/gravitational/teleport/api/client" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" @@ -719,17 +719,12 @@ func GetTraitMappings(cms []types.ClaimMapping) types.TraitMappingSet { return types.TraitMappingSet(tm) } -// ResourceLister is an interface which can list resources. -type ResourceLister interface { - ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error) -} - // RequestValidatorGetter is the interface required by the request validation // functions used to get necessary resources. type RequestValidatorGetter interface { UserGetter RoleGetter - ResourceLister + client.ListResourcesClient GetRoles(ctx context.Context) ([]types.Role, error) GetClusterName(opts ...MarshalOption) (types.ClusterName, error) } @@ -1811,138 +1806,19 @@ func (m *RequestValidator) roleAllowsResource( return true, nil } -type ListResourcesRequestOption func(*proto.ListResourcesRequest) - -func GetResourceDetails(ctx context.Context, clusterName string, lister ResourceLister, ids []types.ResourceID) (map[string]types.ResourceDetails, error) { - var resourceIDs []types.ResourceID - for _, resourceID := range ids { - // We're interested in hostname or friendly name details. These apply to - // nodes, app servers, and user groups. - switch resourceID.Kind { - case types.KindNode, types.KindApp, types.KindUserGroup: - resourceIDs = append(resourceIDs, resourceID) - } - } - - withExtraRoles := func(req *proto.ListResourcesRequest) { - req.UseSearchAsRoles = true - req.UsePreviewAsRoles = true - } - - resources, err := GetResourcesByResourceIDs(ctx, lister, resourceIDs, withExtraRoles) - if err != nil { - return nil, trace.Wrap(err) - } - - result := make(map[string]types.ResourceDetails) - for _, resource := range resources { - friendlyName := FriendlyName(resource) - - // No friendly name was found, so skip to the next resource. - if friendlyName == "" { - continue - } - - id := types.ResourceID{ - ClusterName: clusterName, - Kind: resource.GetKind(), - Name: resource.GetName(), - } - result[types.ResourceIDToString(id)] = types.ResourceDetails{ - FriendlyName: friendlyName, - } - } - - return result, nil +// TODO(atburke): Remove this once teleport.e reference is switched over +func GetResourceDetails(ctx context.Context, clusterName string, lister client.ListResourcesClient, ids []types.ResourceID) (map[string]types.ResourceDetails, error) { + return accessrequest.GetResourceDetails(ctx, clusterName, lister, ids) } -// GetResourceIDsByCluster will return resource IDs grouped by cluster. +// TODO(atburke): Remove this once teleport.e reference is switched over func GetResourceIDsByCluster(r types.AccessRequest) map[string][]types.ResourceID { - resourceIDsByCluster := make(map[string][]types.ResourceID) - for _, resourceID := range r.GetRequestedResourceIDs() { - resourceIDsByCluster[resourceID.ClusterName] = append(resourceIDsByCluster[resourceID.ClusterName], resourceID) - } - return resourceIDsByCluster + return accessrequest.GetResourceIDsByCluster(r) } -func GetResourcesByResourceIDs(ctx context.Context, lister ResourceLister, resourceIDs []types.ResourceID, opts ...ListResourcesRequestOption) ([]types.ResourceWithLabels, error) { - resourceNamesByKind := make(map[string][]string) - for _, resourceID := range resourceIDs { - resourceNamesByKind[resourceID.Kind] = append(resourceNamesByKind[resourceID.Kind], resourceID.Name) - } - var resources []types.ResourceWithLabels - for kind, resourceNames := range resourceNamesByKind { - req := proto.ListResourcesRequest{ - ResourceType: MapResourceKindToListResourcesType(kind), - PredicateExpression: anyNameMatcher(resourceNames), - Limit: int32(len(resourceNames)), - } - for _, opt := range opts { - opt(&req) - } - resp, err := lister.ListResources(ctx, req) - if err != nil { - return nil, trace.Wrap(err) - } - for _, result := range resp.Resources { - leafResources, err := MapListResourcesResultToLeafResource(result, kind) - if err != nil { - return nil, trace.Wrap(err) - } - resources = append(resources, leafResources...) - } - } - return resources, nil -} - -// anyNameMatcher returns a PredicateExpression which matches any of a given list -// of names. Given names will be escaped and quoted when building the expression. -func anyNameMatcher(names []string) string { - matchers := make([]string, len(names)) - for i := range names { - matchers[i] = fmt.Sprintf(`resource.metadata.name == %q`, names[i]) - } - return strings.Join(matchers, " || ") -} - -// MapResourceKindToListResourcesType returns the value to use for ResourceType in a -// ListResourcesRequest based on the kind of resource you're searching for. -// Necessary because some resource kinds don't support ListResources directly, -// so you have to list the parent kind. Use MapListResourcesResultToLeafResource to map back -// to the given kind. -func MapResourceKindToListResourcesType(kind string) string { - switch kind { - case types.KindApp: - return types.KindAppServer - case types.KindDatabase: - return types.KindDatabaseServer - case types.KindKubernetesCluster: - return types.KindKubeServer - default: - return kind - } -} - -// MapListResourcesResultToLeafResource is the inverse of -// MapResourceKindToListResourcesType, after the ListResources call it maps the -// result back to the kind we really want. `hint` should be the name of the -// desired resource kind, used to disambiguate normal SSH nodes and kubernetes -// services which are both returned as `types.Server`. -func MapListResourcesResultToLeafResource(resource types.ResourceWithLabels, hint string) (types.ResourcesWithLabels, error) { - switch r := resource.(type) { - case types.AppServer: - return types.ResourcesWithLabels{r.GetApp()}, nil - case types.KubeServer: - return types.ResourcesWithLabels{r.GetCluster()}, nil - case types.DatabaseServer: - return types.ResourcesWithLabels{r.GetDatabase()}, nil - case types.Server: - if hint == types.KindKubernetesCluster { - return nil, trace.BadParameter("expected kubernetes server, got server") - } - default: - } - return types.ResourcesWithLabels{resource}, nil +// TODO(atburke): Remove this once teleport.e reference is switched over +func GetResourcesByResourceIDs(ctx context.Context, lister client.ListResourcesClient, resourceIDs []types.ResourceID, opts ...accessrequest.ListResourcesRequestOption) ([]types.ResourceWithLabels, error) { + return accessrequest.GetResourcesByResourceIDs(ctx, lister, resourceIDs, opts...) } // resourceMatcherToMatcherSlice returns the resourceMatcher in a RoleMatcher slice @@ -1970,7 +1846,7 @@ func (m *RequestValidator) getUnderlyingResourcesByResourceIDs(ctx context.Conte } } // load the underlying resources. - resources, err := GetResourcesByResourceIDs(ctx, m.getter, searchableResourcesIDs) + resources, err := accessrequest.GetResourcesByResourceIDs(ctx, m.getter, searchableResourcesIDs) return resources, trace.Wrap(err) } diff --git a/lib/services/access_request_test.go b/lib/services/access_request_test.go index f55b9e310c7c9..6c5ef7371b057 100644 --- a/lib/services/access_request_test.go +++ b/lib/services/access_request_test.go @@ -1888,58 +1888,6 @@ func TestValidateAccessRequestClusterNames(t *testing.T) { } } -type mockResourceLister struct { - resources []types.ResourceWithLabels -} - -func (m *mockResourceLister) ListResources(ctx context.Context, _ proto.ListResourcesRequest) (*types.ListResourcesResponse, error) { - return &types.ListResourcesResponse{ - Resources: m.resources, - }, nil -} - -func TestGetResourceDetails(t *testing.T) { - clusterName := "cluster" - - presence := &mockResourceLister{ - resources: []types.ResourceWithLabels{ - newNode(t, "node1", "hostname 1"), - newApp(t, "app1", "friendly app 1", types.OriginDynamic), - newApp(t, "app2", "friendly app 2", types.OriginDynamic), - newApp(t, "app3", "friendly app 3", types.OriginOkta), - newUserGroup(t, "group1", "friendly group 1", types.OriginOkta), - }, - } - resourceIDs := []types.ResourceID{ - newResourceID(clusterName, types.KindNode, "node1"), - newResourceID(clusterName, types.KindApp, "app1"), - newResourceID(clusterName, types.KindApp, "app2"), - newResourceID(clusterName, types.KindApp, "app3"), - newResourceID(clusterName, types.KindUserGroup, "group1"), - } - - ctx := context.Background() - - details, err := GetResourceDetails(ctx, clusterName, presence, resourceIDs) - require.NoError(t, err) - - // Check the resource details to see if friendly names properly propagated. - - // Node should be named for its hostname. - require.Equal(t, "hostname 1", details[types.ResourceIDToString(resourceIDs[0])].FriendlyName) - - // app1 and app2 are expected to be empty because they're not Okta sourced resources. - require.Empty(t, details[types.ResourceIDToString(resourceIDs[1])].FriendlyName) - - require.Empty(t, details[types.ResourceIDToString(resourceIDs[2])].FriendlyName) - - // This Okta sourced app should have a friendly name. - require.Equal(t, "friendly app 3", details[types.ResourceIDToString(resourceIDs[3])].FriendlyName) - - // This Okta sourced user group should have a friendly name. - require.Equal(t, "friendly group 1", details[types.ResourceIDToString(resourceIDs[4])].FriendlyName) -} - func TestMaxDuration(t *testing.T) { // describes a collection of roles and their conditions roleDesc := roleTestSet{ @@ -2175,48 +2123,3 @@ func getMockGetter(t *testing.T, roleDesc roleTestSet, userDesc map[string][]str } return g } - -func newNode(t *testing.T, name, hostname string) types.Server { - node, err := types.NewServer(name, types.KindNode, - types.ServerSpecV2{ - Hostname: hostname, - }) - require.NoError(t, err) - return node -} - -func newApp(t *testing.T, name, description, origin string) types.Application { - app, err := types.NewAppV3(types.Metadata{ - Name: name, - Description: description, - Labels: map[string]string{ - types.OriginLabel: origin, - }, - }, - types.AppSpecV3{ - URI: "https://some-addr.com", - PublicAddr: "https://some-addr.com", - }) - require.NoError(t, err) - return app -} - -func newUserGroup(t *testing.T, name, description, origin string) types.UserGroup { - userGroup, err := types.NewUserGroup(types.Metadata{ - Name: name, - Description: description, - Labels: map[string]string{ - types.OriginLabel: origin, - }, - }, types.UserGroupSpecV1{}) - require.NoError(t, err) - return userGroup -} - -func newResourceID(clusterName, kind, name string) types.ResourceID { - return types.ResourceID{ - ClusterName: clusterName, - Kind: kind, - Name: name, - } -} diff --git a/lib/services/friendlyname.go b/lib/services/friendlyname.go deleted file mode 100644 index 99c6a41b5c214..0000000000000 --- a/lib/services/friendlyname.go +++ /dev/null @@ -1,34 +0,0 @@ -/* -Copyright 2023 Gravitational, Inc. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package services - -import "github.com/gravitational/teleport/api/types" - -// FriendlyName will return the friendly name for a resource if it has one. Otherwise, it -// will return an empty string. -func FriendlyName(resource types.ResourceWithLabels) string { - // Right now, only resources sourced from Okta and nodes have friendly names. - if resource.Origin() == types.OriginOkta { - return resource.GetMetadata().Description - } - - if hn, ok := resource.(interface{ GetHostname() string }); ok { - return hn.GetHostname() - } - - return "" -} diff --git a/lib/services/friendlyname_test.go b/lib/services/friendlyname_test.go deleted file mode 100644 index f5c7f3affb07f..0000000000000 --- a/lib/services/friendlyname_test.go +++ /dev/null @@ -1,81 +0,0 @@ -/* -Copyright 2023 Gravitational, Inc. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package services - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/gravitational/teleport/api/types" -) - -func TestFriendlyName(t *testing.T) { - appNoFriendly, err := types.NewAppV3(types.Metadata{ - Name: "no friendly", - }, types.AppSpecV3{ - URI: "https://some-uri.com", - }, - ) - require.NoError(t, err) - - appFriendly, err := types.NewAppV3(types.Metadata{ - Name: "no friendly", - Description: "friendly name", - Labels: map[string]string{ - types.OriginLabel: types.OriginOkta, - }, - }, types.AppSpecV3{ - URI: "https://some-uri.com", - }, - ) - require.NoError(t, err) - - node, err := types.NewServer("node", types.KindNode, types.ServerSpecV2{ - Hostname: "friendly hostname", - }) - require.NoError(t, err) - - tests := []struct { - name string - resource types.ResourceWithLabels - expected string - }{ - { - name: "no friendly name", - resource: appNoFriendly, - expected: "", - }, - { - name: "friendly app name", - resource: appFriendly, - expected: "friendly name", - }, - { - name: "friendly node name", - resource: node, - expected: "friendly hostname", - }, - } - - for _, test := range tests { - test := test - t.Run(test.name, func(t *testing.T) { - require.Equal(t, test.expected, FriendlyName(test.resource)) - }) - } -} diff --git a/lib/teleterm/clusters/cluster_access_requests.go b/lib/teleterm/clusters/cluster_access_requests.go index c4d24687c6297..70215c32ff36e 100644 --- a/lib/teleterm/clusters/cluster_access_requests.go +++ b/lib/teleterm/clusters/cluster_access_requests.go @@ -21,6 +21,7 @@ import ( "github.com/gravitational/trace" "golang.org/x/exp/slices" + "github.com/gravitational/teleport/api/accessrequest" "github.com/gravitational/teleport/api/types" api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1" "github.com/gravitational/teleport/lib/auth" @@ -272,11 +273,11 @@ func (c *Cluster) AssumeRole(ctx context.Context, req *api.AssumeRoleRequest) er } func getResourceDetails(ctx context.Context, req types.AccessRequest, clt auth.ClientI) (map[string]ResourceDetails, error) { - resourceIDsByCluster := services.GetResourceIDsByCluster(req) + resourceIDsByCluster := accessrequest.GetResourceIDsByCluster(req) resourceDetails := make(map[string]ResourceDetails) for clusterName, resourceIDs := range resourceIDsByCluster { - details, err := services.GetResourceDetails(ctx, clusterName, clt, resourceIDs) + details, err := accessrequest.GetResourceDetails(ctx, clusterName, clt, resourceIDs) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/ui/app.go b/lib/web/ui/app.go index 3e06d1185d0b6..fc01c4dc9e56a 100644 --- a/lib/web/ui/app.go +++ b/lib/web/ui/app.go @@ -23,7 +23,6 @@ import ( "github.com/sirupsen/logrus" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils/aws" ) @@ -123,7 +122,7 @@ func MakeApp(app types.Application, c MakeAppsConfig) App { ClusterID: c.AppClusterName, FQDN: fqdn, AWSConsole: app.IsAWSConsole(), - FriendlyName: services.FriendlyName(app), + FriendlyName: types.FriendlyName(app), UserGroups: userGroupAndDescriptions, SAMLApp: false, } @@ -146,7 +145,7 @@ func MakeSAMLApp(app types.SAMLIdPServiceProvider, c MakeAppsConfig) App { PublicAddr: "", Labels: labels, ClusterID: c.AppClusterName, - FriendlyName: services.FriendlyName(app), + FriendlyName: types.FriendlyName(app), SAMLApp: true, } @@ -182,7 +181,7 @@ func MakeApps(c MakeAppsConfig) []App { ClusterID: c.AppClusterName, FQDN: fqdn, AWSConsole: app.IsAWSConsole(), - FriendlyName: services.FriendlyName(app), + FriendlyName: types.FriendlyName(app), UserGroups: userGroupAndDescriptions, SAMLApp: false, } @@ -202,7 +201,7 @@ func MakeApps(c MakeAppsConfig) []App { PublicAddr: appOrSP.GetPublicAddr(), Labels: labels, ClusterID: c.AppClusterName, - FriendlyName: services.FriendlyName(appOrSP), + FriendlyName: types.FriendlyName(appOrSP), SAMLApp: true, } diff --git a/lib/web/ui/user_groups.go b/lib/web/ui/user_groups.go index bb9b67bffc41e..ee25c8a65d14c 100644 --- a/lib/web/ui/user_groups.go +++ b/lib/web/ui/user_groups.go @@ -18,7 +18,6 @@ package ui import ( "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/services" ) // UserGroup describes a user group. @@ -54,7 +53,7 @@ func MakeUserGroups(userGroups []types.UserGroup, userGroupsToApps map[string]ty for i, app := range apps { appsAndFriendlyNames[i] = ApplicationAndFriendlyName{ Name: app.GetName(), - FriendlyName: services.FriendlyName(app), + FriendlyName: types.FriendlyName(app), } } @@ -62,7 +61,7 @@ func MakeUserGroups(userGroups []types.UserGroup, userGroupsToApps map[string]ty Name: userGroup.GetName(), Description: userGroup.GetMetadata().Description, Labels: uiLabels, - FriendlyName: services.FriendlyName(userGroup), + FriendlyName: types.FriendlyName(userGroup), Applications: appsAndFriendlyNames, }) } diff --git a/tool/tsh/common/access_request.go b/tool/tsh/common/access_request.go index c8e3c22afc607..d97b37ce8b1c2 100644 --- a/tool/tsh/common/access_request.go +++ b/tool/tsh/common/access_request.go @@ -28,6 +28,7 @@ import ( "golang.org/x/exp/slices" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/accessrequest" "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" kubeproto "github.com/gravitational/teleport/api/gen/proto/go/teleport/kube/v1" @@ -434,24 +435,16 @@ func onRequestSearch(cf *CLIConf) error { authClient := proxyClient.CurrentCluster() req := proto.ListResourcesRequest{ - ResourceType: services.MapResourceKindToListResourcesType(cf.ResourceKind), Labels: tc.Labels, PredicateExpression: cf.PredicateExpression, SearchKeywords: tc.SearchKeywords, UseSearchAsRoles: true, } - results, err := client.GetResourcesWithFilters(cf.Context, authClient, req) + resources, err = accessrequest.GetResourcesByKind(cf.Context, authClient, req, cf.ResourceKind) if err != nil { return trace.Wrap(err) } - for _, result := range results { - leafResources, err := services.MapListResourcesResultToLeafResource(result, cf.ResourceKind) - if err != nil { - return trace.Wrap(err) - } - resources = append(resources, leafResources...) - } tableColumns = []string{"Name", "Hostname", "Labels", "Resource ID"} }