diff --git a/app/oauth.go b/app/oauth.go index 85e6769fd5..c1b5043744 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -33,14 +33,14 @@ func (a *App) CreateOAuthApp(app *model.OAuthApp) (*model.OAuthApp, *model.AppEr return nil, model.NewAppError("CreateOAuthApp", "api.oauth.register_oauth_app.turn_off.app_error", nil, "", http.StatusNotImplemented) } - secret := model.NewId() - app.ClientSecret = secret + app.ClientSecret = model.NewId() - if result := <-a.Srv.Store.OAuth().SaveApp(app); result.Err != nil { + result := <-a.Srv.Store.OAuth().SaveApp(app) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.(*model.OAuthApp), nil } + + return result.Data.(*model.OAuthApp), nil } func (a *App) GetOAuthApp(appId string) (*model.OAuthApp, *model.AppError) { @@ -48,11 +48,12 @@ func (a *App) GetOAuthApp(appId string) (*model.OAuthApp, *model.AppError) { return nil, model.NewAppError("GetOAuthApp", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented) } - if result := <-a.Srv.Store.OAuth().GetApp(appId); result.Err != nil { + result := <-a.Srv.Store.OAuth().GetApp(appId) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.(*model.OAuthApp), nil } + + return result.Data.(*model.OAuthApp), nil } func (a *App) UpdateOauthApp(oldApp, updatedApp *model.OAuthApp) (*model.OAuthApp, *model.AppError) { @@ -65,11 +66,12 @@ func (a *App) UpdateOauthApp(oldApp, updatedApp *model.OAuthApp) (*model.OAuthAp updatedApp.CreateAt = oldApp.CreateAt updatedApp.ClientSecret = oldApp.ClientSecret - if result := <-a.Srv.Store.OAuth().UpdateApp(updatedApp); result.Err != nil { + result := <-a.Srv.Store.OAuth().UpdateApp(updatedApp) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.([2]*model.OAuthApp)[0], nil } + + return result.Data.([2]*model.OAuthApp)[0], nil } func (a *App) DeleteOAuthApp(appId string) *model.AppError { @@ -81,7 +83,9 @@ func (a *App) DeleteOAuthApp(appId string) *model.AppError { return err } - a.InvalidateAllCaches() + if err := a.InvalidateAllCaches(); err != nil { + mlog.Error(err.Error()) + } return nil } @@ -91,11 +95,12 @@ 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) } - if result := <-a.Srv.Store.OAuth().GetApps(page*perPage, perPage); result.Err != nil { + result := <-a.Srv.Store.OAuth().GetApps(page*perPage, perPage) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.([]*model.OAuthApp), nil } + + return result.Data.([]*model.OAuthApp), nil } func (a *App) GetOAuthAppsByCreator(userId string, page, perPage int) ([]*model.OAuthApp, *model.AppError) { @@ -103,11 +108,12 @@ 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) } - if result := <-a.Srv.Store.OAuth().GetAppByUser(userId, page*perPage, perPage); result.Err != nil { + result := <-a.Srv.Store.OAuth().GetAppByUser(userId, page*perPage, perPage) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.([]*model.OAuthApp), nil } + + return result.Data.([]*model.OAuthApp), nil } func (a *App) GetOAuthImplicitRedirect(userId string, authRequest *model.AuthorizeRequest) (string, *model.AppError) { @@ -146,12 +152,11 @@ func (a *App) AllowOAuthAppAccessToUser(userId string, authRequest *model.Author authRequest.Scope = model.DEFAULT_SCOPE } - var oauthApp *model.OAuthApp - if result := <-a.Srv.Store.OAuth().GetApp(authRequest.ClientId); result.Err != nil { + result := <-a.Srv.Store.OAuth().GetApp(authRequest.ClientId) + if result.Err != nil { return "", result.Err - } else { - oauthApp = result.Data.(*model.OAuthApp) } + 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) @@ -174,7 +179,7 @@ func (a *App) AllowOAuthAppAccessToUser(userId string, authRequest *model.Author return authRequest.RedirectUri + "?error=server_error&state=" + authRequest.State, nil } - // this saves the OAuth2 app as authorized + // This saves the OAuth2 app as authorized authorizedApp := model.Preference{ UserId: userId, Category: model.PREFERENCE_CATEGORY_AUTHORIZED_OAUTH_APP, @@ -182,7 +187,7 @@ func (a *App) AllowOAuthAppAccessToUser(userId string, authRequest *model.Author Value: authRequest.Scope, } - if result := <-a.Srv.Store.Preference().Save(&model.Preferences{authorizedApp}); result.Err != nil { + if result = <-a.Srv.Store.Preference().Save(&model.Preferences{authorizedApp}); result.Err != nil { mlog.Error(result.Err.Error()) return authRequest.RedirectUri + "?error=server_error&state=" + authRequest.State, nil } @@ -195,7 +200,6 @@ func (a *App) GetOAuthAccessTokenForImplicitFlow(userId string, authRequest *mod return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.disabled.app_error", nil, "", http.StatusNotImplemented) } - var oauthApp *model.OAuthApp oauthApp, err := a.GetOAuthApp(authRequest.ClientId) if err != nil { return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.credentials.app_error", nil, "", http.StatusNotFound) @@ -226,12 +230,11 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.disabled.app_error", nil, "", http.StatusNotImplemented) } - var oauthApp *model.OAuthApp - if result := <-a.Srv.Store.OAuth().GetApp(clientId); result.Err != nil { + result := <-a.Srv.Store.OAuth().GetApp(clientId) + if result.Err != nil { return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.credentials.app_error", nil, "", http.StatusNotFound) - } else { - oauthApp = result.Data.(*model.OAuthApp) } + 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) @@ -241,13 +244,12 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c var accessData *model.AccessData var accessRsp *model.AccessResponse if grantType == model.ACCESS_TOKEN_GRANT_TYPE { - var authData *model.AuthData - if result := <-a.Srv.Store.OAuth().GetAuthData(code); result.Err != nil { + result := <-a.Srv.Store.OAuth().GetAuthData(code) + if result.Err != nil { return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusInternalServerError) - } else { - authData = result.Data.(*model.AuthData) } + authData = result.Data.(*model.AuthData) if authData.IsExpired() { <-a.Srv.Store.OAuth().RemoveAuthData(authData.Code) @@ -258,24 +260,27 @@ 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) } - if result := <-a.Srv.Store.User().Get(authData.UserId); result.Err != nil { + result = <-a.Srv.Store.User().Get(authData.UserId) + if result.Err != nil { return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound) - } else { - user = result.Data.(*model.User) + } + user = result.Data.(*model.User) + + result = <-a.Srv.Store.OAuth().GetPreviousAccessData(user.Id, clientId) + if result.Err != nil { + return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal.app_error", nil, "", http.StatusInternalServerError) } - if result := <-a.Srv.Store.OAuth().GetPreviousAccessData(user.Id, clientId); result.Err != nil { - return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal.app_error", nil, "", http.StatusInternalServerError) - } else if result.Data != nil { + if result.Data != nil { accessData := result.Data.(*model.AccessData) if accessData.IsExpired() { - if access, err := a.newSessionUpdateToken(oauthApp.Name, accessData, user); err != nil { + access, err := a.newSessionUpdateToken(oauthApp.Name, accessData, user) + if err != nil { return nil, err - } else { - accessRsp = access } + accessRsp = access } else { - //return the same token and no need to create a new session + // Return the same token and no need to create a new session accessRsp = &model.AccessResponse{ AccessToken: accessData.Token, TokenType: model.ACCESS_TOKEN_TYPE, @@ -284,12 +289,10 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c } } } else { - // create a new session and return new access token - var session *model.Session - if result, err := a.newSession(oauthApp.Name, user); err != nil { + // Create a new session and return new access token + session, err := a.newSession(oauthApp.Name, user) + if err != nil { return nil, err - } else { - session = result } accessData = &model.AccessData{ClientId: clientId, UserId: user.Id, Token: session.Token, RefreshToken: model.NewId(), RedirectUri: redirectUri, ExpiresAt: session.ExpiresAt, Scope: authData.Scope} @@ -309,31 +312,31 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c <-a.Srv.Store.OAuth().RemoveAuthData(authData.Code) } else { - // when grantType is refresh_token - if result := <-a.Srv.Store.OAuth().GetAccessDataByRefreshToken(refreshToken); result.Err != nil { + // When grantType is refresh_token + result := <-a.Srv.Store.OAuth().GetAccessDataByRefreshToken(refreshToken) + if result.Err != nil { return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.refresh_token.app_error", nil, "", http.StatusNotFound) - } else { - accessData = result.Data.(*model.AccessData) } + accessData = result.Data.(*model.AccessData) - if result := <-a.Srv.Store.User().Get(accessData.UserId); result.Err != nil { + result = <-a.Srv.Store.User().Get(accessData.UserId) + if result.Err != nil { return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound) - } else { - user = result.Data.(*model.User) } + user = result.Data.(*model.User) - if access, err := a.newSessionUpdateToken(oauthApp.Name, accessData, user); err != nil { + access, err := a.newSessionUpdateToken(oauthApp.Name, accessData, user) + if err != nil { return nil, err - } else { - accessRsp = access } + accessRsp = access } return accessRsp, nil } func (a *App) newSession(appName string, user *model.User) (*model.Session, *model.AppError) { - // set new token an session + // Set new token an session session := &model.Session{UserId: user.Id, Roles: user.Roles, IsOAuth: true} session.GenerateCSRF() session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthSSOInDays) @@ -341,29 +344,30 @@ func (a *App) newSession(appName string, user *model.User) (*model.Session, *mod session.AddProp(model.SESSION_PROP_OS, "OAuth2") session.AddProp(model.SESSION_PROP_BROWSER, "OAuth2") - if result := <-a.Srv.Store.Session().Save(session); result.Err != nil { + result := <-a.Srv.Store.Session().Save(session) + if result.Err != nil { return nil, model.NewAppError("newSession", "api.oauth.get_access_token.internal_session.app_error", nil, "", http.StatusInternalServerError) - } else { - session = result.Data.(*model.Session) - a.AddSessionToCache(session) } + session = result.Data.(*model.Session) + + a.AddSessionToCache(session) return session, nil } func (a *App) newSessionUpdateToken(appName string, accessData *model.AccessData, user *model.User) (*model.AccessResponse, *model.AppError) { - var session *model.Session - <-a.Srv.Store.Session().Remove(accessData.Token) //remove the previous session + // Remove the previous session + <-a.Srv.Store.Session().Remove(accessData.Token) - if result, err := a.newSession(appName, user); err != nil { + session, err := a.newSession(appName, user) + if err != nil { return nil, err - } else { - session = result } accessData.Token = session.Token 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)) return nil, model.NewAppError("newSessionUpdateToken", "web.get_access_token.internal_saving.app_error", nil, "", http.StatusInternalServerError) @@ -389,11 +393,12 @@ func (a *App) GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, serv stateProps["redirect_to"] = redirectTo } - if authUrl, err := a.GetAuthorizationCode(w, r, service, stateProps, loginHint); err != nil { + authUrl, err := a.GetAuthorizationCode(w, r, service, stateProps, loginHint) + if err != nil { return "", err - } else { - return authUrl, nil } + + return authUrl, nil } func (a *App) GetOAuthSignupEndpoint(w http.ResponseWriter, r *http.Request, service, teamId string) (string, *model.AppError) { @@ -403,11 +408,12 @@ func (a *App) GetOAuthSignupEndpoint(w http.ResponseWriter, r *http.Request, ser stateProps["team_id"] = teamId } - if authUrl, err := a.GetAuthorizationCode(w, r, service, stateProps, ""); err != nil { + authUrl, err := a.GetAuthorizationCode(w, r, service, stateProps, "") + if err != nil { return "", err - } else { - return authUrl, nil } + + return authUrl, nil } func (a *App) GetAuthorizedAppsForUser(userId string, page, perPage int) ([]*model.OAuthApp, *model.AppError) { @@ -415,17 +421,18 @@ 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) } - if result := <-a.Srv.Store.OAuth().GetAuthorizedApps(userId, page*perPage, perPage); result.Err != nil { + result := <-a.Srv.Store.OAuth().GetAuthorizedApps(userId, page*perPage, perPage) + if result.Err != nil { return nil, result.Err - } else { - apps := result.Data.([]*model.OAuthApp) - for k, a := range apps { - a.Sanitize() - apps[k] = a - } - - return apps, nil } + apps := result.Data.([]*model.OAuthApp) + + for k, a := range apps { + a.Sanitize() + apps[k] = a + } + + return apps, nil } func (a *App) DeauthorizeOAuthAppForUser(userId, appId string) *model.AppError { @@ -433,20 +440,20 @@ func (a *App) DeauthorizeOAuthAppForUser(userId, appId string) *model.AppError { return model.NewAppError("DeauthorizeOAuthAppForUser", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented) } - // revoke app sessions - if result := <-a.Srv.Store.OAuth().GetAccessDataByUserForApp(userId, appId); result.Err != nil { + // Revoke app sessions + result := <-a.Srv.Store.OAuth().GetAccessDataByUserForApp(userId, appId) + if result.Err != nil { return result.Err - } else { - accessData := result.Data.([]*model.AccessData) + } + accessData := result.Data.([]*model.AccessData) - for _, ad := range accessData { - if err := a.RevokeAccessToken(ad.Token); err != nil { - return err - } + 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 rad := <-a.Srv.Store.OAuth().RemoveAccessData(ad.Token); rad.Err != nil { + return rad.Err } } @@ -479,9 +486,7 @@ func (a *App) RevokeAccessToken(token string) *model.AppError { return model.NewAppError("RevokeAccessToken", "api.oauth.revoke_access_token.get.app_error", nil, "", http.StatusBadRequest) } - tchan := a.Srv.Store.OAuth().RemoveAccessData(token) - - if result := <-tchan; result.Err != nil { + if result := <-a.Srv.Store.OAuth().RemoveAccessData(token); result.Err != nil { return model.NewAppError("RevokeAccessToken", "api.oauth.revoke_access_token.del_token.app_error", nil, "", http.StatusInternalServerError) } @@ -516,22 +521,22 @@ func (a *App) CompleteOAuth(service string, body io.ReadCloser, teamId string, p } func (a *App) LoginByOAuth(service string, userData io.Reader, teamId string) (*model.User, *model.AppError) { - buf := bytes.Buffer{} - buf.ReadFrom(userData) - - authData := "" provider := einterfaces.GetOauthProvider(service) if provider == nil { return nil, model.NewAppError("LoginByOAuth", "api.user.login_by_oauth.not_available.app_error", map[string]interface{}{"Service": strings.Title(service)}, "", http.StatusNotImplemented) - } else { - authUser := provider.GetUserFromJson(bytes.NewReader(buf.Bytes())) + } - if authUser.AuthData != nil { - authData = *authUser.AuthData - } else { - authData = "" - } + buf := bytes.Buffer{} + if _, err := buf.ReadFrom(userData); err != nil { + return nil, model.NewAppError("LoginByOAuth", "api.user.login_by_oauth.parse.app_error", + map[string]interface{}{"Service": service}, "", http.StatusBadRequest) + } + authUser := provider.GetUserFromJson(bytes.NewReader(buf.Bytes())) + + authData := "" + if authUser.AuthData != nil { + authData = *authUser.AuthData } if len(authData) == 0 { @@ -583,20 +588,18 @@ func (a *App) LoginByOAuth(service string, userData io.Reader, teamId string) (* return user, nil } -func (a *App) CompleteSwitchWithOAuth(service string, userData io.ReadCloser, email string) (*model.User, *model.AppError) { - authData := "" - ssoEmail := "" +func (a *App) CompleteSwitchWithOAuth(service string, userData io.Reader, email string) (*model.User, *model.AppError) { provider := einterfaces.GetOauthProvider(service) if provider == nil { return nil, model.NewAppError("CompleteSwitchWithOAuth", "api.user.complete_switch_with_oauth.unavailable.app_error", map[string]interface{}{"Service": strings.Title(service)}, "", http.StatusNotImplemented) - } else { - ssoUser := provider.GetUserFromJson(userData) - ssoEmail = ssoUser.Email + } + ssoUser := provider.GetUserFromJson(userData) + ssoEmail := ssoUser.Email - if ssoUser.AuthData != nil { - authData = *ssoUser.AuthData - } + authData := "" + if ssoUser.AuthData != nil { + authData = *ssoUser.AuthData } if len(authData) == 0 { @@ -608,18 +611,17 @@ func (a *App) CompleteSwitchWithOAuth(service string, userData io.ReadCloser, em return nil, model.NewAppError("CompleteSwitchWithOAuth", "api.user.complete_switch_with_oauth.blank_email.app_error", nil, "", http.StatusBadRequest) } - var user *model.User - if result := <-a.Srv.Store.User().GetByEmail(email); result.Err != nil { + result := <-a.Srv.Store.User().GetByEmail(email) + if result.Err != nil { return nil, result.Err - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) if err := a.RevokeAllSessions(user.Id); err != nil { return nil, err } - if result := <-a.Srv.Store.User().UpdateAuthData(user.Id, service, &authData, ssoEmail, true); result.Err != nil { + if result = <-a.Srv.Store.User().UpdateAuthData(user.Id, service, &authData, ssoEmail, true); result.Err != nil { return nil, result.Err } @@ -643,20 +645,17 @@ func (a *App) CreateOAuthStateToken(extra string) (*model.Token, *model.AppError } func (a *App) GetOAuthStateToken(token string) (*model.Token, *model.AppError) { - if result := <-a.Srv.Store.Token().GetByToken(token); result.Err != nil { + result := <-a.Srv.Store.Token().GetByToken(token) + if result.Err != nil { return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, result.Err.Error(), http.StatusBadRequest) - } else { - token := result.Data.(*model.Token) - if token.Type != model.TOKEN_TYPE_OAUTH { - return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, "", http.StatusBadRequest) - } - - return token, nil } -} + mToken := result.Data.(*model.Token) -func generateOAuthStateTokenExtra(email, action, cookie string) string { - return email + ":" + action + ":" + cookie + if mToken.Type != model.TOKEN_TYPE_OAUTH { + return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, "", http.StatusBadRequest) + } + + return mToken, nil } func (a *App) GetAuthorizationCode(w http.ResponseWriter, r *http.Request, service string, props map[string]string, loginHint string) (string, *model.AppError) { @@ -723,13 +722,13 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service return nil, "", nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.unsupported.app_error", nil, "service="+service, http.StatusNotImplemented) } - stateStr := "" - if b, err := b64.StdEncoding.DecodeString(state); err != nil { - return nil, "", nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, err.Error(), http.StatusBadRequest) - } else { - stateStr = string(b) + b, strErr := b64.StdEncoding.DecodeString(state) + if strErr != nil { + return nil, "", nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, strErr.Error(), http.StatusBadRequest) } + stateStr := string(b) + stateProps := model.MapFromJson(strings.NewReader(stateStr)) expectedToken, err := a.GetOAuthStateToken(stateProps["token"]) @@ -743,21 +742,21 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest) } - cookieValue := "" - if cookie, err := r.Cookie(COOKIE_OAUTH); err != nil { + cookie, cookieErr := r.Cookie(COOKIE_OAUTH) + if cookieErr != nil { return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest) - } else { - cookieValue = cookie.Value } - expectedTokenExtra := generateOAuthStateTokenExtra(stateEmail, stateAction, cookieValue) + expectedTokenExtra := generateOAuthStateTokenExtra(stateEmail, stateAction, cookie.Value) if expectedTokenExtra != expectedToken.Extra { return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest) } - a.DeleteToken(expectedToken) + if err = a.DeleteToken(expectedToken); err != nil { + mlog.Error(err.Error()) + } - cookie := &http.Cookie{ + httpCookie := &http.Cookie{ Name: COOKIE_OAUTH, Value: "", Path: "/", @@ -765,7 +764,7 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service HttpOnly: true, } - http.SetCookie(w, cookie) + http.SetCookie(w, httpCookie) teamId := stateProps["team_id"] @@ -776,25 +775,30 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service p.Set("grant_type", model.ACCESS_TOKEN_GRANT_TYPE) p.Set("redirect_uri", redirectUri) - req, _ := http.NewRequest("POST", sso.TokenEndpoint, strings.NewReader(p.Encode())) + req, requestErr := http.NewRequest("POST", sso.TokenEndpoint, strings.NewReader(p.Encode())) + if requestErr != nil { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.token_failed.app_error", nil, requestErr.Error(), http.StatusInternalServerError) + } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Accept", "application/json") - var ar *model.AccessResponse - var bodyBytes []byte - if resp, err := a.HTTPService.MakeClient(true).Do(req); err != nil { - return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.token_failed.app_error", nil, err.Error(), http.StatusInternalServerError) - } else { - bodyBytes, _ = ioutil.ReadAll(resp.Body) - resp.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) + resp, serviceErr := a.HTTPService.MakeClient(true).Do(req) + if serviceErr != nil { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.token_failed.app_error", nil, serviceErr.Error(), http.StatusInternalServerError) + } - ar = model.AccessResponseFromJson(resp.Body) - consumeAndClose(resp) + bodyBytes, readErr := ioutil.ReadAll(resp.Body) + if readErr != nil { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.token_failed.app_error", nil, readErr.Error(), http.StatusInternalServerError) + } + resp.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) - if ar == nil || resp.StatusCode != http.StatusOK { - return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.bad_response.app_error", nil, "response_body="+string(bodyBytes), http.StatusInternalServerError) - } + ar := model.AccessResponseFromJson(resp.Body) + consumeAndClose(resp) + + if ar == nil || resp.StatusCode != http.StatusOK { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.bad_response.app_error", nil, "response_body="+string(bodyBytes), http.StatusInternalServerError) } if strings.ToLower(ar.TokenType) != model.ACCESS_TOKEN_TYPE { @@ -807,27 +811,35 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service p = url.Values{} p.Set("access_token", ar.AccessToken) - req, _ = http.NewRequest("GET", sso.UserApiEndpoint, strings.NewReader("")) + req, requestErr = http.NewRequest("GET", sso.UserApiEndpoint, strings.NewReader("")) + if requestErr != nil { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.service.app_error", map[string]interface{}{"Service": service}, requestErr.Error(), http.StatusInternalServerError) + } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+ar.AccessToken) - if resp, err := a.HTTPService.MakeClient(true).Do(req); err != nil { - return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.service.app_error", map[string]interface{}{"Service": service}, err.Error(), http.StatusInternalServerError) - } else { - bodyBytes, _ = ioutil.ReadAll(resp.Body) - if resp.StatusCode != http.StatusOK { - bodyString := string(bodyBytes) - mlog.Error("Error getting OAuth user: " + bodyString) - if service == model.SERVICE_GITLAB && resp.StatusCode == http.StatusForbidden && strings.Contains(bodyString, "Terms of Service") { - return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "oauth.gitlab.tos.error", nil, "", http.StatusBadRequest) - } - } - resp.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) - return resp.Body, teamId, stateProps, nil + resp, serviceErr = a.HTTPService.MakeClient(true).Do(req) + if serviceErr != nil { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.service.app_error", map[string]interface{}{"Service": service}, serviceErr.Error(), http.StatusInternalServerError) } + bodyBytes, readErr = ioutil.ReadAll(resp.Body) + if readErr != nil { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.token_failed.app_error", nil, readErr.Error(), http.StatusInternalServerError) + } + + if resp.StatusCode != http.StatusOK { + bodyString := string(bodyBytes) + mlog.Error("Error getting OAuth user: " + bodyString) + if service == model.SERVICE_GITLAB && resp.StatusCode == http.StatusForbidden && strings.Contains(bodyString, "Terms of Service") { + return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "oauth.gitlab.tos.error", nil, "", http.StatusBadRequest) + } + } + + resp.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) + return resp.Body, teamId, stateProps, nil } func (a *App) SwitchEmailToOAuth(w http.ResponseWriter, r *http.Request, email, password, code, service string) (string, *model.AppError) { @@ -835,13 +847,12 @@ func (a *App) SwitchEmailToOAuth(w http.ResponseWriter, r *http.Request, email, return "", model.NewAppError("emailToOAuth", "api.user.email_to_oauth.not_available.app_error", nil, "", http.StatusForbidden) } - var user *model.User - var err *model.AppError - if user, err = a.GetUserByEmail(email); err != nil { + user, err := a.GetUserByEmail(email) + if err != nil { return "", err } - if err := a.CheckPasswordAndAllCriteria(user, password, code); err != nil { + if err = a.CheckPasswordAndAllCriteria(user, password, code); err != nil { return "", err } @@ -851,13 +862,14 @@ func (a *App) SwitchEmailToOAuth(w http.ResponseWriter, r *http.Request, email, if service == model.USER_AUTH_SERVICE_SAML { return a.GetSiteURL() + "/login/sso/saml?action=" + model.OAUTH_ACTION_EMAIL_TO_SSO + "&email=" + utils.UrlEncode(email), nil - } else { - if authUrl, err := a.GetAuthorizationCode(w, r, service, stateProps, ""); err != nil { - return "", err - } else { - return authUrl, nil - } } + + authUrl, err := a.GetAuthorizationCode(w, r, service, stateProps, "") + if err != nil { + return "", err + } + + return authUrl, nil } func (a *App) SwitchOAuthToEmail(email, password, requesterId string) (string, *model.AppError) { @@ -865,9 +877,8 @@ func (a *App) SwitchOAuthToEmail(email, password, requesterId string) (string, * return "", model.NewAppError("oauthToEmail", "api.user.oauth_to_email.not_available.app_error", nil, "", http.StatusForbidden) } - var user *model.User - var err *model.AppError - if user, err = a.GetUserByEmail(email); err != nil { + user, err := a.GetUserByEmail(email) + if err != nil { return "", err } @@ -893,3 +904,7 @@ func (a *App) SwitchOAuthToEmail(email, password, requesterId string) (string, * return "/login?extra=signin_change", nil } + +func generateOAuthStateTokenExtra(email, action, cookie string) string { + return email + ":" + action + ":" + cookie +}