Include system-user creation error message in API reply

when system-user creation fails

jupyterhub/apihandlers/users.py

@@ -63,10 +63,10 @@ class UserListAPIHandler(APIHandler):
                 self.db.commit()
             try:
                 yield gen.maybe_future(self.authenticator.add_user(user))
-            except Exception:
+            except Exception as e:
                 self.log.error("Failed to create user: %s" % name, exc_info=True)
                 del self.users[user]
-                raise web.HTTPError(400, "Failed to create user: %s" % name)
+                raise web.HTTPError(400, "Failed to create user %s: %s" % (name, str(e)))
             else:
                 created.append(user)
         

jupyterhub/auth.py

@@ -6,9 +6,10 @@
 from grp import getgrnam
 import pipes
 import pwd
+import re
 from shutil import which
 import sys
-from subprocess import check_call
+from subprocess import Popen, PIPE, STDOUT
 
 from tornado import gen
 import pamela
@@ -271,10 +272,7 @@ class LocalAuthenticator(Authenticator):
     def add_user(self, user):
         """Add a new user
         
-        By default, this just adds the user to the whitelist.
+        If self.create_system_users, the user will attempt to be created.
-        
-        Subclasses may do more extensive things,
-        such as adding actual unix users.
         """
         user_exists = yield gen.maybe_future(self.system_user_exists(user))
         if not user_exists:
@@ -300,7 +298,11 @@ class LocalAuthenticator(Authenticator):
         name = user.name
         cmd = [ arg.replace('USERNAME', name) for arg in self.add_user_cmd ] + [name]
         self.log.info("Creating user: %s", ' '.join(map(pipes.quote, cmd)))
-        check_call(cmd)
+        p = Popen(cmd, stdout=PIPE, stderr=STDOUT)
+        p.wait()
+        if p.returncode:
+            err = p.stdout.read().decode('utf8', 'replace')
+            raise RuntimeError("Failed to create system user %s: %s" % (name, err))
 
 
 class PAMAuthenticator(LocalAuthenticator):

jupyterhub/tests/test_auth.py

@@ -3,7 +3,6 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 
-from subprocess import CalledProcessError
 from unittest import mock
 
 import pytest
@@ -96,12 +95,24 @@ def test_cant_add_system_user(io_loop):
     authenticator.add_user_cmd = ['jupyterhub-fake-command']
     authenticator.create_system_users = True
     
-    def check_call(cmd, *a, **kw):
+    class DummyFile:
-        raise CalledProcessError(1, cmd)
+        def read(self):
+            return b'dummy error'
     
-    with mock.patch.object(auth, 'check_call', check_call):
+    class DummyPopen:
-        with pytest.raises(CalledProcessError):
+        def __init__(self, *args, **kwargs):
+            self.args = args
+            self.kwargs = kwargs
+            self.returncode = 1
+            self.stdout = DummyFile()
+        
+        def wait(self):
+            return
+    
+    with mock.patch.object(auth, 'Popen', DummyPopen):
+        with pytest.raises(RuntimeError) as exc:
             io_loop.run_sync(lambda : authenticator.add_user(user))
+        assert str(exc.value) == 'Failed to create system user lioness4321: dummy error'
 
 
 def test_add_system_user(io_loop):
@@ -111,10 +122,15 @@ def test_add_system_user(io_loop):
     authenticator.add_user_cmd = ['echo', '/home/USERNAME']
     
     record = {}
-    def check_call(cmd, *a, **kw):
+    class DummyPopen:
-        record['cmd'] = cmd
+        def __init__(self, cmd, *args, **kwargs):
+            record['cmd'] = cmd
+            self.returncode = 0
+        
+        def wait(self):
+            return
     
-    with mock.patch.object(auth, 'check_call', check_call):
+    with mock.patch.object(auth, 'Popen', DummyPopen):
         io_loop.run_sync(lambda : authenticator.add_user(user))
     assert record['cmd'] == ['echo', '/home/lioness4321', 'lioness4321']