peburrows / goth

Elixir package for Oauth authentication via Google Cloud APIs
http://hexdocs.pm/goth
MIT License
285 stars 108 forks source link

Support for Google Cloud Platform metadata #6

Closed tazjin closed 8 years ago

tazjin commented 8 years ago

Applications running on Google Cloud Platform can transparently request tokens from the metadata service, if the instance they are running on has appropriate permissions.

This change makes Goth compatible with token retrieval through the metadata service. Existing configuration does not need to be changed, as the presence of the :json key will automatically cause retrieval to use the standard OAuth token provider instead.

Scopes are not currently checked for tokens from the metadata service, however this should be trivial to add by comparing with the output of the /scopes endpoint. I just haven't had the time yet!

Other notes:

peburrows commented 8 years ago

Awesome, thanks! I'll give this a look and get it merged soon.

tazjin commented 8 years ago

Cool!

I've added another change that verifies the scopes from the metadata service as well. This is one additional request per fetched token through the metadata service, but that's probably better than returning tokens for invalid scopes.

tazjin commented 8 years ago

@peburrows Sorry for the delayed response, been a busy week!

I've removed those commits and changed Goth.Config to handle setting the correct token_source on initialisation.

Out of curiosity, in what way is it used in other projects? If some metadata that is only present in the JSON is retrieved then we would most likely have to fetch that data from the metadata server as well, right?

peburrows commented 8 years ago

@tazjin, here's an example of where Goth.Config is used: https://github.com/peburrows/diplomat/blob/master/lib/diplomat/client.ex#L107-L110

You're right, though, Goth.Config will need to expose those configurations regardless of the source (the provided JSON or the metadata service).

tazjin commented 8 years ago

Alright, we can get the project from the metadata service (the config process needs to do that at startup, I'll add it) - however if anyone uses Goth.Config to fetch other configuration keys that may not exist for the metadata service (e.g. the private key) it will fail.

I propose this:

  1. I'll add fetching of project information to the config process to cover some existing use-cases
  2. Goth.Config should be removed in a future (compatibility breaking) release and replaced with more specific functions, such as get_project, as the public API to meta-information.

What do you think?

peburrows commented 8 years ago

@tazjin sounds good!

Thanks again for taking the time to put this together, I appreciate it!

tazjin commented 8 years ago

I have added a commit to support fetching project IDs from the metadata service. Note that I've only tested this with the test I added, not in an actual environment - doing that now.

tazjin commented 8 years ago

Tested and verified live as well.

From my side this is good to go. If fine with you I'd open a separate issue to track the second point I raised.

tazjin commented 8 years ago

Another thing: There is a merge conflict with the change to read configuration from envvars right now. IMHO that should be accomplished with config :goth, json: System.get_env(...) directly in the configuration.

peburrows commented 8 years ago

@tazjin thanks!

Regarding loading the config via ENV vars, using config :goth, json: System.get_env(...) requires the ENV variable to be set at compile time. For example, if you build a release one on system and deploy to another, the ENV variables have to be present on the build system (and values on the target system will have no effect).

I've chosen {:system, VARIABLE} to follow the same convention that Phoenix follows (see here for an example).

Anyway, thanks again for the work on this PR. I'll get the conflicts resolved and merge this later today.

kommen commented 8 years ago

Thank you both @peburrows and @tazjin for your work!