From 464ca0bd00dd14cea1fd791a96161983ef15798a Mon Sep 17 00:00:00 2001
From: Elio Bischof <elio@zitadel.com>
Date: Fri, 11 Oct 2024 10:09:51 +0200
Subject: [PATCH] fix: always create SAML with metadata  (#8696)

# Which Problems Are Solved

Adding a SAML IDPs with an empty metadata XML and URL leads to failed
projection events. The main IDP template projection succeeds, but the
subtable projection for SAML template fails, because the metadata field
is not nullable in that table. The SAML IDP shows up in list queries,
because the list method only queries the main template projection.
However, getting a SAML IDP created without metadata by ID misses the
SAML specific IDP data.

# How the Problems Are Solved

- The command for adding a SAML IDP properly ensures that non-empty
metadata is either given by XML or resolved by URL.
- The console doesn't send requests with empty metadata anymore. This
works by explicitly setting a single oneof option for either XML or URL
and emptying the other one.

# Additional Context

Closes #8443
---
 .../providers/provider-saml-sp/provider-saml-sp.component.ts | 5 ++++-
 internal/command/instance_idp.go                             | 4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/console/src/app/modules/providers/provider-saml-sp/provider-saml-sp.component.ts b/console/src/app/modules/providers/provider-saml-sp/provider-saml-sp.component.ts
index b0741c7b3..6ebd99371 100644
--- a/console/src/app/modules/providers/provider-saml-sp/provider-saml-sp.component.ts
+++ b/console/src/app/modules/providers/provider-saml-sp/provider-saml-sp.component.ts
@@ -198,8 +198,10 @@ export class ProviderSamlSpComponent {
       req.setId(this.provider?.id || this.justCreated$.value);
       req.setName(this.name?.value);
       if (this.metadataXml?.value) {
+        req.setMetadataUrl('');
         req.setMetadataXml(this.metadataXml?.value);
       } else {
+        req.setMetadataXml('');
         req.setMetadataUrl(this.metadataUrl?.value);
       }
       req.setWithSignedRequest(this.withSignedRequest?.value);
@@ -210,7 +212,6 @@ export class ProviderSamlSpComponent {
       req.setTransientMappingAttributeName(this.transientMapping?.value);
       req.setProviderOptions(this.options);
 
-      console.log(req);
       this.loading = true;
       this.service
         .updateSAMLProvider(req)
@@ -234,8 +235,10 @@ export class ProviderSamlSpComponent {
         : new AdminAddSAMLProviderRequest();
     req.setName(this.name?.value);
     if (this.metadataXml?.value) {
+      req.setMetadataUrl('');
       req.setMetadataXml(this.metadataXml?.value);
     } else {
+      req.setMetadataXml('');
       req.setMetadataUrl(this.metadataUrl?.value);
     }
     req.setProviderOptions(this.options);
diff --git a/internal/command/instance_idp.go b/internal/command/instance_idp.go
index 539136a08..c3940c007 100644
--- a/internal/command/instance_idp.go
+++ b/internal/command/instance_idp.go
@@ -1724,14 +1724,14 @@ func (c *Commands) prepareAddInstanceSAMLProvider(a *instance.Aggregate, writeMo
 		if provider.Name = strings.TrimSpace(provider.Name); provider.Name == "" {
 			return nil, zerrors.ThrowInvalidArgument(nil, "INST-o07zjotgnd", "Errors.Invalid.Argument")
 		}
-		if provider.Metadata == nil && provider.MetadataURL != "" {
+		if len(provider.Metadata) == 0 && provider.MetadataURL != "" {
 			data, err := xml.ReadMetadataFromURL(c.httpClient, provider.MetadataURL)
 			if err != nil {
 				return nil, zerrors.ThrowInvalidArgument(err, "INST-8vam1khq22", "Errors.Project.App.SAMLMetadataMissing")
 			}
 			provider.Metadata = data
 		}
-		if provider.Metadata == nil {
+		if len(provider.Metadata) == 0 {
 			return nil, zerrors.ThrowInvalidArgument(nil, "INST-3bi3esi16t", "Errors.Invalid.Argument")
 		}
 		return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {