pow-auth / assent

Multi-provider framework in Elixir
https://powauth.com
MIT License
391 stars 45 forks source link

Should OIDC fetch_user, fetch_userinfo, and validate_id_token allow for dynamic OpenID configuration? #72

Closed oshbec closed 3 years ago

oshbec commented 3 years ago

Hi, and thanks for building and maintaining this 👋.

In the configuration documentation for OIDC, :openid_configuration isn't strictly required since it can be fetched from :openid_configuration_uri if it isn't defined. Similarly, :openid_configuration_uri is also optional, since it defaults to /.well-known/openid-configuration based on :site.

Both authorize_url/1 and callback/3 work this way by calling openid_configuration/1. However, fetch_user/2, fetch_userinfo/2, and validate_id_token/2 are using Config.fetch/2 to resolve configuration, so they aren't getting it dynamically.

Should these work consistently? I'm a bit new to both Elixir and OIDC, so it's quite possible my understanding is off here.

If the intent is to have them all get :openid_configuration dynamically, I'm happy to try submitting a PR.

Thanks for your time!

danschultzer commented 3 years ago

Good call!

Yeah, I didn't think of fetch_user/2, fetch_userinfo/2 or validate_id_token/2 functions being called outside the callback/3 flow even though they are public functions. You are right that we should use the same logic always to fetch :openid_configuration in case you are calling these functions separately.

Feel free to open PR calling openid_configuration(config) instead of Config.fetch(config, :openid_configuration) in fetch_userinfo/2 and validate_id_token/2 (fetch_user/2 calls validate_id_token/2 so only those two a necessary to update) 😄

danschultzer commented 3 years ago

Going to publish new release soon so went ahead and update the functions in #73

oshbec commented 3 years ago

Thanks for your work on this!