[Freeipa-devel] [PATCH 82] Compliant client side session cookie behavior

[Martin Kosek]

On 12/10/2012 01:30 PM, Petr Viktorin wrote:
> On 12/09/2012 04:22 PM, John Dennis wrote:
>> On 12/09/2012 10:04 AM, John Dennis wrote:
>>> On 12/08/2012 11:16 AM, John Dennis wrote:
>>>> I'll send an updated patch shortly with the above fix. I also noticed
>>>> that http_return_ok() omitted the validation for the HttpOnly and Secure
>>>> flags I'll add that too.
>>>
>>> Revised patch attached.
>>
>> Same patch contents but with typo fixed.
>>
> 
> There's some more typos, but let's try to push this through and fix typos later.
> 
> Thanks for filing the Python bug & explaining your approaches. I still think
> this could be improved I'll stop bikeshedding now.
> 
> Just two issues:
> 
> When testing with lite-server listening on localhost, every request outputs
> "ipa: ERROR: not sending session cookie, URL mismatch". Is the message necessary?
> 
> Replying to a previous mail:
> 
>>>> diff --git a/ipalib/session.py b/ipalib/session.py
>>>> index 36beece..900259a 100644
>>>> --- a/ipalib/session.py
>>>> +++ b/ipalib/session.py
>>>> @@ -955,13 +955,18 @@ class MemcacheSessionManager(SessionManager):
> [...]
>>>> +        try:
>>>> +            session_cookie =
>>>> Cookie.get_named_cookie_from_string(cookie_header,
>>>> self.session_cookie_name)
>>>> +        except Exception, e:
>>>> +            session_cookie = None
>>>> +        else:
>>>> +            session_id = session_cookie.value
>>>
>>> When the user first accesses the Web UI, session_cookie will be None,
>>> resulting in an Internal Server Error.
>>
>> Hmm... I didn't see this in testing. I think you mean the cookie_header
>> will be None, not the session_cookie being None. That case should have
>> been caught by the try/except block surrounding
>> get_named_cookie_from_string(). But in any event I added a check for the
>> cookie_header being None at the top of the function. Or am I
>> misunderstanding the problem you saw?
>>
> 
> Yes, different problem.
> No cookie exists, so Cookie.get_named_cookie_from_string returns None,
> session_cookie is set to None, no exception is raised, the else: block accesses
> `session_cookie.value`.
> Tested with a clean install on f18.
> The attached fix solves the problem.
> 

Funny thing is that I was able to reproduce this issue only with Chrome
browser. My Firefox did not reproduce it.

I also hit weird issue when I logged in to Web UI, then cleaned server session
cache (`service ipa_memcached restart`) and then tried to log in again, I was
being rejected for about a minute even though I had valid credentials. But I
checked that this issue is also present in IPA 2.2.1, so it's not a
show-stopper for this release.

Besides that, the patch worked fine in my tests, I also tried joining this
server with older Fedora and some RHEL-6 machines and it worked fine. So I
think that John's patch (with Petr's fix squashed) can be included for 3.1.

Martin