AuthN: Login error handling (#64239)

* Social: Fix type so it appears in error responses

* AuthN: construct errutil.Error from social.Error

* login: Check for errutil.Error and use public message

* Login: redirectURLWithErrorCookie for authn errors

Co-authored-by: Jo <joao.guerreiro@grafana.com>

pkg/api/login.go

@@ -25,6 +25,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
+	"github.com/grafana/grafana/pkg/util/errutil"
 	"github.com/grafana/grafana/pkg/web"
 )
 
@@ -441,5 +442,10 @@ func getLoginExternalError(err error) string {
 		return createTokenErr.ExternalErr
 	}
 
+	gfErr := &errutil.Error{}
+	if errors.As(err, gfErr) {
+		return gfErr.Public().Message
+	}
+
 	return err.Error()
 }

pkg/api/login_oauth.go

@@ -24,7 +24,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/grafana/grafana/pkg/util/errutil"
 	"github.com/grafana/grafana/pkg/web"
 )
 
@@ -90,7 +89,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *contextmodel.ReqContext) {
 		if code == "" {
 			redirect, err := hs.authnService.RedirectURL(ctx.Req.Context(), authn.ClientWithPrefix(name), req)
 			if err != nil {
-				hs.handleAuthnOAuthErr(ctx, "failed to generate oauth redirect url", err)
+				ctx.Redirect(hs.redirectURLWithErrorCookie(ctx, err))
 				return
 			}
 
@@ -109,7 +108,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *contextmodel.ReqContext) {
 		cookies.DeleteCookie(ctx.Resp, OauthStateCookieName, hs.CookieOptionsFromCfg)
 
 		if err != nil {
-			hs.handleAuthnOAuthErr(ctx, "failed to perform login for oauth request", err)
+			ctx.Redirect(hs.redirectURLWithErrorCookie(ctx, err))
 			return
 		}
 
@@ -380,19 +379,6 @@ func (hs *HTTPServer) hashStatecode(code, seed string) string {
 	return hex.EncodeToString(hashBytes[:])
 }
 
-func (hs *HTTPServer) handleAuthnOAuthErr(c *contextmodel.ReqContext, msg string, err error) {
-	gfErr := &errutil.Error{}
-	if errors.As(err, gfErr) {
-		if gfErr.Public().Message != "" {
-			c.Handle(hs.Cfg, gfErr.Public().StatusCode, gfErr.Public().Message, err)
-			return
-		}
-	}
-
-	c.Logger.Warn(msg, "err", err)
-	c.Redirect(hs.Cfg.AppSubURL + "/login")
-}
-
 type LoginError struct {
 	HttpStatus    int
 	PublicMessage string

pkg/login/social/common.go

@@ -12,7 +12,7 @@ import (
 )
 
 var (
-	errMissingGroupMembership = Error{"user not a member of one of the required groups"}
+	errMissingGroupMembership = &Error{"user not a member of one of the required groups"}
 )
 
 type httpGetResponse struct {

pkg/services/authn/clients/oauth.go

@@ -6,6 +6,7 @@ import (
 	"crypto/sha256"
 	"encoding/base64"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -42,11 +43,16 @@ var (
 	errOAuthInvalidState = errutil.NewBase(errutil.StatusUnauthorized, "auth.oauth.state.invalid", errutil.WithPublicMessage("Provided state does not match stored state"))
 
 	errOAuthTokenExchange = errutil.NewBase(errutil.StatusInternal, "auth.oauth.token.exchange", errutil.WithPublicMessage("Failed to get token from provider"))
+	errOAuthUserInfo      = errutil.NewBase(errutil.StatusInternal, "auth.oauth.userinfo.error")
 
-	errOAuthMissingRequiredEmail = errutil.NewBase(errutil.StatusUnauthorized, "auth.oauth.email.missing")
+	errOAuthMissingRequiredEmail = errutil.NewBase(errutil.StatusUnauthorized, "auth.oauth.email.missing", errutil.WithPublicMessage("Provider didn't return an email address"))
-	errOAuthEmailNotAllowed      = errutil.NewBase(errutil.StatusUnauthorized, "auth.oauth.email.not-allowed")
+	errOAuthEmailNotAllowed      = errutil.NewBase(errutil.StatusUnauthorized, "auth.oauth.email.not-allowed", errutil.WithPublicMessage("Required email domain not fulfilled"))
 )
 
+func fromSocialErr(err *social.Error) error {
+	return errutil.NewBase(errutil.StatusUnauthorized, "auth.oauth.userinfo.failed", errutil.WithPublicMessage(err.Error())).Errorf("%w", err)
+}
+
 var _ authn.RedirectClient = new(OAuth)
 
 func ProvideOAuth(
@@ -106,13 +112,17 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
 	// exchange auth code to a valid token
 	token, err := c.connector.Exchange(clientCtx, r.HTTPRequest.URL.Query().Get("code"), opts...)
 	if err != nil {
-		return nil, err
+		return nil, errOAuthTokenExchange.Errorf("failed to exchange code to token: %w", err)
 	}
 	token.TokenType = "Bearer"
 
 	userInfo, err := c.connector.UserInfo(c.connector.Client(clientCtx, token), token)
 	if err != nil {
-		return nil, errOAuthTokenExchange.Errorf("failed to exchange code to token: %w", err)
+		var sErr *social.Error
+		if errors.As(err, &sErr) {
+			return nil, fromSocialErr(sErr)
+		}
+		return nil, errOAuthUserInfo.Errorf("failed to get user info: %w", err)
 	}
 
 	if userInfo.Email == "" {