From e42f8b3101162ccdff3a8cffef1782a4c3ee9e0c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 22 Dec 2022 09:57:57 +0100 Subject: [PATCH] Fix missing HTTP response return in PastedAccount method. Fixes #22 - In case of a HTTP error the PastedAccount method is supposed to return the HTTP response, since this can hold valuable information about the reason why the request failed. Instead, it was returning `nil`. This PR fixes this behaviour. - Additionally, this PR introduces tests to catch such oversights - Finally a proper `error.New()` error has been introduces, to that `error.Is()` can be used on common error that are detected by the module --- paste.go | 22 ++++++++++-------- paste_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 66 insertions(+), 18 deletions(-) diff --git a/paste.go b/paste.go index 7eb7e18..49efcbc 100644 --- a/paste.go +++ b/paste.go @@ -2,11 +2,15 @@ package hibp import ( "encoding/json" + "errors" "fmt" "net/http" "time" ) +// ErrNoAccountID is returned if no account ID is given to the PastedAccount method +var ErrNoAccountID = errors.New("no account ID given") + // PasteAPI is a HIBP pastes API client type PasteAPI struct { hibp *Client // References back to the parent HIBP client @@ -38,19 +42,19 @@ type Paste struct { // PastedAccount returns a single breached site based on its name func (p *PasteAPI) PastedAccount(a string) ([]*Paste, *http.Response, error) { if a == "" { - return nil, nil, fmt.Errorf("no account id given") + return nil, nil, ErrNoAccountID } - apiURL := fmt.Sprintf("%s/pasteaccount/%s", BaseURL, a) - hb, hr, err := p.hibp.HTTPResBody(http.MethodGet, apiURL, nil) + au := fmt.Sprintf("%s/pasteaccount/%s", BaseURL, a) + hb, hr, err := p.hibp.HTTPResBody(http.MethodGet, au, nil) if err != nil { - return nil, nil, err - } - - var pasteDetails []*Paste - if err := json.Unmarshal(hb, &pasteDetails); err != nil { return nil, hr, err } - return pasteDetails, hr, nil + var pd []*Paste + if err := json.Unmarshal(hb, &pd); err != nil { + return nil, hr, err + } + + return pd, hr, nil } diff --git a/paste_test.go b/paste_test.go index 7396caf..835a1dd 100644 --- a/paste_test.go +++ b/paste_test.go @@ -1,21 +1,28 @@ package hibp import ( + "errors" "fmt" "os" "testing" ) -// TestPasteAccount tests the BreachedAccount() method of the breaches API -func TestPasteAccount(t *testing.T) { +// TestPasteAPI_PastedAccount tests the PastedAccount() method of the pastes API +func TestPasteAPI_PastedAccount(t *testing.T) { testTable := []struct { testName string accountName string - isBreached bool + isPasted bool shouldFail bool }{ - {"account-exists is breached once", "account-exists@hibp-integration-tests.com", true, false}, - {"opt-out is not breached", "opt-out-breach@hibp-integration-tests.com", false, true}, + { + "account-exists is found in pastes", "account-exists@hibp-integration-tests.com", + true, false, + }, + { + "opt-out is not found in pastes", "opt-out-breach@hibp-integration-tests.com", + false, true, + }, {"empty account name", "", false, true}, } @@ -29,20 +36,57 @@ func TestPasteAccount(t *testing.T) { pasteDetails, _, err := hc.PasteAPI.PastedAccount(tc.accountName) if err != nil && !tc.shouldFail { t.Error(err) + return } - if pasteDetails == nil && tc.isBreached { - t.Errorf("breach for the account %q is expected, but returned 0 results.", + if pasteDetails == nil && tc.isPasted { + t.Errorf("paste for the account %q is expected, but returned 0 results.", tc.accountName) } - if pasteDetails != nil && !tc.isBreached { - t.Errorf("breach for the account %q is expected to be not breached, but returned breach details.", + if pasteDetails != nil && !tc.isPasted { + t.Errorf("paste for the account %q is expected to be not found, but returned paste details.", tc.accountName) } }) } } +// TestPasteAPI_PastedAccount_WithFailedHTTP tests the PastedAccount() method of the pastes API with a failing HTTP request +func TestPasteAPI_PastedAccount_WithFailedHTTP(t *testing.T) { + apiKey := os.Getenv("HIBP_API_KEY") + if apiKey == "" { + t.SkipNow() + } + hc := New(WithAPIKey(apiKey), WithRateLimitSleep()) + _, res, err := hc.PasteAPI.PastedAccount("öccount-exists@hibp-integration-tests.com") + if err == nil { + t.Errorf("HTTP request for paste should have failed but did not") + return + } + if res == nil { + t.Errorf("HTTP request for paste should have returned the HTTP response but did not") + } +} + +// TestPasteAPI_PastedAccount_Errors tests the errors defined for the PastedAccount() method +func TestPasteAPI_PastedAccount_Errors(t *testing.T) { + apiKey := os.Getenv("HIBP_API_KEY") + if apiKey == "" { + t.SkipNow() + } + hc := New(WithAPIKey(apiKey), WithRateLimitSleep()) + + // No account ID given + _, _, err := hc.PasteAPI.PastedAccount("") + if err == nil { + t.Errorf("HTTP request for paste should have failed but did not") + return + } + if !errors.Is(err, ErrNoAccountID) { + t.Errorf("error response for empty account ID should have been ErrNoAccountID but is not") + } +} + // ExamplePasteAPI_pastedAccount is a code example to show how to fetch a specific paste // based on its name from the HIBP pastes API using the PastedAccount() method func ExamplePasteAPI_pastedAccount() {