Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Userinfo #24

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ lib64
.mr.developer.cfg
parts/
pyvenv.cfg
.python-version
var/

# mxdev
Expand Down
8 changes: 8 additions & 0 deletions src/pas/plugins/oidc/configure.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@
handler=".setuphandlers.activate_challenge_plugin"
/>

<genericsetup:upgradeStep
title="Activate properties plugin"
profile="pas.plugins.oidc:default"
source="1001"
destination="1002"
handler=".setuphandlers.activate_properties_plugin"
/>

<utility
factory=".setuphandlers.HiddenProfiles"
name="pas.plugins.oidc-hiddenprofiles"
Expand Down
116 changes: 98 additions & 18 deletions src/pas/plugins/oidc/plugins.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from AccessControl import ClassSecurityInfo
from AccessControl.class_init import InitializeClass
from BTrees.OOBTree import OOBTree
from contextlib import contextmanager
from oic.oic import Client
from oic.oic.message import OpenIDSchema
Expand All @@ -11,8 +12,10 @@
from Products.CMFCore.utils import getToolByName
from Products.PluggableAuthService.interfaces.plugins import IAuthenticationPlugin
from Products.PluggableAuthService.interfaces.plugins import IChallengePlugin
from Products.PluggableAuthService.interfaces.plugins import IPropertiesPlugin
from Products.PluggableAuthService.interfaces.plugins import IUserAdderPlugin
from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin
from Products.PluggableAuthService.UserPropertySheet import UserPropertySheet
from Products.PluggableAuthService.utils import classImplements
from secrets import choice
from typing import List
Expand Down Expand Up @@ -46,6 +49,9 @@ class IOIDCPlugin(Interface):
""" """


_marker = object()


@implementer(IOIDCPlugin)
class OIDCPlugin(BasePlugin):
"""PAS Plugin OpenID Connect."""
Expand All @@ -63,11 +69,12 @@ class OIDCPlugin(BasePlugin):
create_user = True
create_groups = False
user_property_as_groupid = "groups"
scope = ("profile", "email", "phone")
scope = ("openid", "profile", "email", "phone")
use_pkce = False
use_deprecated_redirect_uri_for_logout = False
use_modified_openid_schema = False
user_property_as_userid = "sub"
userinfo_to_memberdata = ()

_properties = (
dict(id="issuer", type="string", mode="w", label="OIDC/Oauth2 Issuer"),
Expand Down Expand Up @@ -135,8 +142,20 @@ class OIDCPlugin(BasePlugin):
mode="w",
label="User info property used as userid, default 'sub'",
),
dict(
id="userinfo_to_memberdata",
type="lines",
mode="w",
label="Mapping from userinfo to memberdata property. "
"Format: userinfo_propname|memberdata_propname",
),
)

def __init__(self, id, title=None, **kw):
self._setId(id)
self.title = title
self._userdata_by_userid = OOBTree()

def rememberIdentity(self, userinfo):
if not isinstance(userinfo, (OpenIDSchema, dict)):
raise AssertionError(
Expand All @@ -146,7 +165,6 @@ def rememberIdentity(self, userinfo):
# this value is guaranteed to be unique per user, stable over time,
# and never re-used
user_id = userinfo[self.getProperty("user_property_as_userid") or "sub"]
# TODO: configurare userinfo/plone mapping
pas = self._getPAS()
if pas is None:
return
Expand Down Expand Up @@ -195,6 +213,9 @@ def rememberIdentity(self, userinfo):
# if time.time() > user.getProperty(LAST_UPDATE_USER_PROPERTY_KEY) + config.get(autoUpdateUserPropertiesIntervalKey, 0):
with safe_write(self.REQUEST):
self._updateUserProperties(user, userinfo)
elif user is not None:
with safe_write(self.REQUEST):
self._updateUserProperties(user, userinfo)

if self.getProperty("create_groups"):
groupid_property = self.getProperty("user_property_as_groupid")
Expand Down Expand Up @@ -234,23 +255,70 @@ def _updateUserProperties(self, user, userinfo):
This is utilised when first creating a user, and to update
their information when logging in again later.
"""
# TODO: modificare solo se ci sono dei cambiamenti sui dati ?
# TODO: mettere in config il mapping tra metadati che arrivano da oidc e properties su plone
# TODO: warning nel caso non vengono tornati dati dell'utente
userProps = {}
email = userinfo.get("email", "")
name = userinfo.get("name", "")
given_name = userinfo.get("given_name", "")
family_name = userinfo.get("family_name", "")
if email:
userProps["email"] = email
if given_name and family_name:
userProps["fullname"] = f"{given_name} {family_name}"
elif name and family_name:
userProps["fullname"] = f"{name} {family_name}"
userProps = self._get_all_userinfo_properties(userinfo)
# userProps[LAST_UPDATE_USER_PROPERTY_KEY] = time.time()
if userProps:
user.setProperties(**userProps)
if not hasattr(self, "_userdata_by_userid"):
self._userdata_by_userid = OOBTree()
if (
user.getId() not in self._userdata_by_userid
or self._userdata_by_userid[user.getId()]._properties != userProps
):
self._userdata_by_userid[user.getId()] = UserPropertySheet(
user.getId(), **userProps
)
if self.create_user:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With self.create_user we create a standard Plone user like you would get when using the Users and Groups control panel, right?
I doubt that it is then still needed to set self._userdata_by_userid.

Ah, but when create_user is False for a while, and only later switched to True, and we do not update self._userdata_by_userid, it may get outdated and not in sync with the standard member data.
We could remove existing data when create_user is True.
Hard to say for sure what is best without trying it. But should be okay like it is now.

user.setProperties(**userProps)

def _parse_userinfo_to_memberdata(self):
"""Parse the userinfo_to_memberdata property."""
result = []
for line in self.getProperty("userinfo_to_memberdata"):
line = line.strip()
if not line:
continue
if isinstance(line, bytes):
line = line.decode("utf-8")
if line.startswith("#"):
continue
pipes = line.count("|")
if pipes == 1:
userinfo_prop, member_prop = line.split("|")
else:
continue
member_prop = member_prop.strip()
if not member_prop:
continue
userinfo_prop = userinfo_prop.strip()
if not userinfo_prop:
continue
result.append((userinfo_prop, member_prop))
return result

def _get_all_userinfo_properties(self, userinfo):
"""Get all known properties from the userinfo.
Returns a dictionary.
"""
result = {}
logger.debug("userinfo %s", userinfo.items())
for userinfo_prop, member_prop in self._parse_userinfo_to_memberdata():
if userinfo_prop in userinfo:
value = userinfo[userinfo_prop]
result[member_prop] = value
# DEFAULTS
if "email" not in result and "email" in userinfo:
result["email"] = userinfo["email"]
if "fullname" not in result:
if "given_name" in userinfo and "family_name" in userinfo:
result["fullname"] = "{} {}".format(
userinfo["given_name"], userinfo["family_name"]
)
elif "name" in userinfo and "family_name" in userinfo:
result["fullname"] = "{} {}".format(
userinfo["name"], userinfo["family_name"]
)
logger.debug("User properties: %s", result)
return result

def _generatePassword(self):
"""Return a obfuscated password never used for login"""
Expand Down Expand Up @@ -334,6 +402,8 @@ def get_scopes(self):
return [safe_text(scope) for scope in scopes if scope]
return []

# pas_interfaces.plugins.IChallengePlugin
@security.private
def challenge(self, request, response):
"""Assert via the response that credentials will be gathered.

Expand All @@ -353,16 +423,26 @@ def challenge(self, request, response):
response.redirect(url, lock=1)
return True

# pas_interfaces.plugins.IPropertiesPlugin
@security.private
def getPropertiesForUser(self, user, request=None):
if hasattr(self, "_userdata_by_userid"):
propertysheet = self._userdata_by_userid.get(user.getId(), _marker)
if propertysheet is not _marker:
return propertysheet
return None


InitializeClass(OIDCPlugin)


classImplements(
OIDCPlugin,
IOIDCPlugin,
# IExtractionPlugin,
# IAuthenticationPlugin,
IChallengePlugin,
# IPropertiesPlugin,
IPropertiesPlugin,
# IRolesPlugin,
)

Expand Down
2 changes: 1 addition & 1 deletion src/pas/plugins/oidc/profiles/default/metadata.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<metadata>
<version>1001</version>
<version>1002</version>
</metadata>
9 changes: 6 additions & 3 deletions src/pas/plugins/oidc/setuphandlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ def post_install(context):
# Create plugin if it does not exist.
if PLUGIN_ID not in pas.objectIds():
plugin = OIDCPlugin(
id=PLUGIN_ID,
title="OpenID Connect",
)
plugin.id = PLUGIN_ID
pas._setObject(PLUGIN_ID, plugin)
logger.info("Created %s in acl_users.", PLUGIN_ID)
plugin = getattr(pas, PLUGIN_ID)
Expand All @@ -50,7 +50,7 @@ def post_install(context):
for info in plugins.listPluginTypeInfo():
interface_name = info["id"]
# If we support IPropertiesPlugin, it should be added here.
if interface_name in ("IChallengePlugin",):
if interface_name in ("IChallengePlugin", "IPropertiesPlugin"):
iface = plugins._getInterfaceFromName(interface_name)
plugins.movePluginsTop(iface, [PLUGIN_ID])
logger.info(f"Moved {PLUGIN_ID} to top of {interface_name}.")
Expand Down Expand Up @@ -87,6 +87,10 @@ def activate_challenge_plugin(context):
activate_plugin(context, "IChallengePlugin", move_to_top=True)


def activate_properties_plugin(context):
activate_plugin(context, "IPropertiesPlugin", move_to_top=True)


def uninstall(context):
"""Uninstall script"""
from pas.plugins.oidc.utils import PLUGIN_ID
Expand All @@ -96,7 +100,6 @@ def uninstall(context):
# Remove plugin if it exists.
if PLUGIN_ID not in pas.objectIds():
return
from pas.plugins.oidc.plugins import OIDCPlugin

plugin = getattr(pas, PLUGIN_ID)
if not isinstance(plugin, OIDCPlugin):
Expand Down
2 changes: 1 addition & 1 deletion tests/setup/test_setup_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def test_addon_installed(self, installer):

def test_latest_version(self, profile_last_version):
"""Test latest version of default profile."""
assert profile_last_version(f"{PACKAGE_NAME}:default") == "1001"
assert profile_last_version(f"{PACKAGE_NAME}:default") == "1002"

def test_browserlayer(self, browser_layers):
"""Test that IPasPluginsOidcLayer is registered."""
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/test_utils_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def test_authorization_flow_args(self, session_factory):
assert isinstance(result, dict)
assert result["client_id"] == self.plugin.client_id
assert result["response_type"] == "code"
assert result["scope"] == ["profile", "email", "phone"]
assert result["scope"] == ["openid", "profile", "email", "phone"]
assert isinstance(result["state"], str)
assert isinstance(result["nonce"], str)
assert "code_challenge" not in result
Expand Down