Minor code touch-up (#36)

* .install_deps updated so we might be able to pass 3.8
    builds with old deps now
* Added Killerwhile to the Attribution.md file
* Added notes in the README for new additions from
    Killerwhile's contribution, fix some minor grammar
    issues
* Minor doc adjustments on new SAMLAuthenticator traits
* Bump version in setup.py in prep for new version push
    to pypi
* More tests.

.install_deps

@@ -4,7 +4,7 @@ if [[ $DEPS_VERSION = "OLD" ]]; then
     pip install Jinja2==2.4 jupyterhub==0.9.0 lxml==4.2.1 signxml==2.6.0 pytz==2019.1
     pip install pytest==4.0.0 pytest-asyncio==0.10.0 pytest-cov==2.0.0;
 elif [[ $DEPS_VERSION = "AFTER38" ]]; then
-    pip install Jinja2==2.4 jupyterhub==0.9.0 lxml==4.3.4 signxml==2.6.0 pytz==2019.1
+    pip install Jinja2==2.4 jupyterhub==0.9.0 lxml==4.3.5 signxml==2.6.0 pytz==2019.1
     pip install pytest==4.0.0 pytest-asyncio==0.10.0 pytest-cov==2.0.0;
 else
     pip install --upgrade --pre -r requirements.txt

ATTRIBUTION.md

@@ -30,3 +30,4 @@ OTHER DEALINGS IN THE SOFTWARE.
 * @minrk for giving a ton of help on how to integrate with JupyterHub
 * @mwilbz for making a great issue and patch
 * @itsnagaraj for making a great issue, providing a lot of thought and code, and being very patient with development of the project
+* @killerwhile for jumping in and making some really neat additions

README.md

@@ -88,7 +88,11 @@ If the user's servers should be shut down when they logout, set `shutdown_on_log
 
 The SAMLAuthenticator _usually_ attempts to forward users to the SLO URI set in the SAML Metadata. If this is not the desired behavior for whatever reason, set `slo_forward_on_logout` to `False`. This will change the page the user is forwarded to on logout from the page specified in the xml metadata to the standard jupyterhub logout page.
 
-SAMLAuthenticator creates system users by default on successful authentication. If you are running JupyterHub as a non-root user, you may need to turn off this functionality by setting `create_system_users` to `False`.
+The SAMLAuthenticator creates system users by default on successful authentication. If you are running JupyterHub as a non-root user, you may need to turn off this functionality by setting `create_system_users` to `False`.
+
+The default nameid format that the SAMLAuthenticator expects is defined by the SAML Spec as `urn:oasis:names:tc:SAML:2.0:nameid-format:transient`. This can be changed by setting the `nameid_format` field on the SAMLAuthenticator in the JupyterHub Config file.
+
+If the server administrator wants to create local users for each JupyterHub user but doesn't want to use the `useradd` utility, a user can be added with any binary on the host system Set the `create_system_user_binary` field to either a) a full path to the binary or b) the name of a binary on the host's path. Please note, if the binary exits with code 0, the Authenticator will assume that the user add succeeded, and if the binary exits with any code _other than 0_, it will be assumed that creating the user failed.
 
 #### Example Configurations
 
@@ -144,7 +148,19 @@ c.SAMLAuthenticator.organization_display_name = '''My Org's Display Name'''
 c.SAMLAuthenticator.organization_url = 'https://myorg.com'
 
 # Turn off system user creation on authentication
+# This feature added by GitHub user @mwilbz
 c.SAMLAuthenticator.create_system_users = False
+
+# Change nameid format to something else
+# This feature added by GitHub user @killerwhile
+c.SAMLAuthenticator.nameid_format = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
+
+# Change the binary called to create users
+# This feature added by GitHub user @killerwhile
+# If the new_useradd binary isn't on the path, a full path can be provided
+c.SAMLAuthenticator.create_system_user_binary = '/full/path/to/new_useradd'
+# If the new_useradd binary is on the path, we can use the first-found instance
+c.SAMLAuthenticator.create_system_user_binary = 'new_useradd'
 ```
 
 ## Developing and Contributing

samlauthenticator/samlauthenticator.py

@@ -276,11 +276,13 @@ class SAMLAuthenticator(Authenticator):
         allow_none=True,
         config=True,
         help='''
-        When SAMLAuthenticator create system user (also called "just in time user provisioning")
+        When SAMLAuthenticator creates a system user (also called "just in time user provisioning")
         it calls the binary specified in this property in a subprocess to perform the user creation.
-        Default value is useradd, but any other existing binary in the PATH would be called
-        followed by the username to actually add.
-        The binary must follow the same return code than useradd, i.e. return 0 in case of success.
+        Default value is 'useradd'.
+        This can be set to any binary in the host machine's PATH or a full path to an alternate
+        binary not in the host's path. This binary MUST accpet calls of the form
+        "$\{binary_name\} $\{user_name\}" and exit with a status of zero on valid user addition or
+        a non-zero status in the failure case.
         '''
     )
 

setup.py

@@ -22,7 +22,7 @@
 
 from setuptools import setup
 
-version = '0.0.5'
+version = '0.0.6'
 
 with open('requirements.txt', 'r') as req_file, \
         open('test_requirements.txt', 'r') as test_req_file, \

tests/test_authenticator.py

@@ -456,6 +456,35 @@ def test_create_user_fails(self, mock_pwd, mock_subprocess):
         mock_pwd.getpwnam.assert_called_once_with('Bluedata')
         mock_subprocess.call.assert_called_once_with(['useradd', 'Bluedata'])
 
+    @patch('samlauthenticator.samlauthenticator.subprocess')
+    @patch('samlauthenticator.samlauthenticator.pwd')
+    def test_create_user_alternate_binary(self, mock_pwd, mock_subprocess):
+        mock_pwd.getpwnam.side_effect = KeyError('Bad username')
+        mock_subprocess.call.return_value = 0
+        binary_value = 'test_binary'
+
+        a = SAMLAuthenticator()
+        a.create_system_user_binary = binary_value
+
+        assert a._optional_user_add('Bluedata')
+
+        mock_pwd.getpwnam.assert_called_once_with('Bluedata')
+        mock_subprocess.call.assert_called_once_with([binary_value, 'Bluedata'])
+
+    @patch('samlauthenticator.samlauthenticator.subprocess')
+    @patch('samlauthenticator.samlauthenticator.pwd')
+    def test_create_user_alternate_binary_existing_user(self, mock_pwd, mock_subprocess):
+        mock_pwd.getpwnam.return_value = True
+        binary_value = 'test_binary'
+
+        a = SAMLAuthenticator()
+        a.create_system_user_binary = binary_value
+
+        assert a._optional_user_add('Bluedata')
+
+        mock_pwd.getpwnam.assert_called_once_with('Bluedata')
+        mock_subprocess.call.assert_not_called()
+
     def test_check_username_valid_username_no_black_lists(self):
         a = SAMLAuthenticator()
         a._optional_user_add = MagicMock()
@@ -790,6 +819,7 @@ def test_make_full_metadata_nameid_format(self):
 
         assert a._make_sp_metadata(mock_handler_self) == self.full_sp_meta_nameid_format
 
+
 class TestGetHandlers(unittest.TestCase):
     expected_handler_paths = ['/login', '/hub/login', '/logout', '/hub/logout', '/metadata', '/hub/metadata']