Separate authn flow from analytics (#68327)

* separate authn flow from analytics

* lint fix

pkg/api/dtos/models.go

@@ -29,7 +29,6 @@ type LoginCommand struct {
 type CurrentUser struct {
 	IsSignedIn                 bool               `json:"isSignedIn"`
 	Id                         int64              `json:"id"`
-	ExternalUserId             string             `json:"externalUserId"`
 	Login                      string             `json:"login"`
 	Email                      string             `json:"email"`
 	Name                       string             `json:"name"`

pkg/api/index.go

@@ -1,6 +1,11 @@
 package api
 
 import (
+	"context"
+	"crypto/hmac"
+	"crypto/sha256"
+	"encoding/hex"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -12,7 +17,9 @@ import (
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
+	"github.com/grafana/grafana/pkg/services/login"
 	pref "github.com/grafana/grafana/pkg/services/preference"
+	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -92,7 +99,6 @@ func (hs *HTTPServer) setIndexViewData(c *contextmodel.ReqContext) (*dtos.IndexV
 			IsSignedIn:                 c.IsSignedIn,
 			Login:                      c.Login,
 			Email:                      c.Email,
-			ExternalUserId:             c.SignedInUser.ExternalAuthID,
 			Name:                       c.Name,
 			OrgCount:                   c.OrgCount,
 			OrgId:                      c.OrgID,
@@ -108,10 +114,7 @@ func (hs *HTTPServer) setIndexViewData(c *contextmodel.ReqContext) (*dtos.IndexV
 			Language:                   language,
 			HelpFlags1:                 c.HelpFlags1,
 			HasEditPermissionInFolders: hasEditPerm,
-			Analytics: dtos.AnalyticsSettings{
+			Analytics:                  hs.buildUserAnalyticsSettings(c.Req.Context(), c.SignedInUser),
-				Identifier:         c.SignedInUser.Analytics.Identifier,
-				IntercomIdentifier: c.SignedInUser.Analytics.IntercomIdentifier,
-			},
 		},
 		Settings:                            settings,
 		ThemeType:                           theme.Type,
@@ -167,6 +170,35 @@ func (hs *HTTPServer) setIndexViewData(c *contextmodel.ReqContext) (*dtos.IndexV
 	return &data, nil
 }
 
+func (hs *HTTPServer) buildUserAnalyticsSettings(ctx context.Context, signedInUser *user.SignedInUser) dtos.AnalyticsSettings {
+	identifier := signedInUser.Email + "@" + setting.AppUrl
+
+	authInfo, err := hs.authInfoService.GetAuthInfo(ctx, &login.GetAuthInfoQuery{UserId: signedInUser.UserID})
+	if err != nil && !errors.Is(err, user.ErrUserNotFound) {
+		hs.log.Error("Failed to get auth info for analytics", "error", err)
+	}
+
+	if authInfo != nil && authInfo.AuthModule == login.GrafanaComAuthModule {
+		identifier = authInfo.AuthId
+	}
+
+	return dtos.AnalyticsSettings{
+		Identifier:         identifier,
+		IntercomIdentifier: hashUserIdentifier(identifier, hs.Cfg.IntercomSecret),
+	}
+}
+
+func hashUserIdentifier(identifier string, secret string) string {
+	if secret == "" {
+		return ""
+	}
+
+	key := []byte(secret)
+	h := hmac.New(sha256.New, key)
+	h.Write([]byte(identifier))
+	return hex.EncodeToString(h.Sum(nil))
+}
+
 func (hs *HTTPServer) Index(c *contextmodel.ReqContext) {
 	data, err := hs.setIndexViewData(c)
 	if err != nil {

pkg/services/authn/authn.go

@@ -266,8 +266,6 @@ func (i *Identity) SignedInUser() *user.SignedInUser {
 		OrgID:          i.OrgID,
 		OrgName:        i.OrgName,
 		OrgRole:        i.Role(),
-		ExternalAuthModule: i.AuthModule,
-		ExternalAuthID:     i.AuthID,
 		Login:          i.Login,
 		Name:           i.Name,
 		Email:          i.Email,
@@ -327,8 +325,6 @@ func IdentityFromSignedInUser(id string, usr *user.SignedInUser, params ClientPa
 		Teams:          usr.Teams,
 		ClientParams:   params,
 		Permissions:    usr.Permissions,
-		AuthModule:     usr.ExternalAuthModule,
-		AuthID:         usr.ExternalAuthID,
 	}
 }
 

pkg/services/authn/authnimpl/sync/user_sync.go

@@ -392,8 +392,6 @@ func syncSignedInUserToIdentity(usr *user.SignedInUser, identity *authn.Identity
 	identity.LastSeenAt = usr.LastSeenAt
 	identity.IsDisabled = usr.IsDisabled
 	identity.IsGrafanaAdmin = &usr.IsGrafanaAdmin
-	identity.AuthID = usr.ExternalAuthID
-	identity.AuthModule = usr.ExternalAuthModule
 }
 
 func shouldUpdateLastSeen(t time.Time) bool {

pkg/services/contexthandler/contexthandler.go

@@ -3,9 +3,6 @@ package contexthandler
 
 import (
 	"context"
-	"crypto/hmac"
-	"crypto/sha256"
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"net/http"
@@ -113,25 +110,6 @@ func FromContext(c context.Context) *contextmodel.ReqContext {
 	return nil
 }
 
-func hashUserIdentifier(identifier string, secret string) string {
-	key := []byte(secret)
-	h := hmac.New(sha256.New, key)
-	h.Write([]byte(identifier))
-	return hex.EncodeToString(h.Sum(nil))
-}
-
-func setSignedInUser(reqContext *contextmodel.ReqContext, identity *authn.Identity, intercomSecret string) {
-	reqContext.SignedInUser = identity.SignedInUser()
-	if identity.AuthID != "" {
-		reqContext.SignedInUser.Analytics.Identifier = identity.AuthID
-	} else {
-		reqContext.SignedInUser.Analytics.Identifier = identity.Email + "@" + setting.AppUrl
-	}
-	if intercomSecret != "" {
-		reqContext.SignedInUser.Analytics.IntercomIdentifier = hashUserIdentifier(identity.AuthID, intercomSecret)
-	}
-}
-
 // Middleware provides a middleware to initialize the request context.
 func (h *ContextHandler) Middleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -172,8 +150,8 @@ func (h *ContextHandler) Middleware(next http.Handler) http.Handler {
 				// Hack: set all errors on LookupTokenErr, so we can check it in auth middlewares
 				reqContext.LookupTokenErr = err
 			} else {
+				reqContext.SignedInUser = identity.SignedInUser()
 				reqContext.UserToken = identity.SessionToken
-				setSignedInUser(reqContext, identity, h.Cfg.IntercomSecret)
 				reqContext.IsSignedIn = !identity.IsAnonymous
 				reqContext.AllowAnonymous = identity.IsAnonymous
 				reqContext.IsRenderCall = identity.AuthModule == login.RenderModule

pkg/services/user/model.go

@@ -203,8 +203,6 @@ type SignedInUser struct {
 	OrgID            int64 `xorm:"org_id"`
 	OrgName          string
 	OrgRole          roletype.RoleType
-	ExternalAuthModule string
-	ExternalAuthID     string `xorm:"external_auth_id"`
 	Login            string
 	Name             string
 	Email            string
@@ -217,7 +215,6 @@ type SignedInUser struct {
 	HelpFlags1       HelpFlags1
 	LastSeenAt       time.Time
 	Teams            []int64
-	Analytics          AnalyticsSettings
 	// Permissions grouped by orgID and actions
 	Permissions map[int64]map[string][]string `json:"-"`
 }

pkg/services/user/userimpl/store.go

@@ -397,14 +397,11 @@ func (ss *sqlStore) GetSignedInUser(ctx context.Context, query *user.GetSignedIn
 		u.help_flags1         as help_flags1,
 		u.last_seen_at        as last_seen_at,
 		(SELECT COUNT(*) FROM org_user where org_user.user_id = u.id) as org_count,
-		user_auth.auth_module as external_auth_module,
-		user_auth.auth_id     as external_auth_id,
 		org.name              as org_name,
 		org_user.role         as org_role,
 		org.id                as org_id,
 		u.is_service_account  as is_service_account
 		FROM ` + ss.dialect.Quote("user") + ` as u
-		LEFT OUTER JOIN user_auth on user_auth.user_id = u.id
 		LEFT OUTER JOIN org_user on org_user.org_id = ` + orgId + ` and org_user.user_id = u.id
 		LEFT OUTER JOIN org on org.id = org_user.org_id `
 
@@ -438,11 +435,6 @@ func (ss *sqlStore) GetSignedInUser(ctx context.Context, query *user.GetSignedIn
 			signedInUser.OrgName = "Org missing"
 		}
 
-		if signedInUser.ExternalAuthModule != "oauth_grafana_com" {
-			signedInUser.ExternalAuthID = ""
-		}
-
-		signedInUser.Analytics = buildUserAnalyticsSettings(signedInUser, ss.cfg.IntercomSecret)
 		return nil
 	})
 	return &signedInUser, err

pkg/services/user/userimpl/user.go

@@ -2,9 +2,6 @@ package userimpl
 
 import (
 	"context"
-	"crypto/hmac"
-	"crypto/sha256"
-	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"strings"
@@ -466,25 +463,3 @@ func (s *Service) supportBundleCollector() supportbundles.Collector {
 		Fn:                collectorFn,
 	}
 }
-
-func hashUserIdentifier(identifier string, secret string) string {
-	key := []byte(secret)
-	h := hmac.New(sha256.New, key)
-	h.Write([]byte(identifier))
-	return hex.EncodeToString(h.Sum(nil))
-}
-
-func buildUserAnalyticsSettings(signedInUser user.SignedInUser, intercomSecret string) user.AnalyticsSettings {
-	var settings user.AnalyticsSettings
-
-	if signedInUser.ExternalAuthID != "" {
-		settings.Identifier = signedInUser.ExternalAuthID
-	} else {
-		settings.Identifier = signedInUser.Email + "@" + setting.AppUrl
-	}
-
-	if intercomSecret != "" {
-		settings.IntercomIdentifier = hashUserIdentifier(settings.Identifier, intercomSecret)
-	}
-	return settings
-}

pkg/tests/web/index_view_test.go

@@ -1,16 +1,23 @@
 package web
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/login"
+	databaseAuthInfo "github.com/grafana/grafana/pkg/services/login/authinfoservice/database"
+	"github.com/grafana/grafana/pkg/services/secrets/database"
+	secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/tests/testinfra"
 )
@@ -44,38 +51,6 @@ func TestIntegrationIndexView(t *testing.T) {
 		assert.Empty(t, resp.Header.Get("Content-Security-Policy"))
 		assert.Regexp(t, `<script nonce=""`, html)
 	})
-
-	t.Run("Test the exposed user data contains the analytics identifiers", func(t *testing.T) {
-		grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
-			EnableFeatureToggles: []string{"authnService"},
-		})
-
-		addr, store := testinfra.StartGrafana(t, grafDir, cfgPath)
-		createdUser := testinfra.CreateUser(t, store, user.CreateUserCommand{
-			Login:    "admin",
-			Password: "admin",
-			Email:    "admin@grafana.com",
-			OrgID:    1,
-		})
-
-		// insert user_auth relationship
-		query := fmt.Sprintf(`INSERT INTO "user_auth" ("user_id", "auth_module", "auth_id", "created") VALUES ('%d', 'oauth_grafana_com', 'test-id-oauth-grafana', '2023-03-13 14:08:11')`, createdUser.ID)
-		_, err := store.GetEngine().Exec(query)
-		require.NoError(t, err)
-
-		// nolint:bodyclose
-		response, html := makeRequest(t, addr, "admin", "admin")
-		assert.Equal(t, 200, response.StatusCode)
-
-		// parse User.Analytics HTML view into user.AnalyticsSettings model
-		parsedHTML := strings.Split(html, "analytics\":")[1]
-		parsedHTML = strings.Split(parsedHTML, "},\n")[0]
-
-		var analyticsSettings user.AnalyticsSettings
-		require.NoError(t, json.Unmarshal([]byte(parsedHTML), &analyticsSettings))
-
-		require.Equal(t, "test-id-oauth-grafana", analyticsSettings.Identifier)
-	})
 }
 
 func makeRequest(t *testing.T, addr, username, passwowrd string) (*http.Response, string) {
@@ -102,7 +77,99 @@ func makeRequest(t *testing.T, addr, username, passwowrd string) (*http.Response
 	var b strings.Builder
 	_, err = io.Copy(&b, resp.Body)
 	require.NoError(t, err)
-	require.Equal(t, 200, resp.StatusCode)
+	require.Equal(t, 200, resp.StatusCode, b.String())
 
 	return resp, b.String()
 }
+
+// TestIntegrationIndexViewAnalytics tests the Grafana index view has the analytics identifiers.
+func TestIntegrationIndexViewAnalytics(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+
+	testCases := []struct {
+		name           string
+		authModule     string
+		setID          string
+		wantIdentifier string
+		secondModule   string
+		secondID       string
+	}{
+		{
+			name:           "gcom only and last",
+			authModule:     login.GrafanaComAuthModule,
+			setID:          "test-id-oauth-grafana",
+			wantIdentifier: "test-id-oauth-grafana",
+		},
+		{
+			name:           "okta only and last",
+			authModule:     login.OktaAuthModule,
+			setID:          "uuid-1234-5678-9101",
+			wantIdentifier: "admin@grafana.com@http://localhost:3000/",
+		},
+		{
+			name:           "gcom last",
+			authModule:     login.OktaAuthModule,
+			setID:          "test-id-oauth-grafana",
+			wantIdentifier: "60042",
+			secondModule:   login.GrafanaComAuthModule,
+			secondID:       "60042",
+		},
+	}
+
+	// can be removed once ff is removed
+	testCaseFeatures := map[string][]string{"none": {}, "authnService": {featuremgmt.FlagAuthnService}}
+
+	for k, tcFeatures := range testCaseFeatures {
+		for _, tc := range testCases {
+			t.Run(tc.name+"-"+k, func(t *testing.T) {
+				grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
+					EnableFeatureToggles: tcFeatures,
+				})
+				addr, store := testinfra.StartGrafana(t, grafDir, cfgPath)
+				createdUser := testinfra.CreateUser(t, store, user.CreateUserCommand{
+					Login:    "admin",
+					Password: "admin",
+					Email:    "admin@grafana.com",
+					OrgID:    1,
+				})
+
+				secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(store))
+				authInfoStore := databaseAuthInfo.ProvideAuthInfoStore(store, secretsService, nil)
+
+				// insert user_auth relationship
+				err := authInfoStore.SetAuthInfo(context.Background(), &login.SetAuthInfoCommand{
+					AuthModule: tc.authModule,
+					AuthId:     tc.setID,
+					UserId:     createdUser.ID,
+				})
+				require.NoError(t, err)
+				if tc.secondModule != "" {
+					// wait for the user_auth relationship to be inserted. TOFIX: this is a hack
+					time.Sleep(1 * time.Second)
+					err := authInfoStore.SetAuthInfo(context.Background(), &login.SetAuthInfoCommand{
+						AuthModule: tc.secondModule,
+						AuthId:     tc.secondID,
+						UserId:     createdUser.ID,
+					})
+					require.NoError(t, err)
+				}
+
+				// nolint:bodyclose
+				response, html := makeRequest(t, addr, "admin", "admin")
+				assert.Equal(t, http.StatusOK, response.StatusCode)
+
+				// parse User.Analytics HTML view into user.AnalyticsSettings model
+				parsedHTML := strings.Split(html, "analytics\":")[1]
+				parsedHTML = strings.Split(parsedHTML, "},\n")[0]
+
+				var analyticsSettings user.AnalyticsSettings
+				require.NoError(t, json.Unmarshal([]byte(parsedHTML), &analyticsSettings))
+
+				require.NotEmpty(t, analyticsSettings.IntercomIdentifier)
+				require.Equal(t, tc.wantIdentifier, analyticsSettings.Identifier)
+			})
+		}
+	}
+}