From e05edf85cfc0c16f3232a53c106f613ab366f11a Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Wed, 4 Oct 2017 11:04:56 -0400 Subject: PLT-7781 Some more OAuth fixes (#7568) * Some other oauth fixes * Fix unit test --- api4/user.go | 46 ++++++++++++++++++++++++ api4/user_test.go | 106 ++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 129 insertions(+), 23 deletions(-) diff --git a/api4/user.go b/api4/user.go index e46ded670..ae1b2418c 100644 --- a/api4/user.go +++ b/api4/user.go @@ -537,6 +537,20 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) { return } + if c.Session.IsOAuth { + ouser, err := c.App.GetUser(user.Id) + if err != nil { + c.Err = err + return + } + + if ouser.Email != user.Email { + c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) + c.Err.DetailedError += ", attempted email update by oauth app" + return + } + } + if ruser, err := c.App.UpdateUserAsUser(user, c.IsSystemAdmin()); err != nil { c.Err = err return @@ -563,6 +577,20 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) { return } + if c.Session.IsOAuth && patch.Email != nil { + ouser, err := c.App.GetUser(c.Params.UserId) + if err != nil { + c.Err = err + return + } + + if ouser.Email != *patch.Email { + c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) + c.Err.DetailedError += ", attempted email update by oauth app" + return + } + } + if ruser, err := c.App.PatchUser(c.Params.UserId, patch, c.IsSystemAdmin()); err != nil { c.Err = err return @@ -690,6 +718,12 @@ func updateUserMfa(c *Context, w http.ResponseWriter, r *http.Request) { return } + if c.Session.IsOAuth { + c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) + c.Err.DetailedError += ", attempted access by oauth app" + return + } + if !app.SessionHasPermissionToUser(c.Session, c.Params.UserId) { c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) return @@ -729,6 +763,12 @@ func generateMfaSecret(c *Context, w http.ResponseWriter, r *http.Request) { return } + if c.Session.IsOAuth { + c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) + c.Err.DetailedError += ", attempted access by oauth app" + return + } + if !app.SessionHasPermissionToUser(c.Session, c.Params.UserId) { c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) return @@ -1102,6 +1142,12 @@ func createUserAccessToken(c *Context, w http.ResponseWriter, r *http.Request) { return } + if c.Session.IsOAuth { + c.SetPermissionError(model.PERMISSION_CREATE_USER_ACCESS_TOKEN) + c.Err.DetailedError += ", attempted access by oauth app" + return + } + accessToken := model.UserAccessTokenFromJson(r.Body) if accessToken == nil { c.SetInvalidParam("user_access_token") diff --git a/api4/user_test.go b/api4/user_test.go index 12a323137..0913819cc 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" "github.com/stretchr/testify/assert" @@ -998,6 +999,15 @@ func TestUpdateUser(t *testing.T) { } } + session, _ := th.App.GetSession(Client.AuthToken) + session.IsOAuth = true + app.AddSessionToCache(session) + + ruser.Id = user.Id + ruser.Email = GenerateTestEmail() + _, resp = Client.UpdateUser(ruser) + CheckForbiddenStatus(t, resp) + Client.Logout() _, resp = Client.UpdateUser(user) CheckUnauthorizedStatus(t, resp) @@ -1077,6 +1087,15 @@ func TestPatchUser(t *testing.T) { } } + session, _ := th.App.GetSession(Client.AuthToken) + session.IsOAuth = true + app.AddSessionToCache(session) + + patch.Email = new(string) + *patch.Email = GenerateTestEmail() + _, resp = Client.PatchUser(user.Id, patch) + CheckForbiddenStatus(t, resp) + Client.Logout() _, resp = Client.PatchUser(user.Id, patch) CheckUnauthorizedStatus(t, resp) @@ -1518,7 +1537,7 @@ func TestGetUsersNotInChannel(t *testing.T) { CheckNoError(t, resp) } -/*func TestUpdateUserMfa(t *testing.T) { +func TestUpdateUserMfa(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() defer th.TearDown() Client := th.Client @@ -1531,34 +1550,45 @@ func TestGetUsersNotInChannel(t *testing.T) { utils.SetLicense(license) *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = enableMfa }() - utils.IsLicensed()= true + utils.SetIsLicensed(true) utils.SetLicense(&model.License{Features: &model.Features{}}) utils.License().Features.SetDefaults() + *utils.License().Features.MFA = true + *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = true - team := model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN} - rteam, _ := Client.CreateTeam(&team) + session, _ := th.App.GetSession(Client.AuthToken) + session.IsOAuth = true + app.AddSessionToCache(session) - user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", Password: "passwd1"} - ruser, _ := Client.CreateUser(&user) - th.LinkUserToTeam(ruser, rteam) - store.Must(app.Srv.Store.User().VerifyEmail(ruser.Id)) + _, resp := Client.UpdateUserMfa(th.BasicUser.Id, "12345", false) + CheckForbiddenStatus(t, resp) - Client.Logout() - _, resp := Client.UpdateUserMfa(ruser.Id, "12334", true) - CheckUnauthorizedStatus(t, resp) + /* + team := model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN} + rteam, _ := Client.CreateTeam(&team) - Client.Login(user.Email, user.Password) - _, resp = Client.UpdateUserMfa("fail", "56789", false) - CheckBadRequestStatus(t, resp) + user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", Password: "passwd1"} + ruser, _ := Client.CreateUser(&user) + th.LinkUserToTeam(ruser, rteam) + store.Must(app.Srv.Store.User().VerifyEmail(ruser.Id)) - _, resp = Client.UpdateUserMfa(ruser.Id, "", true) - CheckErrorMessage(t, resp, "api.context.invalid_body_param.app_error") + Client.Logout() + _, resp := Client.UpdateUserMfa(ruser.Id, "12334", true) + CheckUnauthorizedStatus(t, resp) - *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = true + Client.Login(user.Email, user.Password) + _, resp = Client.UpdateUserMfa("fail", "56789", false) + CheckBadRequestStatus(t, resp) - _, resp = Client.UpdateUserMfa(ruser.Id, "123456", false) - CheckNotImplementedStatus(t, resp) -}*/ + _, resp = Client.UpdateUserMfa(ruser.Id, "", true) + CheckErrorMessage(t, resp, "api.context.invalid_body_param.app_error") + + *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = true + + _, resp = Client.UpdateUserMfa(ruser.Id, "123456", false) + CheckNotImplementedStatus(t, resp) + */ +} func TestCheckUserMfa(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() @@ -1625,19 +1655,40 @@ func TestGenerateMfaSecret(t *testing.T) { _, resp := Client.GenerateMfaSecret(th.BasicUser.Id) CheckNotImplementedStatus(t, resp) + _, resp = th.SystemAdminClient.GenerateMfaSecret(th.BasicUser.Id) + CheckNotImplementedStatus(t, resp) + _, resp = Client.GenerateMfaSecret("junk") CheckBadRequestStatus(t, resp) + isLicensed := utils.IsLicensed() + license := utils.License() + enableMfa := *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication + defer func() { + utils.SetIsLicensed(isLicensed) + utils.SetLicense(license) + *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = enableMfa + }() + utils.SetIsLicensed(true) + utils.SetLicense(&model.License{Features: &model.Features{}}) + utils.License().Features.SetDefaults() + *utils.License().Features.MFA = true + *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = true + _, resp = Client.GenerateMfaSecret(model.NewId()) CheckForbiddenStatus(t, resp) + session, _ := th.App.GetSession(Client.AuthToken) + session.IsOAuth = true + app.AddSessionToCache(session) + + _, resp = Client.GenerateMfaSecret(th.BasicUser.Id) + CheckForbiddenStatus(t, resp) + Client.Logout() _, resp = Client.GenerateMfaSecret(th.BasicUser.Id) CheckUnauthorizedStatus(t, resp) - - _, resp = th.SystemAdminClient.GenerateMfaSecret(th.BasicUser.Id) - CheckNotImplementedStatus(t, resp) } func TestUpdateUserPassword(t *testing.T) { @@ -2237,6 +2288,15 @@ func TestCreateUserAccessToken(t *testing.T) { if ruser.Id != th.BasicUser.Id { t.Fatal("returned wrong user") } + + Client.AuthToken = oldSessionToken + + session, _ := th.App.GetSession(Client.AuthToken) + session.IsOAuth = true + app.AddSessionToCache(session) + + _, resp = Client.CreateUserAccessToken(th.BasicUser.Id, testDescription) + CheckForbiddenStatus(t, resp) } func TestGetUserAccessToken(t *testing.T) { -- cgit v1.2.3-1-g7c22