fix: redirect after email actions (#139)

* fix: redirect after email actions

* chore: add condition in case the realm name is not yet present in the accountinfo object

* fix: update Dockerfile base image and clean up realm subroutine by removing unused repository manifest

* feat: add baseDomain to subroutine for dynamic redirect URI construction

Author: aaronschweig

Dockerfile

@@ -1,5 +1,5 @@
 # Build the manager binary
-FROM golang:1.25.3-bookworm AS builder
+FROM golang:1.25 AS builder
 ARG TARGETOS
 ARG TARGETARCH
 

cmd/operator.go

@@ -12,6 +12,7 @@ import (
 	kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
 	"github.com/kcp-dev/logicalcluster/v3"
 	"github.com/kcp-dev/multicluster-provider/apiexport"
+	openfgav1 "github.com/openfga/api/proto/openfga/v1"
 	accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
@@ -28,7 +29,6 @@ import (
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
 
-	openfgav1 "github.com/openfga/api/proto/openfga/v1"
 	platformeshcontext "github.com/platform-mesh/golang-commons/context"
 	"github.com/platform-mesh/golang-commons/logger"
 	"github.com/platform-mesh/golang-commons/sentry"

internal/subroutine/invite/subroutine.go

@@ -31,6 +31,7 @@ const (
 
 type subroutine struct {
 	keycloakBaseURL string
+	baseDomain      string
 	keycloak        *http.Client
 	mgr             mcmanager.Manager
 }
@@ -62,6 +63,7 @@ func New(ctx context.Context, cfg *config.Config, mgr mcmanager.Manager, pwd str
 
 	return &subroutine{
 		keycloakBaseURL: cfg.Invite.KeycloakBaseURL,
+		baseDomain:      cfg.BaseDomain,
 		mgr:             mgr,
 		keycloak:        config.Client(ctx, token),
 	}, nil
@@ -110,6 +112,10 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime
 	}
 
 	realm := accountInfo.Spec.Organization.Name
+	if realm == "" {
+		log.Error().Msg("Organization name is empty in AccountInfo")
+		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("organization name is empty in AccountInfo"), true, false)
+	}
 
 	res, err := s.keycloak.Get(fmt.Sprintf("%s/admin/realms/%s/users?%s", s.keycloakBaseURL, realm, v.Encode()))
 	if err != nil { // coverage-ignore
@@ -175,8 +181,13 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime
 
 	log.Debug().Str("email", invite.Spec.Email).Str("id", newUser.ID).Msg("User created")
 
-	req, err := http.NewRequestWithContext(ctx, http.MethodPut, fmt.Sprintf("%s/admin/realms/%s/users/%s/execute-actions-email", s.keycloakBaseURL, realm, newUser.ID), http.NoBody)
-	if err != nil {
+	queryParams := url.Values{
+		"redirect_uri": {"https://%s.%s", realm, s.baseDomain},
+		"client_id":    {realm},
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodPut, fmt.Sprintf("%s/admin/realms/%s/users/%s/execute-actions-email?%s", s.keycloakBaseURL, realm, newUser.ID, queryParams.Encode()), http.NoBody)
+	if err != nil { // coverage-ignore
 		return ctrl.Result{}, errors.NewOperatorError(err, true, true)
 	}
 

internal/subroutine/invite/subroutine_test.go

@@ -224,6 +224,7 @@ func TestSubroutineProcess(t *testing.T) {
 					KeycloakBaseURL:  srv.URL,
 					KeycloakClientID: "admin-cli",
 				},
+				BaseDomain: "portal.dev.local:8443",
 			}, mgr, "password")
 			assert.NoError(t, err)
 

internal/subroutine/manifests/organizationIdp/repository.yaml

@@ -24,8 +24,6 @@ import (
 )
 
 var (
-	//go:embed manifests/organizationIdp/repository.yaml
-	repository string
 
 	//go:embed manifests/organizationIdp/helmrelease.yaml
 	helmRelease string
@@ -62,14 +60,6 @@ func (r *realmSubroutine) Finalize(ctx context.Context, instance runtimeobject.R
 		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get workspace path"), true, false)
 	}
 
-	ociObj, err := unstructuredFromString(repository, nil, log)
-	if err != nil {
-		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to load OCI repository manifest: %w", err), true, true)
-	}
-	if err := r.k8s.Delete(ctx, &ociObj); err != nil {
-		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to delete OCI repository: %w", err), true, true)
-	}
-
 	helmObj, err := unstructuredFromString(helmRelease, nil, log)
 	if err != nil {
 		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to load HelmRelease  manifest: %w", err), true, true)
@@ -100,7 +90,7 @@ func (r *realmSubroutine) Process(ctx context.Context, instance runtimeobject.Ru
 			"client": map[string]any{
 				"name":              workspaceName,
 				"displayName":       workspaceName,
-				"validRedirectUris": append(r.cfg.IDP.AdditionalRedirectURLs, fmt.Sprintf("https://%s.%s/callback*", workspaceName, r.baseDomain)),
+				"validRedirectUris": append(r.cfg.IDP.AdditionalRedirectURLs, fmt.Sprintf("https://%s.%s/*", workspaceName, r.baseDomain)),
 			},
 			"organization": map[string]any{
 				"domain": "example.com", // TODO: change
@@ -151,11 +141,6 @@ func (r *realmSubroutine) Process(ctx context.Context, instance runtimeobject.Ru
 	values := apiextensionsv1.JSON{Raw: marshalledPatch}
 	releaseName := fmt.Sprintf("%s-idp", workspaceName)
 
-	err = applyManifestWithMergedValues(ctx, repository, r.k8s, nil)
-	if err != nil {
-		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to create OCI repository: %w", err), true, true)
-	}
-
 	err = applyReleaseWithValues(ctx, helmRelease, r.k8s, values, releaseName)
 	if err != nil {
 		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to create HelmRelease: %w", err), true, true)
@@ -234,18 +219,3 @@ func ReplaceTemplate(templateData map[string]string, templateBytes []byte) ([]by
 	}
 	return result.Bytes(), nil
 }
-
-func applyManifestWithMergedValues(ctx context.Context, manifest string, k8sClient client.Client, templateData map[string]string) error {
-	log := logger.LoadLoggerFromContext(ctx)
-
-	obj, err := unstructuredFromString(manifest, templateData, log)
-	if err != nil {
-		return err
-	}
-
-	err = k8sClient.Patch(ctx, &obj, client.Apply, client.FieldOwner("security-operator"))
-	if err != nil {
-		return errors.Wrap(err, "Failed to apply manifest (%s/%s)", obj.GetKind(), obj.GetName())
-	}
-	return nil
-}

internal/subroutine/realm.go

@@ -19,14 +19,6 @@ import (
 )
 
 const (
-	repoYAML = `
-apiVersion: v1
-kind: Config
-metadata:
-  name: repo-min
-spec:
-  foo: bar
-`
 	helmReleaseYAML = `
 apiVersion: helm.toolkit.fluxcd.io/v2beta1
 kind: HelmRelease
@@ -100,24 +92,6 @@ spec:
 			},
 			true,
 		},
-		{"manifest: invalid YAML", "manifest", "this: is: : invalid: yaml", nil, true},
-		{
-			"manifest: patch error wrapped",
-			"manifest",
-			trim(`
-apiVersion: v1
-kind: Config
-metadata:
-  name: test-manifest
-spec:
-  foo: bar
-`),
-			func(m *mocks.MockClient) {
-				m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).
-					Return(errors.New("simulated patch error for manifest")).Once()
-			},
-			true,
-		},
 	}
 
 	for _, tc := range cases {
@@ -133,13 +107,6 @@ spec:
 				} else {
 					require.NoError(t, err)
 				}
-			case "manifest":
-				err := applyManifestWithMergedValues(ctx, tc.content, clientMock, nil)
-				if tc.expectErr {
-					require.Error(t, err)
-				} else {
-					require.NoError(t, err)
-				}
 			default:
 				t.Fatalf("unknown call type %q", tc.call)
 			}
@@ -148,20 +115,13 @@ spec:
 }
 
 func TestRealmSubroutine_ProcessAndFinalize(t *testing.T) {
-	origRepo, origHR := repository, helmRelease
-	defer func() { repository, helmRelease = origRepo, origHR }()
+	origHR := helmRelease
+	defer func() { helmRelease = origHR }()
 
 	t.Run("Process", func(t *testing.T) {
 		t.Run("success create repo then helmrelease", func(t *testing.T) {
 			t.Parallel()
 			clientMock := newClientMock(t, func(m *mocks.MockClient) {
-				m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).
-					RunAndReturn(func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error {
-						_, ok := obj.(*unstructured.Unstructured)
-						require.True(t, ok)
-						return nil
-					}).Once()
-
 				m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).
 					RunAndReturn(func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error {
 						hr := obj.(*unstructured.Unstructured)
@@ -174,7 +134,7 @@ func TestRealmSubroutine_ProcessAndFinalize(t *testing.T) {
 					}).Once()
 			})
 
-			repository, helmRelease = trim(repoYAML), trim(helmReleaseYAML)
+			helmRelease = trim(helmReleaseYAML)
 
 			rs := NewRealmSubroutine(clientMock, &config.Config{}, baseDomain)
 			lc := &kcpv1alpha1.LogicalCluster{}
@@ -188,7 +148,7 @@ func TestRealmSubroutine_ProcessAndFinalize(t *testing.T) {
 		t.Run("success create with SMTP config", func(t *testing.T) {
 			t.Parallel()
 			clientMock := newClientMock(t, func(m *mocks.MockClient) {
-				m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Twice()
+				m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
 			})
 
 			cfg := &config.Config{}
@@ -205,14 +165,14 @@ func TestRealmSubroutine_ProcessAndFinalize(t *testing.T) {
 			require.Equal(t, ctrl.Result{}, res)
 		})
 
-		t.Run("oci repository apply fails", func(t *testing.T) {
+		t.Run("helmrelease apply fails", func(t *testing.T) {
 			t.Parallel()
 			clientMock := newClientMock(t, func(m *mocks.MockClient) {
 				m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).
-					Return(errors.New("simulated patch failure for OCI repo")).Once()
+					Return(errors.New("simulated patch failure for HelmRelease")).Once()
 			})
 
-			repository, helmRelease = trim(repoYAML), trim(helmReleaseYAML)
+			helmRelease = trim(helmReleaseYAML)
 			rs := NewRealmSubroutine(clientMock, &config.Config{}, baseDomain)
 			lc := &kcpv1alpha1.LogicalCluster{}
 			lc.Annotations = map[string]string{"kcp.io/path": "root:orgs:test"}
@@ -250,26 +210,17 @@ func TestRealmSubroutine_ProcessAndFinalize(t *testing.T) {
 			expectErr      bool
 			expectedResult ctrl.Result
 		}{
-			{
-				"OCI delete error",
-				func(m *mocks.MockClient) {
-					m.EXPECT().Delete(mock.Anything, mock.Anything).Return(errors.New("failed to delete oci repository")).Once()
-				},
-				true,
-				ctrl.Result{},
-			},
 			{
 				"HelmRelease delete error",
 				func(m *mocks.MockClient) {
-					m.EXPECT().Delete(mock.Anything, mock.Anything).Return(nil).Once()
 					m.EXPECT().Delete(mock.Anything, mock.Anything).Return(errors.New("failed to delete helmRelease")).Once()
 				},
 				true,
 				ctrl.Result{},
 			},
 			{
-				"Both deletes succeed",
-				func(m *mocks.MockClient) { m.EXPECT().Delete(mock.Anything, mock.Anything).Return(nil).Twice() },
+				"Delete succeeds",
+				func(m *mocks.MockClient) { m.EXPECT().Delete(mock.Anything, mock.Anything).Return(nil).Once() },
 				false,
 				ctrl.Result{},
 			},