/__logout__ doesn't expire the session
Because of subject, any data structure linked to the user is still visible to the webserver if the user explicitly types /__logout__ in the URL. That doesn't seem very safe behaviour. The userdb/ in the example directory does this to workaround it: def logout(self, request): request.getSession().expire() request.setComponent(iformless.IRedirectAfterPost, "/"+guard.LOGOUT_AVATAR) but that workaround is insecure, since the user can type __logout__ by himself (if he knows the nevow code at least) and logout without clearing the session. This problem would have caused a (very minor) security issue to me, but it might have more serious implications in other apps. So I'd suggest to expire the session automatically in the __logout__ avatar so that a new fresh (anonymous) session will have to be allocated after logout. The last fix I posted isn't applied yet, so I append it again. My debug code now runs as I expected (i.e. the session is regenerated after logout and I don't need to expire it by hand anymore insecurely like userdb does). Please apply thanks! Index: nevow/rend.py =================================================================== --- nevow/rend.py (revision 1069) +++ nevow/rend.py (working copy) @@ -127,7 +127,8 @@ ctx.remember(request, inevow.IRequest) cf = iformless.IConfigurableFactory(self) c = cf.locateConfigurable(ctx, configurableName) - return self.webFormPost(request, self, c, ctx, bindingName, request.args) + if c: + return self.webFormPost(request, self, c, ctx, bindingName, request.args) return NotFound Index: nevow/guard.py =================================================================== --- nevow/guard.py (revision 1069) +++ nevow/guard.py (working copy) @@ -348,7 +348,7 @@ if segments and segments[0] == LOGIN_AVATAR: return self.login(request, s, self.getCredentials(request), segments[1:]) elif segments and segments[0] == LOGOUT_AVATAR: - s.portalLogout(self.portal) + s.expire() return urlToChild(request, *segments[1:]), () else: r = s.resourceForPortal(self.portal)
On Jan 12, 2005, at 6:21 PM, Andrea Arcangeli wrote:
The last fix I posted isn't applied yet, so I append it again.
I checked in the nevow.rend fix, thanks for that. I'm not comfortable checking in the guard fix without a bit of thought and investigation. The main problem is that glyph originally wrote guard such that each session could be logged in to multiple portals at once. But as I am looking at the current implementation, I can't imagine how this would actually work in practice. I'll comment more about this after some further research. dp
Andrea Arcangeli wrote:
Because of subject, any data structure linked to the user is still visible to the webserver if the user explicitly types /__logout__ in the URL. That doesn't seem very safe behaviour.
The userdb/ in the example directory does this to workaround it:
def logout(self, request): request.getSession().expire() request.setComponent(iformless.IRedirectAfterPost, "/"+guard.LOGOUT_AVATAR)
but that workaround is insecure, since the user can type __logout__ by himself (if he knows the nevow code at least) and logout without clearing the session.
This problem would have caused a (very minor) security issue to me, but it might have more serious implications in other apps.
So I'd suggest to expire the session automatically in the __logout__ avatar so that a new fresh (anonymous) session will have to be allocated after logout.
As I understand it, that removes "too much state"; that is, you only asked to logout from this specific realm, where the same person could be logged into multiple Realms at the same time. Or something like that. When your Realm returns the 3-tuple (inevow.IResource, someResource, logout) that logout callback is called when you call guard's .logout() or visit __logout__ (or when the session expires). Clean up the state there. For instance, if your app-specific state is in session.setComponent(IMyAppState, state), and thus accessed with IMyAppState(inevow.ISession(ctx)), you can clean that with session.unsetComponent(IMyAppState). Things could be cleaner. That's mostly a matter of good docstrings, gentle API evolution, and examples. Things could be simpler. But the design inherits directly from twisted.cred, and that is up to glyph to defend (most like on the main twisted mailing list).
Tommi Virtanen wrote:
When your Realm returns the 3-tuple
(inevow.IResource, someResource, logout)
that logout callback is called when you call guard's .logout() or visit __logout__ (or when the session expires). Clean up the state there. For instance, if your app-specific state is in session.setComponent(IMyAppState, state), and thus accessed with IMyAppState(inevow.ISession(ctx)), you can clean that with session.unsetComponent(IMyAppState).
Maybe this could be in an example somewhere, because I (for one) can't figure out how to do it. How do you access the session from the logout function? It isn't called with any parameters, and from what I can tell about IRealm implementors, the requestAvatar method -- whence the logout function is returned -- doesn't have access to the session either.
Things could be simpler. But the design inherits directly from twisted.cred, and that is up to glyph to defend (most like on the main twisted mailing list).
Glyph did a pretty good job of defending the cred architecture back when it was "new cred". See http://twistedmatrix.com/pipermail/twisted-python/2003-June/004578.html
Things could be cleaner. That's mostly a matter of good docstrings, gentle API evolution, and examples.
Yes, lots of examples :) The architecture is good, it's just not very clear to many end users how to navigate through all the abstractions. -- Alex Levy WWW: http://mesozoic.geecs.org "Never let your sense of morals prevent you from doing what is right." -- Salvor Hardin, Isaac Asimov's _Foundation_
On Thu, Jan 13, 2005 at 04:12:22PM -0500, Alex Levy wrote:
Maybe this could be in an example somewhere, because I (for one) can't figure out how to do it. How do you access the session from the logout function? It isn't called with any parameters, and from what I can tell
This is indeed the problem. I even setup a meaningful login function in the root page instead of the noLogin of most examples: if avatar_id is checkers.ANONYMOUS: resc = guest.root_page_class() resc.realm = self return (inevow.IResource, resc, resc.logout) else: resc = account.root_page_class(avatar_id) resc.remember(avatar_id, iweb.IAccount) resc.realm = self return (inevow.IResource, resc, resc.logout) problem is that the implementation in the root page is like this: def logout(self): return None So it doesnt' get that far. I'd actually prefer to get a callback so I can truly trap the event, but I need some way to reach the ctx to do the cleanup. I understand the twisted auth part must not know anything about the ctx, that's nevow stuff, but we should find a way to pass the ctx parameter (in a opaque way from twisted point of view), from nevow to the logout callback. Now, I don't claim this is really necessairly a relevant problem, if one is careful and never uses a certain interface from the session in the guest part, then there's no risk, but if you use it somewhere (incidentally a page shared between the account and the guest sections), then there can be a problem. I may even be ok to code carefully and never share an interface between guest and account, it's not that bad if you keep it in mind and you make sure not to share interfaces that attaches to the session, it's possible to code it safely, but I just feel safer and relieved from the pain of having to check everything by myself, if it was automatically expired by logout. So if I do a mistake I will get an exception or anyway I don't risk exposing sensitive data to the same session but outside the account area (i.e. after logout). So I've no doubt I feel safer expiring the session completely even if that means a bit more of traffic for cookie regeneration. I'd be fine to do it by hand with a callback though (and if somebody else wants to be more optimzied but less obviously safe, he can use the unsetComponent in the same callback where I do the expiry).
The architecture is good, it's just not very clear to many end users how to navigate through all the abstractions.
Here it doesn't seem just not clear, it seems not possible without any change to the caller to do the cleanup in the logout callback.
On Thu, Jan 13, 2005 at 04:12:22PM -0500, Alex Levy wrote: [...]
Maybe this could be in an example somewhere, because I (for one) can't figure out how to do it. How do you access the session from the logout function? It isn't called with any parameters, and from what I can tell about IRealm implementors, the requestAvatar method -- whence the logout function is returned -- doesn't have access to the session either.
I'm no cred expert, but isn't this is what the mind is for? -Andrew.
Andrew Bennetts wrote:
On Thu, Jan 13, 2005 at 04:12:22PM -0500, Alex Levy wrote:
Maybe this could be in an example somewhere, because I (for one) can't figure out how to do it. How do you access the session from the logout function? It isn't called with any parameters, and from what I can tell about IRealm implementors, the requestAvatar method -- whence the logout function is returned -- doesn't have access to the session either.
I'm no cred expert, but isn't this is what the mind is for?
Yes, you're right; good call. I'd forgotten about that part. So, I've attached an example that should allow you to define code that runs on logout, and also has full access to the session. I've tested it (briefly), so if anyone has any better ideas, they are more than welcome to share. Andrea, this should address your needs. Does anyone else think we should make this (or, better yet, a cleaned-up version of it) part of the Nevow examples? -- Alex Levy WWW: http://mesozoic.geecs.org "Never let your sense of morals prevent you from doing what is right." -- Salvor Hardin, Isaac Asimov's _Foundation_ from twisted.application import strports from twisted.application.service import Application from twisted.cred.checkers import AllowAnonymousAccess from twisted.cred.portal import Portal, IRealm from twisted.cred.credentials import IAnonymous from nevow import appserver from nevow import guard from nevow.inevow import IResource class Mind: def __init__(self, request, credentials): self.request = request self.credentials = credentials class MyRealm: __implements__ = IRealm, def requestAvatar(self, avatar_id, mind, *interfaces): if IResource in interfaces: return ( IResource, None, # This should be whatever resource you use self.createLogout(avatar_id, mind) ) raise NotImplementedError def createLogout(self, avatar_id, mind): def logout(): # This will be a nevow.guard.GuardSession instance session = mind.request.getSession() print 'Logging out for', session return logout application = Application('MySite') portal = Portal(MyRealm()) portal.registerChecker(AllowAnonymousAccess(), IAnonymous) guarded = guard.SessionWrapper(portal, mindFactory=Mind) site = appserver.NevowSite(resource=guarded) svc = strports.service('tcp:8080', site) svc.setServiceParent(application)
On Fri, Jan 14, 2005 at 11:26:30AM -0500, Alex Levy wrote:
Yes, you're right; good call. I'd forgotten about that part.
So, I've attached an example that should allow you to define code that runs on logout, and also has full access to the session. I've tested it (briefly), so if anyone has any better ideas, they are more than welcome to share.
Andrea, this should address your needs. Does anyone else think we should make this (or, better yet, a cleaned-up version of it) part of the Nevow examples?
Yes I see how the mind works now, there was code in nevow just for this purpose! So at least the expiry part I can definitely do it in the logout callback and you can obsolete my proposed patch (unless you feel safer to apply it and to let special aware users not to do the forced expiry by passing a variable when creating the sessionwrapper). However to do everything in the logout callback I need the ctx too, not only the session (then I could drop an intermediate redirecting page). I'll try to find a way to move from request to ctx (they're an 1:1 mapping, going from ctx to request is easy with inevow.IRequest, I'm just not sure about the reverse... ;). Many thanks!
Andrea Arcangeli wrote:
However to do everything in the logout callback I need the ctx too, not only the session (then I could drop an intermediate redirecting page).
I'll try to find a way to move from request to ctx (they're an 1:1 mapping, going from ctx to request is easy with inevow.IRequest, I'm just not sure about the reverse... ;).
This is mostly because the old APIs only passed request to anything, ctx is a recent addition. Things still need to be fixed to have ctx, but backwards compatibility and integration with older code is a challenge. I want ctx in the access.log writing functions, to know which virtual host the request was for..
On Fri, Jan 14, 2005 at 09:04:05PM +0200, Tommi Virtanen wrote:
Andrea Arcangeli wrote:
However to do everything in the logout callback I need the ctx too, not only the session (then I could drop an intermediate redirecting page).
I'll try to find a way to move from request to ctx (they're an 1:1 mapping, going from ctx to request is easy with inevow.IRequest, I'm just not sure about the reverse... ;).
This is mostly because the old APIs only passed request to anything, ctx is a recent addition. Things still need to be fixed to have ctx, but backwards compatibility and integration with older code is a challenge.
I want ctx in the access.log writing functions, to know which virtual host the request was for..
Ok, no problem, logout isn't reliable anyway since the session can expire instead of the user logging out, so I'll simply use the mind to expire the session instead of applying the patch I posted (the security part). Thanks everyone!
Andrea Arcangeli wrote:
Ok, no problem, logout isn't reliable anyway since the session can expire instead of the user logging out, so I'll simply use the mind to expire the session instead of applying the patch I posted (the security part).
As far as I understand things, session timeout causes all the related logout functions to be called. It goes something like this: one session relates to 0..n logged in portals portal logout means pretty much nothing to a session session expiry logs out from all related portals __logout__ logs out from that particular portal if you store data in session, they live until session expiry if you store data in mind, it lives until portal logout (NOTE: this is the only part I do not grok the code for, so I may be wrong here. I am pretty sure about the other points) This should probably be said explicitly in some docstrings.
On Fri, Jan 14, 2005 at 08:33:56PM +0100, Andrea Arcangeli wrote:
expire instead of the user logging out, so I'll simply use the mind to expire the session instead of applying the patch I posted (the security
I can't expire the session from there, it gets in some sort of recursion and tries to expire it twice, and the second time it generates an exception. Not too bad actually but for now I'm fine with a unsetCompontent which doesn't fire the exception (at least I get the bonus that it won't require cookie regeneration). if interface is inevow.IResource: def logout(session): def _logout(): session.unsetComponent(iweb.IAccount) return _logout if avatar_id is checkers.ANONYMOUS or avatar_id.shutdown: resc = guest.root_page_class() resc.realm = self return (inevow.IResource, resc, lambda : None) else: resc = account.root_page_class(avatar_id) resc.remember(avatar_id, iweb.IAccount) resc.realm = self session = mind.request.getSession() return (inevow.IResource, resc, logout(session)) btw, the logout trick looks quite like an hack, but the only real problem is that I can't get to the ctx, so I'm fine with the above trick for now. Thanks everyone.
Alex Levy wrote:
So, I've attached an example that should allow you to define code that runs on logout, and also has full access to the session. I've tested it (briefly), so if anyone has any better ideas, they are more than welcome to share.
Ooh. Thank you. I can use that in atleast one of my apps. If no one else gets to it, I'll put that in nevow/examples. I just need to dig out from under this pile of todo notes..
Tommi Virtanen wrote:
If no one else gets to it, I'll put that in nevow/examples. I just need to dig out from under this pile of todo notes..
I put a couple files in nevow/examples, named logout_guard.py and logout_guard2.py. The first just demonstrates the logout function; the second demonstrates how to save values in the session from a resource, and then retrieve them from the logout function. Please take a look; feedback from everyone is more than welcome. -- Alex Levy WWW: http://mesozoic.geecs.org "Never let your sense of morals prevent you from doing what is right." -- Salvor Hardin, Isaac Asimov's _Foundation_
btw, there is another (minor) problem in the userdb example, that is using formless to do the ILogout, will cause the logout to jump into the home with carryover set. The carryover screwup the cache, so if you happen to type "/" again (like following an home link), you'll find yourself pointing to the "member" page, not to the guest page. Because the last time you've really watched the "/" URL (without carryover) you were on the member part of the site. So I'm not using the redirectafter post anymore to avoid carryover, and everything is working fine now (I must only not share the same paths I use in the memeber context with the guest part, except for the "/" root itself which is handled by nevow itself in the login/logout procedure that disables the cache automatically [via post I guess]). there's an illegal chr in userdb.tac that prevents running it in svn, here the compile fix. Index: examples/userdb/userdb.tac =================================================================== --- examples/userdb/userdb.tac (revision 1084) +++ examples/userdb/userdb.tac (working copy) @@ -482,4 +482,4 @@ ) application = service.Application("UserManager1") -strports.service("8080", site).​setServiceParent(application) +strports.service("8080", site).setServiceParent(application)
participants (5)
-
Alex Levy -
Andrea Arcangeli -
Andrew Bennetts -
Donovan Preston -
Tommi Virtanen