From 71a1bcc81dcf8192ae16e2e1bb4862bb45fb784f Mon Sep 17 00:00:00 2001
From: Jakub Nyckowski <jakub.nyckowski@goteleport.com>
Date: Mon, 14 Nov 2022 14:44:29 -0500
Subject: [PATCH] Add support for user.spec in moderated sessions filters
 (#18402)

As pointed out in #15898, our moderated session filters use user.roles instead of user.spec.roles as the rest of Teleport.
This change adds support for both formats, so we don't break the existing filters and support the desired one.
---
 lib/auth/session_access.go      | 10 +++++++-
 lib/auth/session_access_test.go | 41 +++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/lib/auth/session_access.go b/lib/auth/session_access.go
index e526d23ac5a..882ac1c7b4b 100644
--- a/lib/auth/session_access.go
+++ b/lib/auth/session_access.go
@@ -98,7 +98,15 @@ type SessionAccessContext struct {
 func (ctx *SessionAccessContext) GetIdentifier(fields []string) (interface{}, error) {
 	if fields[0] == "user" {
 		if len(fields) == 2 || len(fields) == 3 {
-			switch fields[1] {
+			checkedFieldIdx := 1
+			// Unify the format. Moderated session originally skipped the spec field (user.roles was used instead of
+			// user.spec.roles) which was not aligned with how our roles filtering works.
+			// Here we try support both cases. We don't want to modify the original fields slice,
+			// as that would change the reported error message (see return below).
+			if len(fields) == 3 && fields[1] == "spec" {
+				checkedFieldIdx = 2
+			}
+			switch fields[checkedFieldIdx] {
 			case "name":
 				return ctx.Username, nil
 			case "roles":
diff --git a/lib/auth/session_access_test.go b/lib/auth/session_access_test.go
index bf3658d50b4..2645860437a 100644
--- a/lib/auth/session_access_test.go
+++ b/lib/auth/session_access_test.go
@@ -73,6 +73,46 @@ func successStartTestCase(t *testing.T) startTestCase {
 	}
 }
 
+func successStartTestCaseSpec(t *testing.T) startTestCase {
+	hostRole, err := types.NewRole("host", types.RoleSpecV5{})
+	require.NoError(t, err)
+	participantRole, err := types.NewRole("participant", types.RoleSpecV5{})
+	require.NoError(t, err)
+
+	hostRole.SetSessionRequirePolicies([]*types.SessionRequirePolicy{{
+		Filter:  "contains(user.spec.roles, \"participant\")",
+		Kinds:   []string{string(types.SSHSessionKind), string(types.KubernetesSessionKind)},
+		Count:   2,
+		OnLeave: types.OnSessionLeaveTerminate,
+		Modes:   []string{"peer"},
+	}})
+
+	participantRole.SetSessionJoinPolicies([]*types.SessionJoinPolicy{{
+		Roles: []string{hostRole.GetName()},
+		Kinds: []string{string(types.SSHSessionKind), string(types.KubernetesSessionKind)},
+		Modes: []string{"*"},
+	}})
+
+	return startTestCase{
+		name:         "success with spec",
+		host:         []types.Role{hostRole},
+		sessionKinds: []types.SessionKind{types.SSHSessionKind, types.KubernetesSessionKind},
+		participants: []SessionAccessContext{
+			{
+				Username: "participant",
+				Roles:    []types.Role{participantRole},
+				Mode:     "peer",
+			},
+			{
+				Username: "participant2",
+				Roles:    []types.Role{participantRole},
+				Mode:     "peer",
+			},
+		},
+		expected: []bool{true, true},
+	}
+}
+
 func failCountStartTestCase(t *testing.T) startTestCase {
 	hostRole, err := types.NewRole("host", types.RoleSpecV5{})
 	require.NoError(t, err)
@@ -173,6 +213,7 @@ func failFilterStartTestCase(t *testing.T) startTestCase {
 func TestSessionAccessStart(t *testing.T) {
 	testCases := []startTestCase{
 		successStartTestCase(t),
+		successStartTestCaseSpec(t),
 		failCountStartTestCase(t),
 		failFilterStartTestCase(t),
 		succeedDiscardPolicySetStartTestCase(t),