Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Shopify detector #875

Merged
merged 6 commits into from Nov 9, 2022
Merged

Conversation

kstilwell
Copy link
Contributor

Specifically, this is adding Shopify access token detection. These tokens are used to authenticate with the Shopify admin API.

Some relevant links:
https://shopify.dev/apps/auth/admin-app-access-tokens
https://shopify.dev/api/admin/getting-started

@kstilwell kstilwell requested a review from a team as a code owner October 27, 2022 16:24
Copy link
Contributor

@dustin-decker dustin-decker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, @kstilwell!

If we use this endpoint for verification it should verify for any oauth scopes assigned to the app: https://shopify.dev/api/admin-rest/2022-10/resources/accessscope#get-admin-oauth-access-scopes

We could also use the response data to add extra information on the scopes that the credential has in the ExtraData field.

@kstilwell
Copy link
Contributor Author

Hey @dustin-decker, thanks for the feedback...made the changes you requested.

Copy link
Contributor

@dustin-decker dustin-decker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Kevin, looks good now. We'll merge this soon after updating the protos.

@kstilwell
Copy link
Contributor Author

Hey @dustin-decker, just wanted to check if there was anything else you needed from me to get this merged.

@ahrav
Copy link
Collaborator

ahrav commented Nov 8, 2022

Hey @dustin-decker, just wanted to check if there was anything else you needed from me to get this merged.

Hey Kevin, looks like there is one linter warning in the detector file. I believe you should be able to omit the fmt.Sprintf since key is already a string. I can get it merged after that.

@ahrav ahrav merged commit ecd2578 into trufflesecurity:main Nov 9, 2022
mac2000 pushed a commit to mac2000/trufflehog that referenced this pull request Nov 16, 2022
* Fixes/work based on testing

* Remove some commented code

* Change how verification happens and grab additional information

* Address linter warnings.

* add shopify detector to default detectors.

Co-authored-by: Dustin Decker <dustin@trufflesec.com>
Co-authored-by: Ahrav Dutta <ahravdutta02@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants