Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/cloudflare_access_application: fix inability to update http_only_cookie_attribute to false #1619

Merged
3 changes: 3 additions & 0 deletions .changelog/1602.txt
@@ -0,0 +1,3 @@
```release-note:bug
resource/cloudflare_access_application: Fix inability to update `http_only_cookie_attribute` to false
```
13 changes: 5 additions & 8 deletions cloudflare/resource_cloudflare_access_application.go
Expand Up @@ -40,18 +40,14 @@ func resourceCloudflareAccessApplicationCreate(ctx context.Context, d *schema.Re
EnableBindingCookie: d.Get("enable_binding_cookie").(bool),
CustomDenyMessage: d.Get("custom_deny_message").(string),
CustomDenyURL: d.Get("custom_deny_url").(string),
HttpOnlyCookieAttribute: d.Get("http_only_cookie_attribute").(bool),
HttpOnlyCookieAttribute: cloudflare.BoolPtr(d.Get("http_only_cookie_attribute").(bool)),
SameSiteCookieAttribute: d.Get("same_site_cookie_attribute").(string),
LogoURL: d.Get("logo_url").(string),
SkipInterstitial: d.Get("skip_interstitial").(bool),
AppLauncherVisible: d.Get("app_launcher_visible").(bool),
ServiceAuth401Redirect: d.Get("service_auth_401_redirect").(bool),
}

if value, ok := d.GetOk("http_only_cookie_attribute"); ok {
newAccessApplication.HttpOnlyCookieAttribute = value.(bool)
}

if len(allowedIDPList) > 0 {
newAccessApplication.AllowedIdps = allowedIDPList
}
Expand Down Expand Up @@ -102,7 +98,8 @@ func resourceCloudflareAccessApplicationRead(ctx context.Context, d *schema.Reso
}

if err != nil {
if strings.Contains(err.Error(), "HTTP status 404") {
var notFoundError *cloudflare.NotFoundError
if errors.As(err, &notFoundError) {
jacobbednarz marked this conversation as resolved.
Show resolved Hide resolved
log.Printf("[INFO] Access Application %s no longer exists", d.Id())
d.SetId("")
return nil
Expand All @@ -120,7 +117,7 @@ func resourceCloudflareAccessApplicationRead(ctx context.Context, d *schema.Reso
d.Set("custom_deny_message", accessApplication.CustomDenyMessage)
d.Set("custom_deny_url", accessApplication.CustomDenyURL)
d.Set("allowed_idps", accessApplication.AllowedIdps)
d.Set("http_only_cookie_attribute", accessApplication.HttpOnlyCookieAttribute)
d.Set("http_only_cookie_attribute", cloudflare.Bool(accessApplication.HttpOnlyCookieAttribute))
d.Set("same_site_cookie_attribute", accessApplication.SameSiteCookieAttribute)
d.Set("skip_interstitial", accessApplication.SkipInterstitial)
d.Set("logo_url", accessApplication.LogoURL)
Expand Down Expand Up @@ -151,7 +148,7 @@ func resourceCloudflareAccessApplicationUpdate(ctx context.Context, d *schema.Re
EnableBindingCookie: d.Get("enable_binding_cookie").(bool),
CustomDenyMessage: d.Get("custom_deny_message").(string),
CustomDenyURL: d.Get("custom_deny_url").(string),
HttpOnlyCookieAttribute: d.Get("http_only_cookie_attribute").(bool),
HttpOnlyCookieAttribute: cloudflare.BoolPtr(d.Get("http_only_cookie_attribute").(bool)),
SameSiteCookieAttribute: d.Get("same_site_cookie_attribute").(string),
LogoURL: d.Get("logo_url").(string),
SkipInterstitial: d.Get("skip_interstitial").(bool),
Expand Down
129 changes: 60 additions & 69 deletions cloudflare/resource_cloudflare_access_application_test.go
Expand Up @@ -10,14 +10,15 @@ import (
"github.com/cloudflare/cloudflare-go"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
"github.com/pkg/errors"
)

var (
zoneID = os.Getenv("CLOUDFLARE_ZONE_ID")
domain = os.Getenv("CLOUDFLARE_DOMAIN")
)

func TestAccCloudflareAccessApplication_Basic(t *testing.T) {
func TestAccCloudflareAccessApplication_BasicZone(t *testing.T) {
rnd := generateRandomResourceName()
name := fmt.Sprintf("cloudflare_access_application.%s", rnd)

Expand All @@ -42,6 +43,11 @@ func TestAccCloudflareAccessApplication_Basic(t *testing.T) {
},
},
})
}

func TestAccCloudflareAccessApplication_BasicAccount(t *testing.T) {
rnd := generateRandomResourceName()
name := fmt.Sprintf("cloudflare_access_application.%s", rnd)

resource.Test(t, resource.TestCase{
PreCheck: func() {
Expand Down Expand Up @@ -215,7 +221,7 @@ func TestAccCloudflareAccessApplication_WithHttpOnlyCookieAttribute(t *testing.T
CheckDestroy: testAccCheckCloudflareAccessApplicationDestroy,
Steps: []resource.TestStep{
{
Config: testAccCloudflareAccessApplicationConfigWithHttpOnlyCookieAttribute(rnd, zoneID, domain),
Config: testAccCloudflareAccessApplicationConfigWithHTTPOnlyCookieAttribute(rnd, zoneID, domain),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(name, "zone_id", zoneID),
resource.TestCheckResourceAttr(name, "name", rnd),
Expand All @@ -229,6 +235,32 @@ func TestAccCloudflareAccessApplication_WithHttpOnlyCookieAttribute(t *testing.T
})
}

func TestAccCloudflareAccessApplication_WithHTTPOnlyCookieAttributeSetToFalse(t *testing.T) {
rnd := generateRandomResourceName()
name := fmt.Sprintf("cloudflare_access_application.%s", rnd)

resource.Test(t, resource.TestCase{
PreCheck: func() {
testAccessAccPreCheck(t)
},
Providers: testAccProviders,
CheckDestroy: testAccCheckCloudflareAccessApplicationDestroy,
Steps: []resource.TestStep{
{
Config: testAccCloudflareAccessApplicationConfigWithHTTPOnlyCookieAttributeSetToFalse(rnd, zoneID, domain),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(name, "zone_id", zoneID),
resource.TestCheckResourceAttr(name, "name", rnd),
resource.TestCheckResourceAttr(name, "domain", fmt.Sprintf("%s.%s", rnd, domain)),
resource.TestCheckResourceAttr(name, "type", "self_hosted"),
resource.TestCheckResourceAttr(name, "session_duration", "24h"),
resource.TestCheckResourceAttr(name, "http_only_cookie_attribute", "false"),
),
},
},
})
}

func TestAccCloudflareAccessApplication_WithSameSiteCookieAttribute(t *testing.T) {
rnd := generateRandomResourceName()
name := fmt.Sprintf("cloudflare_access_application.%s", rnd)
Expand Down Expand Up @@ -424,7 +456,7 @@ resource "cloudflare_access_application" "%[1]s" {
`, rnd, zoneID, domain, accountID)
}

func testAccCloudflareAccessApplicationConfigWithHttpOnlyCookieAttribute(rnd, zoneID, domain string) string {
func testAccCloudflareAccessApplicationConfigWithHTTPOnlyCookieAttribute(rnd, zoneID, domain string) string {
return fmt.Sprintf(`
resource "cloudflare_access_application" "%[1]s" {
zone_id = "%[2]s"
Expand All @@ -437,6 +469,19 @@ resource "cloudflare_access_application" "%[1]s" {
`, rnd, zoneID, domain)
}

func testAccCloudflareAccessApplicationConfigWithHTTPOnlyCookieAttributeSetToFalse(rnd, zoneID, domain string) string {
return fmt.Sprintf(`
resource "cloudflare_access_application" "%[1]s" {
zone_id = "%[2]s"
name = "%[1]s"
domain = "%[1]s.%[3]s"
type = "self_hosted"
session_duration = "24h"
http_only_cookie_attribute = false
}
`, rnd, zoneID, domain)
}

func testAccCloudflareAccessApplicationConfigSameSiteCookieAttribute(rnd, zoneID, domain string) string {
return fmt.Sprintf(`
resource "cloudflare_access_application" "%[1]s" {
Expand Down Expand Up @@ -497,31 +542,27 @@ func testAccCheckCloudflareAccessApplicationDestroy(s *terraform.State) error {
continue
}

_, err := client.AccessApplication(context.Background(), rs.Primary.Attributes["zone_id"], rs.Primary.ID)
if err == nil {
return fmt.Errorf("AccessApplication still exists")
var notFoundError *cloudflare.NotFoundError
if rs.Primary.Attributes["zone_id"] != "" {
_, err := client.ZoneLevelAccessApplication(context.Background(), rs.Primary.Attributes["zone_id"], rs.Primary.ID)
jacobbednarz marked this conversation as resolved.
Show resolved Hide resolved
if !errors.As(err, &notFoundError) {
return fmt.Errorf("AccessApplication still exists")
}
}

_, err = client.AccessApplication(context.Background(), rs.Primary.Attributes["account_id"], rs.Primary.ID)
if err == nil {
return fmt.Errorf("AccessApplication still exists")
if rs.Primary.Attributes["account_id"] != "" {
_, err := client.AccessApplication(context.Background(), rs.Primary.Attributes["account_id"], rs.Primary.ID)
if !errors.As(err, &notFoundError) {
return fmt.Errorf("AccessApplication still exists")
}
}

}

return nil
}

func TestAccCloudflareAccessApplicationWithZoneID(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()
name := "cloudflare_access_application." + rnd
zone := os.Getenv("CLOUDFLARE_DOMAIN")
Expand Down Expand Up @@ -554,16 +595,6 @@ func TestAccCloudflareAccessApplicationWithZoneID(t *testing.T) {
}

func TestAccCloudflareAccessApplicationWithMissingCORSMethods(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")
Expand All @@ -584,16 +615,6 @@ func TestAccCloudflareAccessApplicationWithMissingCORSMethods(t *testing.T) {
}

func TestAccCloudflareAccessApplicationWithMissingCORSOrigins(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")
Expand All @@ -614,16 +635,6 @@ func TestAccCloudflareAccessApplicationWithMissingCORSOrigins(t *testing.T) {
}

func TestAccCloudflareAccessApplicationWithInvalidSessionDuration(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")
Expand All @@ -644,16 +655,6 @@ func TestAccCloudflareAccessApplicationWithInvalidSessionDuration(t *testing.T)
}

func TestAccCloudflareAccessApplicationMisconfiguredCORSCredentialsAllowingAllOrigins(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")
Expand All @@ -674,16 +675,6 @@ func TestAccCloudflareAccessApplicationMisconfiguredCORSCredentialsAllowingAllOr
}

func TestAccCloudflareAccessApplicationMisconfiguredCORSCredentialsAllowingWildcardOrigins(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")
Expand Down
2 changes: 1 addition & 1 deletion cloudflare/schema_cloudflare_access_application.go
Expand Up @@ -133,7 +133,7 @@ func resourceCloudflareAccessApplicationSchema() map[string]*schema.Schema {
"http_only_cookie_attribute": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
Default: true,
},
"same_site_cookie_attribute": {
Type: schema.TypeString,
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/access_application.html.markdown
Expand Up @@ -72,7 +72,7 @@ The following arguments are supported:
dashboard.
* `same_site_cookie_attribute` - (Optional) Defines the same-site cookie setting
for access tokens. Valid values are `none`, `lax`, and `strict`.
* `http_only_cookie_attribute` - (Optional) Option to add the `HttpOnly` cookie flag to access tokens.
* `http_only_cookie_attribute` - (Optional) Option to add the `HttpOnly` cookie flag to access tokens. Defaults to `true`.
* `service_auth_401_redirect` - (Optional) Option to return a 401 status code in
service authentication rules on failed requests.

Expand Down