Unauthorized handshake hook

[shenek]

I added an authorization hook into the handshake function.
The hook allows you to raise an exception and return 401 to the client.

[aaugustin]

Thanks for the PR. I recognize that it's very inconvenient to override the handshake method currently. I'd like to think a bit about the best API. Perhaps it would be better to split the handshake() method.

[shenek]

Perhaps it would be better to split the handshake() method.

Sounds like a good idea.
It might be also nice to have a way to override the server response to handshake without rewriting the whole handler function. https://github.com/aaugustin/websockets/blob/master/websockets/server.py#L65

[JostBaron]

This is really needed - suppose you want to do OAuth authentication/authorization for your websocket server. Then you either need something like this (but ideally even more control over the response), or you need to copy/paste half the server code :(

[aaugustin]

I tried to come up with a more generic API in #154. Would that PR work for your use case?

Things are a bit messy because on one side WebSocketServerProtocol.handler translates internal exceptions to HTTP codes and on another side you can provide an arbitrary response code in get_response_status. However I didn't want to start down the path of introducing an exception class for every possible use case and to code the exception <--> HTTP mapping in handler.

[aaugustin]

To epxand on the "more generic" part: for example my proposal supports returning 401 if authentication info is missing and 403 if authentication info is incorrect.

[shenek]

Well I looked over your branch and I see two things that bother me:

https://github.com/aaugustin/websockets/pull/154/files#diff-a548b853d665035e6e62944845d0a575R133
You are setting headers to self.request_headers and then it could be verified in get_response_status.
Unfortunately I need a part of the path for authentication. So it would be really nice to add something like this to the function:

def read_request_headers(self):
    ...
    self.request_path = path
    ...

But the bigger problem lies here:
https://github.com/aaugustin/websockets/pull/154/files#diff-a548b853d665035e6e62944845d0a575R277
Yes I can return 401 or 403 status. But the connection is initiated anyway. So in the client can just ignore the status and he would have an opened ws connection. So it would bypass the authentication.

[aaugustin]

Both issues are correct. I'll come up with a revised proposal. Thanks for your feedback.

[aaugustin]

After e58c1ac you should be all set.

Let me know if you need anything else!