PLT-7781 Some more OAuth fixes (#7568)
* Some other oauth fixes * Fix unit test
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
affd35071e
Коммит
e05edf85cf
46
api4/user.go
46
api4/user.go
@@ -537,6 +537,20 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
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 {
|
if ruser, err := c.App.UpdateUserAsUser(user, c.IsSystemAdmin()); err != nil {
|
||||||
c.Err = err
|
c.Err = err
|
||||||
return
|
return
|
||||||
@@ -563,6 +577,20 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
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 {
|
if ruser, err := c.App.PatchUser(c.Params.UserId, patch, c.IsSystemAdmin()); err != nil {
|
||||||
c.Err = err
|
c.Err = err
|
||||||
return
|
return
|
||||||
@@ -690,6 +718,12 @@ func updateUserMfa(c *Context, w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
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) {
|
if !app.SessionHasPermissionToUser(c.Session, c.Params.UserId) {
|
||||||
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
|
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
|
||||||
return
|
return
|
||||||
@@ -729,6 +763,12 @@ func generateMfaSecret(c *Context, w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
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) {
|
if !app.SessionHasPermissionToUser(c.Session, c.Params.UserId) {
|
||||||
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
|
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
|
||||||
return
|
return
|
||||||
@@ -1102,6 +1142,12 @@ func createUserAccessToken(c *Context, w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
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)
|
accessToken := model.UserAccessTokenFromJson(r.Body)
|
||||||
if accessToken == nil {
|
if accessToken == nil {
|
||||||
c.SetInvalidParam("user_access_token")
|
c.SetInvalidParam("user_access_token")
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/mattermost/mattermost-server/app"
|
||||||
"github.com/mattermost/mattermost-server/model"
|
"github.com/mattermost/mattermost-server/model"
|
||||||
"github.com/mattermost/mattermost-server/utils"
|
"github.com/mattermost/mattermost-server/utils"
|
||||||
"github.com/stretchr/testify/assert"
|
"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()
|
Client.Logout()
|
||||||
_, resp = Client.UpdateUser(user)
|
_, resp = Client.UpdateUser(user)
|
||||||
CheckUnauthorizedStatus(t, resp)
|
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()
|
Client.Logout()
|
||||||
_, resp = Client.PatchUser(user.Id, patch)
|
_, resp = Client.PatchUser(user.Id, patch)
|
||||||
CheckUnauthorizedStatus(t, resp)
|
CheckUnauthorizedStatus(t, resp)
|
||||||
@@ -1518,7 +1537,7 @@ func TestGetUsersNotInChannel(t *testing.T) {
|
|||||||
CheckNoError(t, resp)
|
CheckNoError(t, resp)
|
||||||
}
|
}
|
||||||
|
|
||||||
/*func TestUpdateUserMfa(t *testing.T) {
|
func TestUpdateUserMfa(t *testing.T) {
|
||||||
th := Setup().InitBasic().InitSystemAdmin()
|
th := Setup().InitBasic().InitSystemAdmin()
|
||||||
defer th.TearDown()
|
defer th.TearDown()
|
||||||
Client := th.Client
|
Client := th.Client
|
||||||
@@ -1531,34 +1550,45 @@ func TestGetUsersNotInChannel(t *testing.T) {
|
|||||||
utils.SetLicense(license)
|
utils.SetLicense(license)
|
||||||
*utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = enableMfa
|
*utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = enableMfa
|
||||||
}()
|
}()
|
||||||
utils.IsLicensed()= true
|
utils.SetIsLicensed(true)
|
||||||
utils.SetLicense(&model.License{Features: &model.Features{}})
|
utils.SetLicense(&model.License{Features: &model.Features{}})
|
||||||
utils.License().Features.SetDefaults()
|
utils.License().Features.SetDefaults()
|
||||||
|
*utils.License().Features.MFA = true
|
||||||
team := model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN}
|
|
||||||
rteam, _ := Client.CreateTeam(&team)
|
|
||||||
|
|
||||||
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))
|
|
||||||
|
|
||||||
Client.Logout()
|
|
||||||
_, resp := Client.UpdateUserMfa(ruser.Id, "12334", true)
|
|
||||||
CheckUnauthorizedStatus(t, resp)
|
|
||||||
|
|
||||||
Client.Login(user.Email, user.Password)
|
|
||||||
_, resp = Client.UpdateUserMfa("fail", "56789", false)
|
|
||||||
CheckBadRequestStatus(t, resp)
|
|
||||||
|
|
||||||
_, resp = Client.UpdateUserMfa(ruser.Id, "", true)
|
|
||||||
CheckErrorMessage(t, resp, "api.context.invalid_body_param.app_error")
|
|
||||||
|
|
||||||
*utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = true
|
*utils.Cfg.ServiceSettings.EnableMultifactorAuthentication = true
|
||||||
|
|
||||||
_, resp = Client.UpdateUserMfa(ruser.Id, "123456", false)
|
session, _ := th.App.GetSession(Client.AuthToken)
|
||||||
CheckNotImplementedStatus(t, resp)
|
session.IsOAuth = true
|
||||||
}*/
|
app.AddSessionToCache(session)
|
||||||
|
|
||||||
|
_, resp := Client.UpdateUserMfa(th.BasicUser.Id, "12345", false)
|
||||||
|
CheckForbiddenStatus(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)
|
||||||
|
|
||||||
|
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))
|
||||||
|
|
||||||
|
Client.Logout()
|
||||||
|
_, resp := Client.UpdateUserMfa(ruser.Id, "12334", true)
|
||||||
|
CheckUnauthorizedStatus(t, resp)
|
||||||
|
|
||||||
|
Client.Login(user.Email, user.Password)
|
||||||
|
_, resp = Client.UpdateUserMfa("fail", "56789", false)
|
||||||
|
CheckBadRequestStatus(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) {
|
func TestCheckUserMfa(t *testing.T) {
|
||||||
th := Setup().InitBasic().InitSystemAdmin()
|
th := Setup().InitBasic().InitSystemAdmin()
|
||||||
@@ -1625,19 +1655,40 @@ func TestGenerateMfaSecret(t *testing.T) {
|
|||||||
_, resp := Client.GenerateMfaSecret(th.BasicUser.Id)
|
_, resp := Client.GenerateMfaSecret(th.BasicUser.Id)
|
||||||
CheckNotImplementedStatus(t, resp)
|
CheckNotImplementedStatus(t, resp)
|
||||||
|
|
||||||
|
_, resp = th.SystemAdminClient.GenerateMfaSecret(th.BasicUser.Id)
|
||||||
|
CheckNotImplementedStatus(t, resp)
|
||||||
|
|
||||||
_, resp = Client.GenerateMfaSecret("junk")
|
_, resp = Client.GenerateMfaSecret("junk")
|
||||||
CheckBadRequestStatus(t, resp)
|
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())
|
_, resp = Client.GenerateMfaSecret(model.NewId())
|
||||||
CheckForbiddenStatus(t, resp)
|
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()
|
Client.Logout()
|
||||||
|
|
||||||
_, resp = Client.GenerateMfaSecret(th.BasicUser.Id)
|
_, resp = Client.GenerateMfaSecret(th.BasicUser.Id)
|
||||||
CheckUnauthorizedStatus(t, resp)
|
CheckUnauthorizedStatus(t, resp)
|
||||||
|
|
||||||
_, resp = th.SystemAdminClient.GenerateMfaSecret(th.BasicUser.Id)
|
|
||||||
CheckNotImplementedStatus(t, resp)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestUpdateUserPassword(t *testing.T) {
|
func TestUpdateUserPassword(t *testing.T) {
|
||||||
@@ -2237,6 +2288,15 @@ func TestCreateUserAccessToken(t *testing.T) {
|
|||||||
if ruser.Id != th.BasicUser.Id {
|
if ruser.Id != th.BasicUser.Id {
|
||||||
t.Fatal("returned wrong user")
|
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) {
|
func TestGetUserAccessToken(t *testing.T) {
|
||||||
|
|||||||
Ссылка в новой задаче
Block a user