Skip to content

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

Merged
merged 12 commits into from
Feb 6, 2025

Conversation

b-ash
Copy link
Member

@b-ash b-ash commented Feb 4, 2025

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

@b-ash b-ash force-pushed the bash/add-scope-authorization-check-to-doctor branch from 025451c to 9cd5148 Compare February 5, 2025 17:40
@joe-yeager joe-yeager self-requested a review February 5, 2025 17:52
joe-yeager
joe-yeager previously approved these changes Feb 5, 2025
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 }}"
Copy link
Contributor

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)

Copy link
Member Author

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 🤷‍♂️

Copy link
Contributor

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)

Copy link
Member Author

Choose a reason for hiding this comment

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

brandenrodgers
brandenrodgers previously approved these changes Feb 5, 2025
Copy link
Contributor

@brandenrodgers brandenrodgers left a 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! 🔥

@b-ash b-ash merged commit dbc44b0 into main Feb 6, 2025
1 check passed
@b-ash b-ash deleted the bash/add-scope-authorization-check-to-doctor branch February 6, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants