Skip to content

Commit

Permalink
fix(auth): Missing JWT plugin activation upgrade
Browse files Browse the repository at this point in the history
Because token generation has been moved into `updateCredentials(...)` [we need an
upgrade step](#1303 (comment))
that enables the JWT token plugin for that PAS plugin interface on existing
installations in order for authentication to work as before.  Also fixes existing
plugins outside of a Plone portal that have been configured to use the keyring.

I tested this locally by:
1. erasing my local data (ZODB)
2. checking out `master` in the `plone/volto` repo
3. running buildout, including `plonesite` in the API to re-create the portal
4. adding a test user in the Plone portal through the Volto UI
5. add `mr.developer` sources and checkouts in the API buildout
6. disable `plonesite` in the API buildout
7. run buildout to update the code to the PR branches
8. test all the upgrade error conditions around login logout
9. run the `v0006 -> v0007` upgrade step for `plone.restapi:default`
10. confirm all the upgrade error conditions around login logout have been resolved

Not that this doesn't address the issue of [existing Zope root `/acl_users/` cookie
login set up](#1304 (comment)).
  • Loading branch information
rpatterson committed Jan 4, 2022
1 parent 9fbf3a5 commit 2208db5
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 32 deletions.
27 changes: 27 additions & 0 deletions src/plone/restapi/pas/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
"""

from App import Management
from Products.CMFCore.utils import getToolByName
from Products.CMFPlone import interfaces as plone_ifaces
from Products import PluggableAuthService # noqa, Ensure PAS patch in place
from Products.PluggableAuthService.interfaces import authservice as authservice_ifaces

import Acquisition


_orig_manage_zmi_logout = Management.Navigation.manage_zmi_logout
Expand All @@ -20,3 +25,25 @@ def manage_zmi_logout(self, REQUEST, RESPONSE):
# Undo the HTTP `Authorization: Basic ...` assumptions
del RESPONSE.headers["WWW-Authenticate"]
RESPONSE.setStatus(200)


def iter_ancestor_pas(context):
"""
Walk up the ZODB OFS returning Pluggableauthservice `./acl_users/` for each level.
"""
uf_parent = Acquisition.aq_inner(context)
while True:
is_plone_site = plone_ifaces.IPloneSiteRoot.providedBy(uf_parent)
uf = getToolByName(uf_parent, "acl_users", default=None)

# Skip ancestor contexts to which we don't/can't apply
if uf is None or not authservice_ifaces.IPluggableAuthService.providedBy(uf):
uf_parent = Acquisition.aq_parent(uf_parent)
continue

yield uf, is_plone_site

# Go up one more level
if uf_parent is uf_parent.getPhysicalRoot():
break
uf_parent = Acquisition.aq_parent(uf_parent)
33 changes: 28 additions & 5 deletions src/plone/restapi/pas/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
from Products.PluggableAuthService.interfaces.plugins import ICredentialsUpdatePlugin
from Products.PluggableAuthService.interfaces.plugins import ICredentialsResetPlugin
from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin
from zope.component import getUtility
from zope import component
from zope.interface import implementer

import jwt
import logging
import time

logger = logging.getLogger(__name__)


manage_addJWTAuthenticationPlugin = PageTemplateFile(
"add_plugin", globals(), __name__="manage_addJWTAuthenticationPlugin"
Expand Down Expand Up @@ -209,9 +212,29 @@ def manage_updateConfig(self, REQUEST):
% (self.absolute_url(), "Configuration+updated.")
)

def _decode_token(self, token, verify=True):
def _use_keyring(self):
"""
Determine whether to use the keyring for this plugin, logging error conditions.
"""
use_keyring = False
keyring_manager = None
if self.use_keyring:
manager = getUtility(IKeyManager)
keyring_manager = component.queryUtility(IKeyManager)
if keyring_manager is None:
logger.error(
"Plugin configured to use keyring but no utility registered: %s",
"/".join(self.getPhysicalPath()),
)
else:
use_keyring = True
return use_keyring, keyring_manager

def _decode_token(self, token, verify=True):
"""
Decode the given JWT token using the keyring if configured to do so.
"""
use_keyring, manager = self._use_keyring()
if use_keyring:
for secret in manager["_system"]:
if secret is None:
continue
Expand All @@ -230,8 +253,8 @@ def _jwt_decode(self, token, secret, verify=True):
pass

def _signing_secret(self):
if self.use_keyring:
manager = getUtility(IKeyManager)
use_keyring, manager = self._use_keyring()
if use_keyring:
return manager.secret() + self._path()
if not self._secret:
self._secret = GenerateSecret()
Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/profiles/default/metadata.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0"?>
<metadata>
<version>0006</version>
<version>0007</version>
</metadata>
22 changes: 21 additions & 1 deletion src/plone/restapi/services/auth/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,27 @@ def reply(self):
)
login_view._post_login()

return {"token": self.request[plugin.cookie_name]}
response = {}
if plugin.cookie_name in self.request:
response["token"] = self.request[plugin.cookie_name]
else:
self.request.response.setStatus(501)
message = (
"JWT authentication token not created, plugin probably not activated "
"for `ICredentialsUpdatePlugin`"
)
logger.error(
"%s: %s",
message,
"/".join(plugin.getPhysicalPath()),
)
return dict(
error=dict(
type="Login failed",
message=message,
)
)
return response

def _find_userfolder(self, userid):
"""Try to find a user folder that contains a user with the given
Expand Down
23 changes: 2 additions & 21 deletions src/plone/restapi/setuphandlers.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
from Acquisition import aq_inner
from Acquisition import aq_parent
from plone.restapi import pas
from plone.restapi.pas.plugin import JWTAuthenticationPlugin
from Products.CMFCore.utils import getToolByName
from Products.CMFPlone import interfaces as plone_ifaces
from Products.CMFPlone.interfaces import INonInstallable
from Products.PluggableAuthService.interfaces import plugins as plugins_ifaces
from Products.PluggableAuthService.interfaces.authservice import (
IPluggableAuthService,
) # noqa: E501
from Products.PluggableAuthService.plugins import CookieAuthHelper
from Products.PluggableAuthService.plugins import HTTPBasicAuthHelper
from zope.component.hooks import getSite
Expand Down Expand Up @@ -40,15 +34,7 @@ def install_pas_plugin(context):
Usually this means it is installed into Plone and into the Zope root.
"""
uf_parent = aq_inner(context)
while True:
is_plone_site = plone_ifaces.IPloneSiteRoot.providedBy(uf_parent)
uf = getToolByName(uf_parent, "acl_users", default=None)

# Skip ancestor contexts to which we don't/can't apply
if uf is None or not IPluggableAuthService.providedBy(uf):
uf_parent = aq_parent(uf_parent)
continue
for uf, is_plone_site in pas.iter_ancestor_pas(context):

# Add the API token plugin if not already installed at this level
if "jwt_auth" not in uf:
Expand Down Expand Up @@ -111,11 +97,6 @@ def install_pas_plugin(context):
basic_plugin.id,
)

# Go up one more level
if uf_parent is uf_parent.getPhysicalRoot():
break
uf_parent = aq_parent(uf_parent)


def post_install_default(context):
"""Post install of default profile"""
Expand Down
9 changes: 6 additions & 3 deletions src/plone/restapi/tests/test_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,15 @@ def _get_upgrade_info(self):
# Set need upgrade state
self.ps.setLastVersionForProfile("plone.restapi:default", "0002")
transaction.commit()
# FIXME: At least the `newVersion` should be extracted from
# `./profiles/default/metadata.xml` so that this test isn't constantly changing
# for unrelated code changes.
self.assertEqual(
{
"available": True,
"hasProfile": True,
"installedVersion": "0002",
"newVersion": "0006",
"newVersion": "0007",
"required": True,
},
_get_upgrade_info(self),
Expand All @@ -135,8 +138,8 @@ def _get_upgrade_info(self):
{
"available": False,
"hasProfile": True,
"installedVersion": "0006",
"newVersion": "0006",
"installedVersion": "0007",
"newVersion": "0007",
"required": False,
},
_get_upgrade_info(self),
Expand Down
3 changes: 2 additions & 1 deletion src/plone/restapi/tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ def test_login_with_zope_user_fails_without_pas_plugin(self):
res = service.reply()
self.assertIn("error", res)
self.assertEqual(
"JWT authentication plugin not installed.", res["error"]["message"]
"jwt authentication plugin not installed",
res["error"]["message"].lower(),
)
self.assertNotIn("token", res)

Expand Down
11 changes: 11 additions & 0 deletions src/plone/restapi/upgrades/configure.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,15 @@
handler="plone.restapi.upgrades.to0006.rename_iface_to_name_in_blocks_behavior"
/>

<genericsetup:upgradeStep
title="Enable new PAS plugin interfaces"
description="After correcting/completing the PAS plugin interfaces, those
interfaces need to be enabled for existing functionality to continue
working."
profile="plone.restapi:default"
source="0006"
destination="0007"
handler="plone.restapi.upgrades.to0007.enable_new_pas_plugin_interfaces"
/>

</configure>
45 changes: 45 additions & 0 deletions src/plone/restapi/upgrades/to0007.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
"""
GenericSetup profile upgrades from version 0006 to 0007.
"""

from plone.restapi import pas
from plone.restapi.pas import plugin
from Products.CMFCore.utils import getToolByName
from Products.PluggableAuthService.interfaces import plugins as plugins_ifaces

import logging

logger = logging.getLogger(__name__)


def enable_new_pas_plugin_interfaces(context):
"""
Enable new PAS plugin interfaces.
After correcting/completing the PAS plugin interfaces, those interfaces need to be
enabled for existing functionality to continue working.
"""
portal = getToolByName(context, "portal_url").getPortalObject()
for uf, is_plone_site in pas.iter_ancestor_pas(portal):
for jwt_plugin in uf.objectValues(plugin.JWTAuthenticationPlugin.meta_type):
if not is_plone_site and jwt_plugin.use_keyring:
logger.info(
"Disabling keyring for plugin outside of Plone: %s",
"/".join(jwt_plugin.getPhysicalPath())
)
jwt_plugin.use_keyring = False
for new_iface in (
plugins_ifaces.ICredentialsUpdatePlugin,
plugins_ifaces.ICredentialsResetPlugin,
):
active_plugin_ids = [
active_plugin_id for active_plugin_id, _ in
uf.plugins.listPlugins(new_iface)
]
if jwt_plugin.id not in active_plugin_ids:
logger.info(
"Activating PAS interface %s: %s",
new_iface.__name__,
"/".join(jwt_plugin.getPhysicalPath())
)
uf.plugins.activatePlugin(new_iface, jwt_plugin.id)

0 comments on commit 2208db5

Please sign in to comment.