auth.login_required does not work correctly on flask-socketio connect handler

[aisotton]

I noticed that using @auth.login_required doesn't work when used on a socketio connect handler. It prevents the function from running, which makes it appear that it works, but it does not actually prevent the connection. This means that unauthenticated clients will be connected, and will receive events. Example:

@socketio.on('connect')
@auth.login_required
def connect():
  # this handler does not run, but the client is still connected
  app.logger.info('connected')

I believe that this is due to the fact that the connection rejection in http handlers and in socketio handlers does not work the same way - socketio connect handlers are supposed to return False to reject a connection, while http handlers send a status code etc. I resorted to using the following approach, which appears to work:

@socketio.on('connect')
@auth.login_required(optional=True)
def connect():
    if not auth.current_user():
        app.logger.info('rejected client')
        # This works - reject clients by returning False.
        return False
    app.logger.info(f'client connected, current_user = {auth.current_user()}')

A full working example is going to be a bit of a pain to write since it requires both a server and a client, but I can do that if it helps.

On a side note, my understanding is that only the connect handler needs to be protected by authentication, and all the other socketio handlers are protected implicitly since clients can't connect. Is this correct?

[miguelgrinberg]

This extension is for Flask routes. You can't use the auth.login_required decorator on any other type of functions.

A Socket.IO connection is permanent, you only need to authenticate the user in the connection handler. But as stated, this extension is not for this task.

[aisotton]

Thanks! I think it would be helpful to mention this in the docs, or detect that this is happening somehow - the fact that it half works (i.e. prevents the function from running but not the connection from working) is a little dangerous since it may give the impression that something is secured when it isn't.

[miguelgrinberg]

@aisotton I'm not sure how to detect this. You are just applying a decorator to a function. The decorator has no way to know if the function is a Flask route or something else. The function has no way to know it has been decorated.

I agree on documentation, but most people do not read docs. There is already a note to this regard:

A common need of applications is to validate the identity of their users. The traditional mechanisms based on web forms and HTTP requests cannot be used in a SocketIO connection, since there is no place to send HTTP requests and responses. If necessary, an application can implement a customized login form that sends credentials to the server as a SocketIO message when the submit button is pressed by the user.

However, in most cases it is more convenient to perform the traditional authentication process before the SocketIO connection is established. The user’s identity can then be recorded in the user session or in a cookie, and later when the SocketIO connection is established that information will be accessible to SocketIO event handlers.

Isn't this sufficient explanation of how to do auth?

[aisotton]

Fair enough. I had read the note in the documentation, but it seems to apply more to login systems with HTML login forms etc than to HTTP auth. In my use case, the login is happening earlier and then the header is set by the browser on all the requests, including socketio and websockets. I added the login_required decorator to the connect handler and checked that the connect handler wasn't running, and incorrectly assumed that this would prevent the connection. I ended up deploying the app and only later noticed that even unauthenticated socketio clients could still connect and receive socketio events.

A couple of possible approaches:

• The login_required decorator could detect that it's not being run on the correct type of function, and raise an exception. Not sure if this is possible by inspecting the context, but it would make the handler unusable, which would break things and make it obvious that this isn't the right decorator.

• Alternatively, flask-socketio might be able to detect that the connect handler is doing something abnormal, e.g. handing the socketio request like an http request. If I read things correctly, the connect handler is supposed to return False to reject the connection, or None to accept it. On the other hand a http handler returns a structure of some sort. Again I'm not sure if this is feasible because I'm not familiar enough with the internals, and it might break existing code.

I totally understand if you don't want to do this, I just thought I'd bring it to your attention.

[miguelgrinberg]

The login_required decorator could detect that it's not being run on the correct type of function

This is someone else's project, so I have no control here.

Alternatively, flask-socketio might be able to detect that the connect handler is doing something abnormal

Maybe, I'll have to add the login_required decorator to the connect handler and inspect what the handler returns, and if it is something that is different than any possible valid response.

[mwri]

I just hit this, actually I was using my own decorator for the WS stuff but then I realised that Flask-HTTPAuth which I use everywhere else must do 99% of what I want so I had a play.

I've settled on this:

@token_auth.verify_token
def _verify_token(token):
    if token_valid(token):
        return token_to_user(token)
    else:
        if request.path[:10] == '/socket.io':
            disconnect()
        return None

@sio.on('connect')
@auth.login_required(role='blah')
def _ws_connect():
    pass

Makes me feel slightly icky, but it works, and it's better than having two different auth paths for WS and other requests.

I'd feel better if I could pass something to the auth.login_required for _ws_connect really, like:

@token_auth.verify_token
def _verify_token(token, data={}):
    if token_valid(token):
        return token_to_user(token)
    else:
        if 'websocket' in data and data['websocket']:
            disconnect()
        return None

@sio.on('connect')
@auth.login_required(role='blah', data={'websocket': True})
def _ws_connect():
    pass

This has no functional advantage I guess for the use case in question, but does stop one coupling the WS path to the auth code, which is a plus as far as I'm concerned.

It would also allow:

@sio.on('message')
@auth.login_required(role='blah', data={'websocket': True})
def _ws_msg(message, x):

This has two effects. First the WS would be disconnected the first time it sends a message after a token expires. Second the current_user() call will then work in the callback.

In truth I'm not particularly bothered about disconnecting the user in this scenario, in fact I'd rather not do so, I'd rather do a soft auth so that current_user() works but doesn't cause a disconnect, but one could do that too with (say) data={'websocket': True, 'allow_expired': True} or something.

[mwri]

Just a comment to anyone reading the above, this is actually a horribly insecure dumb mistake, which I realised during testing! Any connection that provides no auth means _verify_token will never be called and @auth.login_required(role='blah') will exclude _ws_connect from running, so all you have to do to gain access is not offer any authentication, and you get access, duh! You really have to do @auth.login_required(optional=True) to ensure _ws_connect might not be skipped, then check request.current_user(), etc, or decorate to the same effect.