Migrate OAuth store to sync by default (#11319)

* Migrate OAuth store to sync by default

* Removing unnecesary return oldValue for updateApp in Oauth Store
Этот коммит содержится в:
Jesús Espino
2019-06-21 18:21:18 +02:00
коммит произвёл GitHub
родитель e3c8c1251f
Коммит e3504398c7
8 изменённых файлов: 569 добавлений и 592 удалений

Просмотреть файл

@@ -34,24 +34,14 @@ func (a *App) CreateOAuthApp(app *model.OAuthApp) (*model.OAuthApp, *model.AppEr
app.ClientSecret = model.NewId()
result := <-a.Srv.Store.OAuth().SaveApp(app)
if result.Err != nil {
return nil, result.Err
}
return result.Data.(*model.OAuthApp), nil
return a.Srv.Store.OAuth().SaveApp(app)
}
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
}
return result.Data.(*model.OAuthApp), nil
return a.Srv.Store.OAuth().GetApp(appId)
}
func (a *App) UpdateOauthApp(oldApp, updatedApp *model.OAuthApp) (*model.OAuthApp, *model.AppError) {
@@ -64,12 +54,7 @@ func (a *App) UpdateOauthApp(oldApp, updatedApp *model.OAuthApp) (*model.OAuthAp
updatedApp.CreateAt = oldApp.CreateAt
updatedApp.ClientSecret = oldApp.ClientSecret
result := <-a.Srv.Store.OAuth().UpdateApp(updatedApp)
if result.Err != nil {
return nil, result.Err
}
return result.Data.([2]*model.OAuthApp)[0], nil
return a.Srv.Store.OAuth().UpdateApp(updatedApp)
}
func (a *App) DeleteOAuthApp(appId string) *model.AppError {
@@ -77,7 +62,7 @@ func (a *App) DeleteOAuthApp(appId string) *model.AppError {
return model.NewAppError("DeleteOAuthApp", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented)
}
if err := (<-a.Srv.Store.OAuth().DeleteApp(appId)).Err; err != nil {
if err := a.Srv.Store.OAuth().DeleteApp(appId); err != nil {
return err
}
@@ -93,12 +78,7 @@ func (a *App) GetOAuthApps(page, perPage int) ([]*model.OAuthApp, *model.AppErro
return nil, model.NewAppError("GetOAuthApps", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented)
}
result := <-a.Srv.Store.OAuth().GetApps(page*perPage, perPage)
if result.Err != nil {
return nil, result.Err
}
return result.Data.([]*model.OAuthApp), nil
return a.Srv.Store.OAuth().GetApps(page*perPage, perPage)
}
func (a *App) GetOAuthAppsByCreator(userId string, page, perPage int) ([]*model.OAuthApp, *model.AppError) {
@@ -106,12 +86,7 @@ func (a *App) GetOAuthAppsByCreator(userId string, page, perPage int) ([]*model.
return nil, model.NewAppError("GetOAuthAppsByUser", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented)
}
result := <-a.Srv.Store.OAuth().GetAppByUser(userId, page*perPage, perPage)
if result.Err != nil {
return nil, result.Err
}
return result.Data.([]*model.OAuthApp), nil
return a.Srv.Store.OAuth().GetAppByUser(userId, page*perPage, perPage)
}
func (a *App) GetOAuthImplicitRedirect(userId string, authRequest *model.AuthorizeRequest) (string, *model.AppError) {
@@ -134,7 +109,7 @@ func (a *App) GetOAuthCodeRedirect(userId string, authRequest *model.AuthorizeRe
authData := &model.AuthData{UserId: userId, ClientId: authRequest.ClientId, CreateAt: model.GetMillis(), RedirectUri: authRequest.RedirectUri, State: authRequest.State, Scope: authRequest.Scope}
authData.Code = model.NewId() + model.NewId()
if result := <-a.Srv.Store.OAuth().SaveAuthData(authData); result.Err != nil {
if _, err := a.Srv.Store.OAuth().SaveAuthData(authData); err != nil {
return authRequest.RedirectUri + "?error=server_error&state=" + authRequest.State, nil
}
@@ -150,18 +125,16 @@ func (a *App) AllowOAuthAppAccessToUser(userId string, authRequest *model.Author
authRequest.Scope = model.DEFAULT_SCOPE
}
result := <-a.Srv.Store.OAuth().GetApp(authRequest.ClientId)
if result.Err != nil {
return "", result.Err
oauthApp, err := a.Srv.Store.OAuth().GetApp(authRequest.ClientId)
if err != nil {
return "", err
}
oauthApp := result.Data.(*model.OAuthApp)
if !oauthApp.IsValidRedirectURL(authRequest.RedirectUri) {
return "", model.NewAppError("AllowOAuthAppAccessToUser", "api.oauth.allow_oauth.redirect_callback.app_error", nil, "", http.StatusBadRequest)
}
var redirectURI string
var err *model.AppError
switch authRequest.ResponseType {
case model.AUTHCODE_RESPONSE_TYPE:
@@ -215,8 +188,8 @@ func (a *App) GetOAuthAccessTokenForImplicitFlow(userId string, authRequest *mod
accessData := &model.AccessData{ClientId: authRequest.ClientId, UserId: user.Id, Token: session.Token, RefreshToken: "", RedirectUri: authRequest.RedirectUri, ExpiresAt: session.ExpiresAt, Scope: authRequest.Scope}
if result := <-a.Srv.Store.OAuth().SaveAccessData(accessData); result.Err != nil {
mlog.Error(fmt.Sprint(result.Err))
if _, err := a.Srv.Store.OAuth().SaveAccessData(accessData); err != nil {
mlog.Error(fmt.Sprint(err))
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_saving.app_error", nil, "", http.StatusInternalServerError)
}
@@ -228,11 +201,10 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.disabled.app_error", nil, "", http.StatusNotImplemented)
}
result := <-a.Srv.Store.OAuth().GetApp(clientId)
if result.Err != nil {
oauthApp, err := a.Srv.Store.OAuth().GetApp(clientId)
if err != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.credentials.app_error", nil, "", http.StatusNotFound)
}
oauthApp := result.Data.(*model.OAuthApp)
if oauthApp.ClientSecret != secret {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.credentials.app_error", nil, "", http.StatusForbidden)
@@ -243,14 +215,13 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c
var accessRsp *model.AccessResponse
if grantType == model.ACCESS_TOKEN_GRANT_TYPE {
var authData *model.AuthData
result := <-a.Srv.Store.OAuth().GetAuthData(code)
if result.Err != nil {
authData, err = a.Srv.Store.OAuth().GetAuthData(code)
if err != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusBadRequest)
}
authData = result.Data.(*model.AuthData)
if authData.IsExpired() {
<-a.Srv.Store.OAuth().RemoveAuthData(authData.Code)
a.Srv.Store.OAuth().RemoveAuthData(authData.Code)
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusForbidden)
}
@@ -258,21 +229,20 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.redirect_uri.app_error", nil, "", http.StatusBadRequest)
}
var err *model.AppError
user, err = a.Srv.Store.User().Get(authData.UserId)
if err != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound)
}
result = <-a.Srv.Store.OAuth().GetPreviousAccessData(user.Id, clientId)
if result.Err != nil {
accessData, err = a.Srv.Store.OAuth().GetPreviousAccessData(user.Id, clientId)
if err != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal.app_error", nil, "", http.StatusBadRequest)
}
if result.Data != nil {
accessData := result.Data.(*model.AccessData)
if accessData != nil {
if accessData.IsExpired() {
access, err := a.newSessionUpdateToken(oauthApp.Name, accessData, user)
var access *model.AccessResponse
access, err = a.newSessionUpdateToken(oauthApp.Name, accessData, user)
if err != nil {
return nil, err
}
@@ -287,16 +257,17 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c
}
}
} else {
var session *model.Session
// Create a new session and return new access token
session, err := a.newSession(oauthApp.Name, user)
session, err = a.newSession(oauthApp.Name, user)
if err != nil {
return nil, err
}
accessData = &model.AccessData{ClientId: clientId, UserId: user.Id, Token: session.Token, RefreshToken: model.NewId(), RedirectUri: redirectUri, ExpiresAt: session.ExpiresAt, Scope: authData.Scope}
if result := <-a.Srv.Store.OAuth().SaveAccessData(accessData); result.Err != nil {
mlog.Error(fmt.Sprint(result.Err))
if _, err = a.Srv.Store.OAuth().SaveAccessData(accessData); err != nil {
mlog.Error(fmt.Sprint(err))
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_saving.app_error", nil, "", http.StatusInternalServerError)
}
@@ -308,14 +279,13 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c
}
}
<-a.Srv.Store.OAuth().RemoveAuthData(authData.Code)
a.Srv.Store.OAuth().RemoveAuthData(authData.Code)
} else {
// When grantType is refresh_token
result := <-a.Srv.Store.OAuth().GetAccessDataByRefreshToken(refreshToken)
if result.Err != nil {
accessData, err = a.Srv.Store.OAuth().GetAccessDataByRefreshToken(refreshToken)
if err != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.refresh_token.app_error", nil, "", http.StatusNotFound)
}
accessData = result.Data.(*model.AccessData)
user, err := a.Srv.Store.User().Get(accessData.UserId)
if err != nil {
@@ -366,8 +336,8 @@ func (a *App) newSessionUpdateToken(appName string, accessData *model.AccessData
accessData.RefreshToken = model.NewId()
accessData.ExpiresAt = session.ExpiresAt
if result := <-a.Srv.Store.OAuth().UpdateAccessData(accessData); result.Err != nil {
mlog.Error(fmt.Sprint(result.Err))
if _, err := a.Srv.Store.OAuth().UpdateAccessData(accessData); err != nil {
mlog.Error(fmt.Sprint(err))
return nil, model.NewAppError("newSessionUpdateToken", "web.get_access_token.internal_saving.app_error", nil, "", http.StatusInternalServerError)
}
accessRsp := &model.AccessResponse{
@@ -419,11 +389,10 @@ func (a *App) GetAuthorizedAppsForUser(userId string, page, perPage int) ([]*mod
return nil, model.NewAppError("GetAuthorizedAppsForUser", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented)
}
result := <-a.Srv.Store.OAuth().GetAuthorizedApps(userId, page*perPage, perPage)
if result.Err != nil {
return nil, result.Err
apps, err := a.Srv.Store.OAuth().GetAuthorizedApps(userId, page*perPage, perPage)
if err != nil {
return nil, err
}
apps := result.Data.([]*model.OAuthApp)
for k, a := range apps {
a.Sanitize()
@@ -439,19 +408,18 @@ func (a *App) DeauthorizeOAuthAppForUser(userId, appId string) *model.AppError {
}
// Revoke app sessions
result := <-a.Srv.Store.OAuth().GetAccessDataByUserForApp(userId, appId)
if result.Err != nil {
return result.Err
accessData, err := a.Srv.Store.OAuth().GetAccessDataByUserForApp(userId, appId)
if err != nil {
return err
}
accessData := result.Data.([]*model.AccessData)
for _, ad := range accessData {
if err := a.RevokeAccessToken(ad.Token); err != nil {
return err
}
if rad := <-a.Srv.Store.OAuth().RemoveAccessData(ad.Token); rad.Err != nil {
return rad.Err
if err := a.Srv.Store.OAuth().RemoveAccessData(ad.Token); err != nil {
return err
}
}
@@ -469,8 +437,8 @@ func (a *App) RegenerateOAuthAppSecret(app *model.OAuthApp) (*model.OAuthApp, *m
}
app.ClientSecret = model.NewId()
if update := <-a.Srv.Store.OAuth().UpdateApp(app); update.Err != nil {
return nil, update.Err
if _, err := a.Srv.Store.OAuth().UpdateApp(app); err != nil {
return nil, err
}
return app, nil
@@ -485,11 +453,11 @@ func (a *App) RevokeAccessToken(token string) *model.AppError {
close(schan)
}()
if result := <-a.Srv.Store.OAuth().GetAccessData(token); result.Err != nil {
if _, err := a.Srv.Store.OAuth().GetAccessData(token); err != nil {
return model.NewAppError("RevokeAccessToken", "api.oauth.revoke_access_token.get.app_error", nil, "", http.StatusBadRequest)
}
if result := <-a.Srv.Store.OAuth().RemoveAccessData(token); result.Err != nil {
if err := a.Srv.Store.OAuth().RemoveAccessData(token); err != nil {
return model.NewAppError("RevokeAccessToken", "api.oauth.revoke_access_token.del_token.app_error", nil, "", http.StatusInternalServerError)
}

Просмотреть файл

@@ -92,11 +92,10 @@ func TestOAuthRevokeAccessToken(t *testing.T) {
accessData.ClientId = model.NewId()
accessData.ExpiresAt = session.ExpiresAt
if result := <-th.App.Srv.Store.OAuth().SaveAccessData(accessData); result.Err != nil {
t.Fatal(result.Err)
}
_, err := th.App.Srv.Store.OAuth().SaveAccessData(accessData)
require.Nil(t, err)
if err := th.App.RevokeAccessToken(accessData.Token); err != nil {
if err = th.App.RevokeAccessToken(accessData.Token); err != nil {
t.Fatal(err)
}
}
@@ -136,15 +135,14 @@ func TestOAuthDeleteApp(t *testing.T) {
accessData.ClientId = a1.Id
accessData.ExpiresAt = session.ExpiresAt
if result := <-th.App.Srv.Store.OAuth().SaveAccessData(accessData); result.Err != nil {
t.Fatal(result.Err)
}
_, err = th.App.Srv.Store.OAuth().SaveAccessData(accessData)
require.Nil(t, err)
if err := th.App.DeleteOAuthApp(a1.Id); err != nil {
if err = th.App.DeleteOAuthApp(a1.Id); err != nil {
t.Fatal(err)
}
if _, err := th.App.GetSession(session.Token); err == nil {
if _, err = th.App.GetSession(session.Token); err == nil {
t.Fatal("should not get session from cache or db")
}
}

Просмотреть файл

@@ -1436,8 +1436,8 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError {
return result.Err
}
if result := <-a.Srv.Store.OAuth().PermanentDeleteAuthDataByUser(user.Id); result.Err != nil {
return result.Err
if err := a.Srv.Store.OAuth().PermanentDeleteAuthDataByUser(user.Id); err != nil {
return err
}
if err := a.Srv.Store.Webhook().PermanentDeleteIncomingByUser(user.Id); err != nil {