LDAP: Always synchronize Server Admin role through role sync if role sync is enabled (#58820)

fix a bug with role sync

pkg/services/ldap/ldap.go

@@ -456,6 +456,7 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*login.ExternalUserInf
 		return extUser, nil
 	}
 
+	isGrafanaAdmin := false
 	for _, group := range server.Config.Groups {
 		// only use the first match for each org
 		if extUser.OrgRoles[group.OrgId] != "" {
@@ -467,11 +468,12 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*login.ExternalUserInf
 				extUser.OrgRoles[group.OrgId] = group.OrgRole
 			}
 
-			if extUser.IsGrafanaAdmin == nil || !*extUser.IsGrafanaAdmin {
+			if !isGrafanaAdmin && (group.IsGrafanaAdmin != nil && *group.IsGrafanaAdmin) {
-				extUser.IsGrafanaAdmin = group.IsGrafanaAdmin
+				isGrafanaAdmin = true
 			}
 		}
 	}
+	extUser.IsGrafanaAdmin = &isGrafanaAdmin
 
 	// If there are group org mappings configured, but no matching mappings,
 	// the user will not be able to login and will be disabled

pkg/services/ldap/ldap_test.go

@@ -243,6 +243,12 @@ func TestServer_Users(t *testing.T) {
 				{Name: "username", Values: []string{"groot"}},
 				{Name: "name", Values: []string{"I am Groot"}},
 			}}}}
+		babyGrootDN := "dn=babygroot," + usersOU
+		babyGrootSearch := ldap.SearchResult{Entries: []*ldap.Entry{{DN: grootDN,
+			Attributes: []*ldap.EntryAttribute{
+				{Name: "username", Values: []string{"babygroot"}},
+				{Name: "name", Values: []string{"I am baby Groot"}},
+			}}}}
 		peterDN := "dn=peter," + usersOU
 		peterSearch := ldap.SearchResult{Entries: []*ldap.Entry{{DN: peterDN,
 			Attributes: []*ldap.EntryAttribute{
@@ -256,6 +262,12 @@ func TestServer_Users(t *testing.T) {
 				{Name: "member", Values: []string{grootDN}},
 			}}},
 		}
+		babyCreaturesDN := "dn=babycreatures," + groupsOU
+		babyGrootGroups := ldap.SearchResult{Entries: []*ldap.Entry{{DN: babyCreaturesDN,
+			Attributes: []*ldap.EntryAttribute{
+				{Name: "member", Values: []string{babyGrootDN}},
+			}}},
+		}
 		humansDN := "dn=humans," + groupsOU
 		peterGroups := ldap.SearchResult{Entries: []*ldap.Entry{{DN: humansDN,
 			Attributes: []*ldap.EntryAttribute{
@@ -269,6 +281,8 @@ func TestServer_Users(t *testing.T) {
 				switch request.Filter {
 				case "(|(username=groot))":
 					return &grootSearch, nil
+				case "(|(username=babygroot))":
+					return &babyGrootSearch, nil
 				case "(|(username=peter))":
 					return &peterSearch, nil
 				default:
@@ -278,6 +292,8 @@ func TestServer_Users(t *testing.T) {
 				switch request.Filter {
 				case "(member=groot)":
 					return &grootGroups, nil
+				case "(member=babygroot)":
+					return &babyGrootGroups, nil
 				case "(member=peter)":
 					return &peterGroups, nil
 				default:
@@ -288,6 +304,7 @@ func TestServer_Users(t *testing.T) {
 			}
 		})
 
+		isGrafanaAdmin := true
 		cfg := setting.NewCfg()
 		cfg.LDAPAuthEnabled = true
 
@@ -306,9 +323,14 @@ func TestServer_Users(t *testing.T) {
 					{
 						GroupDN:        creaturesDN,
 						OrgId:          2,
-						IsGrafanaAdmin: new(bool),
+						IsGrafanaAdmin: &isGrafanaAdmin,
 						OrgRole:        "Admin",
 					},
+					{
+						GroupDN: babyCreaturesDN,
+						OrgId:   2,
+						OrgRole: "Editor",
+					},
 				},
 			},
 			Connection: conn,
@@ -347,6 +369,22 @@ func TestServer_Users(t *testing.T) {
 			require.True(t, mappingExist)
 			require.Equal(t, roletype.RoleAdmin, role)
 			require.False(t, res[0].IsDisabled)
+			require.NotNil(t, res[0].IsGrafanaAdmin)
+			assert.True(t, *res[0].IsGrafanaAdmin)
+		})
+		t.Run("set Grafana Admin to false by default", func(t *testing.T) {
+			res, err := server.Users([]string{"babygroot"})
+			require.NoError(t, err)
+			require.Len(t, res, 1)
+			require.Equal(t, "I am baby Groot", res[0].Name)
+			require.ElementsMatch(t, res[0].Groups, []string{babyCreaturesDN})
+			require.Len(t, res[0].OrgRoles, 1)
+			role, mappingExist := res[0].OrgRoles[2]
+			require.True(t, mappingExist)
+			require.Equal(t, roletype.RoleEditor, role)
+			require.False(t, res[0].IsDisabled)
+			require.NotNil(t, res[0].IsGrafanaAdmin)
+			assert.False(t, *res[0].IsGrafanaAdmin)
 		})
 	})
 }