Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add auth hooks #16

Closed
wants to merge 1 commit into from
Closed

add auth hooks #16

wants to merge 1 commit into from

Conversation

ccyphers
Copy link

Adding --auth_hook option. This allows
one to filter request on the initial handshake
based on the JS sending an additional string in in the
protocol list:

websocket = new WebSocket(uri, ['base64', user + '-----' + auth])

This allows one to host the html/js under one type of appserver, integrating into the rest of their site's content and to set the two cookies fed to WebSocket. Then whtn the ws or wss handshake takes place, websocket.py, can call the auth_hook and disconnect if not authorized. The --auth_hook is simply an external application the user writes which exits with 0 if allowed.

one to filter request on the initial handshake
based on the JS sending an additional string in in the
protocol list:

websocket = new WebSocket(uri, ['base64', user + '-----' + auth])

where two vars are sent separated by '-----'.  This allows one to have
the web server set cookies which in turn can be then sent with the initial
WebSocket connect request and dropped if the user is not authorized.
@kanaka
Copy link
Member

kanaka commented Oct 25, 2011

@ccyphers, that's a good idea to have an auth_hook callout. However, I would like to ask that you make it more generic:

All the header values should be passed to the auth script as environment variables. The environment variables should be named after the header with a "WEBSOCKET_" prefix, hyphens converted to underscores, and perhaps some of the names normalized across different protocol versions. Then the auth script can key off any of the header values by using the values in its environment. In particular, the origin field is something that will be of interest for people using the auth hook.

Even more importantly, I think there should be a way to have an auth hook as a method that can be overridden by sub-classing. See the top_new_client and new_client methods as a model. I.e. in WebSocketServer, there should be a top_auth_hook method. That method would call out to an external script if specified using --auth_hook on the command line (and fail auth if the command doesn't return 0). Assuming that checks out, then it would call the auth_hook method which would have a stub in the WebSocketServer class but could be overridden when sub-classed to have a python hook.

Other things to clean up:

  • you still have references to pdb in you pull request that should be cleaned up.
  • the auth checks (i.e. the top_auth_hook method) should be called after the headers are parsed. Probably right after the line with "h = self.headers = wsh.headers".

Think you can tackle that? I would myself except that I recently started a new job and have even less time than normal to work on noVNC/websockify.

@ccyphers
Copy link
Author

Joel

All sounds like good recommendations. I'm also in the process of
transitioning in careers and not working on anything that would require time
to be spend in this area. As time permits I'd love to take these
suggestions and refine the feature set. I'll get back with you at that
time.

Cliff.

On Tue, Oct 25, 2011 at 5:38 PM, Joel Martin <
reply@reply.github.com>wrote:

@ccyphers, that's a good idea to have an auth_hook callout. However, I
would like to ask that you make it more generic:

All the header values should be passed to the auth script as environment
variables. The environment variables should be named after the header with a
"WEBSOCKET_" prefix, hyphens converted to underscores, and perhaps some of
the names normalized across different protocol versions. Then the auth
script can key off any of the header values by using the values in its
environment. In particular, the origin field is something that will be of
interest for people using the auth hook.

Even more importantly, I think there should be a way to have an auth hook
as a method that can be overridden by sub-classing. See the top_new_client
and new_client methods as a model. I.e. in WebSocketServer, there should be
a top_auth_hook method. That method would call out to an external script if
specified using --auth_hook on the command line (and fail auth if the
command doesn't return 0). Assuming that checks out, then it would call the
auth_hook method which would have a stub in the WebSocketServer class but
could be overridden when sub-classed to have a python hook.

Other things to clean up:

  • you still have references to pdb in you pull request that should be
    cleaned up.
  • the auth checks (i.e. the top_auth_hook method) should be called after
    the headers are parsed. Probably right after the line with "h = self.headers
    = wsh.headers".

Think you can tackle that? I would myself except that I recently started a
new job and have even less time than normal to work on noVNC/websockify.

Reply to this email directly or view it on GitHub:
#16 (comment)

@kanaka
Copy link
Member

kanaka commented Nov 2, 2012

bump

@DirectXMan12
Copy link
Member

@ccyphers : did you ever get back to this? I've been trying to get some closure on some of the older Websockify pull requests

@DirectXMan12
Copy link
Member

Closing due to no response.

For future reference: this can be implemented in terms of #162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request norespone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants