[fix] - Dropbox detector #3406
[fix] - Dropbox detector #3406
Conversation
pkg/detectors/dropbox/dropbox.go
Outdated
|
|
||
| // Ensure the Scanner satisfies the interface at compile time. | ||
| var _ detectors.Detector = (*Scanner)(nil) | ||
|
|
||
| var ( | ||
| keyPat = regexp.MustCompile(`\b(sl\.[A-Za-z0-9\-\_]{130,140})\b`) | ||
| defaultClient = common.SaneHttpClient() | ||
| keyPat = regexp.MustCompile(`sl\.u\.[0-9a-zA-Z_-]+`) |
There was a problem hiding this comment.
Are all the tokens as long as in the test? It's probably worth putting a minimum range here.
E.g., there's no point in trying for sl.u.a
There was a problem hiding this comment.
Yea, that makes sense. The 5 that I generated were all 1309 characters long.
There was a problem hiding this comment.
The 5 that I generated were all 1309 characters long.
Idk if they're all consistently 1,000+ characters. Even {250,} would likely be sufficient.
There was a problem hiding this comment.
Thanks @ahrav for taking this up. Your changes looks good to me. I tried to generate the access token for an app. The token generated for my account does not seems to be started with sl.u. and the length is 143 characters. I generated multiple times and seeing the same pattern. Could be possible that I am looking at wrong place.
Sorry for the confusion! This PR is for Dropbox, while yours adds a new Box detector.😂 They are similar, but Box is aimed at larger teams, while Dropbox is more for individuals. I don’t think they’re related.😅 |
I was reviewing your PR and got confused too. While creating a token, I noticed it was too big and realized these are two separate services. Since I had the token, I tested our existing Dropbox detector and saw it was broken, so I decided to fix it. Sorry for not making that clearer earlier. |
|
@abmussani dosregard my last comment. You are also referring to Dropbox 🤦 I'll take a second look at thid when I'm back at my laptop. I think you are correct, there might be two different tokens here... |
|
|
||
| // Ensure the Scanner satisfies the interface at compile time. | ||
| var _ detectors.Detector = (*Scanner)(nil) | ||
|
|
||
| var ( | ||
| keyPat = regexp.MustCompile(`\b(sl\.[A-Za-z0-9\-\_]{130,140})\b`) |
There was a problem hiding this comment.
It looks like this pattern matches older tokens. I've found several examples where the tail was 150. Perhaps both the new and old are necessary?
The new is sl.u., the old (?) is sl.B.
\b(sl\.B[A-Za-z0-9\-\_]{129,200})\bThere was a problem hiding this comment.
Yea, that seems to track. I'll update a little later today. Thank you both ❤️
There was a problem hiding this comment.
There's no period after the B.
There was a problem hiding this comment.
There are two types of tokens: an API explorer token, which is longer, and a regular access token, which is shorter. Should I create separate regex patterns for each or use separate detectors? I’m thinking separate detectors.
There was a problem hiding this comment.
Are the endpoints and verification logic the same? You could create separate detectors but reuse the same logic.
There was a problem hiding this comment.
I updated the PR to include both credentials in the test but left everything else unchanged. The endpoint seems to work for both, with the only difference being the token. 😅 Yea, okay i'll do that. 👍
Description:
This PR fixes the Dropbox detector. The original regex was incorrect, and I confirmed that we never successfully verified a Dropbox token, which aligns with the regex issue.
I also added a test token since one wasn't included before.
I think I did this correctly given then new detector format. 🤞
Checklist:
make test-community)?make lintthis requires golangci-lint)?