From c82a84ed765bd9c4d601b93201d93af92f6ee742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 2 Oct 2018 08:04:38 +0200 Subject: MM-12067: Add SetDefaultProfileImage to reset the user profile image to a generated one (#9449) * MM-12067: Add SetDefaultProfileImage to reset the user profile image to a generated one * Allow to get the default profile image for my user * Allowing to reset the last update image date to 0 * PR reviews --- api4/user.go | 62 +++++++++++++++++++++++++++++++ api4/user_test.go | 44 ++++++++++++++++++++++ app/user.go | 76 ++++++++++++++++++++++++++++---------- app/user_test.go | 34 +++++++++++++---- model/client4.go | 27 +++++++++++++- store/sqlstore/user_store.go | 10 +++++ store/store.go | 1 + store/storetest/mocks/UserStore.go | 16 ++++++++ 8 files changed, 242 insertions(+), 28 deletions(-) diff --git a/api4/user.go b/api4/user.go index 2570a6f25..5a8474b8d 100644 --- a/api4/user.go +++ b/api4/user.go @@ -27,8 +27,10 @@ func (api *API) InitUser() { api.BaseRoutes.Users.Handle("/stats", api.ApiSessionRequired(getTotalUsersStats)).Methods("GET") api.BaseRoutes.User.Handle("", api.ApiSessionRequired(getUser)).Methods("GET") + api.BaseRoutes.User.Handle("/image/default", api.ApiSessionRequiredTrustRequester(getDefaultProfileImage)).Methods("GET") api.BaseRoutes.User.Handle("/image", api.ApiSessionRequiredTrustRequester(getProfileImage)).Methods("GET") api.BaseRoutes.User.Handle("/image", api.ApiSessionRequired(setProfileImage)).Methods("POST") + api.BaseRoutes.User.Handle("/image", api.ApiSessionRequired(setDefaultProfileImage)).Methods("DELETE") api.BaseRoutes.User.Handle("", api.ApiSessionRequired(updateUser)).Methods("PUT") api.BaseRoutes.User.Handle("/patch", api.ApiSessionRequired(patchUser)).Methods("PUT") api.BaseRoutes.User.Handle("", api.ApiSessionRequired(deleteUser)).Methods("DELETE") @@ -193,6 +195,35 @@ func getUserByEmail(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(user.ToJson())) } +func getDefaultProfileImage(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireUserId() + if c.Err != nil { + return + } + + users, err := c.App.GetUsersByIds([]string{c.Params.UserId}, c.IsSystemAdmin()) + if err != nil { + c.Err = err + return + } + + if len(users) == 0 { + c.Err = model.NewAppError("getProfileImage", "api.user.get_profile_image.not_found.app_error", nil, "", http.StatusNotFound) + return + } + + user := users[0] + img, err := c.App.GetDefaultProfileImage(user) + if err != nil { + c.Err = err + return + } + + w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%v, public", 24*60*60)) // 24 hrs + w.Header().Set("Content-Type", "image/png") + w.Write(img) +} + func getProfileImage(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireUserId() if c.Err != nil { @@ -286,6 +317,37 @@ func setProfileImage(c *Context, w http.ResponseWriter, r *http.Request) { ReturnStatusOK(w) } +func setDefaultProfileImage(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireUserId() + if c.Err != nil { + return + } + + if !c.App.SessionHasPermissionToUser(c.Session, c.Params.UserId) { + c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) + return + } + + if len(*c.App.Config().FileSettings.DriverName) == 0 { + c.Err = model.NewAppError("setDefaultProfileImage", "api.user.upload_profile_user.storage.app_error", nil, "", http.StatusNotImplemented) + return + } + + user, err := c.App.GetUser(c.Params.UserId) + if err != nil { + c.Err = err + return + } + + if err := c.App.SetDefaultProfileImage(user); err != nil { + c.Err = err + return + } + + c.LogAudit("") + ReturnStatusOK(w) +} + func getTotalUsersStats(c *Context, w http.ResponseWriter, r *http.Request) { if c.Err != nil { return diff --git a/api4/user_test.go b/api4/user_test.go index a9aa967be..d50dfa3b6 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -2309,6 +2309,50 @@ func TestSetProfileImage(t *testing.T) { t.Fatal(err) } } + +func TestSetDefaultProfileImage(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + Client := th.Client + user := th.BasicUser + + ok, resp := Client.SetDefaultProfileImage(user.Id) + if !ok { + t.Fatal(resp.Error) + } + CheckNoError(t, resp) + + ok, resp = Client.SetDefaultProfileImage(model.NewId()) + if ok { + t.Fatal("Should return false, set profile image not allowed") + } + CheckForbiddenStatus(t, resp) + + // status code returns either forbidden or unauthorized + // note: forbidden is set as default at Client4.SetDefaultProfileImage when request is terminated early by server + Client.Logout() + _, resp = Client.SetDefaultProfileImage(user.Id) + if resp.StatusCode == http.StatusForbidden { + CheckForbiddenStatus(t, resp) + } else if resp.StatusCode == http.StatusUnauthorized { + CheckUnauthorizedStatus(t, resp) + } else { + t.Fatal("Should have failed either forbidden or unauthorized") + } + + _, resp = th.SystemAdminClient.SetDefaultProfileImage(user.Id) + CheckNoError(t, resp) + + ruser, err := th.App.GetUser(user.Id) + require.Nil(t, err) + assert.Equal(t, int64(0), ruser.LastPictureUpdate, "Picture should have resetted to default") + + info := &model.FileInfo{Path: "users/" + user.Id + "/profile.png"} + if err := th.cleanupTestFile(info); err != nil { + t.Fatal(err) + } +} + func TestCBALogin(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() diff --git a/app/user.go b/app/user.go index 86f44db4e..1faf2b895 100644 --- a/app/user.go +++ b/app/user.go @@ -745,36 +745,72 @@ func getFont(initialFont string) (*truetype.Font, error) { } func (a *App) GetProfileImage(user *model.User) ([]byte, bool, *model.AppError) { - var img []byte - readFailed := false - if len(*a.Config().FileSettings.DriverName) == 0 { - var err *model.AppError - if img, err = CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont); err != nil { - return nil, false, err + img, appErr := CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont) + if appErr != nil { + return nil, false, appErr } - } else { - path := "users/" + user.Id + "/profile.png" + return img, false, nil + } - if data, err := a.ReadFile(path); err != nil { - readFailed = true + path := "users/" + user.Id + "/profile.png" - if img, err = CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont); err != nil { + data, err := a.ReadFile(path) + if err != nil { + img, appErr := CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont) + if appErr != nil { + return nil, false, appErr + } + + if user.LastPictureUpdate == 0 { + if _, err := a.WriteFile(bytes.NewReader(img), path); err != nil { return nil, false, err } + } + return img, true, nil + } - if user.LastPictureUpdate == 0 { - if _, err := a.WriteFile(bytes.NewReader(img), path); err != nil { - return nil, false, err - } - } + return data, false, nil +} - } else { - img = data - } +func (a *App) GetDefaultProfileImage(user *model.User) ([]byte, *model.AppError) { + img, appErr := CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont) + if appErr != nil { + return nil, appErr } + return img, nil +} - return img, readFailed, nil +func (a *App) SetDefaultProfileImage(user *model.User) *model.AppError { + img, appErr := CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont) + if appErr != nil { + return appErr + } + + path := "users/" + user.Id + "/profile.png" + + if _, err := a.WriteFile(bytes.NewReader(img), path); err != nil { + return err + } + + <-a.Srv.Store.User().ResetLastPictureUpdate(user.Id) + + a.InvalidateCacheForUser(user.Id) + + updatedUser, appErr := a.GetUser(user.Id) + if appErr != nil { + mlog.Error(fmt.Sprintf("Error in getting users profile for id=%v forcing logout", user.Id), mlog.String("user_id", user.Id)) + return nil + } + + options := a.Config().GetSanitizeOptions() + updatedUser.SanitizeProfile(options) + + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_UPDATED, "", "", "", nil) + message.Add("user", updatedUser) + a.Publish(message) + + return nil } func (a *App) SetProfileImage(userId string, imageData *multipart.FileHeader) *model.AppError { diff --git a/app/user_test.go b/app/user_test.go index a007f93d5..465d2a994 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/model" @@ -115,6 +116,25 @@ func TestCreateProfileImage(t *testing.T) { } } +func TestSetDefaultProfileImage(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + err := th.App.SetDefaultProfileImage(&model.User{ + Id: model.NewId(), + Username: "notvaliduser", + }) + require.Error(t, err) + + user := th.BasicUser + + err = th.App.SetDefaultProfileImage(user) + require.Nil(t, err) + + user = getUserFromDB(th.App, user.Id, t) + assert.Equal(t, int64(0), user.LastPictureUpdate) +} + func TestUpdateUserToRestrictedDomain(t *testing.T) { th := Setup() defer th.TearDown() @@ -249,12 +269,12 @@ func TestUpdateOAuthUserAttrs(t *testing.T) { } func getUserFromDB(a *App, id string, t *testing.T) *model.User { - if user, err := a.GetUser(id); err != nil { + user, err := a.GetUser(id) + if err != nil { t.Fatal("user is not found", err) return nil - } else { - return user } + return user } func getGitlabUserPayload(gitlabUser oauthgitlab.GitLabUser, t *testing.T) []byte { @@ -530,10 +550,10 @@ func TestRecordUserServiceTermsAction(t *testing.T) { defer th.TearDown() user := &model.User{ - Email: strings.ToLower(model.NewId()) + "success+test@example.com", - Nickname: "Luke Skywalker", // trying to bring balance to the "Force", one test user at a time - Username: "luke" + model.NewId(), - Password: "passwd1", + Email: strings.ToLower(model.NewId()) + "success+test@example.com", + Nickname: "Luke Skywalker", // trying to bring balance to the "Force", one test user at a time + Username: "luke" + model.NewId(), + Password: "passwd1", AuthService: "", } user, err := th.App.CreateUser(user) diff --git a/model/client4.go b/model/client4.go index 117f6c570..964218a68 100644 --- a/model/client4.go +++ b/model/client4.go @@ -723,7 +723,23 @@ func (c *Client4) AutocompleteUsers(username string, etag string) (*UserAutocomp } } -// GetProfileImage gets user's profile image. Must be logged in or be a system administrator. +// GetDefaultProfileImage gets the default user's profile image. Must be logged in. +func (c *Client4) GetDefaultProfileImage(userId string) ([]byte, *Response) { + r, appErr := c.DoApiGet(c.GetUserRoute(userId)+"/image/default", "") + if appErr != nil { + return nil, BuildErrorResponse(r, appErr) + } + defer closeBody(r) + + data, err := ioutil.ReadAll(r.Body) + if err != nil { + return nil, BuildErrorResponse(r, NewAppError("GetDefaultProfileImage", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } + + return data, BuildResponse(r) +} + +// GetProfileImage gets user's profile image. Must be logged in. func (c *Client4) GetProfileImage(userId, etag string) ([]byte, *Response) { if r, err := c.DoApiGet(c.GetUserRoute(userId)+"/image", etag); err != nil { return nil, BuildErrorResponse(r, err) @@ -1105,6 +1121,15 @@ func (c *Client4) SendVerificationEmail(email string) (bool, *Response) { } } +// SetDefaultProfileImage resets the profile image to a default generated one +func (c *Client4) SetDefaultProfileImage(userId string) (bool, *Response) { + r, err := c.DoApiDelete(c.GetUserRoute(userId) + "/image") + if err != nil { + return false, BuildErrorResponse(r, err) + } + return CheckStatusOK(r), BuildResponse(r) +} + // SetProfileImage sets profile image of the user func (c *Client4) SetProfileImage(userId string, data []byte) (bool, *Response) { body := &bytes.Buffer{} diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 941aa60e9..1b9752064 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -212,6 +212,16 @@ func (us SqlUserStore) UpdateLastPictureUpdate(userId string) store.StoreChannel }) } +func (us SqlUserStore) ResetLastPictureUpdate(userId string) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + if _, err := us.GetMaster().Exec("UPDATE Users SET LastPictureUpdate = :Time, UpdateAt = :Time WHERE Id = :UserId", map[string]interface{}{"Time": 0, "UserId": userId}); err != nil { + result.Err = model.NewAppError("SqlUserStore.UpdateUpdateAt", "store.sql_user.update_last_picture_update.app_error", nil, "user_id="+userId, http.StatusInternalServerError) + } else { + result.Data = userId + } + }) +} + func (us SqlUserStore) UpdateUpdateAt(userId string) store.StoreChannel { return store.Do(func(result *store.StoreResult) { curTime := model.GetMillis() diff --git a/store/store.go b/store/store.go index f375bfb0f..02bbb11ca 100644 --- a/store/store.go +++ b/store/store.go @@ -230,6 +230,7 @@ type UserStore interface { Save(user *model.User) StoreChannel Update(user *model.User, allowRoleUpdate bool) StoreChannel UpdateLastPictureUpdate(userId string) StoreChannel + ResetLastPictureUpdate(userId string) StoreChannel UpdateUpdateAt(userId string) StoreChannel UpdatePassword(userId, newPassword string) StoreChannel UpdateAuthData(userId string, service string, authData *string, email string, resetMfa bool) StoreChannel diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index b505a2f0b..1e0ce8818 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -593,6 +593,22 @@ func (_m *UserStore) PermanentDelete(userId string) store.StoreChannel { return r0 } +// ResetLastPictureUpdate provides a mock function with given fields: userId +func (_m *UserStore) ResetLastPictureUpdate(userId string) store.StoreChannel { + ret := _m.Called(userId) + + var r0 store.StoreChannel + if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + r0 = rf(userId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.StoreChannel) + } + } + + return r0 +} + // Save provides a mock function with given fields: user func (_m *UserStore) Save(user *model.User) store.StoreChannel { ret := _m.Called(user) -- cgit v1.2.3-1-g7c22