fix: KeycloakClient service account users groups aren't being populated correctly

Signed-off-by: Douglass Kirkley <doug.kirkley@gmail.com>

Author: dougkirkley

internal/controller/keycloakclient/chain/service_account.go

@@ -38,19 +38,19 @@ func (el *ServiceAccount) Serve(_ context.Context, keycloakClient *keycloakApi.K
 		return errors.Wrap(err, "unable to sync service account roles")
 	}
 
+	if keycloakClient.Spec.ServiceAccount.Groups != nil {
+		if err := el.keycloakApiClient.SyncServiceAccountGroups(realmName,
+			keycloakClient.Status.ClientID, keycloakClient.Spec.ServiceAccount.Groups, addOnly); err != nil {
+			return errors.Wrap(err, "unable to sync service account groups")
+		}
+	}
+
 	if keycloakClient.Spec.ServiceAccount.Attributes != nil {
 		if err := el.keycloakApiClient.SetServiceAccountAttributes(realmName, keycloakClient.Status.ClientID,
 			keycloakClient.Spec.ServiceAccount.Attributes, addOnly); err != nil {
 			return errors.Wrap(err, "unable to set service account attributes")
 		}
 	}
 
-	if keycloakClient.Spec.ServiceAccount.Groups != nil {
-		if err := el.keycloakApiClient.SetServiceAccountGroups(realmName,
-			keycloakClient.Status.ClientID, keycloakClient.Spec.ServiceAccount.Groups, addOnly); err != nil {
-			return errors.Wrap(err, "unable to sync service account groups")
-		}
-	}
-
 	return nil
 }

internal/controller/keycloakclient/chain/service_account_test.go

@@ -45,10 +45,10 @@ func TestServiceAccount_Serve(t *testing.T) {
 		kc.Spec.ServiceAccount.RealmRoles,
 		map[string][]string{
 			kc.Spec.ServiceAccount.ClientRoles[0].ClientID: kc.Spec.ServiceAccount.ClientRoles[0].Roles}, false).Return(nil)
+	apiClient.On("SyncServiceAccountGroups", realmName, kc.Status.ClientID,
+		kc.Spec.ServiceAccount.Groups, false).Return(nil)
 	apiClient.On("SetServiceAccountAttributes", realmName, kc.Status.ClientID,
 		kc.Spec.ServiceAccount.Attributes, false).Return(nil)
-	apiClient.On("SetServiceAccountGroups", realmName, kc.Status.ClientID,
-		kc.Spec.ServiceAccount.Groups, false).Return(nil)
 
 	sa := NewServiceAccount(apiClient)
 

internal/controller/keycloakclient/keycloakclient_controller_integration_test.go

@@ -95,6 +95,15 @@ var _ = Describe("KeycloakClient controller", Ordered, func() {
 		}, timeout, interval).Should(BeTrue(), "KeycloakClient should be deleted")
 	})
 	It("Should create KeycloakClient with empty secret", func() {
+		By("Creating keycloak api client")
+		client := gocloak.NewClient(keycloakURL)
+		token, err := client.LoginAdmin(ctx, "admin", "admin", "master")
+		Expect(err).ShouldNot(HaveOccurred())
+		By("Creating group for service account")
+		_, err = client.CreateGroup(ctx, token.AccessToken, KeycloakRealmCR, gocloak.Group{
+			Name: gocloak.StringP("test-group"),
+		})
+		Expect(adapter.SkipAlreadyExistsErr(err)).ShouldNot(HaveOccurred())
 		By("Creating a KeycloakClient")
 		keycloakClient := &keycloakApi.KeycloakClient{
 			ObjectMeta: metav1.ObjectMeta{
@@ -187,7 +196,7 @@ var _ = Describe("KeycloakClient controller", Ordered, func() {
 			},
 		}
 		Expect(k8sClient.Create(ctx, clientSecret)).Should(Succeed())
-		By("Crating keycloak api client")
+		By("Creating keycloak api client")
 		client := gocloak.NewClient(keycloakURL)
 		token, err := client.LoginAdmin(ctx, "admin", "admin", "master")
 		Expect(err).ShouldNot(HaveOccurred())

pkg/client/keycloak/adapter/gocloak_adapter_service_account.go

@@ -42,6 +42,15 @@ func (a GoCloakAdapter) SyncServiceAccountRoles(realm, clientID string, realmRol
 	return nil
 }
 
+func (a GoCloakAdapter) SyncServiceAccountGroups(realm, clientID string, groups []string, addOnly bool) error {
+	user, err := a.client.GetClientServiceAccount(context.Background(), a.token.AccessToken, realm, clientID)
+	if err != nil {
+		return errors.Wrap(err, "unable to get client service account")
+	}
+
+	return a.syncUserGroups(context.Background(), realm, *user.ID, groups, addOnly)
+}
+
 func doNotDeleteRealmRoleFromUser(ctx context.Context, token, realm, entityID string, roles []gocloak.Role) error {
 	return nil
 }
@@ -74,34 +83,3 @@ func (a GoCloakAdapter) SetServiceAccountAttributes(realm, clientID string, attr
 
 	return nil
 }
-
-func (a GoCloakAdapter) SetServiceAccountGroups(realm, clientID string, groups []string, addOnly bool) error {
-	user, err := a.client.GetClientServiceAccount(context.Background(), a.token.AccessToken, realm, clientID)
-	if err != nil {
-		return errors.Wrap(err, "unable to get client service account")
-	}
-
-	svcGroups := make(map[string]struct{})
-	if addOnly && user.Groups != nil {
-		for _, group := range *user.Groups {
-			svcGroups[group] = struct{}{}
-		}
-	}
-
-	for _, group := range groups {
-		svcGroups[group] = struct{}{}
-	}
-
-	var newGroups []string
-	for group := range svcGroups {
-		newGroups = append(newGroups, group)
-	}
-
-	user.Groups = &newGroups
-
-	if err = a.client.UpdateUser(context.Background(), a.token.AccessToken, realm, *user); err != nil {
-		return errors.Wrapf(err, "unable to update service account user: %s", clientID)
-	}
-
-	return nil
-}

pkg/client/keycloak/keycloak_client.go

@@ -26,8 +26,8 @@ type Client interface {
 	GetOpenIdConfig(realm *dto.Realm) (string, error)
 	SyncServiceAccountRoles(realm, clientID string, realmRoles []string,
 		clientRoles map[string][]string, addOnly bool) error
+	SyncServiceAccountGroups(realm, clientID string, groups []string, addOnly bool) error
 	SetServiceAccountAttributes(realm, clientID string, attributes map[string]string, addOnly bool) error
-	SetServiceAccountGroups(realm, clientID string, groups []string, addOnly bool) error
 	ExportToken() ([]byte, error)
 }