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

[Petr Viktorin]

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.


-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix.patch
Type: text/x-patch
Size: 503 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121210/080b9ac6/attachment.bin>