[v16] Slack plugin now lists logins permitted by requested role (#45759)

* Include logins

* Add unit tests

* Handle access denied gracefully

* Update documentation with required permissions

* Reduce doc

* Address feedback

* Imported twice

* Improve loggin; Replace none -> -

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

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)

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 <http://www.gnu.org/licenses/>.
+ */
+
+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)
+}

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
 }

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,21 +72,36 @@ 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)
 
 	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
+}

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 <http://www.gnu.org/licenses/>.
+ */
+
+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:          "[email protected]",
+		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*: [email protected]
+	// *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:  "[email protected]",
+		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*: [email protected]
+	// *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
+}

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)

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()
 }

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)"
+}