Skip to content

Commit

Permalink
account_member: Validate presence of accountID (cloudflare#283)
Browse files Browse the repository at this point in the history
To use the `accountID` within the Terraform provider, the `org_id`
attribute needs to be set. However, we have found a case[1] whereby
omitting the value results in it becoming an empty string and gets
passed through to the various account member functions and fails to
build the URL correctly as the organisation/account ID is empty. To
prevent this from happening, I've added additional validation in the
functions that confirm the `accountID` that is getting passed, isn't an
empty string prior to making the calls. This will resolve the issue
upstream too.

Closes terraform-providers/terraform-provider-cloudflare#262

[1]: terraform-providers/terraform-provider-cloudflare#262
  • Loading branch information
jacobbednarz authored and patryk committed Mar 26, 2019
1 parent 796cf82 commit b68229f
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 9 deletions.
20 changes: 20 additions & 0 deletions account_members.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ type AccountMemberInvitation struct {
//
// API reference: https://api.cloudflare.com/#accounts-list-accounts
func (api *API) AccountMembers(accountID string, pageOpts PaginationOptions) ([]AccountMember, ResultInfo, error) {
if accountID == "" {
return []AccountMember{}, ResultInfo{}, errors.New(errMissingAccountID)
}

v := url.Values{}
if pageOpts.PerPage > 0 {
v.Set("per_page", strconv.Itoa(pageOpts.PerPage))
Expand Down Expand Up @@ -87,6 +91,10 @@ func (api *API) AccountMembers(accountID string, pageOpts PaginationOptions) ([]
//
// API reference: https://api.cloudflare.com/#account-members-add-member
func (api *API) CreateAccountMember(accountID string, emailAddress string, roles []string) (AccountMember, error) {
if accountID == "" {
return AccountMember{}, errors.New(errMissingAccountID)
}

uri := "/accounts/" + accountID + "/members"

var newMember = AccountMemberInvitation{
Expand All @@ -111,6 +119,10 @@ func (api *API) CreateAccountMember(accountID string, emailAddress string, roles
//
// API reference: https://api.cloudflare.com/#account-members-remove-member
func (api *API) DeleteAccountMember(accountID string, userID string) error {
if accountID == "" {
return errors.New(errMissingAccountID)
}

uri := fmt.Sprintf("/accounts/%s/members/%s", accountID, userID)

_, err := api.makeRequest("DELETE", uri, nil)
Expand All @@ -125,6 +137,10 @@ func (api *API) DeleteAccountMember(accountID string, userID string) error {
//
// API reference: https://api.cloudflare.com/#account-members-update-member
func (api *API) UpdateAccountMember(accountID string, userID string, member AccountMember) (AccountMember, error) {
if accountID == "" {
return AccountMember{}, errors.New(errMissingAccountID)
}

uri := fmt.Sprintf("/accounts/%s/members/%s", accountID, userID)

res, err := api.makeRequest("PUT", uri, member)
Expand All @@ -145,6 +161,10 @@ func (api *API) UpdateAccountMember(accountID string, userID string, member Acco
//
// API reference: https://api.cloudflare.com/#account-members-member-details
func (api *API) AccountMember(accountID string, memberID string) (AccountMember, error) {
if accountID == "" {
return AccountMember{}, errors.New(errMissingAccountID)
}

uri := fmt.Sprintf(
"/accounts/%s/members/%s",
accountID,
Expand Down
69 changes: 60 additions & 9 deletions account_members_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ var expectedAccountMemberStruct = AccountMember{
ID: "4536bcfad5faccb111b47003c79917fa",
Code: "05dd05cce12bbed97c0d87cd78e89bc2fd41a6cee72f27f6fc84af2e45c0fac0",
User: AccountMemberUserDetails{
ID: "7c5dae5552338874e5053f2534d2767a",
FirstName: "John",
LastName: "Appleseed",
Email: "user@example.com",
ID: "7c5dae5552338874e5053f2534d2767a",
FirstName: "John",
LastName: "Appleseed",
Email: "user@example.com",
TwoFactorAuthenticationEnabled: false,
},
Status: "accepted",
Expand All @@ -36,7 +36,7 @@ var expectedNewAccountMemberStruct = AccountMember{
ID: "4536bcfad5faccb111b47003c79917fa",
Code: "05dd05cce12bbed97c0d87cd78e89bc2fd41a6cee72f27f6fc84af2e45c0fac0",
User: AccountMemberUserDetails{
Email: "user@example.com",
Email: "user@example.com",
TwoFactorAuthenticationEnabled: false,
},
Status: "pending",
Expand All @@ -57,10 +57,10 @@ var newUpdatedAccountMemberStruct = AccountMember{
ID: "4536bcfad5faccb111b47003c79917fa",
Code: "05dd05cce12bbed97c0d87cd78e89bc2fd41a6cee72f27f6fc84af2e45c0fac0",
User: AccountMemberUserDetails{
ID: "7c5dae5552338874e5053f2534d2767a",
FirstName: "John",
LastName: "Appleseeds",
Email: "new-user@example.com",
ID: "7c5dae5552338874e5053f2534d2767a",
FirstName: "John",
LastName: "Appleseeds",
Email: "new-user@example.com",
TwoFactorAuthenticationEnabled: false,
},
Status: "accepted",
Expand Down Expand Up @@ -139,6 +139,17 @@ func TestAccountMembers(t *testing.T) {
}
}

func TestAccountMembersWithoutAccountID(t *testing.T) {
setup()
defer teardown()

_, _, err := client.AccountMembers("", PaginationOptions{})

if assert.Error(t, err) {
assert.Equal(t, err.Error(), errMissingAccountID)
}
}

func TestCreateAccountMember(t *testing.T) {
setup()
defer teardown()
Expand Down Expand Up @@ -193,6 +204,20 @@ func TestCreateAccountMember(t *testing.T) {
}
}

func TestCreateAccountMemberWithoutAccountID(t *testing.T) {
setup()
defer teardown()

_, err := client.CreateAccountMember(
"",
"user@example.com",
[]string{"3536bcfad5faccb999b47003c79917fb"})

if assert.Error(t, err) {
assert.Equal(t, err.Error(), errMissingAccountID)
}
}

func TestUpdateAccountMember(t *testing.T) {
setup()
defer teardown()
Expand Down Expand Up @@ -254,6 +279,21 @@ func TestUpdateAccountMember(t *testing.T) {
}
}

func TestUpdateAccountMemberWithoutAccountID(t *testing.T) {
setup()
defer teardown()

_, err := client.UpdateAccountMember(
"",
"4536bcfad5faccb111b47003c79917fa",
newUpdatedAccountMemberStruct,
)

if assert.Error(t, err) {
assert.Equal(t, err.Error(), errMissingAccountID)
}
}

func TestAccountMember(t *testing.T) {
setup()
defer teardown()
Expand Down Expand Up @@ -306,3 +346,14 @@ func TestAccountMember(t *testing.T) {
assert.Equal(t, expectedAccountMemberStruct, actual)
}
}

func TestAccountMemberWithoutAccountID(t *testing.T) {
setup()
defer teardown()

_, err := client.AccountMember("", "4536bcfad5faccb111b47003c79917fa")

if assert.Error(t, err) {
assert.Equal(t, err.Error(), errMissingAccountID)
}
}
1 change: 1 addition & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const (
errMakeRequestError = "error from makeRequest"
errUnmarshalError = "error unmarshalling the JSON response"
errRequestNotSuccessful = "error reported by API"
errMissingAccountID = "account ID is empty and must be provided"
)

var _ Error = &UserError{}
Expand Down

0 comments on commit b68229f

Please sign in to comment.