Skip to content

Commit

Permalink
resource/cloudfare_api_token: allow empty conditions
Browse files Browse the repository at this point in the history 
In the initial PR for adding API token support to Terraform (cloudflare#862), an
assumption was made that empty `condition` blocks would allow all
traffic; this is incorrect and puts the end user in a state where no
traffic is allowed.

To address this, we need to conditionally build the API token based on
what fields are non-zero values. This requires a bit of a rewrite as
this assumption ran deep into the methods and the structure of the code
itself.

While this increases the test coverage, we do now have a restriction in
the API itself whereby if you create an API token with IP restrictions
and then wish to remove them, you cannot -- you must recreate the token.
This issue is outside the scope of this work so I'll leave it as a known
factor for now.

Closes cloudflare#897
  • Loading branch information
@jacobbednarz
jacobbednarz committed Jan 3, 2021
1 parent 999d226 commit f03b14f
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 68 deletions.
80 changes: 45 additions & 35 deletions cloudflare/resource_cloudflare_api_token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package cloudflare
import (
"encoding/json"
"fmt"
"github.com/cloudflare/cloudflare-go"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"log"
"strings"
"time"

"github.com/cloudflare/cloudflare-go"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
)

func resourceCloudflareApiToken() *schema.Resource {
Expand Down Expand Up @@ -101,19 +102,47 @@ func resourceCloudflareApiToken() *schema.Resource {
}
}

func buildAPIToken(d *schema.ResourceData) cloudflare.APIToken {
token := cloudflare.APIToken{}

token.Name = d.Get("name").(string)
token.Policies = resourceDataToApiTokenPolices(d)

ipsIn := []string{}
ipsNotIn := []string{}
if ips, ok := d.GetOk("condition.0.request_ip.0.in"); ok {
ipsIn = expandInterfaceToStringList(ips)
}

if ips, ok := d.GetOk("condition.0.request_ip.0.not_in"); ok {
ipsNotIn = expandInterfaceToStringList(ips)
}

if len(ipsIn) > 0 || len(ipsNotIn) > 0 {
token.Condition = &cloudflare.APITokenCondition{
RequestIP: &cloudflare.APITokenRequestIPCondition{},
}

if len(ipsIn) > 0 {
token.Condition.RequestIP.In = ipsIn
}

if len(ipsNotIn) > 0 {
token.Condition.RequestIP.NotIn = ipsNotIn
}
}

return token
}

func resourceCloudflareApiTokenCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)

name := d.Get("name").(string)

log.Printf("[INFO] Creating Cloudflare API Token: name %s", name)

t := cloudflare.APIToken{
Name: name,
Policies: resourceDataToApiTokenPolices(d),
Condition: resourceDataToApiTokenCondition(d),
}

t := buildAPIToken(d)
t, err := client.CreateAPIToken(t)
if err != nil {
return fmt.Errorf("error creating Cloudflare API Token %q: %s", name, err)
Expand All @@ -125,25 +154,7 @@ func resourceCloudflareApiTokenCreate(d *schema.ResourceData, meta interface{})
d.Set("modified_on", t.ModifiedOn)
d.Set("value", t.Value)

return nil
}

func resourceDataToApiTokenCondition(d *schema.ResourceData) *cloudflare.APITokenCondition {
ipIn := []string{}
ipNotIn := []string{}
if ips, ok := d.GetOk("condition.0.request_ip.0.in"); ok {
ipIn = expandInterfaceToStringList(ips)
}
if ips, ok := d.GetOk("condition.0.request_ip.0.not_in"); ok {
ipNotIn = expandInterfaceToStringList(ips)
}

return &cloudflare.APITokenCondition{
RequestIP: &cloudflare.APITokenRequestIPCondition{
In: ipIn,
NotIn: ipNotIn,
},
}
return resourceCloudflareApiTokenRead(d, meta)
}

func resourceDataToApiTokenPolices(d *schema.ResourceData) []cloudflare.APITokenPolicies {
Expand Down Expand Up @@ -226,16 +237,19 @@ func resourceCloudflareApiTokenRead(d *schema.ResourceData, meta interface{}) er

var ipIn []string
var ipNotIn []string
if t.Condition != nil && t.Condition.RequestIP != nil {
if t.Condition != nil && t.Condition.RequestIP != nil && t.Condition.RequestIP.In != nil {
ipIn = t.Condition.RequestIP.In
}

if t.Condition != nil && t.Condition.RequestIP != nil && t.Condition.RequestIP.NotIn != nil {
ipNotIn = t.Condition.RequestIP.NotIn
}

if len(ipIn) > 0 || len(ipNotIn) > 0 {
d.Set("condition", []map[string]interface{}{{
"request_ip": []map[string]interface{}{{
"in": ipIn,
"not_in": ipNotIn,
"in": ipIn,
}},
}})
}
Expand All @@ -249,11 +263,7 @@ func resourceCloudflareApiTokenUpdate(d *schema.ResourceData, meta interface{})
name := d.Get("name").(string)
tokenID := d.Id()

t := cloudflare.APIToken{
Name: d.Get("name").(string),
Policies: resourceDataToApiTokenPolices(d),
Condition: resourceDataToApiTokenCondition(d),
}
t := buildAPIToken(d)

log.Printf("[INFO] Updating Cloudflare API Token: name %s", name)

Expand Down
147 changes: 114 additions & 33 deletions cloudflare/resource_cloudflare_api_token_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,22 @@ import (
func TestAccAPIToken(t *testing.T) {
rnd := generateRandomResourceName()
resourceID := "cloudflare_api_token." + rnd
zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAPITokenConfig(rnd, rnd, permissionID, zoneID, true),
Config: testAccCloudflareAPITokenWithoutCondition(rnd, rnd, permissionID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceID, "name", rnd),
resource.TestCheckResourceAttr(resourceID, "condition.0.request_ip.0.in.0", "192.0.2.1/32"),
resource.TestCheckResourceAttr(resourceID, "condition.0.request_ip.0.not_in.0", "198.51.100.1/32"),
),
},
{
Config: testAPITokenConfig(rnd, rnd, permissionID, zoneID, false),
Config: testAccCloudflareAPITokenWithoutCondition(rnd, rnd+"-updated", permissionID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceID, "name", rnd),
resource.TestCheckNoResourceAttr(resourceID, "condition.0.request_ip.0.in.0"),
resource.TestCheckNoResourceAttr(resourceID, "condition.0.request_ip.0.not_in.0"),
resource.TestCheckResourceAttr(resourceID, "name", rnd+"-updated"),
),
},
},
Expand Down Expand Up @@ -60,36 +55,122 @@ func TestAccAPITokenAllowDeny(t *testing.T) {
})
}

func testAPITokenConfig(resourceID, name, permissionID, zoneID string, ips bool) string {
var condition string
func TestAccAPITokenDoesNotSetConditions(t *testing.T) {
rnd := generateRandomResourceName()
name := "cloudflare_api_token." + rnd
permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read

if ips {
condition = `
condition {
request_ip {
in = ["192.0.2.1/32"]
not_in = ["198.51.100.1/32"]
}
}`
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccCloudflareAPITokenWithoutCondition(rnd, rnd, permissionID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(name, "name", rnd),
resource.TestCheckNoResourceAttr(name, "condition.0.request_ip.0.in"),
resource.TestCheckNoResourceAttr(name, "condition.0.request_ip.0.not_in"),
),
},
},
})
}

func testAccCloudflareAPITokenWithoutCondition(resourceName, rnd, permissionID string) string {
return fmt.Sprintf(`
resource "cloudflare_api_token" "%[1]s" {
name = "%[2]s"
policy {
effect = "allow"
permission_groups = [ "%[3]s" ]
resources = { "com.cloudflare.api.account.zone.*" = "*" }
}
}
`, resourceName, rnd, permissionID)
}

func TestAccAPITokenSetIndividualCondition(t *testing.T) {
rnd := generateRandomResourceName()
name := "cloudflare_api_token." + rnd
permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccCloudflareAPITokenWithIndividualCondition(rnd, permissionID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(name, "name", rnd),
resource.TestCheckResourceAttr(name, "condition.0.request_ip.0.in.0", "192.0.2.1/32"),
resource.TestCheckNoResourceAttr(name, "condition.0.request_ip.0.not_in"),
),
},
},
})
}

func testAccCloudflareAPITokenWithIndividualCondition(rnd string, permissionID string) string {
return fmt.Sprintf(`
resource "cloudflare_api_token" "%[1]s" {
name = "%[2]s"
resource "cloudflare_api_token" "%[1]s" {
name = "%[1]s"
%[5]s
policy {
policy {
effect = "allow"
permission_groups = [
"%[3]s",
]
resources = {
"com.cloudflare.api.account.zone.%[4]s" = "*"
permission_groups = [ "%[2]s" ]
resources = { "com.cloudflare.api.account.zone.*" = "*" }
}
condition {
request_ip {
in = ["192.0.2.1/32"]
}
}
}
`, resourceID, name, permissionID, zoneID, condition)
}
`, rnd, permissionID)
}

func TestAccAPITokenSetAllCondition(t *testing.T) {
rnd := generateRandomResourceName()
name := "cloudflare_api_token." + rnd
permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccCloudflareAPITokenWithAllCondition(rnd, permissionID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(name, "name", rnd),
resource.TestCheckResourceAttr(name, "condition.0.request_ip.0.in.0", "192.0.2.1/32"),
resource.TestCheckResourceAttr(name, "condition.0.request_ip.0.not_in.0", "198.51.100.1/32"),
),
},
},
})
}

func testAccCloudflareAPITokenWithAllCondition(rnd string, permissionID string) string {
return fmt.Sprintf(`
resource "cloudflare_api_token" "%[1]s" {
name = "%[1]s"
policy {
effect = "allow"
permission_groups = [ "%[2]s" ]
resources = { "com.cloudflare.api.account.zone.*" = "*" }
}
condition {
request_ip {
in = ["192.0.2.1/32"]
not_in = ["198.51.100.1/32"]
}
}
}
`, rnd, permissionID)
}

func testAPITokenConfigAllowDeny(resourceID, name, permissionID, zoneID string, allowAllZonesExceptOne bool) string {
Expand All @@ -99,7 +180,7 @@ func testAPITokenConfigAllowDeny(resourceID, name, permissionID, zoneID string,
policy {
effect = "deny"
permission_groups = [
"%[1]s",
"%[1]s",
]
resources = {
"com.cloudflare.api.account.zone.%[2]s" = "*"
Expand All @@ -111,11 +192,11 @@ func testAPITokenConfigAllowDeny(resourceID, name, permissionID, zoneID string,
return fmt.Sprintf(`
resource "cloudflare_api_token" "%[1]s" {
name = "%[2]s"
policy {
effect = "allow"
permission_groups = [
"%[3]s",
"%[3]s",
]
resources = {
"com.cloudflare.api.account.zone.*" = "*"
Expand Down

0 comments on commit f03b14f

Please sign in to comment.