From 8b5a3e6217d781a68386c5df450399d0d6fa78b7 Mon Sep 17 00:00:00 2001
From: Julien Tant <785518+JulienTant@users.noreply.github.com>
Date: Mon, 27 Jan 2025 11:03:16 -0700
Subject: [PATCH] [MM-62502] Ensure MFA Check on search routes (#29880)

---
 server/channels/api4/handlers.go  |  2 +-
 server/channels/api4/user_test.go | 66 +++++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/server/channels/api4/handlers.go b/server/channels/api4/handlers.go
index e923f96af1..85bf3505ca 100644
--- a/server/channels/api4/handlers.go
+++ b/server/channels/api4/handlers.go
@@ -183,7 +183,7 @@ func (api *API) APISessionRequiredDisableWhenBusy(h handlerFunc, opts ...APIHand
 		HandlerName:     web.GetHandlerName(h),
 		RequireSession:  true,
 		TrustRequester:  false,
-		RequireMfa:      false,
+		RequireMfa:      true,
 		IsStatic:        false,
 		IsLocal:         false,
 		DisableWhenBusy: true,
diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go
index ae177ec51a..68b7e0fa4e 100644
--- a/server/channels/api4/user_test.go
+++ b/server/channels/api4/user_test.go
@@ -2410,7 +2410,8 @@ func TestUserUnicodeNames(t *testing.T) {
 			Nickname:  "Ender\u2028 Wiggin",
 			Password:  "hello1",
 			Username:  "\ufeffwiggin77",
-			Roles:     model.SystemAdminRoleId + " " + model.SystemUserRoleId}
+			Roles:     model.SystemAdminRoleId + " " + model.SystemUserRoleId,
+		}
 
 		ruser, resp, err := client.CreateUser(context.Background(), &user)
 		require.NoError(t, err)
@@ -6214,7 +6215,7 @@ func TestLoginLockout(t *testing.T) {
 	_, _, err = th.Client.Login(context.Background(), th.BasicUser.Email, "wrong")
 	CheckErrorID(t, err, "api.user.check_user_login_attempts.too_many.app_error")
 
-	//Check if lock is active
+	// Check if lock is active
 	_, _, err = th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
 	CheckErrorID(t, err, "api.user.check_user_login_attempts.too_many.app_error")
 
@@ -6236,7 +6237,7 @@ func TestLoginLockout(t *testing.T) {
 	err = th.Server.Store().User().UpdateMfaActive(th.BasicUser2.Id, false)
 	require.NoError(t, err)
 
-	//Check if lock is active
+	// Check if lock is active
 	_, _, err = th.Client.Login(context.Background(), th.BasicUser2.Email, th.BasicUser2.Password)
 	CheckErrorID(t, err, "api.user.check_user_login_attempts.too_many.app_error")
 }
@@ -6695,6 +6696,7 @@ func TestMigrateAuthToSAML(t *testing.T) {
 		CheckNotImplementedStatus(t, resp)
 	})
 }
+
 func TestUpdatePassword(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()
@@ -7407,7 +7409,8 @@ func TestThreadSocketEvents(t *testing.T) {
 				preMentions: 2,
 				replies:     0,
 				mentions:    0,
-			}, {
+			},
+			{
 				post:        &model.Post{ChannelId: th.BasicChannel.Id, Message: "simple reply", UserId: th.BasicUser2.Id, RootId: rpost.Id},
 				preReplies:  0,
 				preMentions: 0,
@@ -8270,6 +8273,7 @@ func TestGetUsersWithInvalidEmails(t *testing.T) {
 	require.Error(t, err)
 	CheckForbiddenStatus(t, resp)
 }
+
 func TestUserUpdateEvents(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
@@ -8977,3 +8981,57 @@ func TestRevokeAllSessionsForUser(t *testing.T) {
 		CheckForbiddenStatus(t, resp)
 	})
 }
+
+func TestSearchUsersWithMfaEnforced(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.App.Srv().SetLicense(model.NewTestLicense("mfa"))
+
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.EnableMultifactorAuthentication = true
+		*cfg.ServiceSettings.EnforceMultifactorAuthentication = true
+	})
+
+	t.Run("user with MFA active can search users", func(t *testing.T) {
+		userWithMFAOK := th.BasicUser
+		secret, appErr := th.App.GenerateMfaSecret(userWithMFAOK.Id)
+		assert.Nil(t, appErr)
+
+		// Fake user has MFA enabled
+		err := th.Server.Store().User().UpdateMfaActive(userWithMFAOK.Id, true)
+		require.NoError(t, err)
+
+		err = th.Server.Store().User().UpdateMfaSecret(userWithMFAOK.Id, secret.Secret)
+		require.NoError(t, err)
+
+		code := dgoogauth.ComputeCode(secret.Secret, time.Now().UTC().Unix()/30)
+
+		client := th.CreateClient()
+		user, _, err := client.LoginWithMFA(context.Background(), th.BasicUser.Email, th.BasicUser.Password, fmt.Sprintf("%06d", code))
+		require.NoError(t, err)
+		assert.NotNil(t, user)
+
+		_, _, err = client.SearchUsers(context.Background(), &model.UserSearch{
+			Term: "user",
+		})
+
+		require.NoError(t, err)
+	})
+
+	t.Run("user with MFA not active can't search users", func(t *testing.T) {
+		userWithMFANotOk := th.BasicUser2
+		err := th.Server.Store().User().UpdateMfaActive(userWithMFANotOk.Id, false)
+		require.NoError(t, err)
+
+		client := th.CreateClient()
+		_, _, err = client.Login(context.Background(), userWithMFANotOk.Email, userWithMFANotOk.Password)
+		require.NoError(t, err)
+
+		_, resp, err := client.SearchUsers(context.Background(), &model.UserSearch{
+			Term: "user",
+		})
+		CheckErrorID(t, err, "api.context.mfa_required.app_error")
+		CheckForbiddenStatus(t, resp)
+	})
+}