Fix wrap_handler() panic during OAuth login (#49671)

This commit is contained in:
Alexander Zobnin 2022-05-26 13:18:18 +03:00 committed by GitHub
parent a008a01315
commit 9b61d9eb1c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 20 deletions

View File

@ -13,7 +13,6 @@ import (
"golang.org/x/oauth2"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/login"
@ -68,7 +67,7 @@ func genPKCECode() (string, string, error) {
return string(ascii), pkce, nil
}
func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
loginInfo := models.LoginInfo{
AuthModule: "oauth",
}
@ -80,7 +79,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
HttpStatus: http.StatusNotFound,
PublicMessage: "OAuth not enabled",
})
return nil
return
}
connect, err := hs.SocialService.GetConnector(name)
@ -89,7 +88,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
HttpStatus: http.StatusNotFound,
PublicMessage: fmt.Sprintf("No OAuth with name %s configured", name),
})
return nil
return
}
errorParam := ctx.Query("error")
@ -97,7 +96,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
errorDesc := ctx.Query("error_description")
oauthLogger.Error("failed to login ", "error", errorParam, "errorDesc", errorDesc)
hs.handleOAuthLoginErrorWithRedirect(ctx, loginInfo, login.ErrProviderDeniedRequest, "error", errorParam, "errorDesc", errorDesc)
return nil
return
}
code := ctx.Query("code")
@ -129,7 +128,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
HttpStatus: http.StatusInternalServerError,
PublicMessage: "An internal error occurred",
})
return nil
return
}
hashedState := hs.hashStatecode(state, provider.ClientSecret)
@ -139,7 +138,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
}
ctx.Redirect(connect.AuthCodeURL(state, opts...))
return nil
return
}
cookieState := ctx.GetCookie(OauthStateCookieName)
@ -152,7 +151,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
HttpStatus: http.StatusInternalServerError,
PublicMessage: "login.OAuthLogin(missing saved state)",
})
return nil
return
}
queryState := hs.hashStatecode(ctx.Query("state"), provider.ClientSecret)
@ -162,7 +161,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
HttpStatus: http.StatusInternalServerError,
PublicMessage: "login.OAuthLogin(state mismatch)",
})
return nil
return
}
oauthClient, err := hs.SocialService.GetOAuthHttpClient(name)
@ -172,7 +171,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
HttpStatus: http.StatusInternalServerError,
PublicMessage: "login.OAuthLogin(" + err.Error() + ")",
})
return nil
return
}
oauthCtx := context.WithValue(context.Background(), oauth2.HTTPClient, oauthClient)
@ -194,7 +193,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
PublicMessage: "login.OAuthLogin(NewTransportWithCode)",
Err: err,
})
return nil
return
}
// token.TokenType was defaulting to "bearer", which is out of spec, so we explicitly set to "Bearer"
token.TokenType = "Bearer"
@ -217,7 +216,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
Err: err,
})
}
return nil
return
}
oauthLogger.Debug("OAuthLogin got user info", "userInfo", userInfo)
@ -225,26 +224,26 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
// validate that we got at least an email address
if userInfo.Email == "" {
hs.handleOAuthLoginErrorWithRedirect(ctx, loginInfo, login.ErrNoEmail)
return nil
return
}
// validate that the email is allowed to login to grafana
if !connect.IsEmailAllowed(userInfo.Email) {
hs.handleOAuthLoginErrorWithRedirect(ctx, loginInfo, login.ErrEmailNotAllowed)
return nil
return
}
loginInfo.ExternalUser = *hs.buildExternalUserInfo(token, userInfo, name)
loginInfo.User, err = hs.SyncUser(ctx, &loginInfo.ExternalUser, connect)
if err != nil {
hs.handleOAuthLoginErrorWithRedirect(ctx, loginInfo, err)
return nil
return
}
// login
if err := hs.loginUserWithUser(loginInfo.User, ctx); err != nil {
hs.handleOAuthLoginErrorWithRedirect(ctx, loginInfo, err)
return nil
return
}
loginInfo.HTTPStatus = http.StatusOK
@ -255,13 +254,12 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) response.Response {
if err := hs.ValidateRedirectTo(redirectTo); err == nil {
cookies.DeleteCookie(ctx.Resp, "redirect_to", hs.CookieOptionsFromCfg)
ctx.Redirect(redirectTo)
return nil
return
}
ctx.Logger.Debug("Ignored invalid redirect_to cookie value", "redirect_to", redirectTo)
}
ctx.Redirect(setting.AppSubUrl + "/")
return nil
}
// buildExternalUserInfo returns a ExternalUserInfo struct from OAuth user profile

View File

@ -12,7 +12,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/services/hooks"
"github.com/grafana/grafana/pkg/services/licensing"
@ -46,7 +45,7 @@ func setupOAuthTest(t *testing.T, cfg *setting.Cfg) *web.Mux {
m.UseMiddleware(web.Renderer(viewPath, "[[", "]]"))
m.Get("/login/:name", routing.Wrap(hs.OAuthLogin))
m.Get("/login/:name", hs.OAuthLogin)
return m
}