AuthN: add flag for org roles sync (#63507)

* AuthN: Add flag to control org role syncs

* JWT: Only sync org roles if the skip flag for jwt is false

* LDAP: Only sync org role if skip flag for ldap is false

* OAuth: Skip org roles sync if no roles were provided by upstream service

* Grafana: Set SyncOrgRoles to true for authentication through proxy with grafana as backend

pkg/services/authn/authn.go

@@ -47,6 +47,8 @@ type ClientParams struct {
 	FetchSyncedUser bool
 	// SyncTeams will sync the groups from identity to teams in grafana, enterprise only feature
 	SyncTeams bool
+	// SyncOrgRoles will sync the roles from the identity to orgs in grafana
+	SyncOrgRoles bool
 	// CacheAuthProxyKey  if this key is set we will try to cache the user id for proxy client
 	CacheAuthProxyKey string
 	// LookUpParams are the arguments used to look up the entity in the DB.

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

@@ -25,7 +25,7 @@ type OrgSync struct {
 }
 
 func (s *OrgSync) SyncOrgRolesHook(ctx context.Context, id *authn.Identity, _ *authn.Request) error {
-	if !id.ClientParams.SyncUser {
+	if !id.ClientParams.SyncOrgRoles {
 		return nil
 	}
 

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

@@ -79,7 +79,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) {
 					OrgRoles:       map[int64]roletype.RoleType{1: org.RoleAdmin, 2: org.RoleEditor},
 					IsGrafanaAdmin: ptrBool(false),
 					ClientParams: authn.ClientParams{
-						SyncUser: true,
+						SyncOrgRoles: true,
 						LookUpParams: login.UserLookupParams{
 							UserID: nil,
 							Email:  ptrString("test"),
@@ -97,7 +97,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) {
 				OrgID:          1, //set using org
 				IsGrafanaAdmin: ptrBool(false),
 				ClientParams: authn.ClientParams{
-					SyncUser: true,
+					SyncOrgRoles: true,
 					LookUpParams: login.UserLookupParams{
 						UserID: nil,
 						Email:  ptrString("test"),

pkg/services/authn/clients/grafana.go

@@ -38,6 +38,7 @@ func (c *Grafana) AuthenticateProxy(ctx context.Context, r *authn.Request, usern
 			SyncUser:        true,
 			SyncTeams:       true,
 			FetchSyncedUser: true,
+			SyncOrgRoles:    true,
 			AllowSignUp:     c.cfg.AuthProxyAutoSignUp,
 		},
 	}
@@ -69,15 +70,11 @@ func (c *Grafana) AuthenticateProxy(ctx context.Context, r *authn.Request, usern
 	}
 
 	if v, ok := additional[proxyFieldRole]; ok {
-		role := org.RoleType(v)
-		if role.IsValid() {
-			orgID := int64(1)
-			if c.cfg.AutoAssignOrg && c.cfg.AutoAssignOrgId > 0 {
-				orgID = int64(c.cfg.AutoAssignOrgId)
-			}
-			identity.OrgID = orgID
-			identity.OrgRoles = map[int64]org.RoleType{orgID: role}
-		}
+		orgRoles, isGrafanaAdmin, _ := getRoles(c.cfg, func() (org.RoleType, *bool, error) {
+			return org.RoleType(v), nil, nil
+		})
+		identity.OrgRoles = orgRoles
+		identity.IsGrafanaAdmin = isGrafanaAdmin
 	}
 
 	if v, ok := additional[proxyFieldGroups]; ok {

pkg/services/authn/clients/grafana_test.go

@@ -40,7 +40,6 @@ func TestGrafana_AuthenticateProxy(t *testing.T) {
 				proxyFieldEmail:  "email@email.com",
 			},
 			expectedIdentity: &authn.Identity{
-				OrgID:      1,
 				OrgRoles:   map[int64]org.RoleType{1: org.RoleViewer},
 				Login:      "test",
 				Name:       "name",
@@ -53,6 +52,7 @@ func TestGrafana_AuthenticateProxy(t *testing.T) {
 					SyncTeams:       true,
 					AllowSignUp:     true,
 					FetchSyncedUser: true,
+					SyncOrgRoles:    true,
 					LookUpParams: login.UserLookupParams{
 						Email: strPtr("email@email.com"),
 						Login: strPtr("test"),
@@ -71,9 +71,10 @@ func TestGrafana_AuthenticateProxy(t *testing.T) {
 				AuthModule: "authproxy",
 				AuthID:     "test@test.com",
 				ClientParams: authn.ClientParams{
-					SyncUser:    true,
-					SyncTeams:   true,
-					AllowSignUp: true,
+					SyncUser:     true,
+					SyncTeams:    true,
+					AllowSignUp:  true,
+					SyncOrgRoles: true,
 					LookUpParams: login.UserLookupParams{
 						Email: strPtr("test@test.com"),
 						Login: strPtr("test@test.com"),

pkg/services/authn/clients/jwt.go

@@ -10,7 +10,6 @@ import (
 	"github.com/jmespath/go-jmespath"
 
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/models/roletype"
 	"github.com/grafana/grafana/pkg/services/auth"
 	authJWT "github.com/grafana/grafana/pkg/services/auth/jwt"
 	"github.com/grafana/grafana/pkg/services/authn"
@@ -23,11 +22,11 @@ import (
 var _ authn.ContextAwareClient = new(JWT)
 
 var (
-	ErrJWTInvalid = errutil.NewBase(errutil.StatusUnauthorized,
+	errJWTInvalid = errutil.NewBase(errutil.StatusUnauthorized,
 		"jwt.invalid", errutil.WithPublicMessage("Failed to verify JWT"))
-	ErrJWTMissingClaim = errutil.NewBase(errutil.StatusUnauthorized,
+	errJWTMissingClaim = errutil.NewBase(errutil.StatusUnauthorized,
 		"jwt.missing_claim", errutil.WithPublicMessage("Missing mandatory claim in JWT"))
-	ErrJWTInvalidRole = errutil.NewBase(errutil.StatusForbidden,
+	errJWTInvalidRole = errutil.NewBase(errutil.StatusForbidden,
 		"jwt.invalid_role", errutil.WithPublicMessage("Invalid Role in claim"))
 )
 
@@ -55,13 +54,13 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi
 	claims, err := s.jwtService.Verify(ctx, jwtToken)
 	if err != nil {
 		s.log.Debug("Failed to verify JWT", "error", err)
-		return nil, ErrJWTInvalid.Errorf("failed to verify JWT: %w", err)
+		return nil, errJWTInvalid.Errorf("failed to verify JWT: %w", err)
 	}
 
 	sub, _ := claims["sub"].(string)
 	if sub == "" {
 		s.log.Warn("Got a JWT without the mandatory 'sub' claim", "error", err)
-		return nil, ErrJWTMissingClaim.Errorf("missing mandatory 'sub' claim in JWT")
+		return nil, errJWTMissingClaim.Errorf("missing mandatory 'sub' claim in JWT")
 	}
 
 	id := &authn.Identity{
@@ -71,6 +70,7 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi
 		ClientParams: authn.ClientParams{
 			SyncUser:        true,
 			FetchSyncedUser: true,
+			SyncOrgRoles:    !s.cfg.JWTAuthSkipOrgRoleSync,
 			AllowSignUp:     s.cfg.JWTAuthAutoSignUp,
 		}}
 
@@ -87,41 +87,34 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi
 		id.Name = name
 	}
 
-	var role roletype.RoleType
-	var grafanaAdmin bool
-	if !s.cfg.JWTAuthSkipOrgRoleSync {
-		role, grafanaAdmin = s.extractRoleAndAdmin(claims)
+	orgRoles, isGrafanaAdmin, err := getRoles(s.cfg, func() (org.RoleType, *bool, error) {
+		if s.cfg.JWTAuthSkipOrgRoleSync {
+			return "", nil, nil
+		}
+
+		role, grafanaAdmin := s.extractRoleAndAdmin(claims)
 		if s.cfg.JWTAuthRoleAttributeStrict && !role.IsValid() {
-			s.log.Warn("extracted Role is invalid", "role", role, "auth_id", id.AuthID)
-			return nil, ErrJWTInvalidRole.Errorf("invalid role claim in JWT: %s", role)
+			return "", nil, errJWTInvalidRole.Errorf("invalid role claim in JWT: %s", role)
 		}
 
-		if role.IsValid() {
-			var orgID int64
-			// FIXME (jguer): GetIDForNewUser already has the auto assign information
-			// just needs the org role. Find a meaningful way to pass this default
-			// role to it (that doesn't involve id.OrgRoles[0] = role)
-			if s.cfg.AutoAssignOrg && s.cfg.AutoAssignOrgId > 0 {
-				orgID = int64(s.cfg.AutoAssignOrgId)
-				s.log.Debug("The user has a role assignment and organization membership is auto-assigned",
-					"role", role, "orgId", orgID)
-			} else {
-				orgID = int64(1)
-				s.log.Debug("The user has a role assignment and organization membership is not auto-assigned",
-					"role", role, "orgId", orgID)
-			}
-
-			id.OrgRoles[orgID] = role
-			if s.cfg.JWTAuthAllowAssignGrafanaAdmin {
-				id.IsGrafanaAdmin = &grafanaAdmin
-			}
+		if !s.cfg.JWTAuthAllowAssignGrafanaAdmin {
+			return role, nil, nil
 		}
+
+		return role, &grafanaAdmin, nil
+	})
+
+	if err != nil {
+		return nil, err
 	}
 
+	id.OrgRoles = orgRoles
+	id.IsGrafanaAdmin = isGrafanaAdmin
+
 	if id.Login == "" && id.Email == "" {
 		s.log.Debug("Failed to get an authentication claim from JWT",
 			"login", id.Login, "email", id.Email)
-		return nil, ErrJWTMissingClaim.Errorf("missing login and email claim in JWT")
+		return nil, errJWTMissingClaim.Errorf("missing login and email claim in JWT")
 	}
 
 	return id, nil

pkg/services/authn/clients/jwt_test.go

@@ -52,6 +52,7 @@ func TestAuthenticateJWT(t *testing.T) {
 			SyncUser:        true,
 			AllowSignUp:     true,
 			FetchSyncedUser: true,
+			SyncOrgRoles:    true,
 			LookUpParams: login.UserLookupParams{
 				UserID: nil,
 				Email:  stringPtr("eai.doe@cor.po"),

pkg/services/authn/clients/ldap.go

@@ -41,7 +41,7 @@ func (c *LDAP) AuthenticateProxy(ctx context.Context, r *authn.Request, username
 		return nil, err
 	}
 
-	return identityFromLDAPInfo(r.OrgID, info, c.cfg.LDAPAllowSignup), nil
+	return c.identityFromLDAPInfo(r.OrgID, info), nil
 }
 
 func (c *LDAP) AuthenticatePassword(ctx context.Context, r *authn.Request, username, password string) (*authn.Identity, error) {
@@ -66,10 +66,10 @@ func (c *LDAP) AuthenticatePassword(ctx context.Context, r *authn.Request, usern
 		return nil, err
 	}
 
-	return identityFromLDAPInfo(r.OrgID, info, c.cfg.LDAPAllowSignup), nil
+	return c.identityFromLDAPInfo(r.OrgID, info), nil
 }
 
-func identityFromLDAPInfo(orgID int64, info *login.ExternalUserInfo, allowSignup bool) *authn.Identity {
+func (c *LDAP) identityFromLDAPInfo(orgID int64, info *login.ExternalUserInfo) *authn.Identity {
 	return &authn.Identity{
 		OrgID:          orgID,
 		OrgRoles:       info.OrgRoles,
@@ -85,7 +85,8 @@ func identityFromLDAPInfo(orgID int64, info *login.ExternalUserInfo, allowSignup
 			SyncTeams:           true,
 			EnableDisabledUsers: true,
 			FetchSyncedUser:     true,
-			AllowSignUp:         allowSignup,
+			SyncOrgRoles:        !c.cfg.LDAPSkipOrgRoleSync,
+			AllowSignUp:         c.cfg.LDAPAllowSignup,
 			LookUpParams: login.UserLookupParams{
 				Login: &info.Login,
 				Email: &info.Email,

pkg/services/authn/clients/ldap_test.go

@@ -52,6 +52,7 @@ func TestLDAP_AuthenticateProxy(t *testing.T) {
 					SyncTeams:           true,
 					EnableDisabledUsers: true,
 					FetchSyncedUser:     true,
+					SyncOrgRoles:        true,
 					LookUpParams: login.UserLookupParams{
 						Email: strPtr("test@test.com"),
 						Login: strPtr("test"),
@@ -116,6 +117,7 @@ func TestLDAP_AuthenticatePassword(t *testing.T) {
 					SyncTeams:           true,
 					EnableDisabledUsers: true,
 					FetchSyncedUser:     true,
+					SyncOrgRoles:        true,
 					LookUpParams: login.UserLookupParams{
 						Email: strPtr("test@test.com"),
 						Login: strPtr("test"),

pkg/services/authn/clients/oauth.go

@@ -123,22 +123,31 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
 		return nil, errOAuthEmailNotAllowed.Errorf("provided email is not allowed")
 	}
 
+	orgRoles, isGrafanaAdmin, _ := getRoles(c.cfg, func() (org.RoleType, *bool, error) {
+		if c.cfg.OAuthSkipOrgRoleUpdateSync {
+			return "", nil, nil
+		}
+		return userInfo.Role, userInfo.IsGrafanaAdmin, nil
+	})
+
 	return &authn.Identity{
 		Login:          userInfo.Login,
 		Name:           userInfo.Name,
 		Email:          userInfo.Email,
-		IsGrafanaAdmin: userInfo.IsGrafanaAdmin,
+		IsGrafanaAdmin: isGrafanaAdmin,
 		AuthModule:     c.moduleName,
 		AuthID:         userInfo.Id,
 		Groups:         userInfo.Groups,
 		OAuthToken:     token,
-		OrgRoles:       getOAuthOrgRole(userInfo, c.cfg),
+		OrgRoles:       orgRoles,
 		ClientParams: authn.ClientParams{
 			SyncUser:        true,
 			SyncTeams:       true,
 			FetchSyncedUser: true,
 			AllowSignUp:     c.connector.IsSignupAllowed(),
-			LookUpParams:    login.UserLookupParams{Email: &userInfo.Email},
+			// 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},
 		},
 	}, nil
 }
@@ -217,22 +226,3 @@ func hashOAuthState(state, secret, seed string) string {
 	hashBytes := sha256.Sum256([]byte(state + secret + seed))
 	return hex.EncodeToString(hashBytes[:])
 }
-
-func getOAuthOrgRole(userInfo *social.BasicUserInfo, cfg *setting.Cfg) map[int64]org.RoleType {
-	orgRoles := make(map[int64]org.RoleType, 0)
-	if cfg.OAuthSkipOrgRoleUpdateSync {
-		return orgRoles
-	}
-
-	if userInfo.Role == "" || !userInfo.Role.IsValid() {
-		return orgRoles
-	}
-
-	orgID := int64(1)
-	if cfg.AutoAssignOrg && cfg.AutoAssignOrgId > 0 {
-		orgID = int64(cfg.AutoAssignOrgId)
-	}
-
-	orgRoles[orgID] = userInfo.Role
-	return orgRoles
-}

pkg/services/authn/clients/oauth_test.go

@@ -139,6 +139,7 @@ func TestOAuth_Authenticate(t *testing.T) {
 					SyncTeams:       true,
 					AllowSignUp:     true,
 					FetchSyncedUser: true,
+					SyncOrgRoles:    true,
 					LookUpParams:    login.UserLookupParams{Email: strPtr("some@email.com")},
 				},
 			},

pkg/services/authn/clients/utils.go

@@ -0,0 +1,30 @@
+package clients
+
+import (
+	"github.com/grafana/grafana/pkg/services/org"
+	"github.com/grafana/grafana/pkg/setting"
+)
+
+// roleExtractor should return the org role, optional isGrafanaAdmin or an error
+type roleExtractor func() (org.RoleType, *bool, error)
+
+// getRoles only handles one org role for now, could be subject to change
+func getRoles(cfg *setting.Cfg, extract roleExtractor) (map[int64]org.RoleType, *bool, error) {
+	role, isGrafanaAdmin, err := extract()
+	orgRoles := make(map[int64]org.RoleType, 0)
+	if err != nil {
+		return orgRoles, nil, err
+	}
+
+	if role == "" || !role.IsValid() {
+		return orgRoles, nil, nil
+	}
+
+	orgID := int64(1)
+	if cfg.AutoAssignOrg && cfg.AutoAssignOrgId > 0 {
+		orgID = int64(cfg.AutoAssignOrgId)
+	}
+	orgRoles[orgID] = role
+
+	return orgRoles, isGrafanaAdmin, nil
+}