From 2383ebd89d60ad21f69e7fb7b357e69f37d0e7ff Mon Sep 17 00:00:00 2001 From: Kyle Hiller Date: Thu, 24 Oct 2024 22:22:04 -0400 Subject: [PATCH] AUTH-6559 support strictness setting in access application scim mappings --- .changelog/4419.txt | 3 ++ docs/resources/access_application.md | 6 +-- ...urce_cloudflare_access_application_test.go | 3 ++ ...rce_cloudflare_access_identity_provider.go | 9 ++-- ...loudflare_access_identity_provider_test.go | 47 ++++++++++++------- .../schema_cloudflare_access_application.go | 12 ++++- 6 files changed, 57 insertions(+), 23 deletions(-) create mode 100644 .changelog/4419.txt diff --git a/.changelog/4419.txt b/.changelog/4419.txt new file mode 100644 index 0000000000..788379ac7c --- /dev/null +++ b/.changelog/4419.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/cloudflare_access_application: support SCIM schema strictness setting for outbound provisioning +``` \ No newline at end of file diff --git a/docs/resources/access_application.md b/docs/resources/access_application.md index e592ed5379..ebd2d23d2d 100644 --- a/docs/resources/access_application.md +++ b/docs/resources/access_application.md @@ -299,11 +299,11 @@ Required: - `schema` (String) Which SCIM resource type this mapping applies to. Optional: - -- `enabled` (Boolean) Whether or not this mapping is enabled. +- `enabled` (Boolean) Whether this mapping is enabled. - `filter` (String) A [SCIM filter expression](https://datatracker.ietf.org/doc/html/rfc7644#section-3.4.2.2) that matches resources that should be provisioned to this application. -- `operations` (Block List, Max: 1) Whether or not this mapping applies to creates, updates, or deletes. (see [below for nested schema](#nestedblock--scim_config--mappings--operations)) - `transform_jsonata` (String) A [JSONata](https://jsonata.org/) expression that transforms the resource before provisioning it in the application. +- `operations` (Block List, Max: 1) Whether this mapping applies to creates, updates, or deletes. See [below for nested schema](#nestedblock--scim_config--mapping--operations) +- `strictness` (String) Available values: `strict`, `passthrough`. How strictly to adhere to outbound resource schemas when provisioning to this mapping. `strict` will remove unknown values when provisioning, while `passthrough` will pass unknown values to the target. ### Nested Schema for `scim_config.mappings.operations` diff --git a/internal/sdkv2provider/resource_cloudflare_access_application_test.go b/internal/sdkv2provider/resource_cloudflare_access_application_test.go index 946ab45d29..a101aaa5d8 100644 --- a/internal/sdkv2provider/resource_cloudflare_access_application_test.go +++ b/internal/sdkv2provider/resource_cloudflare_access_application_test.go @@ -171,6 +171,7 @@ func TestAccCloudflareAccessApplication_WithSCIMConfigHttpBasic(t *testing.T) { resource.TestCheckResourceAttr(name, "scim_config.0.mappings.0.operations.0.create", "true"), resource.TestCheckResourceAttr(name, "scim_config.0.mappings.0.operations.0.update", "true"), resource.TestCheckResourceAttr(name, "scim_config.0.mappings.0.operations.0.delete", "true"), + resource.TestCheckResourceAttr(name, "scim_config.0.mappings.0.strictness", "passthrough"), ), }, }, @@ -1795,6 +1796,7 @@ resource "cloudflare_zero_trust_access_application" "%[1]s" { update = true delete = true } + strictness = "passthrough" } } } @@ -1886,6 +1888,7 @@ resource "cloudflare_zero_trust_access_application" "%[1]s" { update = true delete = true } + strictness = "strict" } } } diff --git a/internal/sdkv2provider/resource_cloudflare_access_identity_provider.go b/internal/sdkv2provider/resource_cloudflare_access_identity_provider.go index 28d2f10d70..15754db793 100644 --- a/internal/sdkv2provider/resource_cloudflare_access_identity_provider.go +++ b/internal/sdkv2provider/resource_cloudflare_access_identity_provider.go @@ -76,7 +76,10 @@ func resourceCloudflareAccessIdentityProviderRead(ctx context.Context, d *schema d.Set("name", accessIdentityProvider.Name) d.Set("type", accessIdentityProvider.Type) - config := convertAccessIDPConfigStructToSchema(accessIdentityProvider.Config) + // We need to get the current secret and set that in the state as it is only + // returned initially when the resource was created. The value read from the + // server will always be redacted. + config := convertAccessIDPConfigStructToSchema(d.Get("config.0.client_secret").(string), accessIdentityProvider.Config) if configErr := d.Set("config", config); configErr != nil { return diag.FromErr(fmt.Errorf("error setting Access Identity Provider configuration: %w", configErr)) } @@ -285,7 +288,7 @@ func convertScimConfigSchemaToStruct(d *schema.ResourceData) cloudflare.AccessId return ScimConfig } -func convertAccessIDPConfigStructToSchema(options cloudflare.AccessIdentityProviderConfiguration) []interface{} { +func convertAccessIDPConfigStructToSchema(secret string, options cloudflare.AccessIdentityProviderConfiguration) []interface{} { attributes := make([]string, 0) for _, value := range options.Attributes { attributes = append(attributes, value) @@ -301,7 +304,7 @@ func convertAccessIDPConfigStructToSchema(options cloudflare.AccessIdentityProvi "centrify_app_id": options.CentrifyAppID, "certs_url": options.CertsURL, "client_id": options.ClientID, - "client_secret": options.ClientSecret, + "client_secret": secret, "claims": options.Claims, "scopes": options.Scopes, "directory_id": options.DirectoryID, diff --git a/internal/sdkv2provider/resource_cloudflare_access_identity_provider_test.go b/internal/sdkv2provider/resource_cloudflare_access_identity_provider_test.go index 27b3b692ba..251c215927 100644 --- a/internal/sdkv2provider/resource_cloudflare_access_identity_provider_test.go +++ b/internal/sdkv2provider/resource_cloudflare_access_identity_provider_test.go @@ -22,6 +22,30 @@ func init() { }) } +func testSecretPresent(value string) error { + if value == "" { + return errors.New("secret is empty") + } + + if strings.Contains(value, "*") { + return errors.New("secret was redacted") + } + + return nil +} + +func testSecretRedacted(value string) error { + if value == "" { + return errors.New("secret is empty") + } + + if strings.Contains(value, "*") { + return nil + } + + return errors.New("secret was set to secret value") +} + func testSweepCloudflareAccessIdentityProviders(r string) error { ctx := context.Background() accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID") @@ -115,6 +139,7 @@ func TestAccCloudflareAccessIdentityProvider_OAuth(t *testing.T) { accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID") rnd := generateRandomResourceName() resourceName := "cloudflare_zero_trust_access_identity_provider." + rnd + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) @@ -129,7 +154,7 @@ func TestAccCloudflareAccessIdentityProvider_OAuth(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "name", rnd), resource.TestCheckResourceAttr(resourceName, "type", "github"), resource.TestCheckResourceAttr(resourceName, "config.0.client_id", "test"), - resource.TestCheckResourceAttrSet(resourceName, "config.0.client_secret"), + resource.TestCheckResourceAttrWith(resourceName, "config.0.client_secret", testSecretPresent), ), }, }, @@ -155,7 +180,6 @@ func TestAccCloudflareAccessIdentityProvider_OAuthWithUpdate(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "name", rnd), resource.TestCheckResourceAttr(resourceName, "type", "github"), resource.TestCheckResourceAttr(resourceName, "config.0.client_id", "test"), - resource.TestCheckResourceAttrSet(resourceName, "config.0.client_secret"), ), }, { @@ -165,7 +189,6 @@ func TestAccCloudflareAccessIdentityProvider_OAuthWithUpdate(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "name", rnd+"-updated"), resource.TestCheckResourceAttr(resourceName, "type", "github"), resource.TestCheckResourceAttr(resourceName, "config.0.client_id", "test"), - resource.TestCheckResourceAttrSet(resourceName, "config.0.client_secret"), ), }, }, @@ -225,7 +248,7 @@ func TestAccCloudflareAccessIdentityProvider_AzureAD(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "type", "azureAD"), resource.TestCheckResourceAttr(resourceName, "config.0.client_id", "test"), resource.TestCheckResourceAttr(resourceName, "config.0.directory_id", "directory"), - resource.TestCheckResourceAttr(resourceName, "config.0.condtional_access_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "config.0.conditional_access_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "scim_config.0.enabled", "true"), resource.TestCheckResourceAttr(resourceName, "scim_config.0.user_deprovision", "true"), resource.TestCheckResourceAttr(resourceName, "scim_config.0.seat_deprovision", "true"), @@ -247,7 +270,7 @@ func TestAccCloudflareAccessIdentityProvider_OAuth_Import(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "name", rnd), resource.TestCheckResourceAttr(resourceName, "type", "github"), resource.TestCheckResourceAttr(resourceName, "config.0.client_id", "test"), - resource.TestCheckResourceAttrSet(resourceName, "config.0.client_secret"), + resource.TestCheckResourceAttrWith(resourceName, "config.0.client_secret", testSecretPresent), ) resource.Test(t, resource.TestCase{ @@ -267,6 +290,8 @@ func TestAccCloudflareAccessIdentityProvider_OAuth_Import(t *testing.T) { ResourceName: resourceName, ImportStateIdPrefix: fmt.Sprintf("%s/", accountID), Check: checkFn, + // We never return secrets so we should just ignore the import + ImportStateVerifyIgnore: []string{"config.0.client_secret"}, }, }, }) @@ -279,17 +304,7 @@ func TestAccCloudflareAccessIdentityProvider_SCIM_Config_Secret(t *testing.T) { resourceName := "cloudflare_zero_trust_access_identity_provider." + rnd checkFn := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrWith(resourceName, "scim_config.0.secret", func(value string) error { - if value == "" { - return errors.New("secret is empty") - } - - if strings.Contains(value, "*") { - return errors.New("secret was redacted") - } - - return nil - }), + resource.TestCheckResourceAttrWith(resourceName, "scim_config.0.secret", testSecretPresent), ) resource.Test(t, resource.TestCase{ diff --git a/internal/sdkv2provider/schema_cloudflare_access_application.go b/internal/sdkv2provider/schema_cloudflare_access_application.go index e73632b2f9..eae91df2e2 100644 --- a/internal/sdkv2provider/schema_cloudflare_access_application.go +++ b/internal/sdkv2provider/schema_cloudflare_access_application.go @@ -655,7 +655,7 @@ func resourceCloudflareAccessApplicationSchema() map[string]*schema.Schema { "idp_uid": { Type: schema.TypeString, Required: true, - Description: "The UID of the IdP to use as the source for SCIM resources to provision to this application.", + Description: "The UIDs of the IdP to use as the source for SCIM resources to provision to this application.", }, "deactivate_on_delete": { Type: schema.TypeBool, @@ -799,6 +799,11 @@ func resourceCloudflareAccessApplicationSchema() map[string]*schema.Schema { }, }, }, + "strictness": { + Type: schema.TypeString, + Optional: true, + Description: "How strictly to adhere to outbound resource schemas when provisioning to this mapping. \"strict\" will remove unknown values when provisioning, while \"passthrough\" will pass unknown values to the target.", + }, }, }, }, @@ -1108,6 +1113,10 @@ func convertScimConfigMappingsSchemaToStruct(mappingData map[string]interface{}) mapping.TransformJsonata = transformJsonata.(string) } + if strictness, ok := mappingData["strictness"]; ok { + mapping.Strictness = strictness.(string) + } + if operations, ok := mappingData["operations"]; ok { ops := new(cloudflare.AccessApplicationScimMappingOperations) @@ -1420,6 +1429,7 @@ func convertScimConfigMappingsStructsToSchema(mappingsData []*cloudflare.AccessA "enabled": mapping.Enabled, "filter": mapping.Filter, "transform_jsonata": mapping.TransformJsonata, + "strictness": mapping.Strictness, } if mapping.Operations != nil {