diff --git a/docs/pages/includes/plugins/rbac-with-friendly-name.mdx b/docs/pages/includes/plugins/rbac-with-friendly-name.mdx index d390be84751da..553f036c9820b 100644 --- a/docs/pages/includes/plugins/rbac-with-friendly-name.mdx +++ b/docs/pages/includes/plugins/rbac-with-friendly-name.mdx @@ -24,6 +24,11 @@ spec: - resources: ['access_list'] verbs: ['list', 'read'] + # Optional: To display logins permitted by roles, the plugin also needs + # permission to read the role resource. + - resources: ['role'] + verbs: ['read'] + # Optional: To display user-friendly names in resource-based Access # Requests instead of resource IDs, the plugin also needs permission # to list the resources being requested. Include this along with the diff --git a/integrations/access/accessrequest/app.go b/integrations/access/accessrequest/app.go index 9d32482824fba..952e539e3ac6e 100644 --- a/integrations/access/accessrequest/app.go +++ b/integrations/access/accessrequest/app.go @@ -234,12 +234,20 @@ func (a *App) onPendingRequest(ctx context.Context, req types.AccessRequest) err return trace.Wrap(err) } + loginsByRole, err := a.getLoginsByRole(ctx, req) + if trace.IsAccessDenied(err) { + log.Warnf("Missing permissions to get logins by role. Please add role.read to the associated role. error: %s", err) + } else if err != nil { + return trace.Wrap(err) + } + reqData := pd.AccessRequestData{ User: req.GetUser(), Roles: req.GetRoles(), RequestReason: req.GetRequestReason(), SystemAnnotations: req.GetSystemAnnotations(), Resources: resourceNames, + LoginsByRole: loginsByRole, } _, err = a.pluginData.Create(ctx, reqID, PluginData{AccessRequestData: reqData}) @@ -488,6 +496,20 @@ func (a *App) updateMessages(ctx context.Context, reqID string, tag pd.Resolutio return nil } +func (a *App) getLoginsByRole(ctx context.Context, req types.AccessRequest) (map[string][]string, error) { + loginsByRole := make(map[string][]string, len(req.GetRoles())) + + for _, role := range req.GetRoles() { + currentRole, err := a.apiClient.GetRole(ctx, role) + if err != nil { + return nil, trace.Wrap(err) + } + loginsByRole[role] = currentRole.GetLogins(types.Allow) + } + + return loginsByRole, nil +} + func (a *App) getResourceNames(ctx context.Context, req types.AccessRequest) ([]string, error) { resourceNames := make([]string, 0, len(req.GetRequestedResourceIDs())) resourcesByCluster := accessrequest.GetResourceIDsByCluster(req) diff --git a/integrations/access/accessrequest/app_test.go b/integrations/access/accessrequest/app_test.go new file mode 100644 index 0000000000000..70e855ce3a897 --- /dev/null +++ b/integrations/access/accessrequest/app_test.go @@ -0,0 +1,84 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package accessrequest + +import ( + "context" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/integrations/access/common/teleport" +) + +type mockTeleportClient struct { + mock.Mock + teleport.Client +} + +func (m *mockTeleportClient) GetRole(ctx context.Context, name string) (types.Role, error) { + args := m.Called(ctx, name) + return args.Get(0).(types.Role), args.Error(1) +} + +func TestGetLoginsByRole(t *testing.T) { + teleportClient := &mockTeleportClient{} + teleportClient.On("GetRole", mock.Anything, "admin").Return(&types.RoleV6{ + Spec: types.RoleSpecV6{ + Allow: types.RoleConditions{ + Logins: []string{"root", "foo", "bar"}, + }, + }, + }, (error)(nil)) + teleportClient.On("GetRole", mock.Anything, "foo").Return(&types.RoleV6{ + Spec: types.RoleSpecV6{ + Allow: types.RoleConditions{ + Logins: []string{"foo"}, + }, + }, + }, (error)(nil)) + teleportClient.On("GetRole", mock.Anything, "dev").Return(&types.RoleV6{ + Spec: types.RoleSpecV6{ + Allow: types.RoleConditions{ + Logins: []string{}, + }, + }, + }, (error)(nil)) + + app := App{ + apiClient: teleportClient, + } + ctx := context.Background() + loginsByRole, err := app.getLoginsByRole(ctx, &types.AccessRequestV3{ + Spec: types.AccessRequestSpecV3{ + Roles: []string{"admin", "foo", "dev"}, + }, + }) + require.NoError(t, err) + + expected := map[string][]string{ + "admin": {"root", "foo", "bar"}, + "foo": {"foo"}, + "dev": {}, + } + require.Equal(t, expected, loginsByRole) + teleportClient.AssertNumberOfCalls(t, "GetRole", 3) +} diff --git a/integrations/access/accessrequest/bot.go b/integrations/access/accessrequest/bot.go index 2425ded76b996..ea92f962bf564 100644 --- a/integrations/access/accessrequest/bot.go +++ b/integrations/access/accessrequest/bot.go @@ -23,7 +23,6 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/integrations/access/common" - "github.com/gravitational/teleport/integrations/lib/plugindata" pd "github.com/gravitational/teleport/integrations/lib/plugindata" ) @@ -39,5 +38,5 @@ type MessagingBot interface { // This is used to change the access-request status and number of required approval remaining UpdateMessages(ctx context.Context, reqID string, data pd.AccessRequestData, messageData SentMessages, reviews []types.AccessReview) error // NotifyUser notifies the user if their access request status has changed - NotifyUser(ctx context.Context, reqID string, ard plugindata.AccessRequestData) error + NotifyUser(ctx context.Context, reqID string, ard pd.AccessRequestData) error } diff --git a/integrations/access/accessrequest/message.go b/integrations/access/accessrequest/message.go index 68cec7f271c35..b09783b41046e 100644 --- a/integrations/access/accessrequest/message.go +++ b/integrations/access/accessrequest/message.go @@ -21,6 +21,7 @@ package accessrequest import ( "fmt" "net/url" + "slices" "strings" "text/template" "time" @@ -36,7 +37,8 @@ import ( // for message section texts, so we truncate all reasons to a generous but // conservative limit const ( - requestReasonLimit = 500 + requestInlineLimit = 500 + requestReasonLimit resolutionReasonLimit ReviewReasonLimit ) @@ -70,6 +72,8 @@ func MsgStatusText(tag pd.ResolutionTag, reason string) string { return statusText } +// MsgFields constructs and returns the Access Request message. List values are +// constructed in sorted order. func MsgFields(reqID string, reqData pd.AccessRequestData, clusterName string, webProxyURL *url.URL) string { var builder strings.Builder builder.Grow(128) @@ -77,14 +81,27 @@ func MsgFields(reqID string, reqData pd.AccessRequestData, clusterName string, w msgFieldToBuilder(&builder, "ID", reqID) msgFieldToBuilder(&builder, "Cluster", clusterName) + sortedRoles := sortList(reqData.Roles) + if len(reqData.User) > 0 { msgFieldToBuilder(&builder, "User", reqData.User) } - if len(reqData.Roles) > 0 { - msgFieldToBuilder(&builder, "Role(s)", strings.Join(reqData.Roles, ",")) + if len(reqData.LoginsByRole) > 0 { + for _, role := range sortedRoles { + sortedLogins := sortList(reqData.LoginsByRole[role]) + loginStr := "-" + if len(sortedLogins) > 0 { + loginStr = strings.Join(sortedLogins, ", ") + } + msgFieldToBuilder(&builder, "Role", lib.MarkdownEscapeInLine(role, requestInlineLimit), + "Login(s)", lib.MarkdownEscapeInLine(loginStr, requestInlineLimit)) + } + } else if len(reqData.Roles) > 0 { + msgFieldToBuilder(&builder, "Role(s)", lib.MarkdownEscapeInLine(strings.Join(sortedRoles, ","), requestInlineLimit)) } if len(reqData.Resources) > 0 { - msgFieldToBuilder(&builder, "Resource(s)", strings.Join(reqData.Resources, ",")) + sortedResources := sortList(reqData.Resources) + msgFieldToBuilder(&builder, "Resource(s)", lib.MarkdownEscapeInLine(strings.Join(sortedResources, ","), requestInlineLimit)) } if reqData.RequestReason != "" { msgFieldToBuilder(&builder, "Reason", lib.MarkdownEscape(reqData.RequestReason, requestReasonLimit)) @@ -134,10 +151,28 @@ func MsgReview(review types.AccessReview) (string, error) { return builder.String(), nil } -func msgFieldToBuilder(b *strings.Builder, field, value string) { +func msgFieldToBuilder(b *strings.Builder, field, value string, additionalFields ...string) { b.WriteString("*") b.WriteString(field) b.WriteString("*: ") b.WriteString(value) + + for i := 0; i < len(additionalFields)-1; i += 2 { + field := additionalFields[i] + value := additionalFields[i+1] + b.WriteString(" *") + b.WriteString(field) + b.WriteString("*: ") + b.WriteString(value) + } + b.WriteString("\n") } + +// sortedList returns a sorted copy of the src. +func sortList(src []string) []string { + sorted := make([]string, len(src)) + copy(sorted, src) + slices.Sort(sorted) + return sorted +} diff --git a/integrations/access/accessrequest/message_test.go b/integrations/access/accessrequest/message_test.go new file mode 100644 index 0000000000000..ca5ba448e2457 --- /dev/null +++ b/integrations/access/accessrequest/message_test.go @@ -0,0 +1,91 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package accessrequest + +import ( + "fmt" + "net/url" + + "github.com/gravitational/teleport/integrations/lib/plugindata" +) + +func ExampleMsgFields_roles() { + id := "00000000-0000-0000-0000-000000000000" + req := plugindata.AccessRequestData{ + User: "example@goteleport.com", + Roles: []string{"foo", "bar"}, + Resources: []string{"/example.teleport.sh/node/0000"}, + RequestReason: "test", + } + cluster := "example.teleport.sh" + webProxyURL := &url.URL{ + Scheme: "https", + Host: "example.teleport.sh", + RawPath: "web/requests/00000000-0000-0000-0000-000000000000", + } + + msg := MsgFields(id, req, cluster, webProxyURL) + fmt.Println(msg) + + // Output: *ID*: 00000000-0000-0000-0000-000000000000 + // *Cluster*: example.teleport.sh + // *User*: example@goteleport.com + // *Role(s)*: `bar,foo` + // *Resource(s)*: `/example.teleport.sh/node/0000` + // *Reason*: ``` + // test``` + // *Link*: https://example.teleport.sh/web/requests/00000000-0000-0000-0000-000000000000 +} + +func ExampleMsgFields_logins() { + id := "00000000-0000-0000-0000-000000000000" + req := plugindata.AccessRequestData{ + User: "example@goteleport.com", + Roles: []string{"admin", "foo", "bar", "dev"}, + LoginsByRole: map[string][]string{ + "admin": {"foo", "bar", "root"}, + "foo": {"foo"}, + "bar": {"bar"}, + "dev": {}, + }, + Resources: []string{"/example.teleport.sh/node/0000"}, + RequestReason: "test", + } + cluster := "example.teleport.sh" + webProxyURL := &url.URL{ + Scheme: "https", + Host: "example.teleport.sh", + RawPath: "web/requests/00000000-0000-0000-0000-000000000000", + } + + msg := MsgFields(id, req, cluster, webProxyURL) + fmt.Println(msg) + + // Output: *ID*: 00000000-0000-0000-0000-000000000000 + // *Cluster*: example.teleport.sh + // *User*: example@goteleport.com + // *Role*: `admin` *Login(s)*: `bar, foo, root` + // *Role*: `bar` *Login(s)*: `bar` + // *Role*: `dev` *Login(s)*: `-` + // *Role*: `foo` *Login(s)*: `foo` + // *Resource(s)*: `/example.teleport.sh/node/0000` + // *Reason*: ``` + // test``` + // *Link*: https://example.teleport.sh/web/requests/00000000-0000-0000-0000-000000000000 +} diff --git a/integrations/access/common/teleport/client.go b/integrations/access/common/teleport/client.go index 675c9578d939e..081c4c17b6aa8 100644 --- a/integrations/access/common/teleport/client.go +++ b/integrations/access/common/teleport/client.go @@ -36,6 +36,7 @@ type Client interface { types.Events Ping(context.Context) (proto.PingResponse, error) GetAccessRequests(ctx context.Context, filter types.AccessRequestFilter) ([]types.AccessRequest, error) + GetRole(ctx context.Context, name string) (types.Role, 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/escape.go b/integrations/lib/escape.go index 80af0b95d6a8f..c8b9f80ab6126 100644 --- a/integrations/lib/escape.go +++ b/integrations/lib/escape.go @@ -26,15 +26,29 @@ import "strings" // Backticks are escaped and thus count as two runes for the purpose of the // truncation. func MarkdownEscape(t string, n int) string { + return markdownEscape(t, n, "```\n", "```") +} + +// MarkdownEscapeInLine wraps some text `t` in backticks (escaping any backtick +// inside the message), limiting the length of the message to `n` runes (inside +// the single preformatted block). The text is trimmed before escaping. +// Backticks are escaped and thus count as two runes for the purpose of the +// truncation. +func MarkdownEscapeInLine(t string, n int) string { + return markdownEscape(t, n, "`", "`") +} + +func markdownEscape(t string, n int, startBackticks, endBackticks string) string { t = strings.TrimSpace(t) if t == "" { return "(empty)" } + var b strings.Builder - b.WriteString("```\n") + b.WriteString(startBackticks) for i, r := range t { if i >= n { - b.WriteString("``` (truncated)") + b.WriteString(endBackticks + " (truncated)") return b.String() } b.WriteRune(r) @@ -45,6 +59,6 @@ func MarkdownEscape(t string, n int) string { n-- } } - b.WriteString("```") + b.WriteString(endBackticks) return b.String() } diff --git a/integrations/lib/escape_test.go b/integrations/lib/escape_test.go index 930c7b0995a8a..39474fb1b2a0d 100644 --- a/integrations/lib/escape_test.go +++ b/integrations/lib/escape_test.go @@ -31,3 +31,15 @@ func ExampleMarkdownEscape() { // "```\n`\ufefffoo`\ufeff `\ufeffbar`\ufeff```" // "```\n1234567890``` (truncated)" } + +func ExampleMarkdownEscapeInLine() { + fmt.Printf("%q\n", MarkdownEscapeInLine(" ", 1000)) + fmt.Printf("%q\n", MarkdownEscapeInLine("abc", 1000)) + fmt.Printf("%q\n", MarkdownEscapeInLine("`foo` `bar`", 1000)) + fmt.Printf("%q\n", MarkdownEscapeInLine(" 123456789012345 ", 10)) + + // Output: "(empty)" + // "`abc`" + // "``\ufefffoo`\ufeff `\ufeffbar`\ufeff`" + // "`1234567890` (truncated)" +} diff --git a/integrations/lib/plugindata/access_request.go b/integrations/lib/plugindata/access_request.go index 451a849e20aa2..fd70d44927144 100644 --- a/integrations/lib/plugindata/access_request.go +++ b/integrations/lib/plugindata/access_request.go @@ -48,6 +48,7 @@ type AccessRequestData struct { SystemAnnotations map[string][]string Resources []string SuggestedReviewers []string + LoginsByRole map[string][]string } // DecodeAccessRequestData deserializes a string map to PluginData struct. @@ -92,6 +93,17 @@ func DecodeAccessRequestData(dataMap map[string]string) (data AccessRequestData, data.SuggestedReviewers = nil } } + + if str, ok := dataMap["logins_by_role"]; ok { + err = json.Unmarshal([]byte(str), &data.LoginsByRole) + if err != nil { + err = trace.Wrap(err) + return + } + if len(data.LoginsByRole) == 0 { + data.LoginsByRole = nil + } + } return } @@ -135,5 +147,13 @@ func EncodeAccessRequestData(data AccessRequestData) (map[string]string, error) } result["suggested_reviewers"] = string(reviewers) } + + if len(data.LoginsByRole) != 0 { + logins, err := json.Marshal(data.LoginsByRole) + if err != nil { + return nil, trace.Wrap(err) + } + result["logins_by_role"] = string(logins) + } return result, nil } diff --git a/integrations/lib/plugindata/access_request_test.go b/integrations/lib/plugindata/access_request_test.go index 05c128da7aa83..a19a5ebd1c9d0 100644 --- a/integrations/lib/plugindata/access_request_test.go +++ b/integrations/lib/plugindata/access_request_test.go @@ -33,12 +33,15 @@ var sampleAccessRequestData = AccessRequestData{ ResolutionTag: ResolvedApproved, ResolutionReason: "foo ok", SuggestedReviewers: []string{"foouser"}, + LoginsByRole: map[string][]string{ + "role-foo": {"login-foo", "login-bar"}, + }, } func TestEncodeAccessRequestData(t *testing.T) { dataMap, err := EncodeAccessRequestData(sampleAccessRequestData) assert.NoError(t, err) - assert.Len(t, dataMap, 8) + assert.Len(t, dataMap, 9) 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"]) @@ -46,7 +49,9 @@ func TestEncodeAccessRequestData(t *testing.T) { assert.Equal(t, "3", dataMap["reviews_count"]) assert.Equal(t, "APPROVED", dataMap["resolution"]) assert.Equal(t, "foo ok", dataMap["resolve_reason"]) - assert.Equal(t, "[\"foouser\"]", dataMap["suggested_reviewers"]) + assert.Equal(t, `["foouser"]`, dataMap["suggested_reviewers"]) + assert.Equal(t, `{"role-foo":["login-foo","login-bar"]}`, dataMap["logins_by_role"]) + } func TestDecodeAccessRequestData(t *testing.T) { @@ -58,7 +63,8 @@ func TestDecodeAccessRequestData(t *testing.T) { "reviews_count": "3", "resolution": "APPROVED", "resolve_reason": "foo ok", - "suggested_reviewers": "[\"foouser\"]", + "suggested_reviewers": `["foouser"]`, + "logins_by_role": `{"role-foo":["login-foo","login-bar"]}`, }) assert.NoError(t, err) assert.Equal(t, sampleAccessRequestData, pluginData)