From 5d22cc429da5721dfde5666e81dbe7529bf2c11b Mon Sep 17 00:00:00 2001 From: Zorian Motso Date: Fri, 11 Oct 2024 17:54:15 +0300 Subject: [PATCH] fix: Resolve subgroup creation and assignment issues (#95) - Fix subgroup creation for the main group - Fix subgroup assignment to users - Add integration tests for KeycloakRealmGroup - Add integration tests for KeycloakRealmUser with subgroup assignments --- .../chain/put_client_scope_test.go | 8 +- .../keycloakrealmgroup_controller.go | 2 +- ...krealmgroup_controller_integration_test.go | 174 ++++++++++++++ .../keycloakrealmgroup_controller_test.go | 92 -------- controllers/keycloakrealmgroup/suite_test.go | 219 ++++++++++++++++++ .../keycloakrealmgroup/terminator_test.go | 49 ---- ...akrealmuser_controller_integration_test.go | 18 ++ .../keycloak/adapter/gocloak_adapter.go | 2 + .../adapter/gocloak_adapter_groups.go | 167 +++++++++++-- .../adapter/gocloak_adapter_groups_test.go | 183 +++++++++++++++ .../keycloak/adapter/gocloak_adapter_test.go | 127 ---------- .../keycloak/adapter/gocloak_adapter_user.go | 73 +++--- .../adapter/gocloak_adapter_user_test.go | 36 ++- pkg/client/keycloak/keycloak_client.go | 2 +- pkg/client/keycloak/mocks/client_mock.go | 29 +-- .../01-install-keycloak-server.yaml | 11 +- .../01-install-keycloak-server.yaml | 2 +- 17 files changed, 833 insertions(+), 361 deletions(-) create mode 100644 controllers/keycloakrealmgroup/keycloakrealmgroup_controller_integration_test.go delete mode 100644 controllers/keycloakrealmgroup/keycloakrealmgroup_controller_test.go create mode 100644 controllers/keycloakrealmgroup/suite_test.go delete mode 100644 controllers/keycloakrealmgroup/terminator_test.go diff --git a/controllers/keycloakclient/chain/put_client_scope_test.go b/controllers/keycloakclient/chain/put_client_scope_test.go index d7bdc072..6b634c00 100644 --- a/controllers/keycloakclient/chain/put_client_scope_test.go +++ b/controllers/keycloakclient/chain/put_client_scope_test.go @@ -2,8 +2,8 @@ package chain import ( "context" - keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" - "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mocks" + "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -13,7 +13,9 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "testing" + + keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" + "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mocks" ) func TestPutClientScope_Serve(t *testing.T) { diff --git a/controllers/keycloakrealmgroup/keycloakrealmgroup_controller.go b/controllers/keycloakrealmgroup/keycloakrealmgroup_controller.go index 5fd4584a..4dd800bf 100644 --- a/controllers/keycloakrealmgroup/keycloakrealmgroup_controller.go +++ b/controllers/keycloakrealmgroup/keycloakrealmgroup_controller.go @@ -130,7 +130,7 @@ func (r *ReconcileKeycloakRealmGroup) tryReconcile(ctx context.Context, keycloak return fmt.Errorf("unable to get keycloak realm from ref: %w", err) } - id, err := kClient.SyncRealmGroup(gocloak.PString(realm.Realm), &keycloakRealmGroup.Spec) + id, err := kClient.SyncRealmGroup(ctx, gocloak.PString(realm.Realm), &keycloakRealmGroup.Spec) if err != nil { return fmt.Errorf("unable to sync realm group: %w", err) } diff --git a/controllers/keycloakrealmgroup/keycloakrealmgroup_controller_integration_test.go b/controllers/keycloakrealmgroup/keycloakrealmgroup_controller_integration_test.go new file mode 100644 index 00000000..584a9a1c --- /dev/null +++ b/controllers/keycloakrealmgroup/keycloakrealmgroup_controller_integration_test.go @@ -0,0 +1,174 @@ +package keycloakrealmgroup + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/Nerzal/gocloak/v12" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "github.com/epam/edp-keycloak-operator/api/common" + keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" + "github.com/epam/edp-keycloak-operator/controllers/helper" + "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/adapter" + "github.com/epam/edp-keycloak-operator/pkg/objectmeta" +) + +var _ = Describe("KeycloakRealmGroup controller", Ordered, func() { + const ( + groupCR = "test-keycloak-realm-group" + ) + It("Should create KeycloakRealmGroup", func() { + By("Creating role for the group") + _, err := keycloakApiClient.CreateRealmRole(ctx, getKeyCloakToken(), KeycloakRealmCR, gocloak.Role{ + Name: gocloak.StringP("test-group-role"), + }) + Expect(adapter.SkipAlreadyExistsErr(err)).ShouldNot(HaveOccurred()) + + By("Creating a KeycloakRealmGroup subgroup") + group := &keycloakApi.KeycloakRealmGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-subgroup", + Namespace: ns, + }, + Spec: keycloakApi.KeycloakRealmGroupSpec{ + Name: "test-subgroup", + RealmRef: common.RealmRef{ + Kind: keycloakApi.KeycloakRealmKind, + Name: KeycloakRealmCR, + }, + Path: "/test-subgroup", + }, + } + Expect(k8sClient.Create(ctx, group)).Should(Succeed()) + Eventually(func(g Gomega) { + createdGroup := &keycloakApi.KeycloakRealmGroup{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-subgroup", Namespace: ns}, createdGroup) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(createdGroup.Status.Value).Should(Equal(helper.StatusOK)) + }).WithTimeout(time.Second * 20).WithPolling(time.Second).Should(Succeed()) + + By("Creating a KeycloakRealmGroup") + group = &keycloakApi.KeycloakRealmGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: groupCR, + Namespace: ns, + }, + Spec: keycloakApi.KeycloakRealmGroupSpec{ + Name: "test-group", + RealmRef: common.RealmRef{ + Kind: keycloakApi.KeycloakRealmKind, + Name: KeycloakRealmCR, + }, + Path: "/test-group", + RealmRoles: []string{"test-group-role"}, + SubGroups: []string{"test-subgroup"}, + }, + } + Expect(k8sClient.Create(ctx, group)).Should(Succeed()) + Eventually(func(g Gomega) { + createdGroup := &keycloakApi.KeycloakRealmGroup{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: groupCR, Namespace: ns}, createdGroup) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(createdGroup.Status.Value).Should(Equal(helper.StatusOK)) + }).WithTimeout(time.Second * 20).WithPolling(time.Second).Should(Succeed()) + }) + It("Should update KeycloakRealmGroup", func() { + By("Getting KeycloakRealmGroup") + group := &keycloakApi.KeycloakRealmGroup{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: ns, Name: groupCR}, group)).Should(Succeed()) + + By("Updating a parent KeycloakRealmGroup") + group.Spec.SubGroups = []string{} + + Expect(k8sClient.Update(ctx, group)).Should(Succeed()) + Eventually(func(g Gomega) { + updatedGroup := &keycloakApi.KeycloakRealmGroup{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: group.Name, Namespace: ns}, updatedGroup) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(updatedGroup.Status.Value).Should(Equal(helper.StatusOK)) + }, time.Minute, time.Second*5).Should(Succeed()) + }) + It("Should delete KeycloakRealmGroup", func() { + By("Getting KeycloakRealmGroup") + group := &keycloakApi.KeycloakRealmGroup{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: ns, Name: groupCR}, group)).Should(Succeed()) + By("Deleting KeycloakRealmGroup") + Expect(k8sClient.Delete(ctx, group)).Should(Succeed()) + Eventually(func(g Gomega) { + deletedGroup := &keycloakApi.KeycloakRealmGroup{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: group.Name, Namespace: ns}, deletedGroup) + + g.Expect(k8sErrors.IsNotFound(err)).Should(BeTrue()) + }, timeout, interval).Should(Succeed()) + }) + It("Should preserve group with annotation", func() { + By("Creating a KeycloakRealmGroup") + group := &keycloakApi.KeycloakRealmGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-keycloak-realm-group-preserve", + Namespace: ns, + Annotations: map[string]string{ + objectmeta.PreserveResourcesOnDeletionAnnotation: "true", + }, + }, + Spec: keycloakApi.KeycloakRealmGroupSpec{ + RealmRef: common.RealmRef{ + Kind: keycloakApi.KeycloakRealmKind, + Name: KeycloakRealmCR, + }, + Name: "test-group-preserve", + }, + } + Expect(k8sClient.Create(ctx, group)).Should(Succeed()) + Eventually(func(g Gomega) { + createdGroup := &keycloakApi.KeycloakRealmGroup{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: group.Name, Namespace: ns}, createdGroup) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(createdGroup.Status.Value).Should(Equal(helper.StatusOK)) + }).WithTimeout(time.Second * 20).WithPolling(time.Second).Should(Succeed()) + + By("Deleting KeycloakRealmGroup") + Expect(k8sClient.Delete(ctx, group)).Should(Succeed()) + Eventually(func(g Gomega) { + groups, err := keycloakApiClient.GetGroups(ctx, getKeyCloakToken(), KeycloakRealmCR, gocloak.GetGroupsParams{ + Search: gocloak.StringP("test-group-preserve"), + }) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(groups).Should(HaveLen(1)) + }, time.Minute, time.Second*5).Should(Succeed()) + }) + It("Should fail to create KeycloakRealmGroup with not existing subgroup", func() { + By("Creating a KeycloakRealmGroup") + group := &keycloakApi.KeycloakRealmGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-keycloak-realm-group-with-invalid-subgroup", + Namespace: ns, + }, + Spec: keycloakApi.KeycloakRealmGroupSpec{ + RealmRef: common.RealmRef{ + Kind: keycloakApi.KeycloakRealmKind, + Name: KeycloakRealmCR, + }, + Name: "test-group-with-invalid-subgroup", + SubGroups: []string{"not-existing-subgroup"}, + }, + } + Expect(k8sClient.Create(ctx, group)).Should(Succeed()) + + By("Waiting for KeycloakRealmGroup to be processed") + time.Sleep(time.Second * 3) + + By("Checking KeycloakRealmGroup status") + Consistently(func(g Gomega) { + createdGroup := &keycloakApi.KeycloakRealmGroup{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: group.Name, Namespace: ns}, createdGroup) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(createdGroup.Status.Value).Should(ContainSubstring("unable to sync realm group")) + }, time.Second*3, time.Second).Should(Succeed()) + }) +}) diff --git a/controllers/keycloakrealmgroup/keycloakrealmgroup_controller_test.go b/controllers/keycloakrealmgroup/keycloakrealmgroup_controller_test.go deleted file mode 100644 index 1f2afcc1..00000000 --- a/controllers/keycloakrealmgroup/keycloakrealmgroup_controller_test.go +++ /dev/null @@ -1,92 +0,0 @@ -package keycloakrealmgroup - -import ( - "context" - "testing" - "time" - - "github.com/Nerzal/gocloak/v12" - testifymock "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/epam/edp-keycloak-operator/api/common" - keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" - "github.com/epam/edp-keycloak-operator/controllers/helper" - helpermock "github.com/epam/edp-keycloak-operator/controllers/helper/mocks" - "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mock" - "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mocks" -) - -func TestReconcileKeycloakRealmGroup_Reconcile(t *testing.T) { - sch := runtime.NewScheme() - utilruntime.Must(keycloakApi.AddToScheme(sch)) - utilruntime.Must(corev1.AddToScheme(sch)) - - ns := "security" - keycloak := keycloakApi.Keycloak{ - ObjectMeta: metav1.ObjectMeta{Name: "keycloak1", Namespace: ns}, Status: keycloakApi.KeycloakStatus{Connected: true}, - Spec: keycloakApi.KeycloakSpec{Secret: "keycloak-secret"}} - realm := keycloakApi.KeycloakRealm{TypeMeta: metav1.TypeMeta{ - APIVersion: "v1.edp.epam.com/v1", Kind: "KeycloakRealm", - }, - ObjectMeta: metav1.ObjectMeta{Name: "realm1", Namespace: ns, - OwnerReferences: []metav1.OwnerReference{{Name: "keycloak1", Kind: "Keycloak"}}}, - Spec: keycloakApi.KeycloakRealmSpec{RealmName: "ns.realm1"}} - group := keycloakApi.KeycloakRealmGroup{TypeMeta: metav1.TypeMeta{ - APIVersion: "v1.edp.epam.com/v1", Kind: "KeycloakRealmGroup", - }, ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: "group1" /*, DeletionTimestamp: &now*/}, - Spec: keycloakApi.KeycloakRealmGroupSpec{ - RealmRef: common.RealmRef{ - Kind: keycloakApi.KeycloakRealmKind, - Name: realm.Name, - }, - RealmRoles: []string{"role1", "role2"}, Name: "group1"}, - Status: keycloakApi.KeycloakRealmGroupStatus{ID: "id11", Value: helper.StatusOK}} - secret := corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "keycloak-secret", Namespace: ns}, - Data: map[string][]byte{"username": []byte("user"), "password": []byte("pass")}} - - client := fake.NewClientBuilder().WithScheme(sch).WithRuntimeObjects(&group, &realm, &keycloak, &secret).Build() - logger := mock.NewLogr() - h := helpermock.NewControllerHelper(t) - kcMock := mocks.NewMockClient(t) - - h.On("SetRealmOwnerRef", testifymock.Anything, testifymock.Anything).Return(nil) - h.On("CreateKeycloakClientFromRealmRef", testifymock.Anything, testifymock.Anything).Return(kcMock, nil) - h.On("GetKeycloakRealmFromRef", testifymock.Anything, testifymock.Anything, testifymock.Anything). - Return(&gocloak.RealmRepresentation{ - Realm: gocloak.StringP(realm.Spec.RealmName), - }, nil) - h.On("TryToDelete", testifymock.Anything, testifymock.Anything, testifymock.Anything, testifymock.Anything). - Return(false, nil) - kcMock.On("SyncRealmGroup", "ns.realm1", &group.Spec).Return("id11", nil) - - r := ReconcileKeycloakRealmGroup{ - client: client, - helper: h, - successReconcileTimeout: time.Hour, - } - - res, err := r.Reconcile(ctrl.LoggerInto(context.Background(), logger), reconcile.Request{NamespacedName: types.NamespacedName{ - Namespace: ns, - Name: "group1", - }}) - if err != nil { - t.Fatalf("%+v", err) - } - - loggerSink, ok := logger.GetSink().(*mock.Logger) - require.True(t, ok, "wrong logger type") - require.NoError(t, loggerSink.LastError()) - - if res.RequeueAfter != r.successReconcileTimeout { - t.Fatal("success reconcile timeout is not set") - } -} diff --git a/controllers/keycloakrealmgroup/suite_test.go b/controllers/keycloakrealmgroup/suite_test.go new file mode 100644 index 00000000..933a2107 --- /dev/null +++ b/controllers/keycloakrealmgroup/suite_test.go @@ -0,0 +1,219 @@ +package keycloakrealmgroup + +import ( + "context" + "os" + "path/filepath" + "sync" + "testing" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/Nerzal/gocloak/v12" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + "github.com/epam/edp-keycloak-operator/api/common" + keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" + "github.com/epam/edp-keycloak-operator/controllers/helper" + "github.com/epam/edp-keycloak-operator/controllers/keycloak" + "github.com/epam/edp-keycloak-operator/controllers/keycloakrealm" +) + +var ( + cfg *rest.Config + k8sClient client.Client + testEnv *envtest.Environment + ctx context.Context + cancel context.CancelFunc + keycloakApiClient *gocloak.GoCloak + keycloakApiToken string + tokenMutex sync.Mutex +) + +const ( + KeycloakCR = "test-keycloak" + KeycloakRealmCR = "test-group-realm" + ns = "test-group" + + timeout = time.Second * 10 + interval = time.Millisecond * 250 +) + +func TestKeycloakGroup(t *testing.T) { + RegisterFailHandler(Fail) + + if os.Getenv("TEST_KEYCLOAK_URL") == "" { + t.Skip("TEST_KEYCLOAK_URL is not set") + } + + RunSpecs(t, "KeycloakRealmGroup Controller Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + + ctx, cancel = context.WithCancel(context.Background()) + ctx = ctrl.LoggerInto(ctx, logf.Log) + + By("bootstrapping test environment") + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("../..", "config", "crd", "bases")}, + ErrorIfCRDPathMissing: true, + } + + var err error + cfg, err = testEnv.Start() + Expect(err).NotTo(HaveOccurred()) + Expect(cfg).NotTo(BeNil()) + + scheme := runtime.NewScheme() + Expect(keycloakApi.AddToScheme(scheme)).NotTo(HaveOccurred()) + Expect(corev1.AddToScheme(scheme)).NotTo(HaveOccurred()) + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient).NotTo(BeNil()) + + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme, + MetricsBindAddress: "0", + }) + Expect(err).ToNot(HaveOccurred()) + + h := helper.MakeHelper(k8sManager.GetClient(), k8sManager.GetScheme(), "default") + + err = keycloak.NewReconcileKeycloak(k8sManager.GetClient(), k8sManager.GetScheme(), h). + SetupWithManager(k8sManager, 0) + Expect(err).ToNot(HaveOccurred()) + + err = keycloakrealm.NewReconcileKeycloakRealm(k8sManager.GetClient(), k8sManager.GetScheme(), h). + SetupWithManager(k8sManager, 0) + Expect(err).ToNot(HaveOccurred()) + + err = NewReconcileKeycloakRealmGroup(k8sManager.GetClient(), h). + SetupWithManager(k8sManager, 0) + Expect(err).ToNot(HaveOccurred()) + + go func() { + defer GinkgoRecover() + err = k8sManager.Start(ctx) + Expect(err).ToNot(HaveOccurred(), "failed to run manager") + }() + + By("bootstrapping Keycloak and KeycloakRealm") + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: ns, + }, + } + err = k8sClient.Create(ctx, namespace) + Expect(err).To(Not(HaveOccurred())) + By("By creating a Keycloak secret") + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "keycloak-auth-secret", + Namespace: ns, + }, + Data: map[string][]byte{ + "username": []byte("admin"), + "password": []byte("admin"), + }, + } + Expect(k8sClient.Create(ctx, secret)).Should(Succeed()) + By("By creating a Keycloak") + keycloak := &keycloakApi.Keycloak{ + ObjectMeta: metav1.ObjectMeta{ + Name: KeycloakCR, + Namespace: ns, + }, + Spec: keycloakApi.KeycloakSpec{ + Url: os.Getenv("TEST_KEYCLOAK_URL"), + Secret: secret.Name, + }, + } + Expect(k8sClient.Create(ctx, keycloak)).Should(Succeed()) + Eventually(func() bool { + createdKeycloak := &keycloakApi.Keycloak{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: KeycloakCR, Namespace: ns}, createdKeycloak) + Expect(err).ShouldNot(HaveOccurred()) + + return createdKeycloak.Status.Connected + }, time.Second*30, interval).Should(BeTrue()) + By("By creating a KeycloakRealm") + keycloakRealm := &keycloakApi.KeycloakRealm{ + ObjectMeta: metav1.ObjectMeta{ + Name: KeycloakRealmCR, + Namespace: ns, + }, + Spec: keycloakApi.KeycloakRealmSpec{ + RealmName: KeycloakRealmCR, + KeycloakRef: common.KeycloakRef{ + Kind: keycloakApi.KeycloakKind, + Name: keycloak.Name, + }, + }, + } + Expect(k8sClient.Create(ctx, keycloakRealm)).Should(Succeed()) + Eventually(func() bool { + createdKeycloakRealm := &keycloakApi.KeycloakRealm{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: KeycloakRealmCR, Namespace: ns}, createdKeycloakRealm) + Expect(err).ShouldNot(HaveOccurred()) + + return createdKeycloakRealm.Status.Available + }, timeout, interval).Should(BeTrue()) + + keycloakApiClient = gocloak.NewClient(os.Getenv("TEST_KEYCLOAK_URL")) + setKeyCloakToken() + + // To prevent token expiration, we need to refresh it every 30 seconds. + go func() { + defer GinkgoRecover() + + ticker := time.NewTicker(30 * time.Second) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + setKeyCloakToken() + } + } + }() +}) + +var _ = AfterSuite(func() { + cancel() + By("tearing down the test environment") + err := testEnv.Stop() + Expect(err).NotTo(HaveOccurred()) +}) + +func setKeyCloakToken() { + token, err := keycloakApiClient.LoginAdmin(ctx, "admin", "admin", "master") + Expect(err).ShouldNot(HaveOccurred(), "failed to login to keycloak") + + tokenMutex.Lock() + keycloakApiToken = token.AccessToken + tokenMutex.Unlock() +} + +// getKeyCloakToken can be used to concurrently safe get keycloak token. +func getKeyCloakToken() string { + tokenMutex.Lock() + defer tokenMutex.Unlock() + + return keycloakApiToken +} diff --git a/controllers/keycloakrealmgroup/terminator_test.go b/controllers/keycloakrealmgroup/terminator_test.go deleted file mode 100644 index 056a69ee..00000000 --- a/controllers/keycloakrealmgroup/terminator_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package keycloakrealmgroup - -import ( - "context" - "testing" - - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - testifymock "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - ctrl "sigs.k8s.io/controller-runtime" - - "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mock" - "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mocks" -) - -func TestTerminator(t *testing.T) { - lg := mock.NewLogr() - kClient := mocks.NewMockClient(t) - - term := makeTerminator(kClient, "foo", "bar", false) - - kClient.On("DeleteGroup", testifymock.Anything, "foo", "bar").Return(nil).Once() - - err := term.DeleteResource(context.Background()) - require.NoError(t, err) - - kClient.On("DeleteGroup", testifymock.Anything, "foo", "bar").Return(errors.New("fatal")).Once() - - if err := term.DeleteResource(ctrl.LoggerInto(context.Background(), lg)); err == nil { - t.Fatal("no error returned") - } - - loggerSink, ok := lg.GetSink().(*mock.Logger) - require.True(t, ok, "wrong logger type") - assert.NotEmpty(t, loggerSink.InfoMessages(), "no info messages logged") -} - -func TestTerminatorSkipDeletion(t *testing.T) { - term := makeTerminator( - nil, - "realm", - "realmCR", - true, - ) - - err := term.DeleteResource(context.Background()) - require.NoError(t, err) -} diff --git a/controllers/keycloakrealmuser/keycloakrealmuser_controller_integration_test.go b/controllers/keycloakrealmuser/keycloakrealmuser_controller_integration_test.go index ccdb6be6..cb9594c0 100644 --- a/controllers/keycloakrealmuser/keycloakrealmuser_controller_integration_test.go +++ b/controllers/keycloakrealmuser/keycloakrealmuser_controller_integration_test.go @@ -31,6 +31,23 @@ var _ = Describe("KeycloakRealmUser controller", Ordered, func() { }) Expect(adapter.SkipAlreadyExistsErr(err)).ShouldNot(HaveOccurred()) + By("Creating group and subgroup for user") + _, err = keycloakApiClient.CreateGroup(ctx, getKeyCloakToken(), KeycloakRealmCR, gocloak.Group{ + Name: gocloak.StringP("test-user-group2"), + }) + Expect(adapter.SkipAlreadyExistsErr(err)).ShouldNot(HaveOccurred()) + + gr, err := keycloakApiClient.GetGroups(ctx, getKeyCloakToken(), KeycloakRealmCR, gocloak.GetGroupsParams{ + Search: gocloak.StringP("test-user-group2"), + }) + Expect(adapter.SkipAlreadyExistsErr(err)).ShouldNot(HaveOccurred()) + Expect(gr).Should(HaveLen(1)) + + _, err = keycloakApiClient.CreateChildGroup(ctx, getKeyCloakToken(), KeycloakRealmCR, *gr[0].ID, gocloak.Group{ + Name: gocloak.StringP("test-user-group2-subgroup"), + }) + Expect(adapter.SkipAlreadyExistsErr(err)).ShouldNot(HaveOccurred()) + By("Creating role for user") _, err = keycloakApiClient.CreateRealmRole(ctx, getKeyCloakToken(), KeycloakRealmCR, gocloak.Role{ Name: gocloak.StringP("test-user-role"), @@ -74,6 +91,7 @@ var _ = Describe("KeycloakRealmUser controller", Ordered, func() { }, Groups: []string{ "test-user-group", + "test-user-group2-subgroup", }, Attributes: map[string]string{ "attr1": "test-value", diff --git a/pkg/client/keycloak/adapter/gocloak_adapter.go b/pkg/client/keycloak/adapter/gocloak_adapter.go index ddd434f9..eaa2d494 100644 --- a/pkg/client/keycloak/adapter/gocloak_adapter.go +++ b/pkg/client/keycloak/adapter/gocloak_adapter.go @@ -66,6 +66,8 @@ const ( getUserRealmRoleMappings = "/admin/realms/{realm}/users/{id}/role-mappings/realm" getUserGroupMappings = "/admin/realms/{realm}/users/{id}/groups" manageUserGroups = "/admin/realms/{realm}/users/{userID}/groups/{groupID}" + getChildGroups = "/admin/realms/{realm}/groups/{groupID}/children" + getGroup = "/admin/realms/{realm}/groups/{groupID}" logClientDTO = "client dto" ) diff --git a/pkg/client/keycloak/adapter/gocloak_adapter_groups.go b/pkg/client/keycloak/adapter/gocloak_adapter_groups.go index a1bf4a57..2281a1e6 100644 --- a/pkg/client/keycloak/adapter/gocloak_adapter_groups.go +++ b/pkg/client/keycloak/adapter/gocloak_adapter_groups.go @@ -4,9 +4,12 @@ import ( "context" "fmt" "net/http" + "strings" + "sync" "github.com/Nerzal/gocloak/v12" "github.com/pkg/errors" + "golang.org/x/sync/errgroup" keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" ) @@ -37,6 +40,7 @@ func IsErrNotFound(err error) bool { return false } +// GetGroups return top-level groups for a realm. func (a GoCloakAdapter) GetGroups(ctx context.Context, realm string) (map[string]*gocloak.Group, error) { groups, err := a.client.GetGroups( ctx, @@ -61,21 +65,131 @@ func (a GoCloakAdapter) GetGroups(ctx context.Context, realm string) (map[string return groupMap, nil } -func (a GoCloakAdapter) getGroup(realm, groupName string) (*gocloak.Group, error) { - groups, err := a.client.GetGroups(context.Background(), a.token.AccessToken, realm, gocloak.GetGroupsParams{ +func (a GoCloakAdapter) getGroup(ctx context.Context, realm, groupName string) (*gocloak.Group, error) { + groups, err := a.client.GetGroups(ctx, a.token.AccessToken, realm, gocloak.GetGroupsParams{ Search: gocloak.StringP(groupName), }) if err != nil { return nil, errors.Wrap(err, "unable to search groups") } + gr := make([]gocloak.Group, len(groups)) + + for i, g := range groups { + if g != nil { + gr[i] = *g + } + } + + group := getGroupByName(gr, groupName) + if group != nil { + return group, nil + } + + return nil, NotFoundError("group not found") +} + +func (a GoCloakAdapter) getGroupsByNames(ctx context.Context, realm string, groupNames []string) (map[string]gocloak.Group, error) { + groups := make(map[string]gocloak.Group, len(groupNames)) + eg := errgroup.Group{} + m := sync.Mutex{} + + for _, groupName := range groupNames { + groupName := groupName + + eg.Go(func() error { + group, err := a.getGroup(ctx, realm, groupName) + if err != nil { + return err + } + + m.Lock() + defer m.Unlock() + + groups[groupName] = *group + + return nil + }) + } + + if err := eg.Wait(); err != nil { + return nil, fmt.Errorf("failed to get groups by names: %w", err) + } + + return groups, nil +} + +func getGroupByName(groups []gocloak.Group, groupName string) *gocloak.Group { for _, g := range groups { if *g.Name == groupName { - return g, nil + return &g + } + + if g.SubGroups != nil { + return getGroupByName(*g.SubGroups, groupName) } } - return nil, NotFoundError("group not found") + return nil +} + +func (a GoCloakAdapter) getChildGroups(ctx context.Context, realm string, parentGroup *gocloak.Group) ([]gocloak.Group, error) { + var result []gocloak.Group + + resp, err := a.client.RestyClient().R(). + SetContext(ctx). + SetAuthToken(a.token.AccessToken). + SetHeader(contentTypeHeader, contentTypeJson). + SetPathParams(map[string]string{ + keycloakApiParamRealm: realm, + "groupID": *parentGroup.ID, + }). + SetResult(&result). + Get(a.buildPath(getChildGroups)) + + if err = a.checkError(err, resp); err != nil { + // Use workaround for Keycloak versions < 23.0.0 for backward compatibility. + if strings.Contains(err.Error(), "No resource method found for GET, return 405 with Allow header") { + r, err := a.getChildGroupsKCVersionUnder23(ctx, realm, parentGroup) + if err != nil { + return nil, fmt.Errorf("unable to get child groups: %w", err) + } + + return r, nil + } + + return nil, fmt.Errorf("unable to get child groups, rsp: %s", resp.String()) + } + + return result, nil +} + +// getChildGroupsKCVersionUnder23 is a workaround for Keycloak versions < 23.0.0. +// Group model in Keycloak < 23.0.0 contains subgroups. +// In Keycloak >= 23.0.0 to get subgroups we need to use dedicated endpoint. +func (a GoCloakAdapter) getChildGroupsKCVersionUnder23(ctx context.Context, realm string, parentGroup *gocloak.Group) ([]gocloak.Group, error) { + result := &gocloak.Group{} + + resp, err := a.client.RestyClient().R(). + SetContext(ctx). + SetAuthToken(a.token.AccessToken). + SetHeader(contentTypeHeader, contentTypeJson). + SetPathParams(map[string]string{ + keycloakApiParamRealm: realm, + "groupID": *parentGroup.ID, + }). + SetResult(result). + Get(a.buildPath(getGroup)) + + if err = a.checkError(err, resp); err != nil { + return nil, fmt.Errorf("unable to get group: %s", resp.String()) + } + + if result.SubGroups == nil { + return nil, nil + } + + return *result.SubGroups, nil } func (a GoCloakAdapter) syncGroupRoles(realmName, groupID string, spec *keycloakApi.KeycloakRealmGroupSpec) error { @@ -102,9 +216,13 @@ func (a GoCloakAdapter) syncGroupRoles(realmName, groupID string, spec *keycloak return nil } -func (a GoCloakAdapter) syncSubGroups(realm string, group *gocloak.Group, subGroups []string) error { - currentGroups := a.makeCurrentGroups(group) - claimedGroups := make(map[string]struct{}) +func (a GoCloakAdapter) syncSubGroups(ctx context.Context, realm string, group *gocloak.Group, subGroups []string) error { + currentGroups, err := a.makeCurrentGroups(ctx, realm, group) + if err != nil { + return err + } + + claimedGroups := make(map[string]struct{}, len(subGroups)) for _, g := range subGroups { claimedGroups[g] = struct{}{} @@ -112,12 +230,12 @@ func (a GoCloakAdapter) syncSubGroups(realm string, group *gocloak.Group, subGro for _, claimed := range subGroups { if _, ok := currentGroups[claimed]; !ok { - gr, err := a.getGroup(realm, claimed) + gr, err := a.getGroup(ctx, realm, claimed) if err != nil { return errors.Wrapf(err, "unable to get group, realm: %s, group: %s", realm, claimed) } - if _, err := a.client.CreateChildGroup(context.Background(), a.token.AccessToken, realm, *group.ID, *gr); err != nil { + if _, err := a.client.CreateChildGroup(ctx, a.token.AccessToken, realm, *group.ID, *gr); err != nil { return errors.Wrapf(err, "unable to create child group, realm: %s, group: %s", realm, claimed) } } @@ -126,7 +244,7 @@ func (a GoCloakAdapter) syncSubGroups(realm string, group *gocloak.Group, subGro for name, current := range currentGroups { if _, ok := claimedGroups[name]; !ok { // this is strange but if we call create group on subgroup it will be detached from parent group %) - if _, err := a.client.CreateGroup(context.Background(), a.token.AccessToken, realm, current); err != nil { + if _, err := a.client.CreateGroup(ctx, a.token.AccessToken, realm, current); err != nil { return errors.Wrapf(err, "unable to detach subgroup from group, realm: %s, subgroup: %s, group: %+v", realm, name, group) } @@ -136,8 +254,8 @@ func (a GoCloakAdapter) syncSubGroups(realm string, group *gocloak.Group, subGro return nil } -func (a GoCloakAdapter) SyncRealmGroup(realmName string, spec *keycloakApi.KeycloakRealmGroupSpec) (string, error) { - group, err := a.getGroup(realmName, spec.Name) +func (a GoCloakAdapter) SyncRealmGroup(ctx context.Context, realmName string, spec *keycloakApi.KeycloakRealmGroupSpec) (string, error) { + group, err := a.getGroup(ctx, realmName, spec.Name) if err != nil { if !IsErrNotFound(err) { return "", errors.Wrapf(err, "unable to get group with spec %+v", spec) @@ -145,7 +263,7 @@ func (a GoCloakAdapter) SyncRealmGroup(realmName string, spec *keycloakApi.Keycl group = &gocloak.Group{Name: &spec.Name, Path: &spec.Path, Attributes: &spec.Attributes, Access: &spec.Access} - groupID, err := a.client.CreateGroup(context.Background(), a.token.AccessToken, realmName, *group) + groupID, err := a.client.CreateGroup(ctx, a.token.AccessToken, realmName, *group) if err != nil { return "", errors.Wrapf(err, "unable to create group with spec %+v", spec) } @@ -153,7 +271,7 @@ func (a GoCloakAdapter) SyncRealmGroup(realmName string, spec *keycloakApi.Keycl group.ID = &groupID } else { group.Path, group.Access, group.Attributes = &spec.Path, &spec.Access, &spec.Attributes - if err := a.client.UpdateGroup(context.Background(), a.token.AccessToken, realmName, *group); err != nil { + if err := a.client.UpdateGroup(ctx, a.token.AccessToken, realmName, *group); err != nil { return "", errors.Wrapf(err, "unable to update group, realm: %s, group spec: %+v", realmName, spec) } } @@ -162,7 +280,7 @@ func (a GoCloakAdapter) SyncRealmGroup(realmName string, spec *keycloakApi.Keycl return "", errors.Wrapf(err, "unable to sync group realm roles, group: %+v with spec %+v", group, spec) } - if err := a.syncSubGroups(realmName, group, spec.SubGroups); err != nil { + if err := a.syncSubGroups(ctx, realmName, group, spec.SubGroups); err != nil { return "", errors.Wrapf(err, "unable to sync subgroups, group: %+v with spec: %+v", group, spec) } @@ -170,7 +288,7 @@ func (a GoCloakAdapter) SyncRealmGroup(realmName string, spec *keycloakApi.Keycl } func (a GoCloakAdapter) DeleteGroup(ctx context.Context, realm, groupName string) error { - group, err := a.getGroup(realm, groupName) + group, err := a.getGroup(ctx, realm, groupName) if err != nil { return errors.Wrapf(err, "unable to get group, realm: %s, group: %s", realm, groupName) } @@ -182,14 +300,17 @@ func (a GoCloakAdapter) DeleteGroup(ctx context.Context, realm, groupName string return nil } -func (a GoCloakAdapter) makeCurrentGroups(group *gocloak.Group) map[string]gocloak.Group { - currentGroups := make(map[string]gocloak.Group) +func (a GoCloakAdapter) makeCurrentGroups(ctx context.Context, realm string, group *gocloak.Group) (map[string]gocloak.Group, error) { + child, err := a.getChildGroups(ctx, realm, group) + if err != nil { + return nil, err + } + + currentGroups := make(map[string]gocloak.Group, len(child)) - if group.SubGroups != nil { - for i, cg := range *group.SubGroups { - currentGroups[*cg.Name] = (*group.SubGroups)[i] - } + for _, c := range child { + currentGroups[*c.Name] = c } - return currentGroups + return currentGroups, nil } diff --git a/pkg/client/keycloak/adapter/gocloak_adapter_groups_test.go b/pkg/client/keycloak/adapter/gocloak_adapter_groups_test.go index ce84455b..aa118902 100644 --- a/pkg/client/keycloak/adapter/gocloak_adapter_groups_test.go +++ b/pkg/client/keycloak/adapter/gocloak_adapter_groups_test.go @@ -1,12 +1,22 @@ package adapter import ( + "context" "errors" "net/http" + "net/http/httptest" + "strings" "testing" "github.com/Nerzal/gocloak/v12" + "github.com/go-logr/logr" + "github.com/go-resty/resty/v2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" + kcmocks "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/adapter/mocks" ) func TestIsErrNotFound(t *testing.T) { @@ -58,3 +68,176 @@ func TestIsErrNotFound(t *testing.T) { }) } } + +func TestGoCloakAdapter_SyncRealmGroup(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/groups/test-group-old-endpoint-id/children") { + w.WriteHeader(http.StatusMethodNotAllowed) + _, err := w.Write([]byte(`No resource method found for GET, return 405 with Allow header`)) + assert.NoError(t, err) + + return + } + + if strings.Contains(r.URL.Path, "/groups/test-group-old-endpoint-id") { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte(`{"id": "test-group-old-endpoint-id", "name": "test-group"}`)) + assert.NoError(t, err) + + return + } + + if strings.Contains(r.URL.Path, "/children") { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte(`[]`)) + assert.NoError(t, err) + + return + } + })) + t.Cleanup(server.Close) + + tests := []struct { + name string + spec *keycloakApi.KeycloakRealmGroupSpec + client func(t *testing.T) GoCloak + want string + wantErr require.ErrorAssertionFunc + }{ + { + name: "create realm group successfully", + spec: &keycloakApi.KeycloakRealmGroupSpec{ + Name: "test-group", + SubGroups: []string{"sub-group1", "sub-group2"}, + }, + client: func(t *testing.T) GoCloak { + m := kcmocks.NewMockGoCloak(t) + + m.On("GetGroups", mock.Anything, mock.Anything, "master", mock.Anything). + Return(func(ctx context.Context, accessToken, realm string, params gocloak.GetGroupsParams) []*gocloak.Group { + if params.Search != nil && *params.Search == "test-group" { + return []*gocloak.Group{} + } + + if params.Search != nil && *params.Search == "sub-group1" { + return []*gocloak.Group{{ + ID: gocloak.StringP("sub-group1-id"), + Name: gocloak.StringP("sub-group1"), + }} + } + + if params.Search != nil && *params.Search == "sub-group2" { + return []*gocloak.Group{{ + ID: gocloak.StringP("sub-group2-id"), + Name: gocloak.StringP("sub-group2"), + }} + } + + return []*gocloak.Group{} + }, nil) + + m.On("CreateGroup", mock.Anything, mock.Anything, "master", mock.Anything). + Return("test-group-id", nil) + + m.On("GetRoleMappingByGroupID", mock.Anything, mock.Anything, "master", "test-group-id"). + Return(&gocloak.MappingsRepresentation{}, nil) + + m.On("RestyClient").Return(resty.New().SetBaseURL(server.URL)) + + m.On("CreateChildGroup", mock.Anything, mock.Anything, "master", "test-group-id", mock.Anything). + Return("", nil) + + return m + }, + wantErr: require.NoError, + want: "test-group-id", + }, + { + name: "update realm group successfully", + spec: &keycloakApi.KeycloakRealmGroupSpec{ + Name: "test-group", + }, + client: func(t *testing.T) GoCloak { + m := kcmocks.NewMockGoCloak(t) + + m.On("GetGroups", mock.Anything, mock.Anything, "master", mock.Anything). + Return(func(ctx context.Context, accessToken, realm string, params gocloak.GetGroupsParams) []*gocloak.Group { + return []*gocloak.Group{{ + ID: gocloak.StringP("test-group-id"), + Name: gocloak.StringP("test-group"), + }} + }, nil) + + m.On("UpdateGroup", mock.Anything, mock.Anything, "master", mock.Anything). + Return(nil) + + m.On("GetRoleMappingByGroupID", mock.Anything, mock.Anything, "master", "test-group-id"). + Return(&gocloak.MappingsRepresentation{}, nil) + + m.On("RestyClient").Return(resty.New().SetBaseURL(server.URL)) + + return m + }, + wantErr: require.NoError, + want: "test-group-id", + }, + { + name: "use old endpoint to get child groups", + spec: &keycloakApi.KeycloakRealmGroupSpec{ + Name: "test-group", + SubGroups: []string{"sub-group1"}, + }, + client: func(t *testing.T) GoCloak { + m := kcmocks.NewMockGoCloak(t) + + m.On("GetGroups", mock.Anything, mock.Anything, "master", mock.Anything). + Return(func(ctx context.Context, accessToken, realm string, params gocloak.GetGroupsParams) []*gocloak.Group { + if params.Search != nil && *params.Search == "sub-group1" { + return []*gocloak.Group{{ + ID: gocloak.StringP("sub-group1-id"), + Name: gocloak.StringP("sub-group1"), + }} + } + + return []*gocloak.Group{} + }, nil) + + m.On("CreateGroup", mock.Anything, mock.Anything, "master", mock.Anything). + Return("test-group-old-endpoint-id", nil) + + m.On("GetRoleMappingByGroupID", mock.Anything, mock.Anything, "master", "test-group-old-endpoint-id"). + Return(&gocloak.MappingsRepresentation{}, nil) + + m.On("RestyClient").Return(resty.New().SetBaseURL(server.URL)) + + m.On("CreateChildGroup", mock.Anything, mock.Anything, "master", "test-group-old-endpoint-id", mock.Anything). + Return("", nil) + + return m + }, + wantErr: require.NoError, + want: "test-group-old-endpoint-id", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := GoCloakAdapter{ + client: tt.client(t), + token: &gocloak.JWT{ + AccessToken: "token", + }, + log: logr.Discard(), + } + got, err := a.SyncRealmGroup(context.Background(), "master", tt.spec) + + tt.wantErr(t, err) + assert.Equal(t, tt.want, got) + + }) + } +} diff --git a/pkg/client/keycloak/adapter/gocloak_adapter_test.go b/pkg/client/keycloak/adapter/gocloak_adapter_test.go index be5edacf..1ce8836f 100644 --- a/pkg/client/keycloak/adapter/gocloak_adapter_test.go +++ b/pkg/client/keycloak/adapter/gocloak_adapter_test.go @@ -21,7 +21,6 @@ import ( "github.com/stretchr/testify/suite" ctrl "sigs.k8s.io/controller-runtime" - keycloakApi "github.com/epam/edp-keycloak-operator/api/v1" "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/adapter/mocks" "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/dto" "github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mock" @@ -614,132 +613,6 @@ func TestGoCloakAdapter_SyncServiceAccountRoles(t *testing.T) { } } -func TestGoCloakAdapter_SyncRealmGroup_FailureGetGroupsFatal(t *testing.T) { - clMock := mocks.NewMockGoCloak(t) - - adapter := GoCloakAdapter{ - client: clMock, - token: &gocloak.JWT{AccessToken: "token"}, - } - - group := keycloakApi.KeycloakRealmGroupSpec{ - Name: "group1", - } - - clMock.On("GetGroups", testifymock.Anything, "token", "realm1", gocloak.GetGroupsParams{ - Search: &group.Name, - }).Return(nil, errors.New("fatal mock")) - - _, err := adapter.SyncRealmGroup("realm1", &group) - - if err == nil { - t.Fatal("error is not returned") - } - - if errors.Cause(err).Error() != "fatal mock" { - t.Fatalf("wrong error returned: %s", errors.Cause(err).Error()) - } -} - -func TestGoCloakAdapter_SyncRealmGroup(t *testing.T) { - mockClient := mocks.NewMockGoCloak(t) - adapter := GoCloakAdapter{ - client: mockClient, - token: &gocloak.JWT{AccessToken: "token"}, - basePath: "", - log: mock.NewLogr(), - } - - oldChildGroup := gocloak.Group{Name: gocloak.StringP("old-group")} - - mockClient.On("GetGroups", testifymock.Anything, "token", "realm1", - gocloak.GetGroupsParams{Search: gocloak.StringP("group1")}). - Return([]*gocloak.Group{{Name: gocloak.StringP("group1"), ID: gocloak.StringP("1"), - SubGroups: &[]gocloak.Group{oldChildGroup}}}, nil).Once() - mockClient.On("UpdateGroup", testifymock.Anything, "token", "realm1", gocloak.Group{Name: gocloak.StringP("group1"), - Attributes: &map[string][]string{"foo": {"foo", "bar"}}, - Path: gocloak.StringP(""), - Access: &map[string]bool{}, ID: gocloak.StringP("1"), - SubGroups: &[]gocloak.Group{{Name: gocloak.StringP("old-group")}}}).Return(nil) - - oldRole1, oldRole2 := gocloak.Role{Name: gocloak.StringP("old-role-1")}, - gocloak.Role{Name: gocloak.StringP("old-role-2")} - newRole1, newRole2 := gocloak.Role{Name: gocloak.StringP("realm-role1")}, - gocloak.Role{Name: gocloak.StringP("realm-role2")} - oldClientRole1, oldClientRole2, oldClientRole3 := gocloak.Role{Name: gocloak.StringP("oclient-role-1")}, - gocloak.Role{Name: gocloak.StringP("oclient-role-2")}, - gocloak.Role{Name: gocloak.StringP("oclient-role-3")} - newClientRole1, newClientRole2, newClientRole4 := gocloak.Role{Name: gocloak.StringP("client-role1")}, - gocloak.Role{Name: gocloak.StringP("client-role2")}, gocloak.Role{Name: gocloak.StringP("client-role4")} - - mockClient.On("GetRoleMappingByGroupID", testifymock.Anything, "token", "realm1", "1"). - Return(&gocloak.MappingsRepresentation{ - RealmMappings: &[]gocloak.Role{oldRole1, oldRole2}, - ClientMappings: map[string]*gocloak.ClientMappingsRepresentation{ - "old-cl-1": {Client: gocloak.StringP("old-cl-1"), ID: gocloak.StringP("321"), - Mappings: &[]gocloak.Role{oldClientRole1, oldClientRole2}}, - "old-cl-3": {Client: gocloak.StringP("old-cl-3"), ID: gocloak.StringP("3214"), - Mappings: &[]gocloak.Role{oldClientRole3}}, - }, - }, nil) - - subGroup1, subGroup2 := gocloak.Group{Name: gocloak.StringP("subgroup1"), ID: gocloak.StringP("2")}, - gocloak.Group{Name: gocloak.StringP("subgroup2"), ID: gocloak.StringP("3")} - - mockClient.On("GetGroups", testifymock.Anything, "token", "realm1", - testifymock.Anything). - Return([]*gocloak.Group{&subGroup1}, nil).Once() - mockClient.On("GetGroups", testifymock.Anything, "token", "realm1", - testifymock.Anything). - Return([]*gocloak.Group{&subGroup2}, nil) - mockClient.On("CreateChildGroup", testifymock.Anything, "token", "realm1", "1", subGroup1).Return("", nil).Once() - mockClient.On("CreateChildGroup", testifymock.Anything, "token", "realm1", "1", subGroup2).Return("", nil).Once() - mockClient.On("CreateGroup", testifymock.Anything, "token", "realm1", oldChildGroup).Return("", nil) - mockClient.On("GetRealmRole", testifymock.Anything, "token", "realm1", "realm-role1").Return(&newRole1, nil) - mockClient.On("GetRealmRole", testifymock.Anything, "token", "realm1", "realm-role2").Return(&newRole2, nil) - mockClient.On("AddRealmRoleToGroup", testifymock.Anything, "token", "realm1", "1", []gocloak.Role{newRole1, newRole2}).Return(nil) - mockClient.On("DeleteRealmRoleFromGroup", testifymock.Anything, "token", "realm1", "1", testifymock.Anything).Return(nil) - mockClient.On("GetClients", testifymock.Anything, "token", "realm1", - gocloak.GetClientsParams{ClientID: gocloak.StringP("client1")}). - Return([]*gocloak.Client{{ID: gocloak.StringP("clid1"), ClientID: gocloak.StringP("client1")}}, nil) - mockClient.On("GetClients", testifymock.Anything, "token", "realm1", - gocloak.GetClientsParams{ClientID: gocloak.StringP("old-cl-3")}). - Return([]*gocloak.Client{{ID: gocloak.StringP("3214"), ClientID: gocloak.StringP("old-cl-3")}}, nil) - mockClient.On("GetClientRole", testifymock.Anything, "token", "realm1", "clid1", *newClientRole1.Name).Return(&newClientRole1, nil) - mockClient.On("GetClientRole", testifymock.Anything, "token", "realm1", "clid1", *newClientRole2.Name).Return(&newClientRole2, nil) - mockClient.On("GetClientRole", testifymock.Anything, "token", "realm1", "3214", *newClientRole4.Name).Return(&newClientRole4, nil) - mockClient.On("AddClientRoleToGroup", testifymock.Anything, "token", "realm1", "clid1", "1", - testifymock.MatchedBy(func(roles []gocloak.Role) bool { - return len(roles) == 2 - })).Return(nil) - mockClient.On("AddClientRoleToGroup", testifymock.Anything, "token", "realm1", "3214", "1", - []gocloak.Role{newClientRole4}).Return(nil) - - mockClient.On("DeleteClientRoleFromGroup", testifymock.Anything, "token", "realm1", "321", "1", - []gocloak.Role{oldClientRole1, oldClientRole2}).Return(nil) - mockClient.On("DeleteClientRoleFromGroup", testifymock.Anything, "token", "realm1", "3214", "1", - []gocloak.Role{oldClientRole3}).Return(nil) - - groupID, err := adapter.SyncRealmGroup("realm1", &keycloakApi.KeycloakRealmGroupSpec{ - Name: "group1", - Attributes: map[string][]string{"foo": {"foo", "bar"}}, - Access: map[string]bool{}, - SubGroups: []string{"subgroup1", "subgroup2"}, - RealmRoles: []string{"realm-role1", "realm-role2"}, - ClientRoles: []keycloakApi.ClientRole{ - {ClientID: "client1", Roles: []string{"client-role1", "client-role2"}}, - {ClientID: "old-cl-3", Roles: []string{"client-role4"}}, - }, - }) - if err != nil { - t.Fatalf("%+v", err) - } - - if groupID == "" { - t.Fatal("group id is empty") - } -} - func TestGoCloakAdapter_DeleteGroup(t *testing.T) { mockClient := mocks.NewMockGoCloak(t) adapter := GoCloakAdapter{ diff --git a/pkg/client/keycloak/adapter/gocloak_adapter_user.go b/pkg/client/keycloak/adapter/gocloak_adapter_user.go index d6801281..f195d245 100644 --- a/pkg/client/keycloak/adapter/gocloak_adapter_user.go +++ b/pkg/client/keycloak/adapter/gocloak_adapter_user.go @@ -3,6 +3,7 @@ package adapter import ( "context" "fmt" + "slices" "github.com/Nerzal/gocloak/v12" "github.com/pkg/errors" @@ -126,36 +127,47 @@ func (a GoCloakAdapter) GetUserByName(ctx context.Context, realmName, username s } func (a GoCloakAdapter) syncUserGroups(ctx context.Context, realmName string, userID string, groups []string, addOnly bool) error { - if !addOnly { - if err := a.clearUserGroups(ctx, realmName, userID); err != nil { - return errors.Wrap(err, "unable to clear user groups") - } - } - - kcGroups, err := a.client.GetGroups( - ctx, - a.token.AccessToken, - realmName, - gocloak.GetGroupsParams{ - Max: gocloak.IntP(100), - }) + userGroups, err := a.GetUserGroupMappings(ctx, realmName, userID) if err != nil { - return fmt.Errorf("unable to get groups: %w", err) + return err } - groupDict := make(map[string]string, len(kcGroups)) - for _, gr := range kcGroups { - groupDict[*gr.Name] = *gr.ID + groupsToAdd := make([]string, 0, len(groups)) + + for _, gn := range groups { + if !slices.ContainsFunc(userGroups, func(mapping UserGroupMapping) bool { + return mapping.Name == gn + }) { + groupsToAdd = append(groupsToAdd, gn) + } } - for _, gr := range groups { - groupID, ok := groupDict[gr] - if !ok { - return errors.Errorf("group %s not found", gr) + if len(groupsToAdd) > 0 { + var kcGroups map[string]gocloak.Group + + kcGroups, err = a.getGroupsByNames( + ctx, + realmName, + groupsToAdd, + ) + if err != nil { + return fmt.Errorf("unable to get groups: %w", err) } - if err = a.AddUserToGroup(ctx, realmName, userID, groupID); err != nil { - return errors.Wrap(err, "unable to add user to group") + for _, gr := range kcGroups { + if err = a.AddUserToGroup(ctx, realmName, userID, *gr.ID); err != nil { + return fmt.Errorf("failed to add user to group %v: %w", gr.Name, err) + } + } + } + + if !addOnly { + for _, gr := range userGroups { + if !slices.Contains(groups, gr.Name) { + if err = a.RemoveUserFromGroup(ctx, realmName, userID, gr.ID); err != nil { + return fmt.Errorf("unable to remove user from group: %w", err) + } + } } } @@ -277,21 +289,6 @@ func (a GoCloakAdapter) AddUserToGroup(ctx context.Context, realmName, userID, g return nil } -func (a GoCloakAdapter) clearUserGroups(ctx context.Context, realmName, userID string) error { - groups, err := a.GetUserGroupMappings(ctx, realmName, userID) - if err != nil { - return errors.Wrap(err, "unable to get user groups") - } - - for _, gr := range groups { - if err := a.RemoveUserFromGroup(ctx, realmName, userID, gr.ID); err != nil { - return errors.Wrap(err, "unable to remove user from group") - } - } - - return nil -} - func (a GoCloakAdapter) clearUserRealmRoles(ctx context.Context, realmName string, userID string) error { roles, err := a.GetUserRealmRoleMappings(ctx, realmName, userID) if err != nil { diff --git a/pkg/client/keycloak/adapter/gocloak_adapter_user_test.go b/pkg/client/keycloak/adapter/gocloak_adapter_user_test.go index 1bfda7a2..246938a0 100644 --- a/pkg/client/keycloak/adapter/gocloak_adapter_user_test.go +++ b/pkg/client/keycloak/adapter/gocloak_adapter_user_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/http/httptest" + "strings" "testing" "github.com/Nerzal/gocloak/v12" @@ -21,6 +22,15 @@ func TestGoCloakAdapter_SyncRealmUser(t *testing.T) { t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "users/user-with-groups-id/groups") { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + + _, err := w.Write([]byte(`[{"id":"group1-id","name":"group1"},{"id":"group2-id","name":"group2"}]`)) + assert.NoError(t, err) + + return + } w.WriteHeader(http.StatusOK) })) @@ -110,7 +120,7 @@ func TestGoCloakAdapter_SyncRealmUser(t *testing.T) { LastName: "last-name", RequiredUserActions: []string{"change-password"}, Roles: []string{"role1"}, - Groups: []string{"group1"}, + Groups: []string{"group1", "group3"}, Attributes: map[string]string{"attr1": "attr1value"}, Password: "password", }, @@ -119,7 +129,7 @@ func TestGoCloakAdapter_SyncRealmUser(t *testing.T) { m.On("GetUsers", mock.Anything, "", "realm", mock.Anything). Return([]*gocloak.User{{ - ID: gocloak.StringP("user-id"), + ID: gocloak.StringP("user-with-groups-id"), Username: gocloak.StringP("user"), }}, nil) m.On("UpdateUser", @@ -139,7 +149,7 @@ func TestGoCloakAdapter_SyncRealmUser(t *testing.T) { mock.Anything, "", "realm", - "user-id", + "user-with-groups-id", mock.MatchedBy(func(roles []gocloak.Role) bool { return assert.Len(t, roles, 1) && assert.Equal(t, "role1-id", *roles[0].ID) @@ -150,16 +160,26 @@ func TestGoCloakAdapter_SyncRealmUser(t *testing.T) { "", "realm", mock.Anything). - Return([]*gocloak.Group{{ - Name: gocloak.StringP("group1"), - ID: gocloak.StringP("group1-id"), - }}, nil) + Return([]*gocloak.Group{ + { + Name: gocloak.StringP("group1"), + ID: gocloak.StringP("group1-id"), + }, + { + Name: gocloak.StringP("group2"), + ID: gocloak.StringP("group2-id"), + }, + { + Name: gocloak.StringP("group3"), + ID: gocloak.StringP("group3-id"), + }, + }, nil) m.On("RestyClient").Return(resty.New()) m.On("DeleteRealmRoleFromUser", mock.Anything, "", "realm", - "user-id", + "user-with-groups-id", mock.Anything, ).Return(nil) diff --git a/pkg/client/keycloak/keycloak_client.go b/pkg/client/keycloak/keycloak_client.go index 71e80950..ab4409be 100644 --- a/pkg/client/keycloak/keycloak_client.go +++ b/pkg/client/keycloak/keycloak_client.go @@ -49,7 +49,7 @@ type KAuthFlow interface { } type KCloakGroups interface { - SyncRealmGroup(realm string, spec *keycloakApi.KeycloakRealmGroupSpec) (string, error) + SyncRealmGroup(ctx context.Context, realm string, spec *keycloakApi.KeycloakRealmGroupSpec) (string, error) DeleteGroup(ctx context.Context, realm, groupName string) error GetGroups(ctx context.Context, realm string) (map[string]*gocloak.Group, error) } diff --git a/pkg/client/keycloak/mocks/client_mock.go b/pkg/client/keycloak/mocks/client_mock.go index 338a6112..811c67ba 100644 --- a/pkg/client/keycloak/mocks/client_mock.go +++ b/pkg/client/keycloak/mocks/client_mock.go @@ -3528,9 +3528,9 @@ func (_c *MockClient_SyncClientProtocolMapper_Call) RunAndReturn(run func(*dto.C return _c } -// SyncRealmGroup provides a mock function with given fields: realm, spec -func (_m *MockClient) SyncRealmGroup(realm string, spec *v1.KeycloakRealmGroupSpec) (string, error) { - ret := _m.Called(realm, spec) +// SyncRealmGroup provides a mock function with given fields: ctx, realm, spec +func (_m *MockClient) SyncRealmGroup(ctx context.Context, realm string, spec *v1.KeycloakRealmGroupSpec) (string, error) { + ret := _m.Called(ctx, realm, spec) if len(ret) == 0 { panic("no return value specified for SyncRealmGroup") @@ -3538,17 +3538,17 @@ func (_m *MockClient) SyncRealmGroup(realm string, spec *v1.KeycloakRealmGroupSp var r0 string var r1 error - if rf, ok := ret.Get(0).(func(string, *v1.KeycloakRealmGroupSpec) (string, error)); ok { - return rf(realm, spec) + if rf, ok := ret.Get(0).(func(context.Context, string, *v1.KeycloakRealmGroupSpec) (string, error)); ok { + return rf(ctx, realm, spec) } - if rf, ok := ret.Get(0).(func(string, *v1.KeycloakRealmGroupSpec) string); ok { - r0 = rf(realm, spec) + if rf, ok := ret.Get(0).(func(context.Context, string, *v1.KeycloakRealmGroupSpec) string); ok { + r0 = rf(ctx, realm, spec) } else { r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(string, *v1.KeycloakRealmGroupSpec) error); ok { - r1 = rf(realm, spec) + if rf, ok := ret.Get(1).(func(context.Context, string, *v1.KeycloakRealmGroupSpec) error); ok { + r1 = rf(ctx, realm, spec) } else { r1 = ret.Error(1) } @@ -3562,15 +3562,16 @@ type MockClient_SyncRealmGroup_Call struct { } // SyncRealmGroup is a helper method to define mock.On call +// - ctx context.Context // - realm string // - spec *v1.KeycloakRealmGroupSpec -func (_e *MockClient_Expecter) SyncRealmGroup(realm interface{}, spec interface{}) *MockClient_SyncRealmGroup_Call { - return &MockClient_SyncRealmGroup_Call{Call: _e.mock.On("SyncRealmGroup", realm, spec)} +func (_e *MockClient_Expecter) SyncRealmGroup(ctx interface{}, realm interface{}, spec interface{}) *MockClient_SyncRealmGroup_Call { + return &MockClient_SyncRealmGroup_Call{Call: _e.mock.On("SyncRealmGroup", ctx, realm, spec)} } -func (_c *MockClient_SyncRealmGroup_Call) Run(run func(realm string, spec *v1.KeycloakRealmGroupSpec)) *MockClient_SyncRealmGroup_Call { +func (_c *MockClient_SyncRealmGroup_Call) Run(run func(ctx context.Context, realm string, spec *v1.KeycloakRealmGroupSpec)) *MockClient_SyncRealmGroup_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(string), args[1].(*v1.KeycloakRealmGroupSpec)) + run(args[0].(context.Context), args[1].(string), args[2].(*v1.KeycloakRealmGroupSpec)) }) return _c } @@ -3580,7 +3581,7 @@ func (_c *MockClient_SyncRealmGroup_Call) Return(_a0 string, _a1 error) *MockCli return _c } -func (_c *MockClient_SyncRealmGroup_Call) RunAndReturn(run func(string, *v1.KeycloakRealmGroupSpec) (string, error)) *MockClient_SyncRealmGroup_Call { +func (_c *MockClient_SyncRealmGroup_Call) RunAndReturn(run func(context.Context, string, *v1.KeycloakRealmGroupSpec) (string, error)) *MockClient_SyncRealmGroup_Call { _c.Call.Return(run) return _c } diff --git a/tests/e2e/helm-success-path/01-install-keycloak-server.yaml b/tests/e2e/helm-success-path/01-install-keycloak-server.yaml index 62f8a149..982e32ed 100644 --- a/tests/e2e/helm-success-path/01-install-keycloak-server.yaml +++ b/tests/e2e/helm-success-path/01-install-keycloak-server.yaml @@ -62,15 +62,18 @@ spec: - name: configs mountPath: /etc/nginx/conf.d - name: keycloak - image: quay.io/keycloak/keycloak:20.0.3 - args: ["start-dev"] + image: quay.io/keycloak/keycloak:24.0.4 + args: + - "start-dev" + - "--proxy-headers" + - "forwarded" + - "--http-enabled" + - "true" env: - name: KEYCLOAK_ADMIN value: "admin" - name: KEYCLOAK_ADMIN_PASSWORD value: "admin" - - name: KC_PROXY - value: "edge" ports: - name: http containerPort: 8080 diff --git a/tests/e2e/keycloak-selfsigned-certificates/01-install-keycloak-server.yaml b/tests/e2e/keycloak-selfsigned-certificates/01-install-keycloak-server.yaml index cdfed3e8..cff17523 100644 --- a/tests/e2e/keycloak-selfsigned-certificates/01-install-keycloak-server.yaml +++ b/tests/e2e/keycloak-selfsigned-certificates/01-install-keycloak-server.yaml @@ -28,7 +28,7 @@ spec: spec: containers: - name: keycloak - image: quay.io/keycloak/keycloak:24.0.2 + image: quay.io/keycloak/keycloak:24.0.4 args: - "start-dev" - "--https-port=8443"