diff --git a/api4/team_test.go b/api4/team_test.go index 079c623dba..5048124047 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -1826,8 +1826,8 @@ func TestAddTeamMember(t *testing.T) { require.Equal(t, tm.TeamId, team.Id, "team ids should have matched") - _, err = th.App.Srv().Store.Token().GetByToken(token.Token) - require.NotNil(t, err, "The token must be deleted after be used") + _, nErr := th.App.Srv().Store.Token().GetByToken(token.Token) + require.NotNil(t, nErr, "The token must be deleted after be used") tm, resp = Client.AddTeamMemberFromInvite("junk", "") CheckBadRequestStatus(t, resp) diff --git a/api4/user.go b/api4/user.go index f2fc5b75dd..761e8e0e77 100644 --- a/api4/user.go +++ b/api4/user.go @@ -107,10 +107,16 @@ func createUser(c *Context, w http.ResponseWriter, r *http.Request) { var ruser *model.User var err *model.AppError if len(tokenId) > 0 { - var token *model.Token - token, err = c.App.Srv().Store.Token().GetByToken(tokenId) - if err != nil { - c.Err = model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_invalid.app_error", nil, err.Error(), http.StatusBadRequest) + token, nErr := c.App.Srv().Store.Token().GetByToken(tokenId) + if nErr != nil { + var status int + switch nErr.(type) { + case *store.ErrNotFound: + status = http.StatusNotFound + default: + status = http.StatusInternalServerError + } + c.Err = model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_invalid.app_error", nil, nErr.Error(), status) return } auditRec.AddMeta("token_type", token.Type) diff --git a/api4/user_test.go b/api4/user_test.go index 7fabfd8613..84262bc581 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -256,7 +256,7 @@ func TestCreateUserWithToken(t *testing.T) { user := model.User{Email: th.GenerateTestEmail(), Nickname: "Corey Hulen", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID} _, resp := th.Client.CreateUserWithToken(&user, "wrong") - CheckBadRequestStatus(t, resp) + CheckNotFoundStatus(t, resp) CheckErrorMessage(t, resp, "api.user.create_user.signup_link_invalid.app_error") }) diff --git a/app/email.go b/app/email.go index 1f15b7c2e5..327a8f453f 100644 --- a/app/email.go +++ b/app/email.go @@ -571,8 +571,14 @@ func (es *EmailService) CreateVerifyEmailToken(userId string, newEmail string) ( token := model.NewToken(TOKEN_TYPE_VERIFY_EMAIL, string(jsonData)) - if err := es.srv.Store.Token().Save(token); err != nil { - return nil, err + if err = es.srv.Store.Token().Save(token); err != nil { + var appErr *model.AppError + switch { + case errors.As(err, &appErr): + return nil, appErr + default: + return nil, model.NewAppError("CreateVerifyEmailToken", "app.recover.save.app_error", nil, err.Error(), http.StatusInternalServerError) + } } return token, nil diff --git a/app/oauth.go b/app/oauth.go index c1957e76f2..8ae7ab7e46 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -6,6 +6,7 @@ package app import ( "bytes" b64 "encoding/base64" + "errors" "fmt" "io" "io/ioutil" @@ -596,7 +597,13 @@ func (a *App) CreateOAuthStateToken(extra string) (*model.Token, *model.AppError token := model.NewToken(model.TOKEN_TYPE_OAUTH, extra) if err := a.Srv().Store.Token().Save(token); err != nil { - return nil, err + var appErr *model.AppError + switch { + case errors.As(err, &appErr): + return nil, appErr + default: + return nil, model.NewAppError("CreateOAuthStateToken", "app.recover.save.app_error", nil, err.Error(), http.StatusInternalServerError) + } } return token, nil diff --git a/app/team_test.go b/app/team_test.go index 986b0c733e..8cf7344cbe 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -256,8 +256,8 @@ func TestAddUserToTeamByToken(t *testing.T) { _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token) require.Nil(t, err, "Should add user to the team") - _, err = th.App.Srv().Store.Token().GetByToken(token.Token) - require.NotNil(t, err, "The token must be deleted after be used") + _, nErr := th.App.Srv().Store.Token().GetByToken(token.Token) + require.NotNil(t, nErr, "The token must be deleted after be used") members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, ruser.Id) require.Nil(t, err) @@ -356,8 +356,8 @@ func TestAddUserToTeamByToken(t *testing.T) { _, err := th.App.AddUserToTeamByToken(rguest.Id, token.Token) require.Nil(t, err, "Should add user to the team") - _, err = th.App.Srv().Store.Token().GetByToken(token.Token) - require.NotNil(t, err, "The token must be deleted after be used") + _, nErr := th.App.Srv().Store.Token().GetByToken(token.Token) + require.NotNil(t, nErr, "The token must be deleted after be used") members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, rguest.Id) require.Nil(t, err) diff --git a/app/user.go b/app/user.go index 512e40c12b..963be4e148 100644 --- a/app/user.go +++ b/app/user.go @@ -1361,7 +1361,13 @@ func (a *App) CreatePasswordRecoveryToken(userId, email string) (*model.Token, * token := model.NewToken(TOKEN_TYPE_PASSWORD_RECOVERY, string(jsonData)) if err := a.Srv().Store.Token().Save(token); err != nil { - return nil, err + var appErr *model.AppError + switch { + case errors.As(err, &appErr): + return nil, appErr + default: + return nil, model.NewAppError("CreatePasswordRecoveryToken", "app.recover.save.app_error", nil, err.Error(), http.StatusInternalServerError) + } } return token, nil @@ -1379,7 +1385,11 @@ func (a *App) GetPasswordRecoveryToken(token string) (*model.Token, *model.AppEr } func (a *App) DeleteToken(token *model.Token) *model.AppError { - return a.Srv().Store.Token().Delete(token.Token) + err := a.Srv().Store.Token().Delete(token.Token) + if err != nil { + return model.NewAppError("DeleteToken", "app.recover.delete.app_error", nil, err.Error(), http.StatusInternalServerError) + } + return nil } func (a *App) UpdateUserRoles(userId string, newRoles string, sendWebSocketEvent bool) (*model.User, *model.AppError) { diff --git a/app/user_test.go b/app/user_test.go index e3bc7af069..6b9d55498f 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -678,8 +678,8 @@ func TestCreateUserWithToken(t *testing.T) { assert.False(t, newUser.IsGuest()) require.Equal(t, invitationEmail, newUser.Email, "The user email must be the invitation one") - _, err = th.App.Srv().Store.Token().GetByToken(token.Token) - require.NotNil(t, err, "The token must be deleted after be used") + _, nErr := th.App.Srv().Store.Token().GetByToken(token.Token) + require.NotNil(t, nErr, "The token must be deleted after be used") members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, newUser.Id) require.Nil(t, err) @@ -699,8 +699,8 @@ func TestCreateUserWithToken(t *testing.T) { assert.True(t, newGuest.IsGuest()) require.Equal(t, invitationEmail, newGuest.Email, "The user email must be the invitation one") - _, err = th.App.Srv().Store.Token().GetByToken(token.Token) - require.NotNil(t, err, "The token must be deleted after be used") + _, nErr := th.App.Srv().Store.Token().GetByToken(token.Token) + require.NotNil(t, nErr, "The token must be deleted after be used") members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, newGuest.Id) require.Nil(t, err) @@ -744,8 +744,8 @@ func TestCreateUserWithToken(t *testing.T) { require.Nil(t, err) assert.True(t, newGuest.IsGuest()) require.Equal(t, grantedInvitationEmail, newGuest.Email) - _, err = th.App.Srv().Store.Token().GetByToken(grantedDomainToken.Token) - require.NotNil(t, err) + _, nErr := th.App.Srv().Store.Token().GetByToken(grantedDomainToken.Token) + require.NotNil(t, nErr) members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, newGuest.Id) require.Nil(t, err) @@ -781,8 +781,8 @@ func TestCreateUserWithToken(t *testing.T) { require.Nil(t, err) assert.True(t, newGuest.IsGuest()) assert.Equal(t, invitationEmail, newGuest.Email, "The user email must be the invitation one") - _, err = th.App.Srv().Store.Token().GetByToken(token.Token) - require.NotNil(t, err) + _, nErr := th.App.Srv().Store.Token().GetByToken(token.Token) + require.NotNil(t, nErr) members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, newGuest.Id) require.Nil(t, err) diff --git a/i18n/en.json b/i18n/en.json index b945335089..2933b8f196 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3982,6 +3982,14 @@ "id": "app.reaction.save.save.app_error", "translation": "Unable to save reaction." }, + { + "id": "app.recover.delete.app_error", + "translation": "Unable to delete token." + }, + { + "id": "app.recover.save.app_error", + "translation": "Unable to save the token." + }, { "id": "app.role.check_roles_exist.role_not_found", "translation": "The provided role does not exist" @@ -6966,22 +6974,6 @@ "id": "store.sql_preference.update.app_error", "translation": "Unable to update the preference." }, - { - "id": "store.sql_recover.delete.app_error", - "translation": "Unable to delete token." - }, - { - "id": "store.sql_recover.get_by_code.app_error", - "translation": "Unable to get a token with this code." - }, - { - "id": "store.sql_recover.remove_all_tokens_by_type.app_error", - "translation": "Unable to remove all the tokens of a type." - }, - { - "id": "store.sql_recover.save.app_error", - "translation": "Unable to save the token." - }, { "id": "store.sql_role.delete.update.app_error", "translation": "Unable to delete the role." diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index 32c47f4b5e..3af321c77e 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -7486,7 +7486,7 @@ func (s *OpenTracingLayerTokenStore) Cleanup() { } -func (s *OpenTracingLayerTokenStore) Delete(token string) *model.AppError { +func (s *OpenTracingLayerTokenStore) Delete(token string) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TokenStore.Delete") s.Root.Store.SetContext(newCtx) @@ -7504,7 +7504,7 @@ func (s *OpenTracingLayerTokenStore) Delete(token string) *model.AppError { return resultVar0 } -func (s *OpenTracingLayerTokenStore) GetByToken(token string) (*model.Token, *model.AppError) { +func (s *OpenTracingLayerTokenStore) GetByToken(token string) (*model.Token, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TokenStore.GetByToken") s.Root.Store.SetContext(newCtx) @@ -7522,7 +7522,7 @@ func (s *OpenTracingLayerTokenStore) GetByToken(token string) (*model.Token, *mo return resultVar0, resultVar1 } -func (s *OpenTracingLayerTokenStore) RemoveAllTokensByType(tokenType string) *model.AppError { +func (s *OpenTracingLayerTokenStore) RemoveAllTokensByType(tokenType string) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TokenStore.RemoveAllTokensByType") s.Root.Store.SetContext(newCtx) @@ -7540,7 +7540,7 @@ func (s *OpenTracingLayerTokenStore) RemoveAllTokensByType(tokenType string) *mo return resultVar0 } -func (s *OpenTracingLayerTokenStore) Save(recovery *model.Token) *model.AppError { +func (s *OpenTracingLayerTokenStore) Save(recovery *model.Token) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TokenStore.Save") s.Root.Store.SetContext(newCtx) diff --git a/store/sqlstore/tokens_store.go b/store/sqlstore/tokens_store.go index fbc5cdaab1..267a29de36 100644 --- a/store/sqlstore/tokens_store.go +++ b/store/sqlstore/tokens_store.go @@ -5,11 +5,13 @@ package sqlstore import ( "database/sql" - "net/http" + "fmt" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" + + "github.com/pkg/errors" ) type SqlTokenStore struct { @@ -32,33 +34,33 @@ func newSqlTokenStore(sqlStore SqlStore) store.TokenStore { func (s SqlTokenStore) createIndexesIfNotExists() { } -func (s SqlTokenStore) Save(token *model.Token) *model.AppError { +func (s SqlTokenStore) Save(token *model.Token) error { if err := token.IsValid(); err != nil { return err } if err := s.GetMaster().Insert(token); err != nil { - return model.NewAppError("SqlTokenStore.Save", "store.sql_recover.save.app_error", nil, "", http.StatusInternalServerError) + return errors.Wrap(err, "failed to save Token") } return nil } -func (s SqlTokenStore) Delete(token string) *model.AppError { +func (s SqlTokenStore) Delete(token string) error { if _, err := s.GetMaster().Exec("DELETE FROM Tokens WHERE Token = :Token", map[string]interface{}{"Token": token}); err != nil { - return model.NewAppError("SqlTokenStore.Delete", "store.sql_recover.delete.app_error", nil, "", http.StatusInternalServerError) + return errors.Wrapf(err, "failed to delete Token with value %s", token) } return nil } -func (s SqlTokenStore) GetByToken(tokenString string) (*model.Token, *model.AppError) { +func (s SqlTokenStore) GetByToken(tokenString string) (*model.Token, error) { token := &model.Token{} if err := s.GetReplica().SelectOne(token, "SELECT * FROM Tokens WHERE Token = :Token", map[string]interface{}{"Token": tokenString}); err != nil { if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlTokenStore.GetByToken", "store.sql_recover.get_by_code.app_error", nil, err.Error(), http.StatusBadRequest) + return nil, store.NewErrNotFound("Token", fmt.Sprintf("Token=%s", tokenString)) } - return nil, model.NewAppError("SqlTokenStore.GetByToken", "store.sql_recover.get_by_code.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get Token with value %s", tokenString) } return token, nil @@ -72,9 +74,9 @@ func (s SqlTokenStore) Cleanup() { } } -func (s SqlTokenStore) RemoveAllTokensByType(tokenType string) *model.AppError { +func (s SqlTokenStore) RemoveAllTokensByType(tokenType string) error { if _, err := s.GetMaster().Exec("DELETE FROM Tokens WHERE Type = :TokenType", map[string]interface{}{"TokenType": tokenType}); err != nil { - return model.NewAppError("SqlTokenStore.RemoveAllTokensByType", "store.sql_recover.remove_all_tokens_by_type.app_error", nil, err.Error(), http.StatusInternalServerError) + return errors.Wrapf(err, "failed to remove all Tokens with Type=%s", tokenType) } return nil } diff --git a/store/store.go b/store/store.go index be82f5aef0..7bf129c2a6 100644 --- a/store/store.go +++ b/store/store.go @@ -512,11 +512,11 @@ type LicenseStore interface { } type TokenStore interface { - Save(recovery *model.Token) *model.AppError - Delete(token string) *model.AppError - GetByToken(token string) (*model.Token, *model.AppError) + Save(recovery *model.Token) error + Delete(token string) error + GetByToken(token string) (*model.Token, error) Cleanup() - RemoveAllTokensByType(tokenType string) *model.AppError + RemoveAllTokensByType(tokenType string) error } type EmojiStore interface { diff --git a/store/storetest/mocks/TokenStore.go b/store/storetest/mocks/TokenStore.go index 9b7f078e2e..e205e2157e 100644 --- a/store/storetest/mocks/TokenStore.go +++ b/store/storetest/mocks/TokenStore.go @@ -20,23 +20,21 @@ func (_m *TokenStore) Cleanup() { } // Delete provides a mock function with given fields: token -func (_m *TokenStore) Delete(token string) *model.AppError { +func (_m *TokenStore) Delete(token string) error { ret := _m.Called(token) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { r0 = rf(token) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 } // GetByToken provides a mock function with given fields: token -func (_m *TokenStore) GetByToken(token string) (*model.Token, *model.AppError) { +func (_m *TokenStore) GetByToken(token string) (*model.Token, error) { ret := _m.Called(token) var r0 *model.Token @@ -48,45 +46,39 @@ func (_m *TokenStore) GetByToken(token string) (*model.Token, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(token) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // RemoveAllTokensByType provides a mock function with given fields: tokenType -func (_m *TokenStore) RemoveAllTokensByType(tokenType string) *model.AppError { +func (_m *TokenStore) RemoveAllTokensByType(tokenType string) error { ret := _m.Called(tokenType) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { r0 = rf(tokenType) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 } // Save provides a mock function with given fields: recovery -func (_m *TokenStore) Save(recovery *model.Token) *model.AppError { +func (_m *TokenStore) Save(recovery *model.Token) error { ret := _m.Called(recovery) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(*model.Token) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(*model.Token) error); ok { r0 = rf(recovery) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 diff --git a/store/timer_layer.go b/store/timer_layer.go index c324153155..dad3f457ed 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -6760,7 +6760,7 @@ func (s *TimerLayerTokenStore) Cleanup() { } } -func (s *TimerLayerTokenStore) Delete(token string) *model.AppError { +func (s *TimerLayerTokenStore) Delete(token string) error { start := timemodule.Now() resultVar0 := s.TokenStore.Delete(token) @@ -6776,7 +6776,7 @@ func (s *TimerLayerTokenStore) Delete(token string) *model.AppError { return resultVar0 } -func (s *TimerLayerTokenStore) GetByToken(token string) (*model.Token, *model.AppError) { +func (s *TimerLayerTokenStore) GetByToken(token string) (*model.Token, error) { start := timemodule.Now() resultVar0, resultVar1 := s.TokenStore.GetByToken(token) @@ -6792,7 +6792,7 @@ func (s *TimerLayerTokenStore) GetByToken(token string) (*model.Token, *model.Ap return resultVar0, resultVar1 } -func (s *TimerLayerTokenStore) RemoveAllTokensByType(tokenType string) *model.AppError { +func (s *TimerLayerTokenStore) RemoveAllTokensByType(tokenType string) error { start := timemodule.Now() resultVar0 := s.TokenStore.RemoveAllTokensByType(tokenType) @@ -6808,7 +6808,7 @@ func (s *TimerLayerTokenStore) RemoveAllTokensByType(tokenType string) *model.Ap return resultVar0 } -func (s *TimerLayerTokenStore) Save(recovery *model.Token) *model.AppError { +func (s *TimerLayerTokenStore) Save(recovery *model.Token) error { start := timemodule.Now() resultVar0 := s.TokenStore.Save(recovery)