fix: subuser and api key now mostly work

* fix: subuser and api key now work * Improve error handling * Remove enforced adding of 2fa, causes internal server error * Lint failures
trois-six · Mar 10, 2021 · ccc7002 · ccc7002
1 parent c30a589
commit ccc7002
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 42 deletions.
diff --git a/sdk/api_key.go b/sdk/api_key.go
@@ -2,7 +2,9 @@ package sendgrid
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
+	"net/http"
 )
 
 // APIKey is a Sendgrid API key.
@@ -13,50 +15,80 @@ type APIKey struct {
 	Scopes []string `json:"scopes,omitempty"`
 }
 
-func parseAPIKey(respBody string) (*APIKey, error) {
+var (
+	ErrFailedCreatingAPIKey = errors.New("failed creating apiKey")
+	ErrFailedDeletingAPIKey = errors.New("failed deleting apiKey")
+)
+
+func parseAPIKey(respBody string) (*APIKey, RequestError) {
 	var body APIKey
 	if err := json.Unmarshal([]byte(respBody), &body); err != nil {
-		return nil, fmt.Errorf("failed parsing API key: %w", err)
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        fmt.Errorf("failed parsing API key: %w", err),
+		}
 	}
 
-	return &body, nil
+	return &body, RequestError{StatusCode: http.StatusOK, Err: nil}
 }
 
 // CreateAPIKey creates an APIKey and returns it.
-func (c *Client) CreateAPIKey(name string, scopes []string) (*APIKey, error) {
+func (c *Client) CreateAPIKey(name string, scopes []string) (*APIKey, RequestError) {
 	if name == "" {
-		return nil, ErrNameRequired
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        ErrNameRequired,
+		}
 	}
 
-	respBody, _, err := c.Post("POST", "/api_keys", APIKey{
+	respBody, statusCode, err := c.Post("POST", "/api_keys", APIKey{
 		Name:   name,
 		Scopes: scopes,
 	})
 	if err != nil {
-		return nil, fmt.Errorf("failed creating API key: %w", err)
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        fmt.Errorf("failed creating API key: %w", err),
+		}
+	}
+
+	if statusCode >= http.StatusMultipleChoices {
+		return nil, RequestError{
+			StatusCode: statusCode,
+			Err:        fmt.Errorf("%w, status: %d, response: %s", ErrFailedCreatingAPIKey, statusCode, respBody),
+		}
 	}
 
 	return parseAPIKey(respBody)
 }
 
 // ReadAPIKey retreives an APIKey and returns it.
-func (c *Client) ReadAPIKey(id string) (*APIKey, error) {
+func (c *Client) ReadAPIKey(id string) (*APIKey, RequestError) {
 	if id == "" {
-		return nil, ErrAPIKeyIDRequired
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        ErrAPIKeyIDRequired,
+		}
 	}
 
 	respBody, _, err := c.Get("GET", "/api_keys/"+id)
 	if err != nil {
-		return nil, fmt.Errorf("failed reading API key: %w", err)
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        err,
+		}
 	}
 
 	return parseAPIKey(respBody)
 }
 
 // UpdateAPIKey edits an APIKey and returns it.
-func (c *Client) UpdateAPIKey(id, name string, scopes []string) (*APIKey, error) {
+func (c *Client) UpdateAPIKey(id, name string, scopes []string) (*APIKey, RequestError) {
 	if id == "" {
-		return nil, ErrAPIKeyIDRequired
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        ErrAPIKeyIDRequired,
+		}
 	}
 
 	t := APIKey{}
@@ -70,21 +102,38 @@ func (c *Client) UpdateAPIKey(id, name string, scopes []string) (*APIKey, error)
 
 	respBody, _, err := c.Post("PUT", "/api_keys/"+id, t)
 	if err != nil {
-		return nil, fmt.Errorf("failed updating API key: %w", err)
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        err,
+		}
 	}
 
 	return parseAPIKey(respBody)
 }
 
 // DeleteAPIKey deletes an APIKey.
-func (c *Client) DeleteAPIKey(id string) (bool, error) {
+func (c *Client) DeleteAPIKey(id string) (bool, RequestError) {
 	if id == "" {
-		return false, ErrAPIKeyIDRequired
+		return false, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        ErrAPIKeyIDRequired,
+		}
+	}
+
+	responseBody, statusCode, err := c.Get("DELETE", "/api_keys/"+id)
+	if err != nil {
+		return false, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        err,
+		}
 	}
 
-	if _, statusCode, err := c.Get("DELETE", "/api_keys/"+id); statusCode > 299 || err != nil {
-		return false, fmt.Errorf("failed deleting API key: %w", err)
+	if statusCode >= http.StatusMultipleChoices && statusCode != http.StatusNotFound { // ignore not found
+		return false, RequestError{
+			StatusCode: statusCode,
+			Err:        fmt.Errorf("%w, status: %d, response: %s", ErrFailedDeletingAPIKey, statusCode, responseBody),
+		}
 	}
 
-	return true, nil
+	return true, RequestError{StatusCode: http.StatusOK, Err: nil}
 }
diff --git a/sdk/errors.go b/sdk/errors.go
@@ -16,3 +16,8 @@ var (
 	ErrTemplateVersionNameRequired    = errors.New("a template version name is required")
 	ErrTemplateVersionSubjectRequired = errors.New("a template version subject is required")
 )
+
+type RequestError struct {
+	StatusCode int
+	Err        error
+}
diff --git a/sdk/subuser.go b/sdk/subuser.go
@@ -2,7 +2,9 @@ package sendgrid
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
+	"log"
 	"net/http"
 	"net/url"
 )
@@ -11,11 +13,6 @@ type creditAllocation struct {
 	Type string `json:"credit_allocation,omitempty"`
 }
 
-type RequestError struct {
-	StatusCode int
-	Err        error
-}
-
 // SubUser is a Sendgrid SubUser.
 type SubUser struct {
 	ID                 int              `json:"id,omitempty"`
@@ -39,20 +36,38 @@ type subUserErrors struct {
 	Errors []subUserError `json:"errors,omitempty"`
 }
 
+var ErrFailedCreatingSubUser = errors.New("failed creating subUser")
+
+func parseSubUser(respBody string) (*SubUser, RequestError) {
+	var body SubUser
+	if err := json.Unmarshal([]byte(respBody), &body); err != nil {
+		log.Printf("[DEBUG] [parseSubUser] failed parsing subUser, response body: %s", respBody)
+
+		return nil, RequestError{
+			StatusCode: http.StatusInternalServerError,
+			Err:        err,
+		}
+	}
+
+	return &body, RequestError{StatusCode: http.StatusOK, Err: nil}
+}
+
 func parseSubUsers(respBody string) ([]SubUser, RequestError) {
 	var body []SubUser
 	if err := json.Unmarshal([]byte(respBody), &body); err != nil {
+		log.Printf("[DEBUG] [parseSubUsers] failed parsing subUsers, response body: %s", respBody)
+
 		return nil, RequestError{
 			StatusCode: http.StatusInternalServerError,
-			Err:        fmt.Errorf("failed parsing subUsers: %w", err),
+			Err:        err,
 		}
 	}
 
 	return body, RequestError{StatusCode: http.StatusOK, Err: nil}
 }
 
 // CreateSubuser creates a subuser and returns it.
-func (c *Client) CreateSubuser(username, email, password string, ips []string) ([]SubUser, RequestError) {
+func (c *Client) CreateSubuser(username, email, password string, ips []string) (*SubUser, RequestError) {
 	if username == "" {
 		return nil, RequestError{StatusCode: http.StatusNotAcceptable, Err: ErrUsernameRequired}
 	}
@@ -75,14 +90,21 @@ func (c *Client) CreateSubuser(username, email, password string, ips []string) (
 		Password: password,
 		IPs:      ips,
 	})
-	if err != nil || statusCode >= 300 {
+	if err != nil {
 		return nil, RequestError{
 			StatusCode: statusCode,
 			Err:        fmt.Errorf("failed creating subUser: %w", err),
 		}
 	}
 
-	return parseSubUsers(respBody)
+	if statusCode >= http.StatusMultipleChoices {
+		return nil, RequestError{
+			StatusCode: statusCode,
+			Err:        fmt.Errorf("%w, status: %d, response: %s", ErrFailedCreatingSubUser, statusCode, respBody),
+		}
+	}
+
+	return parseSubUser(respBody)
 }
 
 // ReadSubuser retreives a subuser and returns it.

diff --git a/sendgrid/resource_sendgrid_api_key.go b/sendgrid/resource_sendgrid_api_key.go
@@ -89,13 +89,9 @@ func resourceSendgridAPIKeyCreate(ctx context.Context, d *schema.ResourceData, m
 		scopes = append(scopes, "sender_verification_eligible")
 	}
 
-	if ok := scopeInScopes(scopes, "2fa_required"); !ok {
-		scopes = append(scopes, "2fa_required")
-	}
-
 	apiKey, err := c.CreateAPIKey(name, scopes)
-	if err != nil {
-		return diag.FromErr(err)
+	if err.Err != nil {
+		return diag.FromErr(err.Err)
 	}
 
 	d.SetId(apiKey.ID)
@@ -111,8 +107,8 @@ func resourceSendgridAPIKeyRead(_ context.Context, d *schema.ResourceData, m int
 	c.OnBehalfOf = d.Get("sub_user_on_behalf_of").(string)
 
 	apiKey, err := c.ReadAPIKey(d.Id())
-	if err != nil {
-		return diag.FromErr(err)
+	if err.Err != nil {
+		return diag.FromErr(err.Err)
 	}
 
 	//nolint:errcheck
@@ -154,8 +150,8 @@ func resourceSendgridAPIKeyUpdate(ctx context.Context, d *schema.ResourceData, m
 		a.Scopes = scopes
 	}
 
-	if _, err := c.UpdateAPIKey(d.Id(), a.Name, a.Scopes); err != nil {
-		return diag.FromErr(err)
+	if _, err := c.UpdateAPIKey(d.Id(), a.Name, a.Scopes); err.Err != nil {
+		return diag.FromErr(err.Err)
 	}
 
 	return resourceSendgridAPIKeyRead(ctx, d, m)
@@ -167,8 +163,8 @@ func resourceSendgridAPIKeyDelete(_ context.Context, d *schema.ResourceData, m i
 	c.OnBehalfOf = d.Get("sub_user_on_behalf_of").(string)
 
 	_, err := c.DeleteAPIKey(d.Id())
-	if err != nil {
-		return diag.FromErr(err)
+	if err.Err != nil {
+		return diag.FromErr(err.Err)
 	}
 
 	return nil

diff --git a/sendgrid/resource_sendgrid_api_key_test.go b/sendgrid/resource_sendgrid_api_key_test.go
@@ -40,8 +40,8 @@ func testAccCheckSendgridAPIKeyDestroy(s *terraform.State) error {
 		apiKeyID := rs.Primary.ID
 
 		_, err := c.DeleteAPIKey(apiKeyID)
-		if err != nil {
-			return err
+		if err.Err != nil {
+			return err.Err
 		}
 	}
 

diff --git a/sendgrid/resource_sendgrid_subuser.go b/sendgrid/resource_sendgrid_subuser.go
@@ -33,6 +33,10 @@ import (
 
 var ErrSubUserNotFound = errors.New("subUser wasn't found")
 
+func subUserNotFound(name string) error {
+	return fmt.Errorf("%w: %s", ErrSubUserNotFound, name)
+}
+
 func resourceSendgridSubuser() *schema.Resource {
 	return &schema.Resource{
 		CreateContext: resourceSendgridSubuserCreate,
@@ -153,7 +157,7 @@ func resourceSendgridSubuserRead(_ context.Context, d *schema.ResourceData, m in
 	}
 
 	if len(subUser) == 0 {
-		return diag.FromErr(ErrSubUserNotFound)
+		return diag.FromErr(subUserNotFound(d.Id()))
 	}
 
 	//nolint:errcheck