From 887fdaf9d3aafdb8a19a88f97ed94261570f14e7 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 8 Jan 2016 15:45:57 +0100 Subject: [PATCH] add username normalization Handlers call `get_authenticated_user`, which in turn calls - authenticate - normalize_username - check_whitelist get_authenticated_user shouldn't need to be overridden. Normalization can be handled via overriding normalize_username. --- jupyterhub/apihandlers/users.py | 1 + jupyterhub/app.py | 12 +++++-- jupyterhub/auth.py | 61 ++++++++++++++++++++++++++++----- jupyterhub/handlers/base.py | 2 +- jupyterhub/tests/test_auth.py | 25 +++++++++----- 5 files changed, 80 insertions(+), 21 deletions(-) diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 3e79c252..7c2fd151 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -34,6 +34,7 @@ class UserListAPIHandler(APIHandler): to_create = [] for name in usernames: + name = self.authenticator.normalize_username(name) user = self.find_user(name) if user is not None: self.log.warn("User %s already exists" % name) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 3b06b18f..59779577 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -630,7 +630,10 @@ class JupyterHub(Application): "\nUse Authenticator.admin_users instead." ) self.authenticator.admin_users = self.admin_users - admin_users = self.authenticator.admin_users + admin_users = [ + self.authenticator.normalize_username(name) + for name in self.authenticator.admin_users + ] if not admin_users: self.log.warning("No admin users, admin interface will be unavailable.") @@ -651,7 +654,10 @@ class JupyterHub(Application): # the admin_users config variable will never be used after this point. # only the database values will be referenced. - whitelist = self.authenticator.whitelist + whitelist = [ + self.authenticator.normalize_username(name) + for name in self.authenticator.whitelist + ] if not whitelist: self.log.info("Not using whitelist. Any authenticated user will be allowed.") @@ -671,7 +677,7 @@ class JupyterHub(Application): # but changes to the whitelist can occur in the database, # and persist across sessions. for user in db.query(orm.User): - whitelist.add(user.name) + self.authenticator.whitelist.add(user.name) # The whitelist set and the users in the db are now the same. # From this point on, any user changes should be done simultaneously diff --git a/jupyterhub/auth.py b/jupyterhub/auth.py index 4a7898b0..5d11d8d8 100644 --- a/jupyterhub/auth.py +++ b/jupyterhub/auth.py @@ -53,6 +53,56 @@ class Authenticator(LoggingConfigurable): """ ) + def normalize_username(self, username): + """Normalize a username. + + Override in subclasses if usernames should have some normalization. + Default: cast to lowercase, lookup in username_map. + """ + username = username.lower() + return username + + def check_whitelist(self, username): + """Check a username against our whitelist. + + Return True if username is allowed, False otherwise. + No whitelist means any username should be allowed. + + Names are normalized *before* being checked against the whitelist. + """ + if not self.whitelist: + # No whitelist means any name is allowed + return True + return username in self.whitelist + + @gen.coroutine + def get_authenticated_user(self, handler, data): + """This is the outer API for authenticating a user. + + This calls `authenticate`, which should be overridden in subclasses, + normalizes the username if any normalization should be done, + and then validates the name in the whitelist. + + Subclasses should not need to override this method. + The various stages can be overridden separately: + + - authenticate turns formdata into a username + - normalize_username normalizes the username + - check_whitelist checks against the user whitelist + """ + username = yield self.authenticate(handler, data) + if username is None: + return + username = self.normalize_username(username) + if not self.validate_username(username): + self.log.warning("Disallowing invalid username %r.", username) + return + if self.check_whitelist(username): + return username + else: + self.log.warning("User %r not in whitelist.", username) + return + @gen.coroutine def authenticate(self, handler, data): """Authenticate a user with login form data. @@ -60,6 +110,8 @@ class Authenticator(LoggingConfigurable): This must be a tornado gen.coroutine. It must return the username on successful authentication, and return None on failed authentication. + + Checking the whitelist is handled separately by the caller. """ def pre_spawn_start(self, user, spawner): @@ -74,13 +126,6 @@ class Authenticator(LoggingConfigurable): Can be used to do auth-related cleanup, e.g. closing PAM sessions. """ - def check_whitelist(self, user): - """ - Return True if the whitelist is empty or user is in the whitelist. - """ - # Parens aren't necessary here, but they make this easier to parse. - return (not self.whitelist) or (user in self.whitelist) - def add_user(self, user): """Add a new user @@ -240,8 +285,6 @@ class PAMAuthenticator(LocalAuthenticator): Return None otherwise. """ username = data['username'] - if not self.check_whitelist(username): - return try: pamela.authenticate(username, data['password'], service=self.service) except pamela.PAMError as e: diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 0a00af84..94c2e79c 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -247,7 +247,7 @@ class BaseHandler(RequestHandler): def authenticate(self, data): auth = self.authenticator if auth is not None: - result = yield auth.authenticate(self, data) + result = yield auth.get_authenticated_user(self, data) return result else: self.log.error("No authentication function, login is impossible!") diff --git a/jupyterhub/tests/test_auth.py b/jupyterhub/tests/test_auth.py index 802f3711..70471ecf 100644 --- a/jupyterhub/tests/test_auth.py +++ b/jupyterhub/tests/test_auth.py @@ -13,13 +13,13 @@ from jupyterhub import auth, orm def test_pam_auth(io_loop): authenticator = MockPAMAuthenticator() - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'match', 'password': 'match', })) assert authorized == 'match' - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'match', 'password': 'nomatch', })) @@ -27,19 +27,19 @@ def test_pam_auth(io_loop): def test_pam_auth_whitelist(io_loop): authenticator = MockPAMAuthenticator(whitelist={'wash', 'kaylee'}) - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'kaylee', 'password': 'kaylee', })) assert authorized == 'kaylee' - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'wash', 'password': 'nomatch', })) assert authorized is None - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'mal', 'password': 'mal', })) @@ -59,14 +59,14 @@ def test_pam_auth_group_whitelist(io_loop): authenticator = MockPAMAuthenticator(group_whitelist={'group'}) with mock.patch.object(auth, 'getgrnam', getgrnam): - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'kaylee', 'password': 'kaylee', })) assert authorized == 'kaylee' with mock.patch.object(auth, 'getgrnam', getgrnam): - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'mal', 'password': 'mal', })) @@ -75,7 +75,7 @@ def test_pam_auth_group_whitelist(io_loop): def test_pam_auth_no_such_group(io_loop): authenticator = MockPAMAuthenticator(group_whitelist={'nosuchcrazygroup'}) - authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { 'username': 'kaylee', 'password': 'kaylee', })) @@ -144,3 +144,12 @@ def test_handlers(app): assert handlers[0][0] == '/login' +def test_normalize_names(io_loop): + a = MockPAMAuthenticator() + authorized = io_loop.run_sync(lambda : a.get_authenticated_user(None, { + 'username': 'ZOE', + 'password': 'ZOE', + })) + assert authorized == 'zoe' + +