diff --git a/server/channels/app/authentication.go b/server/channels/app/authentication.go index 08115b0346..532beb8fd4 100644 --- a/server/channels/app/authentication.go +++ b/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) diff --git a/server/channels/app/plugin_api_test.go b/server/channels/app/plugin_api_test.go index 9a45e70e8d..8ed8034bf2 100644 --- a/server/channels/app/plugin_api_test.go +++ b/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() diff --git a/server/channels/app/plugin_requests.go b/server/channels/app/plugin_requests.go index 40726d07e0..c2354daab2 100644 --- a/server/channels/app/plugin_requests.go +++ b/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) == "" { diff --git a/server/channels/web/context.go b/server/channels/web/context.go index 56e88b3da5..1437b76ab0 100644 --- a/server/channels/web/context.go +++ b/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 } } diff --git a/server/i18n/en.json b/server/i18n/en.json index bf574dcf44..bbed00cc98 100644 --- a/server/i18n/en.json +++ b/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."