-
Notifications
You must be signed in to change notification settings - Fork 9
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
Adds support for accessToken in config. #3
Conversation
@@ -0,0 +1 @@ | |||
node_modules |
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.
This should just be in your global .gitignore (https://help.github.com/articles/ignoring-files).
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.
Happy to remove this and not start a nitpick battle, but I think it's a good practice for an open source project to maintain its own .gitignore
for files that are generated in the course of development on it. 😄
Consider it gone.
Thank you for this, but I'm not sure I like having this here, since someone who has an access token wouldn't be using this library. Let's just find a place for it to go in gcloud-node, where it won't use this module at all if a token is provided. |
@stephenplusplus that's what I thought at first too, but If you're not convinced, I can certainly go down the path of adding it directly to |
I see your point, however I think this module should just solve the "automatic auth" part. Thinking outside the scope of gcloud-node, the utility function to decorate the request is quite small, I would recommend a user just write it instead of pulling down this library: Object.assign(reqOpts.headers, { Authorization: 'Bearer {token}' }); Also, some functionality would be lost, such as Our APIs that use gRPC (Datastore, Logging, Pub/Sub) require that auth client. I'm not sure if gRPC has a way to simply provide a token. Then there are other libraries gcloud-node uses-- gce-images & gcs-resumable-upload-- that would need to be updated to accept just an access token. That shouldn't be a big deal. |
I guess that's more or less exactly why I wanted to make the change here -- now we're talking about making updates to 3+ separate libraries with unknown ramifications for gRPC calls and so on and so forth. The grpc code seems to maybe just have a similar token authentication to REST, but it's wrapped in so many layers of abstraction it's hard to tell. I'm open to suggestions and willing to do some work, but this was intended to be a quick couple-hour PR and it's sounding more and more like a multi-day effort. 😞 We're probably just going to have to use REST directly and skip gcloud for the moment. |
Yeah, complicated indeed. Thanks for helping out so far. If time allows, feel free to keep digging and share your findings. I'll do the same. |
@mbleigh @stephenplusplus A PR has been sitting here since June 24, 2016. How can we help to make this happen ? Looks like a lot of us need this. |
I started a new PR for this: #48. |
There are situations where an application will already have an access token (see issue). In my case, I have a command-line client that uses an OAuth flow to gain credentials instead of a service account.
This PR adds support for
config.accessToken
which, if specified, will short-circuit the more complex behavior and simply return the token provided. This will allow gcloud to support access token based authentication.