From b86484ba42e1ea3467c1428a4072d6ff0f997529 Mon Sep 17 00:00:00 2001
From: "Nickolaus D. Saint" <nick@globus.org>
Date: Tue, 10 Sep 2019 13:47:25 -0700
Subject: [PATCH] Fixed Globus Logout Handler, added test

---
 oauthenticator/globus.py            | 18 ++++-----
 oauthenticator/tests/test_globus.py | 57 ++++++++++++++++++++++-------
 2 files changed, 51 insertions(+), 24 deletions(-)

diff --git a/oauthenticator/globus.py b/oauthenticator/globus.py
index 418f5bde..fc57bfd2 100644
--- a/oauthenticator/globus.py
+++ b/oauthenticator/globus.py
@@ -40,27 +40,25 @@ class GlobusLogoutHandler(LogoutHandler):
     only the Jupyterhub session is cleared.
     """
     async def get(self):
-        user = self.get_current_user()
-        if user:
-            if self.authenticator.revoke_tokens_on_logout:
-                self.clear_tokens(user)
-            self.clear_login_cookie()
         if self.authenticator.logout_redirect_url:
+            await self.default_handle_logout()
+            await self.handle_logout()
             self.redirect(self.authenticator.logout_redirect_url)
         else:
-            super().get()
+            await super().get()
 
-    async def clear_tokens(self, user):
-        if not self.authenticator.revoke_tokens_on_logout:
-            return
+    async def handle_logout(self):
+        if self.current_user and self.authenticator.revoke_tokens_on_logout:
+                await self.clear_tokens(self.current_user)
 
+    async def clear_tokens(self, user):
         state = await user.get_auth_state()
         if state:
             self.authenticator.revoke_service_tokens(state.get('tokens'))
             self.log.info('Logout: Revoked tokens for user "{}" services: {}'
                           .format(user.name, ','.join(state['tokens'].keys())))
             state['tokens'] = ''
-            user.save_auth_state(state)
+            await user.save_auth_state(state)
 
 
 class GlobusOAuthenticator(OAuthenticator):
diff --git a/oauthenticator/tests/test_globus.py b/oauthenticator/tests/test_globus.py
index 21e42b78..9ec28074 100644
--- a/oauthenticator/tests/test_globus.py
+++ b/oauthenticator/tests/test_globus.py
@@ -54,6 +54,22 @@ def globus_client(client):
     return client
 
 
+@fixture
+def mock_globus_user(mock_globus_sdk):
+    class User:
+        name = 'Wash'
+        state = {'tokens': mock_globus_sdk.by_resource_server}
+
+        @gen.coroutine
+        def get_auth_state(self):
+            return self.state
+
+        @gen.coroutine
+        def save_auth_state(self, state):
+            self.state = state
+    return User()
+
+
 async def test_globus(globus_client, mock_globus_sdk):
     authenticator = GlobusOAuthenticator()
     handler = globus_client.handler_for_user(user_model('wash'))
@@ -63,6 +79,14 @@ async def test_globus(globus_client, mock_globus_sdk):
     assert tokens == ['transfer.api.globus.org']
 
 
+async def test_globus_pre_spawn_start(mock_globus_user):
+    authenticator = GlobusOAuthenticator()
+    spawner = Mock()
+    spawner.environment = {}
+    await authenticator.pre_spawn_start(mock_globus_user, spawner)
+    assert 'GLOBUS_DATA' in spawner.environment
+
+
 async def test_allow_refresh_tokens(globus_client, mock_globus_sdk, monkeypatch):
     authenticator = GlobusOAuthenticator()
     # Sanity check, this field should be set to True
@@ -128,7 +152,7 @@ def test_revoke_tokens(monkeypatch):
     assert ConfidentialAppAuthClient.oauth2_revoke_token.called
 
 
-async def test_custom_logout(monkeypatch):
+async def test_custom_logout(monkeypatch, mock_globus_user):
     custom_logout_url = 'https://universityofindependence.edu/logout'
     authenticator = GlobusOAuthenticator()
     logout_handler = mock_handler(GlobusLogoutHandler,
@@ -139,25 +163,24 @@ async def test_custom_logout(monkeypatch):
         Mock()
     )
     logout_handler.clear_login_cookie = Mock()
-    logout_handler.get_current_user = Mock()
+    logout_handler.get_current_user = Mock(return_value=mock_globus_user)
+    logout_handler._jupyterhub_user = mock_globus_user
+    monkeypatch.setitem(logout_handler.settings, 'statsd', Mock())
+
+    # Sanity check: Ensure the logout handler and url are set on the hub
+    handlers = [handler for _, handler in authenticator.get_handlers(None)]
+    assert any([h == GlobusLogoutHandler for h in handlers])
+    assert authenticator.logout_url('http://myhost') == 'http://myhost/logout'
 
+    # Test the logout handler uses the custom URL
     authenticator.logout_redirect_url = custom_logout_url
     await logout_handler.get()
     logout_handler.redirect.assert_called_once_with(custom_logout_url)
     assert logout_handler.clear_login_cookie.called
 
 
-async def test_logout_revokes_tokens(monkeypatch):
+async def test_logout_revokes_tokens(monkeypatch, mock_globus_user):
 
-    class User:
-        @gen.coroutine
-        def get_auth_state(self):
-            return {'tokens': {}}
-
-        save_auth_state = Mock()
-        name = 'Wash'
-
-    user = User()
     authenticator = GlobusOAuthenticator()
     logout_handler = mock_handler(GlobusLogoutHandler,
                                   authenticator=authenticator)
@@ -166,10 +189,16 @@ def get_auth_state(self):
         'redirect',
         Mock()
     )
+    logout_handler.get_current_user = Mock(return_value=mock_globus_user)
+    logout_handler._jupyterhub_user = mock_globus_user
+    monkeypatch.setitem(logout_handler.settings, 'statsd', Mock())
+    monkeypatch.setitem(logout_handler.settings, 'login_url', '')
+
     logout_handler.clear_login_cookie = Mock()
     authenticator.revoke_service_tokens = Mock()
     authenticator.revoke_tokens_on_logout = True
 
-    await logout_handler.clear_tokens(user)
+    await logout_handler.get()
     assert authenticator.revoke_service_tokens.called
-    assert user.save_auth_state.called
+    auth_state = await mock_globus_user.get_auth_state()
+    assert auth_state == {'tokens': ''}