disconnect() documentation security consideration

[k3an3]

I recently had a problem in my application where my code was nearly exactly like that in the docs for flask_socketio.disconnect:

@socketio.on('admin')
def admin(msg):
    if not current_user.admin:
        disconnect()
    # admin functionality below here

I found out the hard way that disconnect() does not stop execution of the current function when used as a guard clause without an else, allowing for admin functionality to be accessed even after the disconnect(). I assumed (maybe a bit harshly) that it behaved similarly to flask.abort, where an exception is raised and execution of the current request stops dead in its tracks.

Obviously, the docs do not allude to an exception being thrown, but it was not exactly clear whether "terminates the connection with the client" means that the processing of the event stops. I also believe that the example code is misleading, as it succumbs to the same issue:

@socketio.on('message')
def receive_message(msg):
    if is_banned(session['username']):
        disconnect()
    # ...
    # Doesn't matter if you're banned, because this code below is still going to run

So, I am suggesting that the documentation (especially the example) be modified to be more clear that disconnect() cannot be relied on in this way. At a minimum, the example could be modified to include an else or return statement on the line following the disconnect. Perhaps the docstring could have a quick note added to it as well.

Thank you!

[miguelgrinberg]

Yeah, you are right, that example is misleading, there should be an else after the disconnect, just like in the decorator example. Thanks, I'll fix it.

[deounix]

I think best way to use decorator :

from flask_socketio import disconnect
import functools

def login_required(f):
	@functools.wraps(f)
	def wrapped(*args, **kwargs):
		if not current_user.is_authenticated:
			disconnect()
		else:
			return f(*args, **kwargs)
	return wrapped

@socketio.on('message')
@login_required
def receive_message(msg): pass

[deounix]

another way we can use condition in decorator :

from flask_socketio import disconnect
import functools

def login_required(condition=False, *disconnect_args, **disconnect_kwargs):
	def wrapper(fn):
		@functools.wraps(fn)
		def decorated_view(*args, **kwargs):
			if not condition:
				disconnect(*disconnect_args, **disconnect_kwargs)
			else:
				return fn(*args, **kwargs)
		return decorated_view
	return wrapper

@socketio.on('message')
@login_required(condition=current_user.is_authenticated, sid=None, namespace=None)
def receive_message(msg): pass

@socketio.on('message2')
@login_required(condition=current_user.admin, sid=None, namespace=None)
def receive_message2(msg): pass