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 e04a4c9 commit ba3561d
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 23 deletions.
32 changes: 32 additions & 0 deletions src/plone/restapi/pas/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""
A JWT token authentication plugin for PluggableAuthService.
"""

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


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)
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
24 changes: 6 additions & 18 deletions src/plone/restapi/setuphandlers.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
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.interfaces import INonInstallable
from Products.PluggableAuthService.interfaces.authservice import (
IPluggableAuthService,
) # noqa: E501
from zope.component.hooks import getSite
from zope.interface import implementer

Expand All @@ -31,14 +26,12 @@ def getNonInstallableProducts(self): # pragma: no cover


def install_pas_plugin(context):
uf_parent = aq_inner(context)
while True:
uf = getToolByName(uf_parent, "acl_users", default=None)
"""
Install the JWT token PAS plugin in every PAS acl_users here and above.
# 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
Usually this means it is installed into Plone and into the Zope root.
"""
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 All @@ -54,11 +47,6 @@ def install_pas_plugin(context):
],
)

# 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
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>
39 changes: 39 additions & 0 deletions src/plone/restapi/upgrades/to0007.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""
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):
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 ba3561d

Please sign in to comment.