-
Notifications
You must be signed in to change notification settings - Fork 101
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
Issues with authenticated redirects to the same destination host #64
Comments
@mkomitee Yeah, so you're absolutely right here: this limitation most definitely does exist. Requests' redirect following logic simply doesn't have the smarts handle this at this time. I'm open to receiving a PR that changes this: possibly we want to add another method for auth handlers to have called that will allow them to be consulted by requests before requests goes to use .netrc (a kind of "are you still valid on this new host?" question). Thoughts @sigmavirus24? |
Any idea when this might bubble up towards the top of the queue? I'm working on an integration with a SAML Identity Provider (SAML loves 302 redirects!) and unfortunately I'm hitting the same "Context is already fully established" error unless I disable mutual authentication entirely. |
we can't create a patch without changes to requests proper. I wouldn't want
|
This also explicitly disables mutual authentication, which seems to be causing issues with redirects to /start and friends. Context: https://fedoraproject.org/wiki/Changes/kerberos-in-python-modernization requests/requests-kerberos#64 pythongssapi/requests-gssapi#12 pythongssapi/requests-gssapi@498da2e Fixes #263 Signed-off-by: Ernestas Kulik <[email protected]>
This also explicitly disables mutual authentication, which seems to be causing issues with redirects to /start and friends. Context: https://fedoraproject.org/wiki/Changes/kerberos-in-python-modernization requests/requests-kerberos#64 pythongssapi/requests-gssapi#12 pythongssapi/requests-gssapi@498da2e Fixes #263 Signed-off-by: Ernestas Kulik <[email protected]>
I've run into an issue w/ requests & requests_kerberos which I don't think we can fix without changes to requests.
We should not reuse an already established kerberos context on new requests. This means, if we're prompted for authentication, then receive a redirect, we shouldn't reuse the previously generated authorization header when following the redirect, even if the redirect is to the same hostname.
I can update our code to do the right thing (index
HTTPKerberosAuth.context
byresponse.url
instead ofresponse.host
, and pop it off ofself.context
inauthenticate_server
.What we can't do from here is change
requests.Session.rebuild_auth
to remove the Authorization header on a prepared request when following a redirect, even when the redirect is to the same hostname:The code for
rebuild_auth
only has access to theprepared_request
, theresponse
, and thesession
. There's no way to check if the type of authentication object mandates the Authorization header be stripped or not:I looked through the hook documentation and it doesn't look like there's anywhere we can inject ourselves to modify the behavior. Do you have any ideas?
@sigmavirus24 @Lukasa I think I'll need your help on this one.
By the way, I think this is tangentially related to #54, and the reason @danc86 wasn't experiencing his problem with the "latest" requests at the time was because the latest requests happened to include the previous Authorization header on the redirected request. His suggestion to deregister the hook would have caused the latter responses from being mutually authenticated which would have solved his problem, but means he shouldn't trust that response (if he really wanted mutual authentication. If he didn't he could have just disabled it).
Maybe
Session.rebuild_auth
can delegate the responsibility for rebuilding the auth and manipulating headers (or not) to the authentication object if it's not from a built-in authentication mechanism ... but for that to work, we would need to be able to access the authentication object, which we can't if it's not on the session -- which it won't be for most of our users.The text was updated successfully, but these errors were encountered: