diff --git a/tool/common/common.go b/tool/common/common.go index cfb20bbedc294..3c85ec26e9643 100644 --- a/tool/common/common.go +++ b/tool/common/common.go @@ -223,6 +223,50 @@ func FormatResourceName(r types.ResourceWithLabels, verbose bool) string { return r.GetName() } +// FormatResourceAccessID returns the provided ResourceAccessID in its string form, +// appending constraints when present. +func FormatResourceAccessID(rid types.ResourceAccessID) string { + resourceIDString := types.ResourceIDToString(rid.GetResourceID()) + constraintsString := "" + + if c := rid.GetConstraints(); c != nil && c.GetDetails() != nil { + switch d := c.GetDetails().(type) { + case *types.ResourceConstraints_AwsConsole: + if d.AwsConsole == nil { + break + } + constraintsString = fmt.Sprintf("role_arns=%s", strings.Join(d.AwsConsole.RoleArns, ",")) + } + } + + if constraintsString != "" { + return fmt.Sprintf("%s (%s)", resourceIDString, constraintsString) + } + + return resourceIDString +} + +// FormatResourceAccessIDs returns the provided ResourceAccessIDs in string form, +// appending constraints to each when present. Uses JSON.Marshal to +// ensure proper handling for any IDs containing commas/quotes. +func FormatResourceAccessIDs(rids []types.ResourceAccessID) (string, error) { + out := "" + + if len(rids) > 0 { + resourceIDStrings := make([]string, 0, len(rids)) + for _, rid := range rids { + resourceIDStrings = append(resourceIDStrings, FormatResourceAccessID(rid)) + } + bytes, err := json.Marshal(resourceIDStrings) + if err != nil { + return "", trace.Wrap(err, "failed to marshal ResourceAccessIDs") + } + out = string(bytes) + } + + return out, nil +} + // GetDiscoveredResourceName returns the resource original name discovered in // the cloud by the Teleport Discovery Service. func GetDiscoveredResourceName(r types.ResourceWithLabels) (discoveredName string, ok bool) { diff --git a/tool/common/common_test.go b/tool/common/common_test.go index 705411242f7e7..09b0817d86995 100644 --- a/tool/common/common_test.go +++ b/tool/common/common_test.go @@ -21,6 +21,7 @@ package common import ( "bytes" "context" + "fmt" "maps" "testing" @@ -145,3 +146,112 @@ func TestFormatLabels(t *testing.T) { }) } } + +func TestFormatResourceAccessIDs(t *testing.T) { + t.Parallel() + + const ( + ARN1 = "arn:aws:iam::123456789012:role/Role1" + ARN2 = "arn:aws:iam::123456789012:role/Role2" + ) + + rids := []types.ResourceAccessID{ + { + Id: types.ResourceID{ + Kind: types.KindApp, + Name: "aws_console", + ClusterName: "cluster", + }, + Constraints: &types.ResourceConstraints{ + Version: types.V1, + Details: &types.ResourceConstraints_AwsConsole{ + AwsConsole: &types.AWSConsoleResourceConstraints{ + RoleArns: []string{ARN1, ARN2}, + }, + }, + }, + }, + { + Id: types.ResourceID{ + Kind: types.KindNode, + Name: "ssh_server", + ClusterName: "cluster", + }, + Constraints: nil, + }, + } + + t.Run("with aws_console constraints", func(t *testing.T) { + t.Parallel() + + out, err := FormatResourceAccessIDs(rids) + require.NoError(t, err) + require.Equal(t, fmt.Sprintf("[\"/cluster/app/aws_console (role_arns=%s,%s)\",\"/cluster/node/ssh_server\"]", ARN1, ARN2), out) + }) + + t.Run("with empty aws_console constraints", func(t *testing.T) { + t.Parallel() + + rid := types.ResourceAccessID{ + Id: rids[0].Id, + Constraints: &types.ResourceConstraints{ + Version: types.V1, + Details: &types.ResourceConstraints_AwsConsole{ + AwsConsole: &types.AWSConsoleResourceConstraints{ + RoleArns: []string{}, + }, + }, + }, + } + + out, err := FormatResourceAccessIDs([]types.ResourceAccessID{rid}) + require.NoError(t, err) + require.Equal(t, "[\"/cluster/app/aws_console (role_arns=)\"]", out) + }) + + t.Run("with empty or nil list", func(t *testing.T) { + t.Parallel() + + out, err := FormatResourceAccessIDs(nil) + require.NoError(t, err) + require.Empty(t, out) + + out, err = FormatResourceAccessIDs([]types.ResourceAccessID{}) + require.NoError(t, err) + require.Empty(t, out) + }) + + t.Run("with namespace resource with SubResourceName", func(t *testing.T) { + t.Parallel() + + rid := types.ResourceAccessID{ + Id: types.ResourceID{ + ClusterName: "cluster", + Kind: types.KindKubeNamespace, + Name: "my-kube-cluster", + SubResourceName: "production", + }, + } + + out, err := FormatResourceAccessIDs([]types.ResourceAccessID{rid}) + require.NoError(t, err) + require.Equal(t, "[\"/cluster/namespace/my-kube-cluster/production\"]", out) + }) + + t.Run("with kube pod resource with SubResourceName", func(t *testing.T) { + t.Parallel() + + rid := types.ResourceAccessID{ + Id: types.ResourceID{ + ClusterName: "cluster", + Kind: types.AccessRequestPrefixKindKubeNamespaced + "pods", + Name: "my-kube-cluster", + SubResourceName: "default/nginx", + }, + } + + out, err := FormatResourceAccessIDs([]types.ResourceAccessID{rid}) + require.NoError(t, err) + require.Equal(t, "[\"/cluster/kube:ns:pods/my-kube-cluster/default/nginx\"]", out) + }) +} diff --git a/tool/tctl/common/access_request_command.go b/tool/tctl/common/access_request_command.go index a95dc6e3d9030..271fd47574e50 100644 --- a/tool/tctl/common/access_request_command.go +++ b/tool/tctl/common/access_request_command.go @@ -39,6 +39,7 @@ import ( "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/tlsca" + "github.com/gravitational/teleport/tool/common" commonclient "github.com/gravitational/teleport/tool/tctl/common/client" tctlcfg "github.com/gravitational/teleport/tool/tctl/common/config" ) @@ -487,7 +488,9 @@ func printRequestsOverview(reqs []types.AccessRequest, format string) error { "Full reason was truncated, use the `tctl requests get` subcommand to view the full reason.", ) for _, req := range reqs { - resourceIDsString, err := types.ResourceIDsToString(req.GetRequestedResourceIDs()) + // This table isn't a comprehensive overview of each request; omit constraints on resources for brevity + // and only print their stringified ResourceIDs. + resourceIDsString, err := types.ResourceIDsToString(types.RiskyExtractResourceIDs(req.GetAllRequestedResourceIDs())) if err != nil { return trace.Wrap(err) } @@ -518,7 +521,7 @@ func printRequestsDetailed(reqs []types.AccessRequest, format string) error { switch format { case teleport.Text: for _, req := range reqs { - resourceIDsString, err := types.ResourceIDsToString(req.GetRequestedResourceIDs()) + resourceIDsString, err := common.FormatResourceAccessIDs(req.GetAllRequestedResourceIDs()) if err != nil { return trace.Wrap(err) } diff --git a/tool/tsh/common/access_request.go b/tool/tsh/common/access_request.go index 0bbd9d3741b6d..036aaffd4e0c3 100644 --- a/tool/tsh/common/access_request.go +++ b/tool/tsh/common/access_request.go @@ -184,12 +184,9 @@ func printRequest(cf *CLIConf, req types.AccessRequest) error { reviewers = strings.Join(r, ", ") } - resourcesStr := "" - if resources := req.GetRequestedResourceIDs(); len(resources) > 0 { - var err error - if resourcesStr, err = types.ResourceIDsToString(resources); err != nil { - return trace.Wrap(err) - } + resourcesStr, err := common.FormatResourceAccessIDs(req.GetAllRequestedResourceIDs()) + if err != nil { + return trace.Wrap(err) } table := asciitable.MakeHeadlessTable(2) @@ -210,7 +207,7 @@ func printRequest(cf *CLIConf, req types.AccessRequest) error { } table.AddRow([]string{"Status:", req.GetState().String()}) - _, err := table.AsBuffer().WriteTo(cf.Stdout()) + _, err = table.AsBuffer().WriteTo(cf.Stdout()) if err != nil { return trace.Wrap(err) } @@ -363,7 +360,9 @@ func showRequestTable(cf *CLIConf, reqs []types.AccessRequest) error { if now.After(req.GetAccessExpiry()) { continue } - resourceIDsString, err := types.ResourceIDsToString(req.GetRequestedResourceIDs()) + // This table isn't a comprehensive overview of each request; omit constraints on resources for brevity + // and only print their stringified ResourceIDs. + resourceIDsString, err := types.ResourceIDsToString(types.RiskyExtractResourceIDs(req.GetAllRequestedResourceIDs())) if err != nil { return trace.Wrap(err) } diff --git a/tool/tsh/common/access_request_test.go b/tool/tsh/common/access_request_test.go index e933f6aece241..2eadcf727495c 100644 --- a/tool/tsh/common/access_request_test.go +++ b/tool/tsh/common/access_request_test.go @@ -277,6 +277,69 @@ func TestShowRequestTable(t *testing.T) { }, wantPresent: []string{"someName", "someUser", "role1,role2", assumeStartTime.UTC().Format(time.RFC822)}, }, + { + name: "Access Requests with constrained resources", + reqs: []types.AccessRequest{ + &types.AccessRequestV3{ + Metadata: types.Metadata{ + Name: "constrainedRequest", + Expires: &expiresTime, + }, + Spec: types.AccessRequestSpecV3{ + User: "someUser", + Roles: []string{"aws-access"}, + Expires: expiresTime, + SessionTTL: expiresTime, + Created: createdAtTime, + RequestedResourceAccessIDs: []types.ResourceAccessID{ + { + Id: types.ResourceID{ + ClusterName: "test-cluster", + Kind: types.KindApp, + Name: "awsconsole", + }, + Constraints: &types.ResourceConstraints{ + Version: types.V1, + Details: &types.ResourceConstraints_AwsConsole{ + AwsConsole: &types.AWSConsoleResourceConstraints{ + RoleArns: []string{"arn:aws:iam::123456789012:role/Admin"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantPresent: []string{"constrainedRequest", "someUser", "aws-access"}, + }, + { + name: "Access Requests with namespace resources", + reqs: []types.AccessRequest{ + &types.AccessRequestV3{ + Metadata: types.Metadata{ + Name: "namespaceRequest", + Expires: &expiresTime, + }, + Spec: types.AccessRequestSpecV3{ + User: "someUser", + Roles: []string{"kube-access"}, + Expires: expiresTime, + SessionTTL: expiresTime, + Created: createdAtTime, + RequestedResourceIDs: []types.ResourceID{ + { + ClusterName: "test-cluster", + Kind: types.KindKubeNamespace, + Name: "my-kube-cluster", + SubResourceName: "production", + }, + }, + }, + }, + }, + wantPresent: []string{"namespaceRequest", "someUser", "kube-access"}, + }, } for _, tc := range tests { @@ -295,6 +358,196 @@ func TestShowRequestTable(t *testing.T) { } } +func TestPrintRequest(t *testing.T) { + t.Parallel() + + createdAtTime := time.Now() + expiresTime := time.Now().Add(time.Hour) + + tests := []struct { + name string + req types.AccessRequest + wantPresent []string + wantAbsent []string + }{ + { + name: "basic request without resources", + req: &types.AccessRequestV3{ + Metadata: types.Metadata{ + Name: "basic-request", + Expires: &expiresTime, + }, + Spec: types.AccessRequestSpecV3{ + User: "testuser", + Roles: []string{"admin", "developer"}, + Expires: expiresTime, + SessionTTL: expiresTime, + Created: createdAtTime, + }, + }, + wantPresent: []string{ + "Request ID:", + "basic-request", + "Username:", + "testuser", + "Roles:", + "admin, developer", + }, + wantAbsent: []string{"Resources:"}, + }, + { + name: "request with constrained AWS console resources", + req: &types.AccessRequestV3{ + Metadata: types.Metadata{ + Name: "aws-request", + Expires: &expiresTime, + }, + Spec: types.AccessRequestSpecV3{ + User: "testuser", + Roles: []string{"aws-access"}, + Expires: expiresTime, + SessionTTL: expiresTime, + Created: createdAtTime, + RequestedResourceAccessIDs: []types.ResourceAccessID{ + { + Id: types.ResourceID{ + ClusterName: "test-cluster", + Kind: types.KindApp, + Name: "awsconsole", + }, + Constraints: &types.ResourceConstraints{ + Version: types.V1, + Details: &types.ResourceConstraints_AwsConsole{ + AwsConsole: &types.AWSConsoleResourceConstraints{ + RoleArns: []string{ + "arn:aws:iam::123456789012:role/Admin", + "arn:aws:iam::123456789012:role/ReadOnly", + }, + }, + }, + }, + }, + }, + }, + }, + wantPresent: []string{ + "Request ID:", + "aws-request", + "Username:", + "testuser", + "Roles:", + "aws-access", + "Resources:", + "/test-cluster/app/awsconsole", + "role_arns=", + "arn:aws:iam::123456789012:role/Admin", + "arn:aws:iam::123456789012:role/ReadOnly", + }, + }, + { + name: "request with kubernetes namespace resources", + req: &types.AccessRequestV3{ + Metadata: types.Metadata{ + Name: "kube-request", + Expires: &expiresTime, + }, + Spec: types.AccessRequestSpecV3{ + User: "testuser", + Roles: []string{"kube-access"}, + Expires: expiresTime, + SessionTTL: expiresTime, + Created: createdAtTime, + RequestedResourceIDs: []types.ResourceID{ + { + ClusterName: "test-cluster", + Kind: types.KindKubeNamespace, + Name: "my-kube-cluster", + SubResourceName: "production", + }, + }, + }, + }, + wantPresent: []string{ + "Request ID:", + "kube-request", + "Username:", + "testuser", + "Roles:", + "kube-access", + "Resources:", + "/test-cluster/namespace/my-kube-cluster/production", + }, + }, + { + name: "request with mixed resources and constraints", + req: &types.AccessRequestV3{ + Metadata: types.Metadata{ + Name: "mixed-request", + Expires: &expiresTime, + }, + Spec: types.AccessRequestSpecV3{ + User: "testuser", + Roles: []string{"multi-access"}, + Expires: expiresTime, + SessionTTL: expiresTime, + Created: createdAtTime, + RequestedResourceIDs: []types.ResourceID{ + { + ClusterName: "test-cluster", + Kind: types.KindNode, + Name: "my-server", + }, + }, + RequestedResourceAccessIDs: []types.ResourceAccessID{ + { + Id: types.ResourceID{ + ClusterName: "test-cluster", + Kind: types.KindApp, + Name: "awsconsole", + }, + Constraints: &types.ResourceConstraints{ + Version: types.V1, + Details: &types.ResourceConstraints_AwsConsole{ + AwsConsole: &types.AWSConsoleResourceConstraints{ + RoleArns: []string{"arn:aws:iam::123456789012:role/Admin"}, + }, + }, + }, + }, + }, + }, + }, + wantPresent: []string{ + "Request ID:", + "mixed-request", + "Resources:", + "/test-cluster/node/my-server", + "/test-cluster/app/awsconsole", + "role_arns=", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + captureStdout := new(bytes.Buffer) + cf := &CLIConf{ + OverrideStdout: captureStdout, + } + err := printRequest(cf, tc.req) + require.NoError(t, err) + output := captureStdout.String() + for _, wanted := range tc.wantPresent { + require.Contains(t, output, wanted, "expected output to contain %q", wanted) + } + for _, unwanted := range tc.wantAbsent { + require.NotContains(t, output, unwanted, "expected output to not contain %q", unwanted) + } + }) + } +} + func TestRequestSearchRequestableRoles(t *testing.T) { ctx := t.Context() tmpHomePath := t.TempDir() diff --git a/tool/tsh/common/db_test.go b/tool/tsh/common/db_test.go index ea70e255d9060..14ce65ce6cfe0 100644 --- a/tool/tsh/common/db_test.go +++ b/tool/tsh/common/db_test.go @@ -552,7 +552,7 @@ func updateAccessRequestForDB(t *testing.T, s *suite, wantRequestReason, wantDBN if accessRequest.GetRequestReason() != wantRequestReason { continue } - for _, resourceID := range accessRequest.GetRequestedResourceIDs() { + for _, resourceID := range types.RiskyExtractResourceIDs(accessRequest.GetAllRequestedResourceIDs()) { if resourceID.Kind == types.KindDatabase && resourceID.Name == wantDBName { accessRequestID = accessRequest.GetName() diff --git a/tool/tsh/common/kube_test.go b/tool/tsh/common/kube_test.go index b2ae32ccb1e85..3256bd9e0af9d 100644 --- a/tool/tsh/common/kube_test.go +++ b/tool/tsh/common/kube_test.go @@ -741,7 +741,7 @@ func TestKubeSelection(t *testing.T) { } equal := reflect.DeepEqual( - accessRequests[0].GetRequestedResourceIDs(), + types.RiskyExtractResourceIDs(accessRequests[0].GetAllRequestedResourceIDs()), []types.ResourceID{ { ClusterName: s.root.Config.Auth.ClusterName.GetClusterName(),