diff --git a/app/oauth.go b/app/oauth.go index 6812b3a356..322060281c 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -46,7 +46,6 @@ func (a *App) GetOAuthApp(appId string) (*model.OAuthApp, *model.AppError) { if !*a.Config().ServiceSettings.EnableOAuthServiceProvider { return nil, model.NewAppError("GetOAuthApp", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented) } - result := <-a.Srv.Store.OAuth().GetApp(appId) if result.Err != nil { return nil, result.Err @@ -354,7 +353,9 @@ func (a *App) newSession(appName string, user *model.User) (*model.Session, *mod func (a *App) newSessionUpdateToken(appName string, accessData *model.AccessData, user *model.User) (*model.AccessResponse, *model.AppError) { // Remove the previous session - <-a.Srv.Store.Session().Remove(accessData.Token) + if err := a.Srv.Store.Session().Remove(accessData.Token); err != nil { + mlog.Error(fmt.Sprint(err)) + } session, err := a.newSession(appName, user) if err != nil { @@ -477,7 +478,12 @@ func (a *App) RegenerateOAuthAppSecret(app *model.OAuthApp) (*model.OAuthApp, *m func (a *App) RevokeAccessToken(token string) *model.AppError { session, _ := a.GetSession(token) - schan := a.Srv.Store.Session().Remove(token) + + schan := make(chan *model.AppError, 1) + go func() { + schan <- a.Srv.Store.Session().Remove(token) + close(schan) + }() if result := <-a.Srv.Store.OAuth().GetAccessData(token); result.Err != nil { return model.NewAppError("RevokeAccessToken", "api.oauth.revoke_access_token.get.app_error", nil, "", http.StatusBadRequest) @@ -487,7 +493,7 @@ func (a *App) RevokeAccessToken(token string) *model.AppError { return model.NewAppError("RevokeAccessToken", "api.oauth.revoke_access_token.del_token.app_error", nil, "", http.StatusInternalServerError) } - if result := <-schan; result.Err != nil { + if err := <-schan; err != nil { return model.NewAppError("RevokeAccessToken", "api.oauth.revoke_access_token.del_session.app_error", nil, "", http.StatusInternalServerError) } diff --git a/app/session.go b/app/session.go index 15837f2366..bc38695e79 100644 --- a/app/session.go +++ b/app/session.go @@ -102,8 +102,8 @@ func (a *App) RevokeAllSessions(userId string) *model.AppError { if session.IsOAuth { a.RevokeAccessToken(session.Token) } else { - if result := <-a.Srv.Store.Session().Remove(session.Id); result.Err != nil { - return result.Err + if err := a.Srv.Store.Session().Remove(session.Id); err != nil { + return err } } } @@ -195,8 +195,8 @@ func (a *App) RevokeSession(session *model.Session) *model.AppError { return err } } else { - if result := <-a.Srv.Store.Session().Remove(session.Id); result.Err != nil { - return result.Err + if err := a.Srv.Store.Session().Remove(session.Id); err != nil { + return err } } diff --git a/store/sqlstore/session_store.go b/store/sqlstore/session_store.go index c11a0bc21a..7cb41b86f1 100644 --- a/store/sqlstore/session_store.go +++ b/store/sqlstore/session_store.go @@ -144,13 +144,12 @@ func (me SqlSessionStore) GetSessionsWithActiveDeviceIds(userId string) ([]*mode return sessions, nil } -func (me SqlSessionStore) Remove(sessionIdOrToken string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - _, err := me.GetMaster().Exec("DELETE FROM Sessions WHERE Id = :Id Or Token = :Token", map[string]interface{}{"Id": sessionIdOrToken, "Token": sessionIdOrToken}) - if err != nil { - result.Err = model.NewAppError("SqlSessionStore.RemoveSession", "store.sql_session.remove.app_error", nil, "id="+sessionIdOrToken+", err="+err.Error(), http.StatusInternalServerError) - } - }) +func (me SqlSessionStore) Remove(sessionIdOrToken string) *model.AppError { + _, err := me.GetMaster().Exec("DELETE FROM Sessions WHERE Id = :Id Or Token = :Token", map[string]interface{}{"Id": sessionIdOrToken, "Token": sessionIdOrToken}) + if err != nil { + return model.NewAppError("SqlSessionStore.RemoveSession", "store.sql_session.remove.app_error", nil, "id="+sessionIdOrToken+", err="+err.Error(), http.StatusInternalServerError) + } + return nil } func (me SqlSessionStore) RemoveAllSessions() *model.AppError { diff --git a/store/store.go b/store/store.go index 38caa2b6df..6d8bc4bc37 100644 --- a/store/store.go +++ b/store/store.go @@ -316,7 +316,7 @@ type SessionStore interface { Save(session *model.Session) (*model.Session, *model.AppError) GetSessions(userId string) ([]*model.Session, *model.AppError) GetSessionsWithActiveDeviceIds(userId string) ([]*model.Session, *model.AppError) - Remove(sessionIdOrToken string) StoreChannel + Remove(sessionIdOrToken string) *model.AppError RemoveAllSessions() *model.AppError PermanentDeleteSessionsByUser(teamId string) *model.AppError UpdateLastActivityAt(sessionId string, time int64) *model.AppError diff --git a/store/storetest/mocks/SessionStore.go b/store/storetest/mocks/SessionStore.go index 79246becf1..016a58c636 100644 --- a/store/storetest/mocks/SessionStore.go +++ b/store/storetest/mocks/SessionStore.go @@ -6,7 +6,6 @@ package mocks import mock "github.com/stretchr/testify/mock" import model "github.com/mattermost/mattermost-server/model" -import store "github.com/mattermost/mattermost-server/store" // SessionStore is an autogenerated mock type for the SessionStore type type SessionStore struct { @@ -133,15 +132,15 @@ func (_m *SessionStore) PermanentDeleteSessionsByUser(teamId string) *model.AppE } // Remove provides a mock function with given fields: sessionIdOrToken -func (_m *SessionStore) Remove(sessionIdOrToken string) store.StoreChannel { +func (_m *SessionStore) Remove(sessionIdOrToken string) *model.AppError { ret := _m.Called(sessionIdOrToken) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { r0 = rf(sessionIdOrToken) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } diff --git a/store/storetest/session_store.go b/store/storetest/session_store.go index 6a4ca7b9c4..4653ecea1e 100644 --- a/store/storetest/session_store.go +++ b/store/storetest/session_store.go @@ -124,8 +124,8 @@ func testSessionRemove(t *testing.T, ss store.Store) { } } - store.Must(ss.Session().Remove(s1.Id)) - + removeErr := ss.Session().Remove(s1.Id) + require.Nil(t, removeErr) if _, err := ss.Session().Get(s1.Id); err == nil { t.Fatal("should have been removed") } @@ -192,7 +192,8 @@ func testSessionRemoveToken(t *testing.T, ss store.Store) { } } - store.Must(ss.Session().Remove(s1.Token)) + removeErr := ss.Session().Remove(s1.Token) + require.Nil(t, removeErr) if _, err := ss.Session().Get(s1.Id); err == nil { t.Fatal("should have been removed") @@ -333,6 +334,9 @@ func testSessionCleanup(t *testing.T, ss store.Store) { _, err = ss.Session().Get(s4.Id) assert.NotNil(t, err) - store.Must(ss.Session().Remove(s1.Id)) - store.Must(ss.Session().Remove(s2.Id)) + removeErr := ss.Session().Remove(s1.Id) + require.Nil(t, removeErr) + + removeErr = ss.Session().Remove(s2.Id) + require.Nil(t, removeErr) }