From 0298f656d657a80962a422d8f875cfa3f063f127 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Fri, 21 May 2021 14:51:56 +1000 Subject: [PATCH] access_application: prevent bad CORS config with allowing all origins and credentials Updates the Access Application resource to better protect against scenarios where people unknowning violate a CORS restriction where you cannot allow all origins and use credentials[1]. The service prevents this however the Terraform resource did not resulting in bad state if you ever attempted this. Fixes #1059 [1]: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS/Errors/CORSNotSupportingCredentials --- .../resource_cloudflare_access_application.go | 9 ++ ...urce_cloudflare_access_application_test.go | 104 +++++++++++++++++- 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/cloudflare/resource_cloudflare_access_application.go b/cloudflare/resource_cloudflare_access_application.go index 4f7cafc4a5..047a2d991d 100644 --- a/cloudflare/resource_cloudflare_access_application.go +++ b/cloudflare/resource_cloudflare_access_application.go @@ -350,6 +350,15 @@ func convertCORSSchemaToStruct(d *schema.ResourceData) (*cloudflare.AccessApplic CORSConfig.AllowCredentials = d.Get("cors_headers.0.allow_credentials").(bool) CORSConfig.MaxAge = d.Get("cors_headers.0.max_age").(int) + // Prevent misconfigurations of CORS when `Access-Control-Allow-Origin` is + // a wildcard (aka all origins) and using credentials. + // See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS/Errors/CORSNotSupportingCredentials + if CORSConfig.AllowCredentials { + if contains(CORSConfig.AllowedOrigins, "*") || CORSConfig.AllowAllOrigins { + return nil, errors.New("CORS credentials are not permitted when all origins are allowed") + } + } + // Ensure that should someone forget to set allowed methods (either // individually or *), we throw an error to prevent getting into an // unrecoverable state. diff --git a/cloudflare/resource_cloudflare_access_application_test.go b/cloudflare/resource_cloudflare_access_application_test.go index 70559357c5..ea0ed257ad 100644 --- a/cloudflare/resource_cloudflare_access_application_test.go +++ b/cloudflare/resource_cloudflare_access_application_test.go @@ -199,11 +199,11 @@ func TestAccCloudflareAccessApplicationWithADefinedIdps(t *testing.T) { func testAccCloudflareAccessApplicationConfigBasic(rnd string, domain string, identifier AccessIdentifier) string { return fmt.Sprintf(` resource "cloudflare_access_application" "%[1]s" { - %[3]s_id = "%[4]s" - name = "%[1]s" - domain = "%[1]s.%[2]s" - session_duration = "24h" - auto_redirect_to_identity = false + %[3]s_id = "%[4]s" + name = "%[1]s" + domain = "%[1]s.%[2]s" + session_duration = "24h" + auto_redirect_to_identity = false } `, rnd, domain, identifier.Type, identifier.Value) } @@ -258,7 +258,7 @@ resource "cloudflare_access_application" "%[1]s" { domain = "%[1]s.%[3]s" session_duration = "24h" custom_deny_message = "denied!" - custom_deny_url = "https://www.cloudflare.com" + custom_deny_url = "https://www.cloudflare.com" } `, rnd, zoneID, domain) } @@ -435,6 +435,66 @@ func TestAccCloudflareAccessApplicationWithInvalidSessionDuration(t *testing.T) }) } +func TestAccessApplicationMisconfiguredCORSCredentialsAllowingAllOrigins(t *testing.T) { + // Temporarily unset CLOUDFLARE_API_TOKEN if it is set as the Access + // service does not yet support the API tokens and it results in + // misleading state error messages. + if os.Getenv("CLOUDFLARE_API_TOKEN") != "" { + defer func(apiToken string) { + os.Setenv("CLOUDFLARE_API_TOKEN", apiToken) + }(os.Getenv("CLOUDFLARE_API_TOKEN")) + os.Setenv("CLOUDFLARE_API_TOKEN", "") + } + + rnd := generateRandomResourceName() + zone := os.Getenv("CLOUDFLARE_DOMAIN") + zoneID := os.Getenv("CLOUDFLARE_ZONE_ID") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccessAccPreCheck(t) + testAccPreCheckAccount(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccessApplicationMisconfiguredCORSAllowAllOriginsWithCredentials(rnd, zone, zoneID), + ExpectError: regexp.MustCompile(regexp.QuoteMeta(`CORS credentials are not permitted when all origins are allowed`)), + }, + }, + }) +} + +func TestAccessApplicationMisconfiguredCORSCredentialsAllowingWildcardOrigins(t *testing.T) { + // Temporarily unset CLOUDFLARE_API_TOKEN if it is set as the Access + // service does not yet support the API tokens and it results in + // misleading state error messages. + if os.Getenv("CLOUDFLARE_API_TOKEN") != "" { + defer func(apiToken string) { + os.Setenv("CLOUDFLARE_API_TOKEN", apiToken) + }(os.Getenv("CLOUDFLARE_API_TOKEN")) + os.Setenv("CLOUDFLARE_API_TOKEN", "") + } + + rnd := generateRandomResourceName() + zone := os.Getenv("CLOUDFLARE_DOMAIN") + zoneID := os.Getenv("CLOUDFLARE_ZONE_ID") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccessAccPreCheck(t) + testAccPreCheckAccount(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccessApplicationMisconfiguredCORSAllowWildcardOriginWithCredentials(rnd, zone, zoneID), + ExpectError: regexp.MustCompile(regexp.QuoteMeta(`CORS credentials are not permitted when all origins are allowed`)), + }, + }, + }) +} + func testAccessApplicationWithZoneID(resourceID, zone, zoneID string) string { return fmt.Sprintf(` resource "cloudflare_access_application" "%[1]s" { @@ -493,3 +553,35 @@ func testAccessApplicationWithInvalidSessionDuration(resourceID, zone, zoneID st } `, resourceID, zone, zoneID) } + +func testAccessApplicationMisconfiguredCORSAllowAllOriginsWithCredentials(resourceID, zone, zoneID string) string { + return fmt.Sprintf(` + resource "cloudflare_access_application" "%[1]s" { + name = "%[1]s-updated" + zone_id = "%[3]s" + domain = "%[1]s.%[2]s" + + cors_headers { + allowed_methods = ["GET"] + allow_all_origins = true + allow_credentials = true + } + } + `, resourceID, zone, zoneID) +} + +func testAccessApplicationMisconfiguredCORSAllowWildcardOriginWithCredentials(resourceID, zone, zoneID string) string { + return fmt.Sprintf(` + resource "cloudflare_access_application" "%[1]s" { + name = "%[1]s-updated" + zone_id = "%[3]s" + domain = "%[1]s.%[2]s" + + cors_headers { + allowed_methods = ["GET"] + allowed_origins = ["*"] + allow_credentials = true + } + } + `, resourceID, zone, zoneID) +}