Auth: Remove Email Lookup from oauth integrations (#894)

Remove email lookup from oauth integrations

Co-authored-by: ievaVasiljeva <ieva.vasiljeva@grafana.com>

pkg/api/login_oauth.go

@@ -339,16 +339,17 @@ func (hs *HTTPServer) SyncUser(
 	connect social.SocialConnector,
 ) (*user.User, error) {
 	oauthLogger.Debug("Syncing Grafana user with corresponding OAuth profile")
+	lookupParams := loginservice.UserLookupParams{}
+	if hs.Cfg.OAuthAllowInsecureEmailLookup {
+		lookupParams.Email = &extUser.Email
+	}
+
 	// add/update user in Grafana
 	cmd := &loginservice.UpsertUserCommand{
-		ReqContext:    ctx,
+		ReqContext:       ctx,
-		ExternalUser:  extUser,
+		ExternalUser:     extUser,
-		SignupAllowed: connect.IsSignupAllowed(),
+		SignupAllowed:    connect.IsSignupAllowed(),
-		UserLookupParams: loginservice.UserLookupParams{
+		UserLookupParams: lookupParams,
-			Email:  &extUser.Email,
-			UserID: nil,
-			Login:  nil,
-		},
 	}
 
 	upsertedUser, err := hs.Login.UpsertUser(ctx.Req.Context(), cmd)

pkg/services/authn/clients/oauth.go

@@ -140,6 +140,11 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
 		return userInfo.Role, userInfo.IsGrafanaAdmin, nil
 	})
 
+	lookupParams := login.UserLookupParams{}
+	if c.cfg.OAuthAllowInsecureEmailLookup {
+		lookupParams.Email = &userInfo.Email
+	}
+
 	return &authn.Identity{
 		Login:          userInfo.Login,
 		Name:           userInfo.Name,
@@ -158,7 +163,7 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
 			AllowSignUp:     c.connector.IsSignupAllowed(),
 			// skip org role flag is checked and handled in the connector. For now we can skip the hook if no roles are passed
 			SyncOrgRoles: len(orgRoles) > 0,
-			LookUpParams: login.UserLookupParams{Email: &userInfo.Email},
+			LookUpParams: lookupParams,
 		},
 	}, nil
 }

pkg/services/authn/clients/oauth_test.go

@@ -20,9 +20,10 @@ import (
 
 func TestOAuth_Authenticate(t *testing.T) {
 	type testCase struct {
-		desc     string
+		desc                  string
-		req      *authn.Request
+		req                   *authn.Request
-		oauthCfg *social.OAuthInfo
+		oauthCfg              *social.OAuthInfo
+		allowInsecureTakeover bool
 
 		addStateCookie   bool
 		stateCookieValue string
@@ -127,6 +128,45 @@ func TestOAuth_Authenticate(t *testing.T) {
 				Role:   "Admin",
 				Groups: []string{"grp1", "grp2"},
 			},
+			expectedIdentity: &authn.Identity{
+				Email:      "some@email.com",
+				AuthModule: "oauth_azuread",
+				AuthID:     "123",
+				Name:       "name",
+				Groups:     []string{"grp1", "grp2"},
+				OAuthToken: &oauth2.Token{},
+				OrgRoles:   map[int64]org.RoleType{1: org.RoleAdmin},
+				ClientParams: authn.ClientParams{
+					SyncUser:        true,
+					SyncTeams:       true,
+					AllowSignUp:     true,
+					FetchSyncedUser: true,
+					SyncOrgRoles:    true,
+					LookUpParams:    login.UserLookupParams{},
+				},
+			},
+		},
+		{
+			desc: "should return identity for valid request - and lookup user by email",
+			req: &authn.Request{HTTPRequest: &http.Request{
+				Header: map[string][]string{},
+				URL:    mustParseURL("http://grafana.com/?state=some-state"),
+			},
+			},
+			oauthCfg:              &social.OAuthInfo{UsePKCE: true},
+			allowInsecureTakeover: true,
+			addStateCookie:        true,
+			stateCookieValue:      "some-state",
+			addPKCECookie:         true,
+			pkceCookieValue:       "some-pkce-value",
+			isEmailAllowed:        true,
+			userInfo: &social.BasicUserInfo{
+				Id:     "123",
+				Name:   "name",
+				Email:  "some@email.com",
+				Role:   "Admin",
+				Groups: []string{"grp1", "grp2"},
+			},
 			expectedIdentity: &authn.Identity{
 				Email:      "some@email.com",
 				AuthModule: "oauth_azuread",
@@ -151,6 +191,10 @@ func TestOAuth_Authenticate(t *testing.T) {
 		t.Run(tt.desc, func(t *testing.T) {
 			cfg := setting.NewCfg()
 
+			if tt.allowInsecureTakeover {
+				cfg.OAuthAllowInsecureEmailLookup = true
+			}
+
 			if tt.addStateCookie {
 				v := tt.stateCookieValue
 				if v != "" {

pkg/setting/setting.go

@@ -301,8 +301,9 @@ type Cfg struct {
 	AuthProxySyncTTL          int
 
 	// OAuth
-	OAuthAutoLogin    bool
+	OAuthAutoLogin                bool
-	OAuthCookieMaxAge int
+	OAuthCookieMaxAge             int
+	OAuthAllowInsecureEmailLookup bool
 
 	// JWT Auth
 	JWTAuthEnabled                 bool
@@ -1477,7 +1478,6 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) {
 	auth := iniFile.Section("auth")
 
 	cfg.LoginCookieName = valueAsString(auth, "login_cookie_name", "grafana_session")
-
 	const defaultMaxInactiveLifetime = "7d"
 	maxInactiveDurationVal := valueAsString(auth, "login_maximum_inactive_lifetime_duration", defaultMaxInactiveLifetime)
 	cfg.LoginMaxInactiveLifetime, err = gtime.ParseDuration(maxInactiveDurationVal)
@@ -1485,6 +1485,8 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) {
 		return err
 	}
 
+	cfg.OAuthAllowInsecureEmailLookup = auth.Key("oauth_allow_insecure_email_lookup").MustBool(false)
+
 	const defaultMaxLifetime = "30d"
 	maxLifetimeDurationVal := valueAsString(auth, "login_maximum_lifetime_duration", defaultMaxLifetime)
 	cfg.LoginMaxLifetime, err = gtime.ParseDuration(maxLifetimeDurationVal)