Added Onfido API Token detector #3261
Conversation
removed comment
|
Hello @zricethezav , |
| if err != nil { | ||
| continue | ||
| } | ||
| req.Header.Add("Authorization", fmt.Sprintf("Token token=%s", resMatch)) |
There was a problem hiding this comment.
We are not handling verification API response code correctly and setting verification errors accordingly in case of failure.
There was a problem hiding this comment.
What do you mean? It only validate if the status code is 200, it fails in every other cases - see line 75-78
There was a problem hiding this comment.
If any errors happens during verification, I guess it's good to set it as verification error but I'll let @zricethezav comment here if that is required or if we can ignore that.
| Raw: []byte(resMatch), | ||
| } | ||
|
|
||
| if verify { |
There was a problem hiding this comment.
I would recommend to keep the verification logic in a separate func.
There was a problem hiding this comment.
@lucasan1 Along with my comments, Can you write Pattern test for Onfido detector? Here is the easyinsight's example to do it.
| } | ||
|
|
||
| // Ensure the Scanner satisfies the interface at compile time. | ||
| var _ detectors.Detector = (*Scanner)(nil) |
There was a problem hiding this comment.
Unfortunately, With the recent changes, There is another func Description introduce in Scanner interface. Code will not compile without that implementation.
| defaultClient = common.SaneHttpClientTimeOut(time.Second * 10) | ||
|
|
||
| // Make sure that your group is surrounded in boundary characters such as below to reduce false positives. | ||
| keyPat = regexp.MustCompile(`\b(?:api_live(?:_[a-zA-Z]{2})?\.[a-zA-Z0-9-_]{11}\.[-_a-zA-Z0-9]{32})\b`) |
There was a problem hiding this comment.
Onfido providers do support sandbox token, starting with api_sandbox.
| // FromData will find and optionally verify Onfido secrets in a given set of bytes. | ||
| func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (results []detectors.Result, err error) { | ||
| dataStr := string(data) | ||
| matches := keyPat.FindAllStringSubmatch(dataStr, -1) |
There was a problem hiding this comment.
Filtering unique values for key before looping through, can improve performance, especially if there are a lot of duplicates.
|
|
||
| for _, match := range matches { | ||
| // There are no capturing group in the regex, so match[0] is the only one we need. | ||
| resMatch := strings.TrimSpace(match[0]) |
There was a problem hiding this comment.
Trimming is redundant as regex is not capturing any spaces.
| client := s.client | ||
| if client == nil { | ||
| client = defaultClient | ||
| } | ||
|
|
||
| // Determine the region code based on the prefix of resMatch | ||
| region := "eu" // Default region | ||
| if strings.HasPrefix(resMatch, "api_live_ca.") { | ||
| region = "ca" | ||
| } else if strings.HasPrefix(resMatch, "api_live_us.") { | ||
| region = "us" | ||
| } | ||
|
|
||
| // Construct the URL using the region variable | ||
| url := fmt.Sprintf("https://api.%s.onfido.com/v3/validate_api_token", region) | ||
| req, err := http.NewRequestWithContext(ctx, "POST", url, nil) | ||
|
|
||
| if err != nil { | ||
| continue | ||
| } | ||
| req.Header.Add("Authorization", fmt.Sprintf("Token token=%s", resMatch)) |
There was a problem hiding this comment.
Nit: Moving verification logic to a func can improve resource utilization.
|
|
||
| // Construct the URL using the region variable | ||
| url := fmt.Sprintf("https://api.%s.onfido.com/v3/validate_api_token", region) | ||
| req, err := http.NewRequestWithContext(ctx, "POST", url, nil) |
| req.Header.Add("Authorization", fmt.Sprintf("Token token=%s", resMatch)) | ||
| res, err := client.Do(req) | ||
| if err == nil { | ||
| defer res.Body.Close() |
There was a problem hiding this comment.
It's worthwhile to close the body manually (#2086).
Description:
Add Onfido API Token detector to recognize this type of secrets. Documentation on API token can be found here: https://documentation.onfido.com/api/latest/#api-tokens.
Note that Onfido is a GitHub secret scanning partner and those tokens are detected by the built-in GitHub scanner.
Test
Tested in local, it worked fine for both non verified and verified detection. To test the former, it's possible to use this repo which is an official GitHub secret scanning test repository, to test the latter please reach out to me as i can generate valid API tokens to validate.
Cli command:
./trufflehog git https://github.com/dry-runs-test/test-new-repo-2/ --include-detectors=onfido