From 1e2c2edab6d4233b81b8c21a36c426dc99084bbd Mon Sep 17 00:00:00 2001
From: SteveTheEngineer <sthesengineer@gmail.com>
Date: Mon, 20 Jun 2022 18:37:54 +0300
Subject: [PATCH] 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.
---
routers/web/auth/oauth.go | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go
index f70d69509c..714ae96fa4 100644
--- a/routers/web/auth/oauth.go
+++ b/routers/web/auth/oauth.go
@@ -37,6 +37,7 @@ import (
"gitea.com/go-chi/binding"
"github.com/golang-jwt/jwt/v4"
"github.com/markbates/goth"
+ "github.com/markbates/goth/gothic"
)
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) {
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)
if err != nil {
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)
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
}