Client secret not required for Public client apps

[reevesjeremy]

RE: Office365

AzureAD App client secrets are usually used in a Confidential client configuration where an application is running as a service/daemon of some kind. Since I just want the proxy to work to authenticate a user, I would instead use an Azure AD app configured as a Public client. In this situation, a client secret would not necessarily be required since the user will be authenticating as itself.

In my testing I set the client_secret blank in my config file and made the following modification in the code

email-oauth2-proxy/emailproxy.py

Line 280 in 89f2f7c

if not (permission_url and token_url and oauth2_scope and redirect_uri and client_id and client_secret):

To:

        if not (permission_url and token_url and oauth2_scope and redirect_uri and client_id):

This works perfectly and still allows for a client_secret to be used if it were going to be used a different way.

What were your thoughts when making client_secret a required field, and would you consider republishing as optional?

[simonrob]

Thanks for raising this – it has been on my todo list for a while to check this particular configuration for O365, so it's great that you've confirmed it works.

dc8b717 updates the proxy to account for this, making sure not to send client_secret=None, and instead omitting the value entirely if it is not present. Please could you check that accounts with no client_secret value present work properly with this update and a client configured in this way?

[reevesjeremy]

making sure not to send client_secret=None, and instead omitting the value entirely

Yep, I tried that as part of my original test, and found that if an invalid secret value is included, it will return 401 Unauthorized.

I did find that deleting the property from the config file also works, and advising that when there is no client_secret might allow users to make sense of what they are doing there.

I was also thinking of this in the debug log. client_secret is no longer required.

email-oauth2-proxy/emailproxy.py

Line 285 in dc8b717

'and client_secret)' % (APP_NAME, username))

Please could you check that accounts with no client_secret value present work properly with this update and a client configured in this way?

Works!

Although, you could check if *** your client secret here *** is found, remove the param, and if *** your client id here *** is found, throw an error. Since this must be changed.

I also recommend commenting all the configs so users have to opt in/enable them. Otherwise, by default, I have 6 ports open and 4 accounts configured but I'm only using 1 of each, if I didnt know to comment the ones I'm not using.

Edit: I just realized when you re-write the config file back, it removes all the commented lines.

[simonrob]

Thanks for testing this. Yes - the messages can probably be improved, though I'd like to keep things as simple as possible for the normal use-case. It might be worth throwing errors if the default values are found, though, as you suggest.

Re: config file examples, this is a trade-off with the configparser module. The proxy needs to write back to the configuration file, but that module doesn't really support comments. Given that the proxy's main usage is designed to be entirely local I think this isn't a major issue, but I'm open to suggestions for improvement