search

peburrows / goth

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

Allow for ingesting multiple SAs from one JSON file. Allow for select… #41

Closed apognu closed 6 years ago

apognu commented 6 years ago

This allows the user to use goth with multiple service accounts stored in one JSON credentials file (#35).

The auto-formatter from Elixir 1.6 changed the format of the files, I hope that's OK.

peburrows commented 6 years ago

I don't hate this, but it still requires an app to know about all possible service accounts it might need at startup. I know that's also a limitation of the current version of Goth, but the more I think about Goth configuration and account selection, the more I think we might ought to decouple it from the application config.

I need to think about how best to go about doing that. In the meantime, I may still merge this PR as it seems to solve the problem you're trying to solve in a way that's fairly consistent with how Goth has always handled json credentials.

apognu commented 6 years ago

I understand, I would be interested to pitch in on that discussion, or even participate in code. What did you have in mind? A way to dynamically pass in a service account? Or a dynamic overridable credentials fetcher?

Maybe create a specific issue for discussing this?

parkerjm commented 6 years ago

but the more I think about Goth configuration and account selection, the more I think we might ought to decouple it from the application config

I agree with you entirely, but this PR doesn't introduce the pattern (as you mentioned), and it would add needed functionality. Perhaps merging this now and re-designing the configuration scheme later would be the best path forward?

apognu commented 6 years ago

I would be interested in discussing how we could inject and use service accounts not predefined in app configuration, if this is the direction you want goth to take.

peburrows commented 6 years ago

I'm gonna go ahead and merge this so people who need it can use it.

At some point in the near future, I'm also going to work through refactoring how credentials are handled in general.

peburrows commented 6 years ago

Merged and published to hex as 0.9.0

parkerjm commented 6 years ago

thanks @peburrows! please feel free to reach out to me if you want a second/third set of eyes on the credential loading re-design.