OAuth: Refactor user syncing (#26721)

* OAuth: Refactor user syncing

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Don't ignore error

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/api/login_oauth.go

@@ -96,7 +96,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
 		return
 	}
 
-	// handle call back
+	// handle callback
 	tr := &http.Transport{
 		Proxy: http.ProxyFromEnvironment,
 		TLSClientConfig: &tls.Config{
@@ -125,6 +125,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
 			ctx.Handle(500, "login.OAuthLogin(Failed to setup TlsClientCa)", nil)
 			return
 		}
+
 		caCertPool := x509.NewCertPool()
 		caCertPool.AppendCertsFromPEM(caCert)
 
@@ -172,8 +173,38 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
 		return
 	}
 
+	user, err := syncUser(ctx, token, userInfo, name, connect)
+	if err != nil {
+		hs.redirectWithError(ctx, err)
+		return
+	}
+
+	// login
+	if err := hs.loginUserWithUser(user, ctx); err != nil {
+		hs.redirectWithError(ctx, err)
+		return
+	}
+
+	metrics.MApiLoginOAuth.Inc()
+
+	if redirectTo, err := url.QueryUnescape(ctx.GetCookie("redirect_to")); err == nil && len(redirectTo) > 0 {
+		if err := hs.ValidateRedirectTo(redirectTo); err == nil {
+			middleware.DeleteCookie(ctx.Resp, "redirect_to", hs.CookieOptionsFromCfg)
+			ctx.Redirect(redirectTo)
+			return
+		}
+		log.Debugf("Ignored invalid redirect_to cookie value: %v", redirectTo)
+	}
+
+	ctx.Redirect(setting.AppSubUrl + "/")
+}
+
+// syncUser syncs a Grafana user profile with the corresponding OAuth profile.
+func syncUser(ctx *models.ReqContext, token *oauth2.Token, userInfo *social.BasicUserInfo, name string,
+	connect social.SocialConnector) (*models.User, error) {
+	oauthLogger.Debug("Syncing Grafana user with corresponding OAuth profile")
 	extUser := &models.ExternalUserInfo{
-		AuthModule: "oauth_" + name,
+		AuthModule: fmt.Sprintf("oauth_%s", name),
 		OAuthToken: token,
 		AuthId:     userInfo.Id,
 		Name:       userInfo.Name,
@@ -190,53 +221,35 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
 			var orgID int64
 			if setting.AutoAssignOrg && setting.AutoAssignOrgId > 0 {
 				orgID = int64(setting.AutoAssignOrgId)
+				logger.Debug("The user has a role assignment and organization membership is auto-assigned",
+					"role", userInfo.Role, "orgId", orgID)
 			} else {
 				orgID = int64(1)
+				logger.Debug("The user has a role assignment and organization membership is not auto-assigned",
+					"role", userInfo.Role, "orgId", orgID)
 			}
 			extUser.OrgRoles[orgID] = rt
 		}
 	}
 
-	// add/update user in grafana
+	// add/update user in Grafana
 	cmd := &models.UpsertUserCommand{
 		ReqContext:    ctx,
 		ExternalUser:  extUser,
 		SignupAllowed: connect.IsSignupAllowed(),
 	}
-
-	err = bus.Dispatch(cmd)
-	if err != nil {
-		hs.redirectWithError(ctx, err)
-		return
+	if err := bus.Dispatch(cmd); err != nil {
+		return nil, err
 	}
 
 	// Do not expose disabled status,
 	// just show incorrect user credentials error (see #17947)
 	if cmd.Result.IsDisabled {
 		oauthLogger.Warn("User is disabled", "user", cmd.Result.Login)
-		hs.redirectWithError(ctx, login.ErrInvalidCredentials)
-		return
-	}
-
-	// login
-	err = hs.loginUserWithUser(cmd.Result, ctx)
-	if err != nil {
-		hs.redirectWithError(ctx, err)
-		return
+		return nil, login.ErrInvalidCredentials
 	}
 
-	metrics.MApiLoginOAuth.Inc()
-
-	if redirectTo, _ := url.QueryUnescape(ctx.GetCookie("redirect_to")); len(redirectTo) > 0 {
-		if err := hs.ValidateRedirectTo(redirectTo); err == nil {
-			middleware.DeleteCookie(ctx.Resp, "redirect_to", hs.CookieOptionsFromCfg)
-			ctx.Redirect(redirectTo)
-			return
-		}
-		log.Debugf("Ignored invalid redirect_to cookie value: %v", redirectTo)
-	}
-
-	ctx.Redirect(setting.AppSubUrl + "/")
+	return cmd.Result, nil
 }
 
 func hashStatecode(code, seed string) string {