diff --git a/i18n/en.json b/i18n/en.json index 2c82d22fe0..ccd7c81332 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1130,6 +1130,10 @@ "id": "api.context.404.app_error", "translation": "Sorry, we could not find the page." }, + { + "id": "api.context.get_user.app_error", + "translation": "Unable to get user from session UserID." + }, { "id": "api.context.invalid_body_param.app_error", "translation": "Invalid or missing {{.Name}} in request body." diff --git a/web/context.go b/web/context.go index 9c25854d3c..32e1eed644 100644 --- a/web/context.go +++ b/web/context.go @@ -154,35 +154,36 @@ func (c *Context) MfaRequired() { return } - if user, err := c.App.GetUser(c.App.Session().UserId); err != nil { - c.Err = model.NewAppError("", "api.context.session_expired.app_error", nil, "MfaRequired", http.StatusUnauthorized) + user, err := c.App.GetUser(c.App.Session().UserId) + if err != nil { + c.Err = model.NewAppError("MfaRequired", "api.context.get_user.app_error", nil, err.Error(), http.StatusUnauthorized) return - } else { - if user.IsGuest() && !*c.App.Config().GuestAccountsSettings.EnforceMultifactorAuthentication { - return - } - // Only required for email and ldap accounts - if user.AuthService != "" && - user.AuthService != model.USER_AUTH_SERVICE_EMAIL && - user.AuthService != model.USER_AUTH_SERVICE_LDAP { - return - } + } - // Special case to let user get themself - subpath, _ := utils.GetSubpathFromConfig(c.App.Config()) - if c.App.Path() == path.Join(subpath, "/api/v4/users/me") { - return - } + if user.IsGuest() && !*c.App.Config().GuestAccountsSettings.EnforceMultifactorAuthentication { + return + } + // Only required for email and ldap accounts + if user.AuthService != "" && + user.AuthService != model.USER_AUTH_SERVICE_EMAIL && + user.AuthService != model.USER_AUTH_SERVICE_LDAP { + return + } - // Bots are exempt - if user.IsBot { - return - } + // Special case to let user get themself + subpath, _ := utils.GetSubpathFromConfig(c.App.Config()) + if c.App.Path() == path.Join(subpath, "/api/v4/users/me") { + return + } - if !user.MfaActive { - c.Err = model.NewAppError("", "api.context.mfa_required.app_error", nil, "MfaRequired", http.StatusForbidden) - 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 } } diff --git a/web/context_test.go b/web/context_test.go index f6278af242..c196f34ae2 100644 --- a/web/context_test.go +++ b/web/context_test.go @@ -7,6 +7,10 @@ import ( "net/http" "testing" + "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/plugin/plugintest/mock" + "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -27,3 +31,38 @@ func TestRequireHookId(t *testing.T) { require.Equal(t, http.StatusBadRequest, c.Err.StatusCode, "Should have set status as 400") }) } + +func TestMfaRequired(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() + + mockStore := th.App.Srv().Store.(*mocks.Store) + mockUserStore := mocks.UserStore{} + mockUserStore.On("Count", mock.Anything).Return(int64(10), nil) + mockUserStore.On("Get", "userid").Return(nil, model.NewAppError("Userstore.Get", "storeerror", nil, "store error", http.StatusInternalServerError)) + mockPostStore := mocks.PostStore{} + mockPostStore.On("GetMaxPostSize").Return(65535, nil) + mockSystemStore := mocks.SystemStore{} + mockSystemStore.On("GetByName", "InstallationDate").Return(&model.System{Name: "InstallationDate", Value: "10"}, nil) + + mockStore.On("User").Return(&mockUserStore) + mockStore.On("Post").Return(&mockPostStore) + mockStore.On("System").Return(&mockSystemStore) + + th.App.Srv().SetLicense(model.NewTestLicense("mfa")) + + th.App.SetSession(&model.Session{Id: "abc", UserId: "userid"}) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableMultifactorAuthentication = true + *cfg.ServiceSettings.EnforceMultifactorAuthentication = true + }) + + c := &Context{ + App: th.App, + } + + c.MfaRequired() + + assert.Equal(t, c.Err.Id, "api.context.get_user.app_error") +}