enh: added authentication ability for webapp #736

Merged
merged 6 commits into from Sep 6, 2011

Conversation

Projects
None yet
4 participants
Contributor

satra commented Aug 27, 2011

this probably needs to be tested a fair bit. but i put it up there so that we can discuss and improve this. but i believe this provides basic authentication, which coupled with running over SSL/TLS would at least satisfy some concerns.

you have to set the --IPythonNotebookApp.keyword for it to seek authentication. otherwise it will behave as before.

@minrk minrk and 1 other commented on an outdated diff Aug 29, 2011

IPython/frontend/html/notebook/notebookapp.py
@@ -94,6 +95,8 @@ class NotebookWebApplication(web.Application):
settings = dict(
template_path=os.path.join(os.path.dirname(__file__), "templates"),
static_path=os.path.join(os.path.dirname(__file__), "static"),
+ cookie_secret="61oETzKXQAGaYdkL5gEmGeJJFuYh7EQnp2XdTP1o/Vo=",
@minrk

minrk Aug 29, 2011

Owner

Does it make sense to hardcode the cookie secret? Or should this be generated on a per-instance basis? Is there any value in having a cookie secret that is publicly accessible?

@ellisonbg

ellisonbg Aug 30, 2011

Owner

The cookie secret needs to be 1) either generated anew each time the server starts or 2) set using a configuration variable. Hardcoding will means that it would not be difficult to compromise the security of the notebook.

@minrk minrk commented on an outdated diff Aug 29, 2011

IPython/frontend/html/notebook/handlers.py
@@ -35,13 +35,36 @@ except ImportError:
# Top-level handlers
#-----------------------------------------------------------------------------
-
-class NBBrowserHandler(web.RequestHandler):
+class BaseHandler(web.RequestHandler):
+ def get_current_user(self):
+ user_id = self.get_secure_cookie("user")
+ keyword = self.get_secure_cookie("keyword")
+ if self.application.keyword and self.application.keyword != keyword:
+ return None
@minrk

minrk Aug 29, 2011

Owner

I think this is responsible for setting the default username to 'None', when I try to login the first time.

@minrk minrk commented on an outdated diff Aug 29, 2011

IPython/frontend/html/notebook/notebookapp.py
})
notebook_aliases = [u'port', u'ip', u'keyfile', u'certfile', u'ws-hostname',
- u'notebook-dir']
+ u'notebook-dir', u'keyword']
@minrk

minrk Aug 29, 2011

Owner

I don't think we want to encourage setting the keyword at the command-line, as it completely invalidates protection from other users on localhost, who can view your command-line args. Also note that all options are still available on the command-line without being aliases, in their full --Class.trait=value form:

ipython notebook --IPythonNotebookApp.keyword=secret

which should only be used if you trust everyone who has access to your machine. Otherwise, setting via a config file should be the way to go.

@ellisonbg ellisonbg commented on an outdated diff Aug 30, 2011

IPython/frontend/html/notebook/handlers.py
def get(self):
nbm = self.application.notebook_manager
project = nbm.notebook_dir
self.render('nbbrowser.html', project=project)
+class LoginHandler(BaseHandler):
+ def get(self):
+ user_id = self.get_secure_cookie("user")
+ self.write('<html><body><form action="/login" method="post">'
+ 'Name: <input type="text" name="name" value=%s>'
+ 'Keyword: <input type="password" name="keyword">'
+ '<input type="submit" value="Sign in">'
+ '</form></body></html>'%user_id)
+
+ def post(self):
+ self.set_secure_cookie("user", self.get_argument("name", default=u''))
@ellisonbg

ellisonbg Aug 30, 2011

Owner

The setting of secure cookies and redirection should only be done if authentication succeeds.

@ellisonbg ellisonbg commented on an outdated diff Aug 30, 2011

IPython/frontend/html/notebook/handlers.py
def get(self):
nbm = self.application.notebook_manager
project = nbm.notebook_dir
self.render('nbbrowser.html', project=project)
+class LoginHandler(BaseHandler):
+ def get(self):
+ user_id = self.get_secure_cookie("user")
+ self.write('<html><body><form action="/login" method="post">'
@ellisonbg

ellisonbg Aug 30, 2011

Owner

I would rather use a form that has the notebook header and it styled according using the jquery ui classes and buttons.

@ellisonbg ellisonbg commented on an outdated diff Aug 30, 2011

IPython/frontend/html/notebook/notebookapp.py
@@ -122,11 +125,12 @@ aliases.update({
'keyfile': 'IPythonNotebookApp.keyfile',
'certfile': 'IPythonNotebookApp.certfile',
'ws-hostname': 'IPythonNotebookApp.ws_hostname',
- 'notebook-dir': 'NotebookManager.notebook_dir'
+ 'notebook-dir': 'NotebookManager.notebook_dir',
+ 'keyword' : 'IPythonNotebookApp.keyword'
@ellisonbg

ellisonbg Aug 30, 2011

Owner

Use password instead of keyword everywhere. Keyword sound like there is some tag associated with the notebook that is unrelated to security.

Owner

ellisonbg commented Aug 30, 2011

What about using standard Basic or Digest HTTP Auth instead of rolling our own? I know that Basic sends credentials in plain text, so that may not be a good idea if https is not enabled though. I did see that there are Basic and Digest auth mixin classes available for tornado, but I don't know how solid they are. What do others think?

Owner

minrk commented Aug 30, 2011

I think the form+cookie is fine, with the changes suggested (keyword->password, remove alias, new cookie sig each Server instance). We probably also want to add the @web.authenticated to the notebook pages, in addition to the frontpage.

Owner

ellisonbg commented Aug 30, 2011

Also, we need to look into seeing if the WebSocket handlers support
@web.authenticated. Otherwise, the actual kernel will be wide open
as well.

On Tue, Aug 30, 2011 at 10:58 AM, minrk
reply@reply.github.com
wrote:

I think the form+cookie is fine, with the changes suggested (keyword->password, remove alias, new cookie sig each Server instance).  We probably also want to add the @web.authenticated to the notebook pages, in addition to the frontpage.

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

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

minrk added some commits Aug 30, 2011

@minrk minrk notebook auth adjustments
* keyword -> password
* removed password alias
* add login.html as template
* new cookie key for each Server instance
f97967f
@minrk minrk Authenticate all notebook requests (except websockets)
* BaseHandler renamed AuthenticatedHandler
* also clears cookies if invalid, to prevent repeated 'Invalid cookie signature' warning messages.
4d5cfe6
Owner

minrk commented Aug 30, 2011

The kernel is protected by our own HMAC auth (once we turn that on by default), so I'm less concerned about them. I also don't know how the websocket machinery works, but a quick check showed 'method not supported on websockets' when I tried to authenticate in the same way as other connections.

I've updated this code in my own enh/httpauth branch, to reflect the changes we have discussed so far. All page requests are now authenticated, and the cookie secret is generated with os.urandom(). @satra, either you can merge my branch into yours, or we can issue a new PR from mine.

New diff: https://github.com/minrk/ipython/compare/enh/httpauth

Owner

ellisonbg commented Aug 30, 2011

The kernel is protected by our own HMAC auth (once we turn that on by default), so I'm less concerned about them.  I also don't know how the websocket machinery works, but a quick check showed 'method not supported on websockets' when I tried to authenticate in the same way as other connections.

But the HMAC auth will only protect the notebook server to kernel
connection. The WebSocket handlers in handlers.py will simply forward
any WS traffic to the kernel, so if the WS connections are not
authenticated, the HMAC won't protect. Here is a rough idea of how I
imagine this working.'

  • We need to make sure that the WebSocket connection in the browser
    sends the cookies along with the connection request.
  • We need to define the get_current_user method on the WebScoket
    handlers we have.
  • The open method of the WebSocket handlers need to check the cookies
    to make sure they match.
  • If the cookies don't match or are empty, then the handler should
    close the websocket connection.

I've updated this code in my own enh/httpauth branch, to reflect the changes we have discussed so far.  All page requests are now authenticated, and the cookie secret is generated with os.urandom().  @satra, either you can merge my branch into yours, or we can issue a new PR from mine.

New diff: https://github.com/minrk/ipython/compare/enh/httpauth

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

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

Owner

minrk commented Aug 30, 2011

Ah, good point. I forgot that the websocket connection is not passthrough. Then we do indeed need to authorize the websocket requests.

Is it possible (and if so, how?) to establish the websocket connections without ever being able to make a successful page request?

@minrk minrk authenticate Websockets with the session cookie
Now all Notebook connections are authenticated.
c28e4cb
Owner

minrk commented Aug 30, 2011

Websockets are now authenticated with the same cookie in my branch, so now all requests are authenticated.

Contributor

satra commented Aug 30, 2011

@min: i'm going to discard this request, please do create your own request. i did try a branch with a random cookie secret, but that did not seem to work well. i'll download your branch and hopefully test it on my flight back.

satra closed this Aug 31, 2011

satra reopened this Aug 31, 2011

Contributor

satra commented Aug 31, 2011

i reopened this and merged min's code and ran it. looks good. i don't know enough about web-authentication.

@min: do you think we should use "xsrf_cookies": True?

Owner

minrk commented Aug 31, 2011

@satra, I honestly have no idea on the xsrf cookie flag. I'm not familiar with its use case.

Owner

minrk commented Aug 31, 2011

I should note that the one thing I don't like about changing the cookie secret at each launch is that there will be warning about 'Invalid cookie signature' on the first connection from a browser, since the browser will still offer its old cookies to check if they are valid. It's not a big deal, except that if you run a logged in session followed by a passwordless one, you can continue to see the warnings, as fresh cookies are not created to replace the stale ones.

@ellisonbg ellisonbg merged commit c28e4cb into ipython:master Sep 6, 2011

robbyki commented Feb 3, 2014

Just setup the notebook at work on AWS EC2 and it works as expected except for the [tornado.general] WARNING | Invalid cookie signature with subsequent browser launches. Just want to confirm that this is still expected behavior and there is no way to resolve the message. --thank you

Owner

ellisonbg commented Feb 3, 2014

We may revisit this in the next few months but for now it is the expected behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment