From 474afe4671a2fbfefd611784b7a24186f82b6fab Mon Sep 17 00:00:00 2001
From: Jim <jlambert@hashicorp.com>
Date: Tue, 27 Jun 2023 12:13:13 -0400
Subject: [PATCH] =?UTF-8?q?fix:=20allow=20ldap=20managed=20grps=20to=20be?=
 =?UTF-8?q?=20set=20and=20removed=20as=20principals=E2=80=A6=20(#3363)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 CHANGELOG.md                                  |  6 +++--
 .../controller/handlers/roles/role_service.go |  6 +++--
 .../handlers/roles/role_service_test.go       | 23 +++++++++++++++++++
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76fbed8b96..b22c4682a3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,8 +22,10 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
 
 ### Bug Fixes
 
-* LDAP managed groups: adding a principal to a role now works properly when it's
-  an LDAP managed group. ([PR](https://github.com/hashicorp/boundary/pull/3361))
+* LDAP managed groups: adding/setting/removing a principal to a role now works
+  properly when it's an LDAP managed group.
+  ([PR](https://github.com/hashicorp/boundary/pull/3361) and
+  [PR](https://github.com/hashicorp/boundary/pull/3363))  
 
 ## 0.13 (2023/06/13)
 
diff --git a/internal/daemon/controller/handlers/roles/role_service.go b/internal/daemon/controller/handlers/roles/role_service.go
index 28b4b1887e..cba03b170a 100644
--- a/internal/daemon/controller/handlers/roles/role_service.go
+++ b/internal/daemon/controller/handlers/roles/role_service.go
@@ -1006,7 +1006,8 @@ func validateSetRolePrincipalsRequest(req *pbs.SetRolePrincipalsRequest) error {
 	for _, id := range req.GetPrincipalIds() {
 		if !handlers.ValidId(handlers.Id(id), globals.GroupPrefix) &&
 			!handlers.ValidId(handlers.Id(id), globals.UserPrefix) &&
-			!handlers.ValidId(handlers.Id(id), globals.OidcManagedGroupPrefix) {
+			!handlers.ValidId(handlers.Id(id), globals.OidcManagedGroupPrefix) &&
+			!handlers.ValidId(handlers.Id(id), globals.LdapManagedGroupPrefix) {
 			badFields["principal_ids"] = "Must only have valid user, group, and/or managed group ids."
 			break
 		}
@@ -1035,7 +1036,8 @@ func validateRemoveRolePrincipalsRequest(req *pbs.RemoveRolePrincipalsRequest) e
 	for _, id := range req.GetPrincipalIds() {
 		if !handlers.ValidId(handlers.Id(id), globals.GroupPrefix) &&
 			!handlers.ValidId(handlers.Id(id), globals.UserPrefix) &&
-			!handlers.ValidId(handlers.Id(id), globals.OidcManagedGroupPrefix) {
+			!handlers.ValidId(handlers.Id(id), globals.OidcManagedGroupPrefix) &&
+			!handlers.ValidId(handlers.Id(id), globals.LdapManagedGroupPrefix) {
 			badFields["principal_ids"] = "Must only have valid user, group, and/or managed group ids."
 			break
 		}
diff --git a/internal/daemon/controller/handlers/roles/role_service_test.go b/internal/daemon/controller/handlers/roles/role_service_test.go
index 0c4303cdca..4a208422e6 100644
--- a/internal/daemon/controller/handlers/roles/role_service_test.go
+++ b/internal/daemon/controller/handlers/roles/role_service_test.go
@@ -1246,6 +1246,9 @@ func TestSetPrincipal(t *testing.T) {
 		oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://www.alice.com/callback")[0]),
 	)
 
+	ldapAuthMethod := ldap.TestAuthMethod(t, conn, databaseWrapper, o.PublicId, []string{"ldaps://ldap1"})
+	ldapManagedGroup := ldap.TestManagedGroup(t, conn, ldapAuthMethod, []string{"admin"})
+
 	users := []*iam.User{
 		iam.TestUser(t, iamRepo, o.GetPublicId()),
 		iam.TestUser(t, iamRepo, o.GetPublicId()),
@@ -1332,6 +1335,14 @@ func TestSetPrincipal(t *testing.T) {
 			setManagedGroups:    []string{managedGroups[1].GetPublicId()},
 			resultManagedGroups: []string{managedGroups[1].GetPublicId()},
 		},
+		{
+			name: "Set LDAP managed group on populated role",
+			setup: func(r *iam.Role) {
+				iam.TestManagedGroupRole(t, conn, r.GetPublicId(), managedGroups[0].GetPublicId())
+			},
+			setManagedGroups:    []string{ldapManagedGroup.GetPublicId()},
+			resultManagedGroups: []string{ldapManagedGroup.GetPublicId()},
+		},
 		{
 			name:     "Set invalid u_recovery on role",
 			setup:    func(r *iam.Role) {},
@@ -1436,6 +1447,9 @@ func TestRemovePrincipal(t *testing.T) {
 		oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://www.alice.com/callback")[0]),
 	)
 
+	ldapAuthMethod := ldap.TestAuthMethod(t, conn, databaseWrapper, o.PublicId, []string{"ldaps://ldap1"})
+	ldapManagedGroup := ldap.TestManagedGroup(t, conn, ldapAuthMethod, []string{"admin"})
+
 	users := []*iam.User{
 		iam.TestUser(t, iamRepo, o.GetPublicId()),
 		iam.TestUser(t, iamRepo, o.GetPublicId()),
@@ -1569,6 +1583,15 @@ func TestRemovePrincipal(t *testing.T) {
 			removeManagedGroups: []string{managedGroups[0].GetPublicId(), managedGroups[1].GetPublicId()},
 			resultManagedGroups: []string{},
 		},
+		{
+			name: "Remove LDAP managed groups from role",
+			setup: func(r *iam.Role) {
+				iam.TestManagedGroupRole(t, conn, r.GetPublicId(), ldapManagedGroup.GetPublicId())
+				iam.TestManagedGroupRole(t, conn, r.GetPublicId(), managedGroups[0].GetPublicId())
+			},
+			removeManagedGroups: []string{ldapManagedGroup.GetPublicId()},
+			resultManagedGroups: []string{managedGroups[0].GetPublicId()},
+		},
 	}
 
 	for _, tc := range addCases {