-
Notifications
You must be signed in to change notification settings - Fork 116
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
feat(hyper-rustls): default hyper client support https and http #180
Conversation
Thank you for this PR; as an alternative, could you slightly refactor the current function to have one In any case, thank you for taking the time to fix this! |
Should I also change the default http client built for |
Most of the checks fail due to #178 which renamed some type parameters. It should be enough to rebase and rename
Sorry, which default client build do you mean? |
Well since the metadata server is always over http, should I also manage any default constructors for the authenticator? or would it be too risky? |
I think one blocking problem is that a "secure" builder is built by default (https://docs.rs/yup-oauth2/latest/src/yup_oauth2/authenticator.rs.html#441). I'm sorry for not having this in mind earlier (I've grown somewhat estranged from this code base, unfortunately - need to take the time to re-learn it). Maybe, after all, your original PR (allowing HTTP) is still the best solution. I wonder if there is a practical threat scenario in which this would make a difference; if somebody has access to change your token URL, they could conceivably do more dangerous things in the first place. I'm sorry for the confusion! This crate has grown quite complex in the meantime. |
If an attacker could manipulate URLs for token retrieval etc., they could wreak considerably more havoc than a downgrade attack.
After giving it some thought, I realize it is probably better to remain secure by default and have a deliberate implementation of an insecure client when using the metadata server. So here is my proposal: |
Additional documentation sounds good, although note that my commit already enabled HTTP fallback (for all I can see, the token URLs etc. are supplied by the application developer, and they should be HTTPS). |
If an attacker could manipulate URLs for token retrieval etc., they could wreak considerably more havoc than a downgrade attack.
Support http requests for the metadata server requests
Author: Pierre Seguin [email protected]
Resolves: #179