Merge pull request #108 from zopefoundation/zmi_logout#107

Improve the PAS override for Zope's `manage_zmi_logout`
zopefoundation · Jan 4, 2022 · 44ac67f · 44ac67f
2 parents 2a678b5 + 1e92e50
commit 44ac67f
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 25 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -4,6 +4,9 @@ Change Log
 2.6.5 (unreleased)
 ------------------
 
+- Improve the PAS override for Zope's ``manage_zmi_logout`` (`#107
+  <https://github.com/zopefoundation/Products.PluggableAuthService/issues/107>`_)
+
 - Fixed deprecation warning for ``AccessControl.AuthEncoding``.
 
 - Pass the login name as ``updateCredentials``'s ``login`` parameter

diff --git a/src/Products/PluggableAuthService/__init__.py b/src/Products/PluggableAuthService/__init__.py
@@ -23,7 +23,6 @@
 
 from AccessControl.Permissions import manage_users as ManageUsers
 from App.Management import Navigation
-from zExceptions import Unauthorized
 
 from Products.GenericSetup import BASE
 from Products.GenericSetup import profile_registry
@@ -73,34 +72,30 @@
 
 
 # monkey patch Zope to cause zmi logout to be PAS-aware
+zope_manage_zmi_logout = Navigation.manage_zmi_logout
+
+
 def manage_zmi_logout(self, REQUEST, RESPONSE):
     """Logout current user"""
+    # keep despite doubt that it will work
     p = getattr(REQUEST, '_logout_path', None)
     if p is not None:
         return self.restrictedTraverse(p)
 
-    acl_users = self.acl_users
-    realm = RESPONSE.realm
-    RESPONSE.setHeader('WWW-Authenticate', 'basic realm="%s"' % realm, 1)
+    from AccessControl import getSecurityManager
+    user = getSecurityManager().getUser()
+    if user is None or 'Authenticated' not in user.getRoles():
+        return 'You are not/no longer logged in'
+
+    acl_users = user.aq_parent
 
     if IPluggableAuthService.providedBy(acl_users):
-        acl_users.resetCredentials(REQUEST, RESPONSE)
+        return acl_users.logout(REQUEST)
     else:
-        raise Unauthorized('<p>You have been logged out.</p>')
-
-    RESPONSE.setStatus(401)
-    RESPONSE.setBody("""<html>
-<head><title>Logout</title></head>
-<body>
-<p>
-You have been logged out.
-</p>
-</body>
-</html>""")
+        return zope_manage_zmi_logout(self, REQUEST, RESPONSE)
 
 
 Navigation.manage_zmi_logout = manage_zmi_logout
-del manage_zmi_logout
 
 
 def initialize(context):

diff --git a/src/Products/PluggableAuthService/tests/test_UserFolder.py b/src/Products/PluggableAuthService/tests/test_UserFolder.py
@@ -254,14 +254,6 @@ def test__doAddUser_with_preencrypted_passwords(self):
 
         self.assertEqual(uid_and_info, (USER_ID, USER_ID))
 
-    def test_manage_zmi_logout(self):
-        request = self.app.REQUEST
-        response = request.RESPONSE
-        self.folder.manage_zmi_logout(request, response)
-        self.assertEqual(response.status, 401)
-        self.assertEqual(response.headers.get('WWW-Authenticate'),
-                         'basic realm="%s"' % response.realm)
-
 
 class UserTests(pastc.PASTestCase):
 

diff --git a/src/Products/PluggableAuthService/tests/test_zmi_logout.py b/src/Products/PluggableAuthService/tests/test_zmi_logout.py
@@ -0,0 +1,77 @@
+##############################################################################
+#
+# Copyright (c) 2022 Zope Foundation and Contributors
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this
+# distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##############################################################################
+
+from Testing.ZopeTestCase import ZopeTestCase
+from zope.interface import directlyProvides
+
+from ..interfaces.plugins import ICredentialsResetPlugin
+from ..interfaces.plugins import ICredentialsUpdatePlugin
+from ..interfaces.plugins import IExtractionPlugin
+from .pastc import PASTestCase
+from .pastc import user_name
+from .pastc import user_password
+from .test_PluggableAuthService import DummyCredentialsStore
+
+
+class PasMixin(object):
+    def _setupUserFolder(self):
+        super(PasMixin, self)._setupUserFolder()
+        pas = self.folder.acl_users
+        plugins = pas.plugins
+        creds_store = DummyCredentialsStore('creds')
+        directlyProvides(creds_store, IExtractionPlugin,
+                         ICredentialsUpdatePlugin, ICredentialsResetPlugin)
+        pas._setObject('creds', creds_store)
+        plugins.deactivatePlugin(IExtractionPlugin, 'http_auth')
+        plugins.activatePlugin(IExtractionPlugin, 'creds')
+        plugins.activatePlugin(ICredentialsUpdatePlugin, 'creds')
+        plugins.activatePlugin(ICredentialsResetPlugin, 'creds')
+        request = self.folder.REQUEST
+        request["login"] = user_name
+        creds_store.updateCredentials(
+            request, request.response, user_name, user_password)
+
+    def verify_logout(self):
+        folder = self.folder
+        request = folder.REQUEST
+        return not folder.acl_users.creds.extractCredentials(request)
+
+
+class ZopeMixin(object):
+    def verify_logout(self):
+        response = self.folder.REQUEST.response
+        return response.getStatus() == 401
+
+
+class Tests(object):
+    def test_logout(self):
+        folder = self.folder
+        request = folder.REQUEST
+        folder.manage_zmi_logout(request, request.response)
+        self.assertTrue(self.verify_logout())
+
+
+class PasLogoutTests(PasMixin, Tests, PASTestCase):
+    def test_not_logged_in(self):
+        from AccessControl.SecurityManagement import noSecurityManager
+        noSecurityManager()
+        folder = self.folder
+        request = folder.REQUEST
+        self.assertEqual(
+            folder.manage_zmi_logout(request, request.response),
+            "You are not/no longer logged in")
+
+
+class ZopeLogoutTests(ZopeMixin, Tests, ZopeTestCase):
+    pass