Issues with authenticated redirects to the same destination host

[mkomitee]

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 by response.url instead of response.host, and pop it off of self.context in authenticate_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 the prepared_request, the response, and the session. There's no way to check if the type of authentication object mandates the Authorization header be stripped or not:

 def rebuild_auth(self, prepared_request, response):
        """
        When being redirected we may want to strip authentication from the
        request to avoid leaking credentials. This method intelligently removes
        and reapplies authentication where possible to avoid credential loss.
        """
        headers = prepared_request.headers
        url = prepared_request.url

        if 'Authorization' in headers:
            # If we get redirected to a new host, we should strip out any
            # authentication headers.
            original_parsed = urlparse(response.request.url)
            redirect_parsed = urlparse(url)

            if (original_parsed.hostname != redirect_parsed.hostname):
                del headers['Authorization']

        # .netrc might have more auth for us on our new host.
        new_auth = get_netrc_auth(url) if self.trust_env else None
        if new_auth is not None:
            prepared_request.prepare_auth(new_auth)

        return

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.

[Lukasa]

@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?

[flv7a]

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.

[Lukasa]

@flv7a Unfortunately we don't have any schedule on it, we're purely volunteer based so it'll get worked on when someone has time. I'm sure @mkomitee would be happy to guide you through providing a patch if that's something you're willing to do!

[mkomitee]

we can't create a patch without changes to requests proper. I wouldn't want
to do the work in changing requests without some buy in on how you'd like
the change designed.
On Sat, May 28, 2016 at 10:45 AM Cory Benfield notifications@github.com
wrote:

@flv7a https://github.com/flv7a Unfortunately we don't have any
schedule on it, we're purely volunteer based so it'll get worked on when
someone has time. I'm sure @mkomitee https://github.com/mkomitee would
be happy to guide you through providing a patch if that's something you're
willing to do!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#64 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAADOvpddZwQLoy8-JL17wOUHLmeRavtks5qGFULgaJpZM4HuC85
.