test1234
/
gitea
forked from gitea/gitea
1
0
Fork 0
Code Pull Requests Activity

Catch the error before the response is processed by goth. (#20000) 

The code introduced by #18185 gets the error from response after it was processed by goth.

That is incorrect, as goth (and golang.org/x/oauth) doesn't really care about the error, and it sends a token request with an empty authorization code to the server anyway, which always results in a `oauth2: cannot fetch token: 400 Bad Request` error from goth.
It means that unless the "state" parameter is omitted from the error response (which is required to be present, according to [RFC 6749, Section 4.1.2.1](https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1)) or the page is reloaded (makes the session invalid), a 500 Internal Server Error page will be displayed.
This fixes it by handling the error before the request is passed to goth.
This commit is contained in:
SteveTheEngineer 2022-06-20 18:37:54 +03:00 committed by GitHub
parent 0649c54275
commit 1e2c2edab6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 20 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
routers/web/auth/oauth.go
View File

@ -37,6 +37,7 @@ import (
"gitea.com/go-chi/binding" "gitea.com/go-chi/binding"
"github.com/golang-jwt/jwt/v4" "github.com/golang-jwt/jwt/v4"
"github.com/markbates/goth" "github.com/markbates/goth"
"github.com/markbates/goth/gothic"
) )
const ( const (
@ -1098,24 +1099,31 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
func oAuth2UserLoginCallback(authSource *auth.Source, request *http.Request, response http.ResponseWriter) (*user_model.User, goth.User, error) { func oAuth2UserLoginCallback(authSource *auth.Source, request *http.Request, response http.ResponseWriter) (*user_model.User, goth.User, error) {
oauth2Source := authSource.Cfg.(*oauth2.Source) oauth2Source := authSource.Cfg.(*oauth2.Source)
// Make sure that the response is not an error response.
errorName := request.FormValue("error")
if len(errorName) > 0 {
errorDescription := request.FormValue("error_description")
// Delete the goth session
err := gothic.Logout(response, request)
if err != nil {
return nil, goth.User{}, err
}
return nil, goth.User{}, errCallback{
Code: errorName,
Description: errorDescription,
}
}
// Proceed to authenticate through goth.
gothUser, err := oauth2Source.Callback(request, response) gothUser, err := oauth2Source.Callback(request, response)
if err != nil { if err != nil {
if err.Error() == "securecookie: the value is too long" || strings.Contains(err.Error(), "Data too long") { if err.Error() == "securecookie: the value is too long" || strings.Contains(err.Error(), "Data too long") {
log.Error("OAuth2 Provider %s returned too long a token. Current max: %d. Either increase the [OAuth2] MAX_TOKEN_LENGTH or reduce the information returned from the OAuth2 provider", authSource.Name, setting.OAuth2.MaxTokenLength) log.Error("OAuth2 Provider %s returned too long a token. Current max: %d. Either increase the [OAuth2] MAX_TOKEN_LENGTH or reduce the information returned from the OAuth2 provider", authSource.Name, setting.OAuth2.MaxTokenLength)
err = fmt.Errorf("OAuth2 Provider %s returned too long a token. Current max: %d. Either increase the [OAuth2] MAX_TOKEN_LENGTH or reduce the information returned from the OAuth2 provider", authSource.Name, setting.OAuth2.MaxTokenLength) err = fmt.Errorf("OAuth2 Provider %s returned too long a token. Current max: %d. Either increase the [OAuth2] MAX_TOKEN_LENGTH or reduce the information returned from the OAuth2 provider", authSource.Name, setting.OAuth2.MaxTokenLength)
} }
// goth does not provide the original error message
// https://github.com/markbates/goth/issues/348
if strings.Contains(err.Error(), "server response missing access_token") || strings.Contains(err.Error(), "could not find a matching session for this request") {
errorCode := request.FormValue("error")
errorDescription := request.FormValue("error_description")
if errorCode != "" || errorDescription != "" {
return nil, goth.User{}, errCallback{
Code: errorCode,
Description: errorDescription,
}
}
}
return nil, goth.User{}, err return nil, goth.User{}, err
} }