From 5ec0f82baa02fc78bafc1bf1dc752fadfe7aae56 Mon Sep 17 00:00:00 2001 From: Jo Date: Mon, 15 May 2023 18:38:54 +0200 Subject: [PATCH] Separate authn flow from analytics (#68327) * separate authn flow from analytics * lint fix --- pkg/api/dtos/models.go | 1 - pkg/api/index.go | 42 +++++- pkg/services/authn/authn.go | 34 ++--- .../authn/authnimpl/sync/user_sync.go | 2 - pkg/services/contexthandler/contexthandler.go | 24 +--- pkg/services/user/model.go | 35 +++-- pkg/services/user/userimpl/store.go | 8 -- pkg/services/user/userimpl/user.go | 25 ---- pkg/tests/web/index_view_test.go | 133 +++++++++++++----- 9 files changed, 169 insertions(+), 135 deletions(-) diff --git a/pkg/api/dtos/models.go b/pkg/api/dtos/models.go index 8339940ac78..b092cc75022 100644 --- a/pkg/api/dtos/models.go +++ b/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"` diff --git a/pkg/api/index.go b/pkg/api/index.go index 456a60dcd8c..987e5df8576 100644 --- a/pkg/api/index.go +++ b/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{ - Identifier: c.SignedInUser.Analytics.Identifier, - IntercomIdentifier: c.SignedInUser.Analytics.IntercomIdentifier, - }, + Analytics: hs.buildUserAnalyticsSettings(c.Req.Context(), c.SignedInUser), }, 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 { diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index 2f7da220426..f2b84ea7559 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -262,23 +262,21 @@ func (i *Identity) SignedInUser() *user.SignedInUser { } u := &user.SignedInUser{ - UserID: 0, - OrgID: i.OrgID, - OrgName: i.OrgName, - OrgRole: i.Role(), - ExternalAuthModule: i.AuthModule, - ExternalAuthID: i.AuthID, - Login: i.Login, - Name: i.Name, - Email: i.Email, - OrgCount: i.OrgCount, - IsGrafanaAdmin: isGrafanaAdmin, - IsAnonymous: i.IsAnonymous, - IsDisabled: i.IsDisabled, - HelpFlags1: i.HelpFlags1, - LastSeenAt: i.LastSeenAt, - Teams: i.Teams, - Permissions: i.Permissions, + UserID: 0, + OrgID: i.OrgID, + OrgName: i.OrgName, + OrgRole: i.Role(), + Login: i.Login, + Name: i.Name, + Email: i.Email, + OrgCount: i.OrgCount, + IsGrafanaAdmin: isGrafanaAdmin, + IsAnonymous: i.IsAnonymous, + IsDisabled: i.IsDisabled, + HelpFlags1: i.HelpFlags1, + LastSeenAt: i.LastSeenAt, + Teams: i.Teams, + Permissions: i.Permissions, } namespace, id := i.NamespacedID() @@ -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, } } diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index 20c120f262f..b4b38e2dbcc 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/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 { diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 5c3760af292..6ba160c8e39 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/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 diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index 989b2ecc7b4..d0ff601382b 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -199,25 +199,22 @@ type AnalyticsSettings struct { } type SignedInUser struct { - UserID int64 `xorm:"user_id"` - OrgID int64 `xorm:"org_id"` - OrgName string - OrgRole roletype.RoleType - ExternalAuthModule string - ExternalAuthID string `xorm:"external_auth_id"` - Login string - Name string - Email string - ApiKeyID int64 `xorm:"api_key_id"` - IsServiceAccount bool `xorm:"is_service_account"` - OrgCount int - IsGrafanaAdmin bool - IsAnonymous bool - IsDisabled bool - HelpFlags1 HelpFlags1 - LastSeenAt time.Time - Teams []int64 - Analytics AnalyticsSettings + UserID int64 `xorm:"user_id"` + OrgID int64 `xorm:"org_id"` + OrgName string + OrgRole roletype.RoleType + Login string + Name string + Email string + ApiKeyID int64 `xorm:"api_key_id"` + IsServiceAccount bool `xorm:"is_service_account"` + OrgCount int + IsGrafanaAdmin bool + IsAnonymous bool + IsDisabled bool + HelpFlags1 HelpFlags1 + LastSeenAt time.Time + Teams []int64 // Permissions grouped by orgID and actions Permissions map[int64]map[string][]string `json:"-"` } diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index f59c6e40302..cbd3b5a4ce4 100644 --- a/pkg/services/user/userimpl/store.go +++ b/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 diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index f5aded11836..9bb06ef129b 100644 --- a/pkg/services/user/userimpl/user.go +++ b/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 -} diff --git a/pkg/tests/web/index_view_test.go b/pkg/tests/web/index_view_test.go index 7a17a816dff..fee29a2664a 100644 --- a/pkg/tests/web/index_view_test.go +++ b/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, `