appcues / ueberauth_okta

Okta strategy for Überauth
MIT License
9 stars 18 forks source link

Issue with potential logging of sensitive data #41

Open jordan0day opened 1 month ago

jordan0day commented 1 month ago

Looking through application logs today, I noticed that I was seeing some log messages like:

[notice] Invalid option {token,#{'__struct__' => 'Elixir.OAuth2.AccessToken',
                        access_token =>
                            <<"...snipped...">>, ...}} ignored

and

[notice] Invalid option {client_secret,<<"...snipped...">} ignored.

It appears these log messages are coming from httpc, which is the default HTTP client used in the OAuth2.Request module (via Tesla.Adapter.Httpc).

These are cropping up as part of the handle_callback! flow, during fetch_user/2.

In fetch_user/2, it looks like we're loading up all the oauth config options (via add_oauth_options/1) and passing them through as-is ever since #23. Maybe these more-sensitive options should be getting stripped out before being passed-along in Ueberauth.Strategy.Okta.Oauth.get_user_info/2? (Or maybe opts doesn't need to be passed along at all to Client.get/4, as the opts were already used to initalize the client?)

mustela commented 1 month ago

I can confirm that removing the opts here fixes the issue. I guess the goal is to be able to easily pass options to the client downstream to the Tesla adapter. However, as @jordan0day suggests, we should probably remove the unnecessary attributes. I'm happy to open a pull request if you have any thoughts.