SSO: Fix and improve SAML validations (#1542)

* fix and improve SAML validations

* fix indentation

Author: dmihai

internal/resources/grafana/resource_sso_settings.go

@@ -536,11 +536,11 @@ func UpdateSSOSettings(ctx context.Context, d *schema.ResourceData, meta interfa
 		}
 
 		settings = mergeCustomFields(settings)
+	}
 
-		err = validateOAuth2Settings(provider, settings)
-		if err != nil {
-			return diag.FromErr(err)
-		}
+	err = validateSSOSettings(provider, settings)
+	if err != nil {
+		return diag.FromErr(err)
 	}
 
 	ssoSettings := models.UpdateProviderSettingsParamsBody{
@@ -681,9 +681,15 @@ var validationsByProvider = map[string][]validateFunc{
 		ssoValidateEmpty("token_url"),
 		ssoValidateEmpty("api_url"),
 	},
+	"saml": {
+		ssoValidateOnlyOneOf("certificate", "certificate_path"),
+		ssoValidateOnlyOneOf("private_key", "private_key_path"),
+		ssoValidateOnlyOneOf("idp_metadata", "idp_metadata_path", "idp_metadata_url"),
+		ssoValidateURL("idp_metadata_url"),
+	},
 }
 
-func validateOAuth2Settings(provider string, settings map[string]any) error {
+func validateSSOSettings(provider string, settings map[string]any) error {
 	validators := validationsByProvider[provider]
 	for _, validateF := range validators {
 		err := validateF(settings, provider)
@@ -823,9 +829,27 @@ func ssoValidateEmpty(key string) validateFunc {
 
 func ssoValidateURL(key string) validateFunc {
 	return func(settingsMap map[string]any, provider string) error {
-		if !isValidURL(settingsMap[key].(string)) {
+		if settingsMap[key].(string) != "" && !isValidURL(settingsMap[key].(string)) {
 			return fmt.Errorf("%s must be a valid http/https URL for the provider %s", key, provider)
 		}
 		return nil
 	}
 }
+
+func ssoValidateOnlyOneOf(keys ...string) validateFunc {
+	return func(settingsMap map[string]any, provider string) error {
+		configuredKeys := 0
+
+		for _, key := range keys {
+			if settingsMap[key].(string) != "" {
+				configuredKeys++
+			}
+		}
+
+		if configuredKeys != 1 {
+			return fmt.Errorf("exactly one of %v must be configured for provider %s", keys, provider)
+		}
+
+		return nil
+	}
+}

internal/resources/grafana/resource_sso_settings_test.go

@@ -82,23 +82,13 @@ func TestSSOSettings_basic_saml(t *testing.T) {
 		CheckDestroy:             checkSsoSettingsReset(api, provider, defaultSettings.Payload),
 		Steps: []resource.TestStep{
 			{
-				Config: testConfigForSamlProvider("new"),
+				Config: testConfigForSamlProvider,
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "provider_name", provider),
 					resource.TestCheckResourceAttr(resourceName, "saml_settings.#", "1"),
-					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.certificate_path", "/var/certificate_new"),
-					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.private_key_path", "/var/private_key_new"),
-					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.idp_metadata_path", "/var/idp_metadata_new"),
-				),
-			},
-			{
-				Config: testConfigForSamlProvider("updated"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "provider_name", provider),
-					resource.TestCheckResourceAttr(resourceName, "saml_settings.#", "1"),
-					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.certificate_path", "/var/certificate_updated"),
-					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.private_key_path", "/var/private_key_updated"),
-					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.idp_metadata_path", "/var/idp_metadata_updated"),
+					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.certificate_path", "devenv/docker/blocks/auth/saml-enterprise/cert.crt"),
+					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.private_key_path", "devenv/docker/blocks/auth/saml-enterprise/key.pem"),
+					resource.TestCheckResourceAttr(resourceName, "saml_settings.0.idp_metadata_url", "https://nexus.microsoftonline-p.com/federationmetadata/saml20/federationmetadata.xml"),
 				),
 			},
 			{
@@ -305,16 +295,15 @@ func testConfigForOAuth2Provider(provider string, prefix string) string {
 }`, prefix, provider, urls)
 }
 
-func testConfigForSamlProvider(prefix string) string {
-	return fmt.Sprintf(`resource "grafana_sso_settings" "saml_sso_settings" {
+// the SAML configuration needs a valid certificate, private_key and idp_metadata to be accepted by Grafana API
+const testConfigForSamlProvider = `resource "grafana_sso_settings" "saml_sso_settings" {
   provider_name = "saml"
   saml_settings {
-    certificate_path  = "/var/certificate_%[1]s"
-    private_key_path  = "/var/private_key_%[1]s"
-	idp_metadata_path = "/var/idp_metadata_%[1]s"
+    certificate_path = "devenv/docker/blocks/auth/saml-enterprise/cert.crt"
+    private_key_path = "devenv/docker/blocks/auth/saml-enterprise/key.pem"
+    idp_metadata_url = "https://nexus.microsoftonline-p.com/federationmetadata/saml20/federationmetadata.xml"
   }
-}`, prefix)
-}
+}`
 
 const testConfigWithCustomFields = `resource "grafana_sso_settings" "sso_settings" {
   provider_name = "github"
@@ -453,5 +442,21 @@ var testConfigsWithValidationErrors = []string{
     client_id = "client_id"
     api_url  = "https://login.microsoftonline.com/12345/oauth2/v2.0/userinfo"
   }
+}`,
+	// certificate and certificate_path are both configured for saml
+	`resource "grafana_sso_settings" "saml_sso_settings" {
+  provider_name = "saml"
+  saml_settings {
+    certificate = "this-is-a-valid-certificate"
+    certificate_path = "/valid/certificate/path"
+  }
+}`,
+	// missing idp_metadata for saml
+	`resource "grafana_sso_settings" "saml_sso_settings" {
+  provider_name = "saml"
+  saml_settings {
+    certificate = "this-is-a-valid-certificate"
+    private_key = "this-is-a-valid-private-key"
+  }
 }`,
 }