[AI assisted]: MM-62914: Added MFA authentication for plugin requests as well (#30160)

We wipe the token if MFA authentication is enabled. Also added a test case
to lock in the functionality.

https://mattermost.atlassian.net/browse/MM-62914

```release-note
NONE
```

server/channels/app/authentication.go

@@ -6,12 +6,14 @@ package app
 import (
 	"errors"
 	"net/http"
+	"path"
 	"strings"
 
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/mattermost/mattermost/server/public/shared/mlog"
 	"github.com/mattermost/mattermost/server/public/shared/request"
 	"github.com/mattermost/mattermost/server/v8/channels/app/users"
+	"github.com/mattermost/mattermost/server/v8/channels/utils"
 	"github.com/mattermost/mattermost/server/v8/platform/shared/mfa"
 )
 
@@ -233,6 +235,55 @@ func (a *App) CheckUserMfa(rctx request.CTX, user *model.User, token string) *mo
 	return nil
 }
 
+func (a *App) MFARequired(rctx request.CTX) *model.AppError {
+	if license := a.Channels().License(); license == nil || !*license.Features.MFA || !*a.Config().ServiceSettings.EnableMultifactorAuthentication || !*a.Config().ServiceSettings.EnforceMultifactorAuthentication {
+		return nil
+	}
+
+	session := rctx.Session()
+	// Session cannot be nil or empty if MFA is to be enforced.
+	if session == nil || session.Id == "" {
+		return model.NewAppError("MfaRequired", "api.context.get_session.app_error", nil, "", http.StatusUnauthorized)
+	}
+
+	// OAuth integrations are excepted
+	if session.IsOAuth {
+		return nil
+	}
+
+	user, err := a.GetUser(session.UserId)
+	if err != nil {
+		return model.NewAppError("MfaRequired", "api.context.get_user.app_error", nil, "", http.StatusUnauthorized).Wrap(err)
+	}
+
+	if user.IsGuest() && !*a.Config().GuestAccountsSettings.EnforceMultifactorAuthentication {
+		return nil
+	}
+	// Only required for email and ldap accounts
+	if user.AuthService != "" &&
+		user.AuthService != model.UserAuthServiceEmail &&
+		user.AuthService != model.UserAuthServiceLdap {
+		return nil
+	}
+
+	// Special case to let user get themself
+	subpath, _ := utils.GetSubpathFromConfig(a.Config())
+	if rctx.Path() == path.Join(subpath, "/api/v4/users/me") {
+		return nil
+	}
+
+	// Bots are exempt
+	if user.IsBot {
+		return nil
+	}
+
+	if !user.MfaActive {
+		return model.NewAppError("MfaRequired", "api.context.mfa_required.app_error", nil, "", http.StatusForbidden)
+	}
+
+	return nil
+}
+
 func checkUserLoginAttempts(user *model.User, max int) *model.AppError {
 	if user.FailedAttempts >= max {
 		return model.NewAppError("checkUserLoginAttempts", "api.user.check_user_login_attempts.too_many.app_error", nil, "user_id="+user.Id, http.StatusUnauthorized)

server/channels/app/plugin_api_test.go

@@ -1896,6 +1896,91 @@ func TestPluginHTTPConnHijack(t *testing.T) {
 	require.Equal(t, "OK", string(body))
 }
 
+func TestPluginMFAEnforcement(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.App.Srv().SetLicense(model.NewTestLicense("mfa"))
+
+	pluginCode := `
+	package main
+
+	import (
+		"net/http"
+		"github.com/mattermost/mattermost/server/public/plugin"
+	)
+
+	type MyPlugin struct {
+		plugin.MattermostPlugin
+	}
+
+	func (p *MyPlugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) {
+		// Simply return the value of Mattermost-User-Id header
+		userID := r.Header.Get("Mattermost-User-Id")
+		w.Write([]byte(userID))
+	}
+
+	func main() {
+		plugin.ClientMain(&MyPlugin{})
+	}
+	`
+
+	// Create and setup plugin
+	tearDown, ids, errs := SetAppEnvironmentWithPlugins(t, []string{pluginCode}, th.App, th.NewPluginAPI)
+	defer tearDown()
+	require.NoError(t, errs[0])
+	require.Len(t, ids, 1)
+
+	pluginID := ids[0]
+
+	// Create user that requires MFA
+	user := th.CreateUser()
+
+	// Create session
+	session, appErr := th.App.CreateSession(th.Context, &model.Session{
+		UserId: user.Id,
+	})
+	require.Nil(t, appErr)
+
+	client := &http.Client{}
+	makeRequest := func() string {
+		reqURL := fmt.Sprintf("http://localhost:%d/plugins/%s", th.Server.ListenAddr.Port, pluginID)
+		req, err := http.NewRequest("GET", reqURL, nil)
+		require.NoError(t, err)
+		req.Header.Set(model.HeaderAuth, model.HeaderToken+" "+session.Token)
+
+		resp, err := client.Do(req)
+		require.NoError(t, err)
+		defer resp.Body.Close()
+
+		body, err := io.ReadAll(resp.Body)
+		require.NoError(t, err)
+		return string(body)
+	}
+
+	t.Run("MFA not enforced", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.EnableMultifactorAuthentication = true
+			*cfg.ServiceSettings.EnforceMultifactorAuthentication = false
+		})
+
+		// Should return user ID since MFA is not enforced
+		userID := makeRequest()
+		assert.Equal(t, user.Id, userID)
+	})
+
+	t.Run("MFA enforced", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.EnableMultifactorAuthentication = true
+			*cfg.ServiceSettings.EnforceMultifactorAuthentication = true
+		})
+
+		// Should return empty string since MFA is enforced but not active
+		userID := makeRequest()
+		assert.Empty(t, userID)
+	})
+}
+
 func TestPluginHTTPUpgradeWebSocket(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()

server/channels/app/plugin_requests.go

@@ -16,6 +16,7 @@ import (
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/mattermost/mattermost/server/public/plugin"
 	"github.com/mattermost/mattermost/server/public/shared/mlog"
+	"github.com/mattermost/mattermost/server/public/shared/request"
 	"github.com/mattermost/mattermost/server/v8/channels/utils"
 )
 
@@ -146,16 +147,33 @@ func (ch *Channels) servePluginRequest(w http.ResponseWriter, r *http.Request, h
 		token = r.URL.Query().Get("access_token")
 	}
 
+	// If MFA is required and user has not activated it, we wipe the token.
+	app := New(ServerConnector(ch))
+	rctx := request.EmptyContext(ch.srv.Log()).WithPath(r.URL.Path)
+
+	// The appErr is later used at L176 and L226.
+	session, appErr := app.GetSession(token)
+	if session != nil {
+		rctx = rctx.WithSession(session)
+	}
+
+	if mfaAppErr := app.MFARequired(rctx); mfaAppErr != nil {
+		pluginID := mux.Vars(r)["plugin_id"]
+		ch.srv.Log().Warn("Treating session as unauthenticated since MFA required",
+			mlog.String("plugin_id", pluginID),
+			mlog.String("url", r.URL.Path),
+			mlog.Err(mfaAppErr),
+		)
+		token = ""
+	}
+
 	// Mattermost-Plugin-ID can only be set by inter-plugin requests
 	r.Header.Del("Mattermost-Plugin-ID")
 
 	r.Header.Del("Mattermost-User-Id")
 	if token != "" {
-		session, appErr := New(ServerConnector(ch)).GetSession(token)
-
 		csrfCheckPassed := false
-
-		if session != nil && appErr == nil && cookieAuth && r.Method != "GET" {
+		if (session != nil && session.Id != "") && appErr == nil && cookieAuth && r.Method != "GET" {
 			sentToken := ""
 
 			if r.Header.Get(model.HeaderCsrfToken) == "" {

server/channels/web/context.go

@@ -5,7 +5,6 @@ package web
 
 import (
 	"net/http"
-	"path"
 	"regexp"
 	"strings"
 
@@ -158,46 +157,8 @@ func (c *Context) RemoteClusterTokenRequired() {
 }
 
 func (c *Context) MfaRequired() {
-	// Must be licensed for MFA and have it configured for enforcement
-	if license := c.App.Channels().License(); license == nil || !*license.Features.MFA || !*c.App.Config().ServiceSettings.EnableMultifactorAuthentication || !*c.App.Config().ServiceSettings.EnforceMultifactorAuthentication {
-		return
-	}
-
-	// OAuth integrations are excepted
-	if c.AppContext.Session().IsOAuth {
-		return
-	}
-
-	user, err := c.App.GetUser(c.AppContext.Session().UserId)
-	if err != nil {
-		c.Err = model.NewAppError("MfaRequired", "api.context.get_user.app_error", nil, "", http.StatusUnauthorized).Wrap(err)
-		return
-	}
-
-	if user.IsGuest() && !*c.App.Config().GuestAccountsSettings.EnforceMultifactorAuthentication {
-		return
-	}
-	// Only required for email and ldap accounts
-	if user.AuthService != "" &&
-		user.AuthService != model.UserAuthServiceEmail &&
-		user.AuthService != model.UserAuthServiceLdap {
-		return
-	}
-
-	// Special case to let user get themself
-	subpath, _ := utils.GetSubpathFromConfig(c.App.Config())
-	if c.AppContext.Path() == path.Join(subpath, "/api/v4/users/me") {
-		return
-	}
-
-	// Bots are exempt
-	if user.IsBot {
-		return
-	}
-
-	if !user.MfaActive {
-		c.Err = model.NewAppError("MfaRequired", "api.context.mfa_required.app_error", nil, "", http.StatusForbidden)
-		return
+	if appErr := c.App.MFARequired(c.AppContext); appErr != nil {
+		c.Err = appErr
 	}
 }
 

server/i18n/en.json

@@ -1705,6 +1705,10 @@
     "id": "api.context.404.app_error",
     "translation": "Sorry, we could not find the page."
   },
+  {
+    "id": "api.context.get_session.app_error",
+    "translation": "Session not found."
+  },
   {
     "id": "api.context.get_user.app_error",
     "translation": "Unable to get user from session UserID."