Open antonymott opened 2 years ago
oberstet
commented
2 years ago crossbar.io v22.6.1
yeah, this should be fixed, as the fix was
https://github.com/crossbario/crossbar/issues/2025
here is the testing that was done
https://github.com/crossbario/crossbar-examples/pull/156#issuecomment-1155590858 https://github.com/crossbario/crossbar-examples/blob/cfe7d866b2872eebdce3de5d311c6894c2868809/authentication/cookie/client.py#L111
antonymott
commented
2 years ago The fix #2025 and testing verified client-initiated logout works, but admin-initiated logout ("wamp.close.logout") of an authid appears broken. Was anyone able to replicate? "wamp.close.logout" triggers onleave, but does not appear to interact with the cookiestore so users automatically log in again with their still-valid cookie.
oberstet
commented
2 years ago but admin-initiated logout ("wamp.close.logout") of an authid appears broken
If you kill another session from your admin session using the WAMP meta API procedure wamp.session.kill and provide reason=="logout", the router side code that implements this meta API procedure
https://github.com/crossbario/crossbar/blob/master/crossbar/router/service.py#L433
will do the same as when the client itself had left its session with reason=="logout"
further, once the router-side session object of a session that is to be left (ended / removed from router), this will run
which will then call cs.delAuth(cbtid) to delete the cookie on that session from the cookiestore. it will also (potentially) kill all other sessions with the same cookie.
when a session is logged out, either from the session itself, or via the meta API, in both cases, this will hence log
do you see that log message, but only for "client-initiated logout", and not for "admin-initiated logout"?
antonymott
commented
2 years ago do you see that log message, but only for "client-initiated logout", and not for "admin-initiated logout"?
I see it only for client-initiated logout.
In particular, the logs are different depending on if I self logout as a client or using the WAMP meta API procedure(s). If I logout as client: connection.logout('wamp.close.logout') works and I get these logs (I added log messages in cookiestore.py to help me see what's going on, in particular if cs.delAuth(cbtid) is called):
2022-10-12T14:18:53-0400 [Router 97919]
I tried two of the WAMP meta API procedures: wamp.session.kill and wamp.session.kill_by_authid. And for each I used two reasons, the reason you asked me to use: reason=='logout' as well as reason=='wamp.close.logout' as the docs indicate to use all three words separated by dots and that does work for client self-logout. The logs are below, but I'll summarize: router session does detach...but not cs.delAuth(cbtid). Then the logs show the client attempts to login again using the same cookie which the router parses and authenticates. Missing but needed before the client attempts to login again are the bright red
Logs from trying the meta API procedures:
2022-10-12T14:28:40-0400 [Router 99434]
I connect using monitor.html as that makes it very easy to test, I first make sure a client is cookie-authenticated, then repeatedly to try to kill by session, by authid, pasting in each of the reasons into the form fields. As long as the cookie's lifetime was not expired, the client re-authenticates successfully with a new session every time.
At your end, do you find that when you kill by session or authid using the WAMP meta API procedure that it works, and you do see the cookie being deleted?
oberstet
commented
2 years ago I see it only for client-initiated logout.
ok. that is unexpected. as in: it should log in both cases, and I don't understand why it wouldn't. IOW: this needs more investigation.
we do have automated tests for authentication, including cookie, and including "logout", and also retesting with cookie deleted (expecting auth to fail then to succeed the test) - but only from the session itself, not via WAMP meta API
https://github.com/crossbario/crossbar-examples/blob/master/authentication/test_cookie.sh https://github.com/crossbario/crossbar-examples/blob/cfe7d866b2872eebdce3de5d311c6894c2868809/authentication/cookie/client.py#L111
At your end, do you find that when you kill by session or authid using the WAMP meta API procedure that it works, and you do see the cookie being deleted?
I'm not using cookies myself, I only created above test as I was working on auth related features for a customer project. and this test is missing a test via meta API .. which is more complicated (the test) to automate as 2 sessions are needed then .. anyways, yeah, sorry, don't know why it doesn't work ..
antonymott
commented
2 years ago For the time being, I patched my use case with a most dreadful kluge, but it may make you smile. I can now log anyone (white-hat clients) out using WAMP meta API: I replaced the default argument of message='some custom message that nobody uses and could be removed' with a flattened json object that flags some monkey-patch js in the browser to trigger client-assisted logout...which does the job. I may be the only CB user who ever used it, but thanks for leaving the "message" parameter in, it helped! 😊
antonymott
commented
2 years ago Update ! Good morning! I made a fix...tested and now WAMP meta API "kill by..." works so we can boot anyone off by session or authid and their cookie(s) deleted.
I didn't do a pull request yet because when you look over my fix, you may want to solve it a better way than I did (mine is not as DRY as it could be as there is similar code in onLeave), perhaps even in a different file as you know the innards so well. However, if you think it looks robust enough I'll happily do a pull request if that would help.
I followed the logic that WAMP meta API calls activate session.leave which is in class RouterSession(BaseSession) in service.py, but the main problem is that session.leave doesn't trigger "onLeave". I traced the "leave" call which does successfully fire leave in session.py (where the def is). Looking closer at "leave" I see it serves as a kind of Goodbye or aftereffect/cleanup. So probably the order that the existing “leave” needs to be fired should be after the important session.onLeave which actually does the job of deleting (“de-authenticating”) the cookie, closing transport protos and other stuff.
To the "leave" def (line 756 of session.py), I basically added the "if reason == " part and modified a "self.log.info(..." so it would work with the "leave" arguments which are type string, not type autobahn.wamp.types CloseDetails like those of the "onLeave" def:
def leave(self, reason=None, message=None):
"""
Implements :func:`autobahn.wamp.interfaces.ISession.leave`
"""
session_id = self._session_id or self._previous_session_id
cookie_deleted = None
cnt_kicked = 0
if reason == 'wamp.close.logout':
# if cookie was set on transport
if self._transport and hasattr(self._transport, '_cbtid') and self._transport._cbtid and self._transport.factory._cookiestore:
cbtid = self._transport._cbtid
cs = self._transport.factory._cookiestore
print(f'🎠 {__name__} 🌱 🐝 🐞 \n\n\n\n 🎠 🎠 🎠 this seems to work great! 🎠 🎠 🎠 \n\n\n\n 🐝 🐞 ')
# set cookie to "not authenticated"
# cs.setAuth(cbtid, None, None, None, None, None)
cs.delAuth(cbtid)
cookie_deleted = cbtid
# kick all transport protos (eg WampWebSocketServerProtocol) for the same auth cookie
for proto in cs.getProtos(cbtid):
# but don't kick ourselves
if proto != self._transport:
proto.sendClose()
cnt_kicked += 1
self.log.info(
'{func} {action} completed with message {message} for session {session_id} (cookie authentication deleted: '
'"{cookie_deleted}", pro-actively kicked (other) sessions: {cnt_kicked})',
action=hlval('wamp.close.logout', color='red'),
session_id=hlid(session_id),
cookie_deleted=hlval(cookie_deleted, color='red') if cookie_deleted else 'none',
cnt_kicked=hlval(cnt_kicked, color='red') if cnt_kicked else 'none',
message=hlval(message, color='blue') if message else 'no message',
func=hltype(self.onLeave))
if not self._goodbye_sent:
if reason:
msg = wamp.message.Goodbye(reason, message)
else:
msg = wamp.message.Goodbye(message=message)
self._transport.send(msg)
self._goodbye_sent = True
else:
raise SessionNotReady("Already requested to close the session")thanks for posting what you did! fwiw, we need an automated test of cookie auth testing cookie delete via WAMP meta API. this test should fail first, and then we can add a change to crossbar to fix it ...
crossbar.io v22.6.1
Calls to wamp.session.kill_by_authid with any reason are handled by router/service.py so do not appear to cause the router to set the cookie to "not authenticated", unlike when the client explicitly closes the connection with reason "wamp.close.logout" which does work (router/session.pyLn1225).
After admin issues "wamp.session.kill_by_authid", the session is detached, but on the client, connection.open() automatically reauthenticates the user as the cookie in the browser persists as valid. Wampcra is bypassed when using cookie authentication.
To repeat: authmethods: ["cookie", "anonymous", "wampcra"], then change to another user/session (i.e. admin or monitor) and call "wamp.session.kill_by_authid"
I started to try to fix this and do a pull request in router/service.py, something like (pseudocode):
#if details.reason == "wamp.close.logout", set cookie to "not authenticated"
set self._transport.factory._cookiestore.delAuth(cbtid)# kick all transport protos (eg WampWebSocketServerProtocol) for the same auth cookie
for proto in self._transport.factory._cookiestore.getProtos(cbtid): if proto != self._transport: proto.sendClose() cnt_kicked += 1but 'wamp.session.killby..." calls are in a different module (around line 444 of router/service.py) so it was not obvious to me how to properly acquire scope of transports or cookiestore.
This security/control feature was implemented 4 years ago after this discussion: https://github.com/wamp-proto/wamp-proto/issues/314. So I wonder if it might be user-error - no bug at all - and that I'm the one incorrectly using "wamp.session.killby[session/authid]".
On the other hand, I searched for "cookie" in the discussion and no hits...so maybe cookie-authenticated users slipped through 😬
Here are server logs specific to the issue...note the authid does have its session detached by "wamp.session.kill_by_authid"...then immediately is able to reauthenticate using the same authid:
2022-10-01T00:16:09-0400 [Router 202987] router session detached from realm "realm1" (session=7750981284023503, detached_session_ids=1, authid="XQXG-7R9K-GHTT-4LML-T7AJ-57GV", authrole="public", authmethod="anonymous", authprovider="dynamic")
2022-10-01T00:16:11-0400 [Router 202987] : parsed tracking/authentication cookie cbtid "rgI7hRwLQd87oovxcvR2BquX" from HTTP request headers
2022-10-01T00:16:11-0400 [Router 202987] : tracking/authentication cookie cbtid "rgI7hRwLQd87oovxcvR2BquX" already set and stored
2022-10-01T00:16:11-0400 [Router 202987] authenticated client via cookie cbtid=rgI7hRwLQd87oovxcvR2BquX as authid="XQXG-7R9K-GHTT-4LML-T7AJ-57GV", authrole="public", authmethod="anonymous", authprovider="cookie", authrealm="realm1"