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

Add all scopes as optional #1

Closed
wants to merge 1 commit into from

Conversation

MattMSumner
Copy link

Why:

For the node hubspot api client, I would like to have an example to
show how to obtain an auth token with the appropriate scopes for running
the test suite with mocking disabled.

This PR:

Adds all the hubspot scopes as "Optional" meaning that they'll only be
asked for access if those features are enabled.

Why:

For the [node hubspot api client], I would like to have an example to
show how to obtain an auth token with the appropriate scopes for running
the test suite with mocking disabled.

[node hubspot api client]: https://github.com/MadKudu/node-hubspot

This PR:

Adds all the hubspot scopes as "Optional" meaning that they'll only be
asked for access if those features are enabled.
Copy link
Contributor

@egoodhall egoodhall left a comment

Choose a reason for hiding this comment

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

Hey @MattMSumner, thanks for making this PR!
I like the idea of adding optional scopes to this project to show how they're used, but I think it should be an empty array in the main repo (since it's only using the contacts scope). Maybe the Node client's example could reference a fork of this project that has OPTIONAL_SCOPES filled in instead? Maybe the scopes/optional scopes be read in from an environment variable instead, with defaults set if nothing is supplied

@@ -30,6 +45,7 @@ const authUrl =
'https://app.hubspot.com/oauth/authorize' +
`?client_id=${encodeURIComponent(CLIENT_ID)}` + // app's client ID
`&scope=${encodeURIComponent(SCOPES)}` + // scopes being requested by the app
`&optional_scope=${encodeURIComponent(OPTIONAL_SCOPES)}` +
Copy link
Contributor

Choose a reason for hiding this comment

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

This query parameter should only be added if there are optional scopes being requested

@MattMSumner
Copy link
Author

I love the idea of an ENV variable. I can take another look at this on Friday.

@egoodhall egoodhall mentioned this pull request Mar 12, 2019
@egoodhall
Copy link
Contributor

Resolved by #4 - scopes can now be specified with a SCOPE environment variable

@egoodhall egoodhall closed this Mar 12, 2019
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.

2 participants