Skip to content

Commit a316f3f

Browse files
SteveTheEngineerSysoev, Vladimir
authored and
Sysoev, Vladimir
committed
Catch the error before the response is processed by goth. (go-gitea#20000)
The code introduced by go-gitea#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.
1 parent f9c0f34 commit a316f3f

File tree

1 file changed

+20
-12
lines changed

1 file changed

+20
-12
lines changed

routers/web/auth/oauth.go

+20-12
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"gitea.com/go-chi/binding"
3838
"github.com/golang-jwt/jwt/v4"
3939
"github.com/markbates/goth"
40+
"github.com/markbates/goth/gothic"
4041
)
4142

4243
const (
@@ -1098,24 +1099,31 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
10981099
func oAuth2UserLoginCallback(authSource *auth.Source, request *http.Request, response http.ResponseWriter) (*user_model.User, goth.User, error) {
10991100
oauth2Source := authSource.Cfg.(*oauth2.Source)
11001101

1102+
// Make sure that the response is not an error response.
1103+
errorName := request.FormValue("error")
1104+
1105+
if len(errorName) > 0 {
1106+
errorDescription := request.FormValue("error_description")
1107+
1108+
// Delete the goth session
1109+
err := gothic.Logout(response, request)
1110+
if err != nil {
1111+
return nil, goth.User{}, err
1112+
}
1113+
1114+
return nil, goth.User{}, errCallback{
1115+
Code: errorName,
1116+
Description: errorDescription,
1117+
}
1118+
}
1119+
1120+
// Proceed to authenticate through goth.
11011121
gothUser, err := oauth2Source.Callback(request, response)
11021122
if err != nil {
11031123
if err.Error() == "securecookie: the value is too long" || strings.Contains(err.Error(), "Data too long") {
11041124
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)
11051125
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)
11061126
}
1107-
// goth does not provide the original error message
1108-
// https://github.com/markbates/goth/issues/348
1109-
if strings.Contains(err.Error(), "server response missing access_token") || strings.Contains(err.Error(), "could not find a matching session for this request") {
1110-
errorCode := request.FormValue("error")
1111-
errorDescription := request.FormValue("error_description")
1112-
if errorCode != "" || errorDescription != "" {
1113-
return nil, goth.User{}, errCallback{
1114-
Code: errorCode,
1115-
Description: errorDescription,
1116-
}
1117-
}
1118-
}
11191127
return nil, goth.User{}, err
11201128
}
11211129

0 commit comments

Comments
 (0)