[Kimchi-devel] [PATCH 2/5] Code cleanup.

[Leonardo Augusto Guimarães Garcia]

On 02/11/2014 10:45 AM, Mark Wu wrote:
> On 02/10/2014 10:32 AM, Leonardo Garcia wrote:
>> From: Leonardo Garcia <lagarcia at br.ibm.com>
>>
>> Remove useless statements and improve debug message.
>>
>> Signed-off-by: Leonardo Garcia <lagarcia at br.ibm.com>
>> ---
>>   src/kimchi/auth.py | 9 +++------
>>   1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
>> index 242fdcf..f9873ca 100644
>> --- a/src/kimchi/auth.py
>> +++ b/src/kimchi/auth.py
>> @@ -87,13 +87,11 @@ def check_auth_session():
>>       for the user.
>>       """
>>       try:
>> -        s = cherrypy.session[SESSION_USER]
>> -        user = cherrypy.request.login = cherrypy.session[SESSION_USER]
>> -        debug("Authenticated with session: %s, for user: %s" % (s,
>> user))
>> +        user = cherrypy.session[USER_ID]
> It seems USER_ID is defined in the following up patch in this series, 
> so it needs a rebase.
Agree. I'll fix this in v2.
>> +        debug("Session authenticated for user %s" % user)
>>       except KeyError:
>>           debug("Session not found")
>>           return False
>> -    debug("Session found for user %s" % user)
>>       return True
>>
>>
>> @@ -135,8 +133,7 @@ def login(userid, password):
>>
>>   def logout():
>>       cherrypy.session.acquire_lock()
>> -    userid = cherrypy.session.get(SESSION_USER, None)
>> -    cherrypy.session[SESSION_USER] = cherrypy.request.login = None
>> +    cherrypy.session[USER_ID] = None
> we reset the session var to None on logout,  but in
> check_auth_session, we check if the key exists.
> So check_auth_session will always hold true even after logout.
> @shaohe,  it's a but, correct?
>
> I know this problem exists in the original patch,  but I think we
> should not keep the bug when it's changed.
I'll take a look on this as well. I agree that we should fix it even if
it is a bug that already exists in the previous code.

Best regards,

Leonardo Garcia
>>       cherrypy.session.release_lock()
>>       cherrypy.lib.sessions.expire()
>>
>