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 the option to get the client secret dynamically. #79

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

alejandrodevs
Copy link
Contributor

Similar to what was done in: ueberauth/ueberauth_google#101

Add the possibility to get the client secret from a module in order to rotate the secret at least every 6 months (which is a the Microsoft recommendation) without the need to be changing the config or env var and restart the application. Similar to the implementation of ueberauth_apple: https://github.com/ueberauth/ueberauth_apple/blob/main/guides/getting-started.md#generating-the-client-secret

Configuration would look like this:

config :ueberauth, Ueberauth.Strategy.Microsoft.OAuth,
  client_id: System.get_env("MICROSOFT_CLIENT_ID"),
  client_secret: {MyApp.Microsoft, :client_secret}

and then:

defmodule MyApp.Microsoft
  @spec client_secret(config :: keyword) :: String.t()
  def client_secret(_config \\ []) do
    # Get client secret from db, cache or any other custom implementation.
  end
end

This does not affect to the current implementation, it is backwards compatible.
What do you think? @swelham

@swelham
Copy link
Owner

swelham commented Sep 21, 2023

Hi @alejandrodevs, thanks for the PR!

This looks good to me and makes sense to support this.

Are the config file changes required just for testing? If so, would it make sense to move these into a config/test.exs file?

@alejandrodevs
Copy link
Contributor Author

Hi @swelham,

Config file is just for testing, I wanted to keep it as other projects in the ueberauth account but it makes sense to change it to config/test.exs. Besides that, do you want me to bump the package version and add the entry to the changelog file?

@swelham
Copy link
Owner

swelham commented Sep 22, 2023

Thanks for the updates, this looks good to go.

I don't mind the version/changelog being bumped separatly to this so I can get this merged. I will bump them as I publish a new release to hex.

@swelham swelham merged commit 3dfc4c3 into swelham:master Sep 22, 2023
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