From d934dc59bed3d418349076c4155bf724634b1f46 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 12:49:44 +0000 Subject: [PATCH 01/10] Adding oauthenticator to the setup --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index cd85e761f..f2bd77690 100644 --- a/setup.py +++ b/setup.py @@ -16,6 +16,7 @@ "jupyter_client>=4.3.0", "click>=6.6", "tabulate>=0.7.5", + "oauthenticator>=0.5", ] # Unfortunately RTD cannot install jupyterhub because jupyterhub needs bower, From 3530cf35d910d3f194c5d1c0d43444900948f1ff Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 12:53:48 +0000 Subject: [PATCH 02/10] Added oauthenticator to the requirements file --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index f2ca9f4aa..8562b70fe 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,3 +13,4 @@ jupyter_client==4.3.0 click==6.6 tabulate==0.7.5 git+git://github.com/simphony/tornado-webapi.git#egg=tornadowebapi +oauthenticator==0.5.1 From fa2a044a9ccb68b3dae48097d25c60e388c39db8 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 13:36:29 +0000 Subject: [PATCH 03/10] Moved world authenticator in a separate directory --- remoteappmanager/jupyterhub/auth/__init__.py | 1 + .../jupyterhub/{auth.py => auth/world_authenticator.py} | 0 2 files changed, 1 insertion(+) create mode 100644 remoteappmanager/jupyterhub/auth/__init__.py rename remoteappmanager/jupyterhub/{auth.py => auth/world_authenticator.py} (100%) diff --git a/remoteappmanager/jupyterhub/auth/__init__.py b/remoteappmanager/jupyterhub/auth/__init__.py new file mode 100644 index 000000000..8b0c7ddb9 --- /dev/null +++ b/remoteappmanager/jupyterhub/auth/__init__.py @@ -0,0 +1 @@ +from .world_authenticator import WorldAuthenticator diff --git a/remoteappmanager/jupyterhub/auth.py b/remoteappmanager/jupyterhub/auth/world_authenticator.py similarity index 100% rename from remoteappmanager/jupyterhub/auth.py rename to remoteappmanager/jupyterhub/auth/world_authenticator.py From f11b500966a39b86cd53abd3a8786598597ae73e Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 13:46:11 +0000 Subject: [PATCH 04/10] changed quotes --- remoteappmanager/jupyterhub/auth/world_authenticator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/jupyterhub/auth/world_authenticator.py b/remoteappmanager/jupyterhub/auth/world_authenticator.py index 3f218ee26..127f5b8dc 100644 --- a/remoteappmanager/jupyterhub/auth/world_authenticator.py +++ b/remoteappmanager/jupyterhub/auth/world_authenticator.py @@ -7,7 +7,7 @@ class WorldAuthenticator(Authenticator): - ''' This authenticator authenticates everyone ''' + """ This authenticator authenticates everyone """ @gen.coroutine def authenticate(self, handler, data): From 07daf9d56e544084d289ef50dc5682ace0b2587f Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 13:46:24 +0000 Subject: [PATCH 05/10] added tests for the world authenticator --- .../jupyterhub/auth/tests/__init__.py | 0 .../auth/tests/test_world_authenticator.py | 15 +++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 remoteappmanager/jupyterhub/auth/tests/__init__.py create mode 100644 remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py diff --git a/remoteappmanager/jupyterhub/auth/tests/__init__.py b/remoteappmanager/jupyterhub/auth/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py b/remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py new file mode 100644 index 000000000..9b86a990f --- /dev/null +++ b/remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py @@ -0,0 +1,15 @@ +from tornado.testing import AsyncTestCase, gen_test +try: + from unittest.mock import Mock +except ImportError: + from mock import Mock + +from remoteappmanager.jupyterhub.auth import WorldAuthenticator + + +class TestWorldAuthenticator(AsyncTestCase): + @gen_test + def test_basic_auth(self): + auth = WorldAuthenticator() + response = yield auth.authenticate(Mock(), {"username": "foo"}) + self.assertEqual(response, "foo") From 83489a80aec03bc968334a13e7d68da489ee91fc Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 14:50:45 +0000 Subject: [PATCH 06/10] Added github whitelist authenticator --- remoteappmanager/jupyterhub/auth/__init__.py | 1 + .../auth/github_whitelist_authenticator.py | 63 +++++++++++++++++++ .../test_github_whitelist_authenticator.py | 55 ++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py create mode 100644 remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py diff --git a/remoteappmanager/jupyterhub/auth/__init__.py b/remoteappmanager/jupyterhub/auth/__init__.py index 8b0c7ddb9..80a6a1b65 100644 --- a/remoteappmanager/jupyterhub/auth/__init__.py +++ b/remoteappmanager/jupyterhub/auth/__init__.py @@ -1 +1,2 @@ from .world_authenticator import WorldAuthenticator +from .github_whitelist_authenticator import GitHubWhitelistAuthenticator diff --git a/remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py b/remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py new file mode 100644 index 000000000..4e5a1dfbd --- /dev/null +++ b/remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py @@ -0,0 +1,63 @@ +import os +from oauthenticator import GitHubOAuthenticator +from traitlets.config import LoggingConfigurable +from traitlets import Unicode, Float, Set + + +class FileWhitelistMixin(LoggingConfigurable): + """ + """ + + #: The path of the whitelist file. + whitelist_file = Unicode() + + #: When the file was last modified, so that we can reload appropriately. + _whitelist_file_last_modified = Float() + + #: Cached whitelist to return every time the file hasn't changed. + _whitelist = Set() + + @property + def whitelist(self): + try: + cur_mtime = os.path.getmtime(self.whitelist_file) + print(os.path.getsize(self.whitelist_file)) + except FileNotFoundError: + # empty set means everybody is allowed + return {} + except Exception: + # For other exceptions, assume the file is broken, log it + # and return what we have. + self.log.exception("Unable to access whitelist.") + return self._whitelist + + if cur_mtime <= self._whitelist_file_last_modified: + # File older than last change. + # keep using the current cached whitelist + return self._whitelist + + self.log.info("Whitelist file more recent than the old one. " + "Updating whitelist.") + + # File has changed, parse its new content and reapply + try: + with open(self.whitelist_file, "r") as f: + whitelisted_users = set(x.strip() for x in f.readlines()) + except FileNotFoundError: + return {} + except Exception: + self.log.exception("Unable to access whitelist.") + # For other exceptions, assume the file is broken + return self._whitelist + + self._whitelist = whitelisted_users + self._whitelist_file_last_modified = cur_mtime + + return self._whitelist + + +class GitHubWhitelistAuthenticator(FileWhitelistMixin, GitHubOAuthenticator): + """A github authenticator that also verifies that the + user belongs to a specified whitelisted user as provided + by an external file (so that we don't have to restart + the service to change the whitelisted users""" diff --git a/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py new file mode 100644 index 000000000..a0f8a4d11 --- /dev/null +++ b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py @@ -0,0 +1,55 @@ +import os + +import time +from tornado.testing import AsyncTestCase, gen_test + +from remoteappmanager.tests.temp_mixin import TempMixin +from remoteappmanager.tests.utils import mock_coro_factory + +try: + from unittest.mock import Mock +except ImportError: + from mock import Mock + +from remoteappmanager.jupyterhub.auth import GitHubWhitelistAuthenticator + + +class TestGithubWhiteListAuthenticator(TempMixin, AsyncTestCase): + @gen_test + def test_basic_auth(self): + auth = GitHubWhitelistAuthenticator() + auth.authenticate = mock_coro_factory(return_value="foo") + + response = yield auth.authenticate(Mock(), {"username": "foo"}) + self.assertEqual(response, "foo") + + @gen_test + def test_basic_auth_with_whitelist_file(self): + whitelist_path = os.path.join(self.tempdir, "whitelist.txt") + with open(whitelist_path, "w") as f: + f.write("foo\n") + f.write("bar\n") + + auth = GitHubWhitelistAuthenticator() + auth.authenticate = mock_coro_factory(return_value="foo") + auth.whitelist_file = whitelist_path + + response = yield auth.get_authenticated_user(Mock(), + {"username": "foo"}) + self.assertEqual(response, "foo") + + # Check again to touch the code that does not trigger another load + response = yield auth.get_authenticated_user(Mock(), + {"username": "foo"}) + self.assertEqual(response, "foo") + + # wait one second, so that we see a change in mtime. + time.sleep(1) + + # Change the file and see if we get a different behavior + with open(whitelist_path, "w") as f: + f.write("bar\n") + + response = yield auth.get_authenticated_user(Mock(), + {"username": "foo"}) + self.assertEqual(response, None) From 29239db60e0ee9c594f3fbe8beaf469a83436921 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 16:53:09 +0000 Subject: [PATCH 07/10] Removed flake testing from init module --- remoteappmanager/jupyterhub/auth/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/remoteappmanager/jupyterhub/auth/__init__.py b/remoteappmanager/jupyterhub/auth/__init__.py index 80a6a1b65..8df2b6918 100644 --- a/remoteappmanager/jupyterhub/auth/__init__.py +++ b/remoteappmanager/jupyterhub/auth/__init__.py @@ -1,2 +1,2 @@ -from .world_authenticator import WorldAuthenticator -from .github_whitelist_authenticator import GitHubWhitelistAuthenticator +from .world_authenticator import WorldAuthenticator # noqa +from .github_whitelist_authenticator import GitHubWhitelistAuthenticator # noqa From 14b65e278cb3f21b2e4d870f361458ab032468d2 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 17:40:35 +0000 Subject: [PATCH 08/10] Coverage --- .../auth/github_whitelist_authenticator.py | 28 ++++------ .../test_github_whitelist_authenticator.py | 51 ++++++++++++++++--- 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py b/remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py index 4e5a1dfbd..0219410b0 100644 --- a/remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py +++ b/remoteappmanager/jupyterhub/auth/github_whitelist_authenticator.py @@ -21,33 +21,23 @@ class FileWhitelistMixin(LoggingConfigurable): def whitelist(self): try: cur_mtime = os.path.getmtime(self.whitelist_file) - print(os.path.getsize(self.whitelist_file)) - except FileNotFoundError: - # empty set means everybody is allowed - return {} - except Exception: - # For other exceptions, assume the file is broken, log it - # and return what we have. - self.log.exception("Unable to access whitelist.") - return self._whitelist - - if cur_mtime <= self._whitelist_file_last_modified: - # File older than last change. - # keep using the current cached whitelist - return self._whitelist + if cur_mtime <= self._whitelist_file_last_modified: + # File older than last change. + # keep using the current cached whitelist + return self._whitelist - self.log.info("Whitelist file more recent than the old one. " - "Updating whitelist.") + self.log.info("Whitelist file more recent than the old one. " + "Updating whitelist.") - # File has changed, parse its new content and reapply - try: with open(self.whitelist_file, "r") as f: whitelisted_users = set(x.strip() for x in f.readlines()) except FileNotFoundError: + # empty set means everybody is allowed return {} except Exception: + # For other exceptions, assume the file is broken, log it + # and return what we have. self.log.exception("Unable to access whitelist.") - # For other exceptions, assume the file is broken return self._whitelist self._whitelist = whitelisted_users diff --git a/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py index a0f8a4d11..a5bd68ba4 100644 --- a/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py +++ b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py @@ -6,19 +6,20 @@ from remoteappmanager.tests.temp_mixin import TempMixin from remoteappmanager.tests.utils import mock_coro_factory -try: - from unittest.mock import Mock -except ImportError: - from mock import Mock +from unittest.mock import Mock, patch from remoteappmanager.jupyterhub.auth import GitHubWhitelistAuthenticator class TestGithubWhiteListAuthenticator(TempMixin, AsyncTestCase): + def setUp(self): + self.auth = GitHubWhitelistAuthenticator() + self.auth.authenticate = mock_coro_factory(return_value="foo") + super().setUp() + @gen_test def test_basic_auth(self): - auth = GitHubWhitelistAuthenticator() - auth.authenticate = mock_coro_factory(return_value="foo") + auth = self.auth response = yield auth.authenticate(Mock(), {"username": "foo"}) self.assertEqual(response, "foo") @@ -30,8 +31,7 @@ def test_basic_auth_with_whitelist_file(self): f.write("foo\n") f.write("bar\n") - auth = GitHubWhitelistAuthenticator() - auth.authenticate = mock_coro_factory(return_value="foo") + auth = self.auth auth.whitelist_file = whitelist_path response = yield auth.get_authenticated_user(Mock(), @@ -53,3 +53,38 @@ def test_basic_auth_with_whitelist_file(self): response = yield auth.get_authenticated_user(Mock(), {"username": "foo"}) self.assertEqual(response, None) + + @gen_test + def test_basic_auth_without_whitelist_file(self): + auth = self.auth + auth.whitelist_file = "/does/not/exist.txt" + + response = yield auth.get_authenticated_user(Mock(), + {"username": "foo"}) + + # Should be equivalent to no whitelist, so everybody allowed + self.assertEqual(response, "foo") + + @gen_test + def test_exception_during_read(self): + whitelist_path = os.path.join(self.tempdir, "whitelist.txt") + with open(whitelist_path, "w") as f: + f.write("bar\n") + + auth = self.auth + auth.whitelist_file = whitelist_path + + # Do the first triggering, so that we load the file content. + response = yield auth.get_authenticated_user(Mock(), + {"username": "foo"}) + + self.assertEqual(response, None) + + # Then try again with an exception occurring + with patch("os.path.getmtime") as p: + p.side_effect = Exception("BOOM!") + + response = yield auth.get_authenticated_user(Mock(), + {"username": "foo"}) + self.assertEqual(response, None) + From bd3dc11051e3129379a46ceeb1ace31822cb4262 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 17:48:03 +0000 Subject: [PATCH 09/10] flake --- .../jupyterhub/auth/tests/test_github_whitelist_authenticator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py index a5bd68ba4..cebe870a6 100644 --- a/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py +++ b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py @@ -87,4 +87,3 @@ def test_exception_during_read(self): response = yield auth.get_authenticated_user(Mock(), {"username": "foo"}) self.assertEqual(response, None) - From 79b736a82650a31b1c9b7ddd6750a17c927d9ba6 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 6 Mar 2017 17:57:49 +0000 Subject: [PATCH 10/10] Removed irrelevant import for mock 2.7 --- .../jupyterhub/auth/tests/test_world_authenticator.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py b/remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py index 9b86a990f..5f4d04ef5 100644 --- a/remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py +++ b/remoteappmanager/jupyterhub/auth/tests/test_world_authenticator.py @@ -1,8 +1,5 @@ from tornado.testing import AsyncTestCase, gen_test -try: - from unittest.mock import Mock -except ImportError: - from mock import Mock +from unittest.mock import Mock from remoteappmanager.jupyterhub.auth import WorldAuthenticator