Set cookie on 401

If a 401 response contains a Set-Cookie header, that header is not returned on the subsequent AUTH.

An application requiring that cookie ( e.g. session id ) on the AUTH fails.


`HTTPDigestAuth.handle_401()` needs to persist cookies. Thanks for reporting this! I'm marking this as contributor friendly, anyone who wants it should take it. I'm looking at @gazpachoking and @sigmavirus24, they're the cookie masters here.


No problem, thanks for the awesome library.

The is a simple workaround in the time being if anyone needs it.  It may not work in all situations but it should for regular session ids..

""" GET on the URL that requires authentication, just for the cookies """
req = requests.get(authUrl)

""" Start the session already holding the cookies """
sess = requests.Session()
sess.cookies = req.cookies

""" Session should now auth with the session cookies present """


Hmm, I took a bit of a peek. So, should a `Session` persist response cookies before and after calling hooks then?


I'm not sure what the division of responsibilities should be here. In principle a hook could change _everything_ about the response, but if that happened I'd be tempted to argue that the previous version of the response shouldn't be persisted. I wonder if the auth handler should do this itself (or whether it can without a huge rewrite).


Yeah, I was having the same trouble deciding what should do what. It certainly didn't seem right that the auth handler would do anything with session cookies. I like how the auth handler catches the 401 and makes a totally new response, but I'm not sure how the session should get access to the headers from the first response to persist the cookies. The 401 handler could certainly read the set-cookies headers from the first response and send those cookies when it retries, but I'm not sure how the set-cookie headers ever get to the session in that case.


Yeah, this is a thorny one. I'll have a ponder about it and see if I can think of a good flow here.


So I have two ideas for how to handle this:

The first is the least magic-y and is that we extract the cookies before dispatching the hook and compare the old response to the new one. If they're different, extract the cookies from the new one too.

The second is write a function that will take the session object and return a hook to be placed as the default hook for responses. When users add their own hooks, it will persist on the session and it will be called when the other hooks are called. This would remove the need for line 456 in `Session.send` too. We'd instantiate the hook upon Session initialization and so long as the user doesn't remove it, we'll be fine.


New option after a [conversation](https://botbot.me/freenode/python-requests/msg/2960213/) with @gazpachoking (which I would link to if botbot.me was working; _edit_ added the link since it's now up and running):

Since extracting the cookies before the hook could result in cookies being stored that hook authors might not want stored, we can safely move the extraction to after all redirects have been handled because then the history will be available. We also know that the history and the responses in the history will be correct because if there were redirects, the hooks are called on those responses too. So we can do something akin to:

``` python
r = dispatch_hook('..')

# resolve redirects

history = ['list comprehension']

if history:
    # fix the history

for resp in r.history:
    extract_cookies_to_jar(self.cookies, request, resp.raw)

extract_cookies_to_jar(self.cookies, request, r.raw)
```

This way the cookies are set and expired in the right order and only the cookies the hook author wants us to see are extracted. We're in the clear with this because the Auth Handlers properly also manage history (at least the built-in ones do and requests-kerberos does too. I'll check ntlm later).


@sigmavirus24: If I understand this right, we'd also have to add code to `DigestAuth` to propagate cookies for the redirects it handles. Correct?


Depends on which of the three you're talking about.


Your newest option.


@Lukasa the cookie extraction would take place on the history. DigestAuth won't require any changes at all. It already maintains history and we'll be parsing that history. 


That's not my concern. My worry is the second request that is made by `handle_401`. If a `Set-Cookie` header comes in on the 401, the auth handler needs to extract that cookie and send it back on the new request. Right?


Ah, I see what you mean. Yeah that means DigestAuth will have to do some extra 
work.


Wouldn't that already be broken now if it's a problem?


It is broken now, that's exactly what this issue is about. =P


> It is broken now, that's exactly what this issue is about. =P

Doh!


Sucks a bit that we have to repeat the cookie parsing 3 places, don't have any better ideas right now though.


Shouldn't `r.cookies` return the proper cookies set by the server? In that case, what if we just clone the old request and call `p.prepare_cookies(r.cookies)` to add those cookies? It'll give us a proper PreparedRequest for the new response and will have the cookies in tow.

In fact, I just checked `requests/adapters.py` and before the response is returned to us the cookies are in fact parsed and returned on `Response.cookies`. Let's just use that!


Won't that just contain the cookies that the server set on that this response? And so we'd be missing the cookies from the session?


Hm. The cookies on the session should be in the Set-Cookie header on `r.request`. Any relevant ones were prepared there. Is it possible to extract those cookies and add them to the jar before preparing the new request?


It should be. I think that's the way to go. =)


So to recapitulate:

In `requests/auth.py` we're going to extract the old cookies from the `Cookie` header in `r.request` add that to a Jar and then add any other cookies in `r.cookies`. Then we're going to make a new PreparedRequest and send for the Authentication.

I proposed earlier that in `requests/sessions.py` we move the cookie extraction after the history has been completed but I've just now realized this is problematic for the following reason:

Let's say we hit a server (either with authentication or otherwise) that redirects us and sets a cookie. If we only extract the cookies to the session's jar after we follow the redirect, we won't have the proper cookie for retrieving the page we're redirected to. So in my opinion, that should stay exactly where it is. I believe all of this is happening in `Session#send` so every time we get to the cookie extraction point (on the redirect requests) cookies are updated on the session's jar anyway. This does not need to be changed. If you guys think otherwise, or can think of a reason why my proposed situation is impossible, let me know. But I'm 90% sure that change is wholly unnecessary.


Yeah, we definitely need to keep the cookie persistence in the redirect code, which is the same problem we have to solve in the auth code, i.e. recomputing new cookies based on previous ones and `Set-Cookie` headers in the response for the new request.

I'm not sure adding `r.request.headers['Cookies']` to the `r.cookies` cookiejar  in the auth code is exactly right though. For one, if the original response was trying to unset a cookie, it wouldn't work.


> I'm not sure adding r.request.headers['Cookies'] to the r.cookies cookiejar in the auth code is exactly right though. For one, if the original response was trying to unset a cookie, it wouldn't work.

But if we turn `r.request.headers['Cookies']` into a jar first, then update it with `r.cookies` won't that unset the cookies you're talking about? Just like how they work in the `send` method.


@sigmavirus24 I think so, yeah.


A cookiejar doesn't contain the unset commands though, it is already lost when we parsed the `Set-Cookie` header into the `r.cookies` jar.


To clarify, I think we need to parse the `Set-Cookie` header from the response into a jar that contains the request's cookies rather than adding the request's cookies into the jar created from the response's `Set-Cookies` header.


I thought that's what I said. Anyway can't talk now. Out to dinner with an old friend.


I meant we can't use `r.cookies`, we have to use the `Set-Cookie` header directly from `r`