ueberauth / ueberauth_google

Google OAuth2 Strategy for Überauth.
MIT License
166 stars 85 forks source link

support {:system, "X"} for provider configuration #40

Closed evadne closed 7 years ago

evadne commented 7 years ago

Our use case requires configuring provider details via environment variables. Planning to fork the library and file a PR for this change, which involves changing Ueberauth.Strategy.Google.OAuth.client. Is it something you’ll take?

Thanks in advance.

doomspork commented 7 years ago

Hi @evadne, this is not something we should support. In fact, we just had a conversation about this support in a Guardian pull request see here: https://github.com/ueberauth/guardian/pull/377#discussion_r135806789

Could you help me understand why you need something like this? Are you using Distillery for releases?

evadne commented 7 years ago

We use Distillery. However I’d like to allow operators to rotate keys without requiring developers to build a new release and also allow testing of a release independently outside of production using credentials specifically keyed for non-production use so as to verify the integrity of a release, etc.

It seems init/2 is the way forward. if I do it that way would you take it?

evadne commented 7 years ago

Proposed change is unnecessary

$  grep Ueberauth.Strategy.Google.OAuth -A 2 config/config.exs
config :ueberauth, Ueberauth.Strategy.Google.OAuth,
  client_id: "${GOOGLE_CLIENT_ID}",
  client_secret: "${GOOGLE_CLIENT_SECRET}"

$ mix release --env=prod # && …

$ GOOGLE_CLIENT_ID=gci GOOGLE_CLIENT_SECRET=gcs … REPLACE_OS_VARS=true ./releases/application/bin/application console

Interactive Elixir (1.5.0) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Application.get_env :ueberauth, Ueberauth.Strategy.Google.OAuth
[client_id: "gci", client_secret: "gcs"]
evadne commented 7 years ago

Second iteration, involving config/config.exs,

envar = fn (name) ->
  case List.keyfind(Application.loaded_applications, :distillery, 0) do
    {_, _, _} -> "${#{name}}"
    _ -> System.get_env(name)
  end
end
config :ueberauth, Ueberauth.Strategy.Google.OAuth,
  client_id: envar.("GOOGLE_CLIENT_ID"),
  client_secret: envar.("GOOGLE_CLIENT_SECRET")

This allows me to emit ${…} strings as part of sys.config in a release which Distillery will recognise at runtime in its scripts if $REPLACE_OS_VARS is set (see _init_configs(), template in Distillery repository, priv/templates/boot.eex) but only when Distillery is loaded (NB: heuristics) due to the Distillery application not being loaded ordinarily and always explicitly loaded in Mix.Tasks.Release (see Distillery repository, lib/distillery/tasks/release.ex).

The benefit of doing this is that envars will now always work whether inside or outside of a release, and that a single config/config.exs can be reused for all environments without duplicating statements to accommodate ${…} vs System.get_env(…).

ps. Checking Mix.env is not ideal because it is valid to build a release with Mix.env not being :prod