-
Notifications
You must be signed in to change notification settings - Fork 67
feat: add scope authorization check to hs doctor #1367
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
Conversation
025451c
to
9cd5148
Compare
lang/en.lyaml
Outdated
incomplete: | ||
one: "Personal access key is valid. {{ count }} scope available, but not included in your key." | ||
other: "Personal access key is valid. {{ count }} scopes available, but not included in your key." | ||
incompleteSecondary: "To add the available scopes, run {{ command}} and re-authenticate your account with a new key that has those scopes. Visit hubspot.com to view selected and available scopes for your personal access key. {{ link }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We refer to it as " visit HubSpot" instead of "visit hubspot.com" (small change but worth the consistency)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood (I thought the hubspot.com was a little wacky too). The existing copy for when you need to paste in your key is "Open hubspot.com to copy your personal access key?"
- should we change that also? My thinking is yes, and there are a few others as well. I followed convention with mine 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, I didn't realize we had those inconsistencies tbh. But yeah it would be great to update that too if you don't mind! (not a blocker though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally and this looks great! 🔥
Description and Context
Adds scope authorization checking to
hs doctor
. The goal is to alert the user if there are potential issues we detect with their local configuration. In this situation, we're highlighting when the personal access key being used for auth has fewer scopes than the current user has access to. If so, we'll detail that in the doctor response.I'll add some tests before merging but wanted to get some eyes on the shape of things ahead of that.
Depends on HubSpot/hubspot-local-dev-lib#232