fix: fixed verification endpoint and verification logic for brand fetch #3470
Conversation
| "domain": "www.example.com" | ||
| }`) | ||
| req, err := http.NewRequestWithContext(ctx, "POST", "https://api.brandfetch.io/v1/color", payload) | ||
| // API upgraded to v2 from v1, new API doc: |
There was a problem hiding this comment.
API documentation link is missing.
There was a problem hiding this comment.
added, thanks @kashifkhan0771
| }`) | ||
| req, err := http.NewRequestWithContext(ctx, "POST", "https://api.brandfetch.io/v1/color", payload) | ||
| // API upgraded to v2 from v1, new API doc: https://docs.brandfetch.com/reference/brand-api | ||
| req, err := http.NewRequestWithContext(ctx, "GET", "https://api.brandfetch.io/v2/brands/google.com", nil) |
There was a problem hiding this comment.
The current v1 APIs with the old tokens are still functioning properly. Therefore, rather than updating the existing version, I would recommend adding a new version of the detector.
| @@ -20,7 +21,7 @@ var ( | |||
| client = common.SaneHttpClient() | |||
|
|
|||
| // Make sure that your group is surrounded in boundary characters such as below to reduce false positives. | |||
| keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"brandfetch"}) + `\b([0-9A-Za-z]{40})\b`) | |||
| keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"brandfetch"}) + `\b([a-zA-Z0-9=]{44})`) | |||
There was a problem hiding this comment.
The token I received included a / character as well. Its length was 44, which seems correct. I recommend generating multiple tokens to confirm the pattern.
There was a problem hiding this comment.
Enhanced regex and created new version for the API v2
| @@ -1642,6 +1642,8 @@ func DefaultDetectors() []detectors.Detector { | |||
| meraki.Scanner{}, | |||
| saladcloudapikey.Scanner{}, | |||
| boxoauth.Scanner{}, | |||
| brandfetchV1.Scanner{}, | |||
There was a problem hiding this comment.
Missing Versioner interface implementation. For reference see any detector which has two versions (e.g Github, Gitlab etc)
There was a problem hiding this comment.
I’ll create an issue to document this better since it can be confusing for most contributors. Even I find the interface smuggling confusing at times, so it’s likely unclear to others as well.
There was a problem hiding this comment.
Missing Versioner interface implementation
Can you give me an example of PR where this is implemented.
There was a problem hiding this comment.
Here's the implementation
There was a problem hiding this comment.
I have made the relevant changes.
Signed-off-by: Sahil Silare <[email protected]>
| "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" | ||
| ) | ||
|
|
||
| func TestBrandfetch_FromChunk(t *testing.T) { |
There was a problem hiding this comment.
These test cases for v2 might fail as the regex is different and these v2 secrets does not exist in the vault.
I am not sure what to do here so tagging experts 👨🏻💻 @zricethezav @mcastorina @abmussani
And please add the the pattern test cases for both versions separately.
There was a problem hiding this comment.
Didn't get your point here, tests are passing, do you mean to add the tests which will fail in v2 but are of v1 pattern?
Description:
Fixes #3469 .
Checklist:
make test-community)?make lintthis requires golangci-lint)?