Skip to content

Commit

Permalink
fix(cfapi): only parse SignResponse when it exists
Browse files Browse the repository at this point in the history
If the Cloudflare API returns an error, it is unlikely to have also sent
a valid response, resulting in failure trying to parse the expiration
time formats.

This changeset delays parsing the result field until after verifying the
error field.

Fixes #46
  • Loading branch information
terinjokes committed Dec 7, 2021
1 parent 1001a9a commit b912d75
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 6 deletions.
33 changes: 27 additions & 6 deletions internal/cfapi/cfapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"net/url"
"time"
)

Expand All @@ -16,12 +17,14 @@ type Interface interface {
type Client struct {
serviceKey []byte
client *http.Client
endpoint string
}

func New(serviceKey []byte, options ...Options) *Client {
c := &Client{
serviceKey: serviceKey,
client: http.DefaultClient,
endpoint: "https://api.cloudflare.com/client/v4/certificates",
}

for _, opt := range options {
Expand All @@ -39,6 +42,19 @@ func WithClient(client *http.Client) Options {
}
}

func WithEndpoint(endpoint string) (Options, error) {
u, err := url.Parse(endpoint)
if err != nil {
return nil, err
}

u.Path = "/client/v4/certificates"

return func(c *Client) {
c.endpoint = u.String()
}, nil
}

type SignRequest struct {
Hostnames []string `json:"hostnames"`
Validity int `json:"requested_validity"`
Expand All @@ -57,10 +73,10 @@ type SignResponse struct {
}

type APIResponse struct {
Success bool `json:"success"`
Errors []APIError `json:"errors"`
Messages []string `json:"messages"`
Result SignResponse `json:"result"`
Success bool `json:"success"`
Errors []APIError `json:"errors"`
Messages []string `json:"messages"`
Result json.RawMessage `json:"result"`
}

type APIError struct {
Expand All @@ -78,7 +94,7 @@ func (c *Client) Sign(ctx context.Context, req *SignRequest) (*SignResponse, err
return nil, err
}

r, err := http.NewRequestWithContext(ctx, "POST", "https://api.cloudflare.com/client/v4/certificates", bytes.NewBuffer(p))
r, err := http.NewRequestWithContext(ctx, "POST", c.endpoint, bytes.NewBuffer(p))
if err != nil {
return nil, err
}
Expand All @@ -101,7 +117,12 @@ func (c *Client) Sign(ctx context.Context, req *SignRequest) (*SignResponse, err
return nil, &api.Errors[0]
}

return &api.Result, nil
signResp := SignResponse{}
if err := json.Unmarshal(api.Result, &signResp); err != nil {
return nil, err
}

return &signResp, nil
}

// adapted from http://choly.ca/post/go-json-marshalling/
Expand Down
94 changes: 94 additions & 0 deletions internal/cfapi/cfapi_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package cfapi

import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"testing"
"time"

Expand Down Expand Up @@ -65,3 +69,93 @@ func TestSignResponse_Unmarshal(t *testing.T) {
})
}
}

func TestSign(t *testing.T) {
expectedTime := time.Date(2020, time.December, 25, 6, 27, 0, 0, time.UTC)
tests := []struct {
name string
handler http.Handler
response *SignResponse
error string
}{
{name: "API success",
handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, `{
"success": true,
"errors": [],
"message": [],
"result": {
"id":"9001",
"certificate":"-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n",
"expires_on":"2020-12-25T06:27:00Z",
"request_type":"origin-ecc",
"hostnames":["example.com"],
"csr":"-----BEGIN CERTIFICATE REQUEST-----\n-----END CERTIFICATE REQUEST-----",
"requested_validity":7
}
}`)
}),
response: &SignResponse{
Id: "9001",
Certificate: "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n",
Hostnames: []string{"example.com"},
Expiration: expectedTime,
Type: "origin-ecc",
Validity: 7,
CSR: "-----BEGIN CERTIFICATE REQUEST-----\n-----END CERTIFICATE REQUEST-----",
},
error: "",
},
{
name: "API error",
handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, `{
"success": false,
"errors": [{"code": 9001, "message": "Over Nine Thousand!"}],
"message": [],
"result": {}
}`)
}),
response: nil,
error: "Cloudflare API Error code=9001 message=Over Nine Thousand!",
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ts := httptest.NewTLSServer(tt.handler)
defer ts.Close()

client := New([]byte("v1.0-FFFF-FFFF"),
WithClient(ts.Client()),
Must(WithEndpoint(ts.URL)),
)
resp, err := client.Sign(context.Background(), &SignRequest{
Hostnames: []string{"example.com"},
Validity: 3600,
Type: "MD4",
CSR: "Lorem ipsum dolor sit amet, consectetur adipiscing elit.",
})

if diff := cmp.Diff(resp, tt.response); diff != "" {
t.Fatalf("diff: (-want +got)\n%s", diff)
}

if tt.error != "" {
if diff := cmp.Diff(err.Error(), tt.error); diff != "" {
t.Fatalf("diff: (-want +got)\n%s", diff)
}
}
})
}

}

func Must(opt Options, err error) Options {
if err != nil {
panic("option constructo returned error " + err.Error())
}

return opt
}

0 comments on commit b912d75

Please sign in to comment.