From da2a7aacd5bfe9eddf45a016a4299fd90bc6f468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Tue, 29 Sep 2026 19:57:31 -0300 Subject: [PATCH 1/6] Add pas.plugins.identity with Person as the user content type Install pas.plugins.identity with the dependencies profile, and represent each user account as a Person in /users. Groups stay in source_groups, and Person objects that are not accounts, like team pages, stay plain content. - Patch pas.plugins.identity 1.0.0a11 to use Person instead of the hardcoded UserProfile, and no group content type - Person schema extends IUserContent; Person gets the user attributes (userid, login, fullname, emails, email, verified_emails) and an optional email addresses behavior - A Person created with a login is marked as a user profile - Disallow UserProfile and UserGroup globally - Upgrade step 20260929001 installs the add-on and creates a Person for each user in source_users - New identity-keycloak, identity-oidc, identity-github and identity-google site creation providers, side by side with the pas.plugins.oidc and pas.plugins.authomatic ones Upstream: https://github.com/collective/pas-plugins-identity/issues/127 Refs: https://gitlab.kitconcept.io/kitconcept/distribution-kitconcept-intranet/-/issues/604 --- backend/news/604.feature | 1 + backend/pyproject.toml | 1 + backend/src/kitconcept/core/__init__.py | 3 + backend/src/kitconcept/core/_types.py | 21 ++- .../kitconcept/core/behaviors/configure.zcml | 7 + .../core/behaviors/person_emails.py | 52 +++++++ backend/src/kitconcept/core/configure.zcml | 1 + backend/src/kitconcept/core/content/person.py | 144 ++++++++++++++++++ backend/src/kitconcept/core/dependencies.zcml | 1 + backend/src/kitconcept/core/factory.py | 3 + .../src/kitconcept/core/patches/identity.py | 65 ++++++++ .../core/profiles/base/metadata.xml | 2 +- .../core/profiles/dependencies/metadata.xml | 1 + ...ntrolpanel.interfaces.IProfileSettings.xml | 27 ++++ .../profiles/dependencies/types/Person.xml | 4 +- .../types/PrincipalsContainer.xml | 13 ++ .../profiles/dependencies/types/UserGroup.xml | 9 ++ .../dependencies/types/UserProfile.xml | 9 ++ .../kitconcept/core/subscribers/__init__.py | 32 ++++ .../core/subscribers/configure.zcml | 10 ++ .../testing/distributions/testing/schema.json | 6 +- .../kitconcept/core/upgrades/configure.zcml | 1 + .../core/upgrades/v20260929001/__init__.py | 34 +++++ .../core/upgrades/v20260929001/configure.zcml | 21 +++ .../core/utils/authentication/__init__.py | 5 + .../core/utils/authentication/identity.py | 89 +++++++++++ backend/src/kitconcept/core/utils/scripts.py | 31 ++++ backend/src/kitconcept/core/utils/users.py | 53 +++++++ backend/tests/conftest.py | 2 +- backend/tests/content_types/test_ct_person.py | 2 + .../tests/creation/authentication/conftest.py | 10 ++ .../identity_github/__init__.py | 0 .../identity_github/conftest.py | 19 +++ .../test_auth_identity_github_creation.py | 37 +++++ .../identity_google/__init__.py | 0 .../identity_google/conftest.py | 20 +++ .../test_auth_identity_google_creation.py | 30 ++++ .../identity_keycloak/__init__.py | 0 .../identity_keycloak/conftest.py | 23 +++ .../test_auth_identity_keycloak_creation.py | 57 +++++++ .../authentication/identity_oidc/__init__.py | 0 .../authentication/identity_oidc/conftest.py | 21 +++ .../test_auth_identity_oidc_creation.py | 36 +++++ backend/tests/identity/__init__.py | 0 backend/tests/identity/test_identity_setup.py | 73 +++++++++ backend/tests/identity/test_identity_users.py | 134 ++++++++++++++++ backend/tests/setup/test_upgrades.py | 34 +++++ .../utils/authentication/test_identity.py | 82 ++++++++++ backend/tests/utils/test_utils_scripts.py | 17 +++ backend/uv.lock | 47 ++++++ 50 files changed, 1285 insertions(+), 5 deletions(-) create mode 100644 backend/news/604.feature create mode 100644 backend/src/kitconcept/core/behaviors/person_emails.py create mode 100644 backend/src/kitconcept/core/content/person.py create mode 100644 backend/src/kitconcept/core/patches/identity.py create mode 100644 backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IProfileSettings.xml create mode 100644 backend/src/kitconcept/core/profiles/dependencies/types/PrincipalsContainer.xml create mode 100644 backend/src/kitconcept/core/profiles/dependencies/types/UserGroup.xml create mode 100644 backend/src/kitconcept/core/profiles/dependencies/types/UserProfile.xml create mode 100644 backend/src/kitconcept/core/subscribers/__init__.py create mode 100644 backend/src/kitconcept/core/subscribers/configure.zcml create mode 100644 backend/src/kitconcept/core/upgrades/v20260929001/__init__.py create mode 100644 backend/src/kitconcept/core/upgrades/v20260929001/configure.zcml create mode 100644 backend/src/kitconcept/core/utils/authentication/identity.py create mode 100644 backend/src/kitconcept/core/utils/users.py create mode 100644 backend/tests/creation/authentication/identity_github/__init__.py create mode 100644 backend/tests/creation/authentication/identity_github/conftest.py create mode 100644 backend/tests/creation/authentication/identity_github/test_auth_identity_github_creation.py create mode 100644 backend/tests/creation/authentication/identity_google/__init__.py create mode 100644 backend/tests/creation/authentication/identity_google/conftest.py create mode 100644 backend/tests/creation/authentication/identity_google/test_auth_identity_google_creation.py create mode 100644 backend/tests/creation/authentication/identity_keycloak/__init__.py create mode 100644 backend/tests/creation/authentication/identity_keycloak/conftest.py create mode 100644 backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py create mode 100644 backend/tests/creation/authentication/identity_oidc/__init__.py create mode 100644 backend/tests/creation/authentication/identity_oidc/conftest.py create mode 100644 backend/tests/creation/authentication/identity_oidc/test_auth_identity_oidc_creation.py create mode 100644 backend/tests/identity/__init__.py create mode 100644 backend/tests/identity/test_identity_setup.py create mode 100644 backend/tests/identity/test_identity_users.py create mode 100644 backend/tests/utils/authentication/test_identity.py diff --git a/backend/news/604.feature b/backend/news/604.feature new file mode 100644 index 00000000..3284042f --- /dev/null +++ b/backend/news/604.feature @@ -0,0 +1 @@ +Added `pas.plugins.identity` as an always installed dependency, with `Person` as the user content type: each user account is a `Person` in the new `/users` folder, while other `Person` objects stay plain content. An upgrade step installs the add-on on existing sites and creates a `Person` for each user in `source_users`. Site creation gains the `identity-keycloak`, `identity-oidc`, `identity-github` and `identity-google` authentication providers, configured with `identity-*` answers or `SITE_AUTHENTICATION_IDENTITY-*` environment variables, side by side with the existing `pas.plugins.oidc` and `pas.plugins.authomatic` providers. @ericof diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 7ce5fc80..05de6034 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -34,6 +34,7 @@ dependencies = [ "collective.honeypot", "collective.person==1.0.0b6", "pas.plugins.authomatic==2.0.0", + "pas.plugins.identity==1.0.0a11", "pas.plugins.keycloakgroups==1.0.0b1", "pas.plugins.oidc==2.0.0", ] diff --git a/backend/src/kitconcept/core/__init__.py b/backend/src/kitconcept/core/__init__.py index 5b8eb057..e69f76d8 100644 --- a/backend/src/kitconcept/core/__init__.py +++ b/backend/src/kitconcept/core/__init__.py @@ -19,7 +19,10 @@ def initialize(context): + from kitconcept.core.patches import identity from kitconcept.core.tools import migration + + identity.apply_patch(logger=logger) from Products.CMFPlone.utils import ToolInit tools = (migration.MigrationTool,) diff --git a/backend/src/kitconcept/core/_types.py b/backend/src/kitconcept/core/_types.py index 9af5c12c..1d28637d 100644 --- a/backend/src/kitconcept/core/_types.py +++ b/backend/src/kitconcept/core/_types.py @@ -1,5 +1,6 @@ from dataclasses import dataclass from typing import Literal +from typing import NotRequired from typing import overload from typing import Protocol from typing import TypedDict @@ -60,7 +61,23 @@ class DistributionInfo(TypedDict): AnswersAuthomatic = AnswersAuthomaticGoogle | AnswersAuthomaticGitHub -AuthAnswers = AnswersKeycloak | AnswersAuthomatic | AnswersOIDC +AnswersIdentity = TypedDict( + "AnswersIdentity", + { + "provider": Literal[ + "identity-keycloak", "identity-oidc", "identity-github", "identity-google" + ], + "identity-server_url": NotRequired[str], + "identity-realm_name": NotRequired[str], + "identity-issuer": NotRequired[str], + "identity-client_id": str, + "identity-client_secret": str, + "identity-site-url": NotRequired[str], + "identity-scope": NotRequired[list[str]], + }, +) + +AuthAnswers = AnswersKeycloak | AnswersAuthomatic | AnswersOIDC | AnswersIdentity class AuthSetupHandler(Protocol): @@ -70,6 +87,8 @@ def __call__(self, answers: AnswersKeycloak) -> None: ... def __call__(self, answers: AnswersOIDC) -> None: ... @overload def __call__(self, answers: AnswersAuthomatic) -> None: ... + @overload + def __call__(self, answers: AnswersIdentity) -> None: ... def __call__(self, answers: AuthAnswers) -> None: ... diff --git a/backend/src/kitconcept/core/behaviors/configure.zcml b/backend/src/kitconcept/core/behaviors/configure.zcml index 2fa9886a..3fd432de 100644 --- a/backend/src/kitconcept/core/behaviors/configure.zcml +++ b/backend/src/kitconcept/core/behaviors/configure.zcml @@ -40,4 +40,11 @@ provides=".person_image.IPersonImage" /> + + diff --git a/backend/src/kitconcept/core/behaviors/person_emails.py b/backend/src/kitconcept/core/behaviors/person_emails.py new file mode 100644 index 00000000..99443655 --- /dev/null +++ b/backend/src/kitconcept/core/behaviors/person_emails.py @@ -0,0 +1,52 @@ +from kitconcept.core import _ +from plone.autoform.directives import read_permission +from plone.autoform.directives import write_permission +from plone.autoform.interfaces import IFormFieldProvider +from plone.schema import Email +from plone.supermodel import model +from zope import schema +from zope.interface import provider + + +@provider(IFormFieldProvider) +class IPersonEmails(model.Schema): + """Email addresses of a Person that represents a user. + + Same fields as the ``pas.plugins.identity.email_addresses`` behavior, but + optional: a Person that is not a user does not need an address. + """ + + model.fieldset( + "email", + label=_("Email"), + fields=["emails", "email"], + ) + + emails = schema.Tuple( + title=_("label_person_emails", default="Email addresses"), + description=_( + "help_person_emails", + default="The addresses this person uses to sign in, most preferred first.", + ), + value_type=Email(title=_("Email")), + required=False, + missing_value=(), + default=(), + ) + + email = Email( + title=_("Email"), + description=_( + "help_person_email", + default="The first verified address above, or the first one when " + "none is verified.", + ), + required=False, + readonly=True, + ) + + write_permission(emails="pas.plugins.identity.content.edit") + read_permission( + emails="pas.plugins.identity.content.viewpii", + email="pas.plugins.identity.content.viewpii", + ) diff --git a/backend/src/kitconcept/core/configure.zcml b/backend/src/kitconcept/core/configure.zcml index 8467d1b9..630fa32c 100644 --- a/backend/src/kitconcept/core/configure.zcml +++ b/backend/src/kitconcept/core/configure.zcml @@ -27,6 +27,7 @@ + tuple[str, str]: + """Split a full name into first and last name. + + :param fullname: The full name. + :returns: First name and last name, empty when there is no full name. + """ + parts = fullname.strip().split(maxsplit=1) + if not parts: + return "", "" + return parts[0], parts[1] if len(parts) > 1 else "" + + +# Attributes copied into collective.person's Person class +USER_ATTRIBUTES = ( + "login", + "email_confirmation_pending", + "userid", + "fullname", + "emails", + "email", + "verified_emails", +) + + +class PersonUserMixin: + """User attributes of a Person, mirroring pas.plugins.identity UserProfile.""" + + if TYPE_CHECKING: + title: str + first_name: str + last_name: str + + def getId(self) -> str: ... + + login = "" + + email_confirmation_pending = False + + @property + def userid(self) -> str: + """Return the userid, which is the object id. + + :returns: The userid. + """ + return self.getId() + + @userid.setter + def userid(self, value: str) -> None: + """Ignore writes: the userid is derived from the object id. + + :param value: The value being written. + """ + if value and value != self.getId(): + logger.warning( + f"Ignoring an attempt to set userid to {value!r} on {self.getId()!r}" + ) + + @property + def fullname(self) -> str: + """Return the full name, which is the Person title. + + :returns: The full name. + """ + return self.title or "" + + @fullname.setter + def fullname(self, value: str) -> None: + """Store a full name in the first and last name fields. + + :param value: The full name. An empty value is ignored. + """ + first_name, last_name = split_fullname(value or "") + if not first_name: + return + self.first_name = first_name + self.last_name = last_name + + @property + def emails(self) -> tuple[str, ...]: + """Return the addresses of this person, most preferred first. + + :returns: The addresses. + """ + return getattr(self, "_emails", ()) + + @emails.setter + def emails(self, value: object) -> None: + """Store the addresses, normalized and de-duplicated. + + :param value: The addresses. + """ + self._emails = clean(value) + + @property + def email(self) -> str: + """Return the address that stands for this person. + + :returns: The first verified address, else the first address. + """ + return preferred_address(self.getId(), self.emails) + + @email.setter + def email(self, value: str) -> None: + """Move an address to the front of :attr:`emails`. + + :param value: The address. + """ + address = normalize(value) + if not address: + return + self.emails = (address, *(a for a in self.emails if a != address)) + + @property + def verified_emails(self) -> tuple[str, ...]: + """Return the addresses this site verified for this person. + + :returns: The verified addresses. + """ + return verified_addresses(self.getId(), self.emails) diff --git a/backend/src/kitconcept/core/dependencies.zcml b/backend/src/kitconcept/core/dependencies.zcml index 59f0c631..3589b8ac 100644 --- a/backend/src/kitconcept/core/dependencies.zcml +++ b/backend/src/kitconcept/core/dependencies.zcml @@ -13,6 +13,7 @@ + diff --git a/backend/src/kitconcept/core/factory.py b/backend/src/kitconcept/core/factory.py index 0cd572c5..6a84826f 100644 --- a/backend/src/kitconcept/core/factory.py +++ b/backend/src/kitconcept/core/factory.py @@ -79,6 +79,9 @@ "souper.plone", "collective.volto.otp", "pas.plugins.authomatic", + "pas.plugins.identity", + # Identity provider (server) layer of pas.plugins.identity, not offered yet + "pas.plugins.identity.server", "pas.plugins.oidc", "pas.plugins.keycloakgroups", ] diff --git a/backend/src/kitconcept/core/patches/identity.py b/backend/src/kitconcept/core/patches/identity.py new file mode 100644 index 00000000..dee7f772 --- /dev/null +++ b/backend/src/kitconcept/core/patches/identity.py @@ -0,0 +1,65 @@ +"""Use ``Person`` as the user content type of ``pas.plugins.identity``. + +``pas.plugins.identity`` hardcodes ``UserProfile`` as the portal type of a +user in several places (``PROFILE_PORTAL_TYPE``), and resets the +``user_content_type`` and ``group_content_type`` registry records to its own +types whenever the container settings change. Until the add-on reads the type +from the registry, point every copy of those constants at ``Person``, and at no +group type, so groups stay in ``source_groups``. + +The ``Person`` class also gets the attributes the identity catalog and the +PAS plugins read from a ``UserProfile``. +""" + +from importlib import import_module +from logging import Logger + + +PROFILE_PORTAL_TYPE = "Person" + +# Empty: groups are not content, they stay in source_groups +GROUP_PORTAL_TYPE = "" + +# Modules holding a copy of PROFILE_PORTAL_TYPE +PROFILE_TYPE_MODULES = ( + "pas.plugins.identity.core.catalog", + "pas.plugins.identity.core.completeness", + "pas.plugins.identity.core.doctor", + "pas.plugins.identity.core.profiles", + "pas.plugins.identity.core.subscribers.principals", + "pas.plugins.identity.core.pas.profile", + "pas.plugins.identity.core.services.types", + "pas.plugins.identity.core.services.myprofile", + "pas.plugins.identity.core.services.groups", + "pas.plugins.identity.exportimport.importer", + "pas.plugins.identity.exportimport.exporter", +) + + +def _patch_portal_types(logger: Logger): + for name in PROFILE_TYPE_MODULES: + vars(import_module(name))["PROFILE_PORTAL_TYPE"] = PROFILE_PORTAL_TYPE + catalog = vars(import_module("pas.plugins.identity.core.catalog")) + catalog["CATALOGUED_TYPES"] = (PROFILE_PORTAL_TYPE, catalog["GROUP_PORTAL_TYPE"]) + # Only used by sync_core_records, to fill group_content_type + principals = vars(import_module("pas.plugins.identity.core.subscribers.principals")) + principals["GROUP_PORTAL_TYPE"] = GROUP_PORTAL_TYPE + logger.info(f"Patched pas.plugins.identity to use {PROFILE_PORTAL_TYPE}") + + +def _patch_person(logger: Logger): + from collective.person.content.person import Person + from kitconcept.core.content import person + + for name in person.USER_ATTRIBUTES: + setattr(Person, name, getattr(person.PersonUserMixin, name)) + logger.info("Patched collective.person Person with user attributes") + + +def apply_patch(logger: Logger): + """Apply the pas.plugins.identity patches. + + :param logger: Logger used to report the patches. + """ + _patch_portal_types(logger) + _patch_person(logger) diff --git a/backend/src/kitconcept/core/profiles/base/metadata.xml b/backend/src/kitconcept/core/profiles/base/metadata.xml index ccebb115..cc78d055 100644 --- a/backend/src/kitconcept/core/profiles/base/metadata.xml +++ b/backend/src/kitconcept/core/profiles/base/metadata.xml @@ -1,4 +1,4 @@ - 20260918001 + 20260929001 diff --git a/backend/src/kitconcept/core/profiles/dependencies/metadata.xml b/backend/src/kitconcept/core/profiles/dependencies/metadata.xml index b077b5f6..86dd6a52 100644 --- a/backend/src/kitconcept/core/profiles/dependencies/metadata.xml +++ b/backend/src/kitconcept/core/profiles/dependencies/metadata.xml @@ -10,5 +10,6 @@ profile-kitconcept.voltolighttheme:default profile-plonegovbr.socialmedia:default profile-plone.formblock:default + profile-pas.plugins.identity:default diff --git a/backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IProfileSettings.xml b/backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IProfileSettings.xml new file mode 100644 index 00000000..f3e13a62 --- /dev/null +++ b/backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IProfileSettings.xml @@ -0,0 +1,27 @@ + + + + + + + users + Users + PrincipalsContainer + + + + private + pending + published + + + + False + + + + diff --git a/backend/src/kitconcept/core/profiles/dependencies/types/Person.xml b/backend/src/kitconcept/core/profiles/dependencies/types/Person.xml index 2d17adcd..ef796a7c 100644 --- a/backend/src/kitconcept/core/profiles/dependencies/types/Person.xml +++ b/backend/src/kitconcept/core/profiles/dependencies/types/Person.xml @@ -6,7 +6,8 @@ > - collective.person.content.person.IPerson + + kitconcept.core.content.person.IPerson @@ -18,6 +19,7 @@ + diff --git a/backend/src/kitconcept/core/profiles/dependencies/types/PrincipalsContainer.xml b/backend/src/kitconcept/core/profiles/dependencies/types/PrincipalsContainer.xml new file mode 100644 index 00000000..ef827411 --- /dev/null +++ b/backend/src/kitconcept/core/profiles/dependencies/types/PrincipalsContainer.xml @@ -0,0 +1,13 @@ + + + + + + + diff --git a/backend/src/kitconcept/core/profiles/dependencies/types/UserGroup.xml b/backend/src/kitconcept/core/profiles/dependencies/types/UserGroup.xml new file mode 100644 index 00000000..40ad9cf7 --- /dev/null +++ b/backend/src/kitconcept/core/profiles/dependencies/types/UserGroup.xml @@ -0,0 +1,9 @@ + + + + False + diff --git a/backend/src/kitconcept/core/profiles/dependencies/types/UserProfile.xml b/backend/src/kitconcept/core/profiles/dependencies/types/UserProfile.xml new file mode 100644 index 00000000..1e7f44c2 --- /dev/null +++ b/backend/src/kitconcept/core/profiles/dependencies/types/UserProfile.xml @@ -0,0 +1,9 @@ + + + + False + diff --git a/backend/src/kitconcept/core/subscribers/__init__.py b/backend/src/kitconcept/core/subscribers/__init__.py new file mode 100644 index 00000000..fc41dc7f --- /dev/null +++ b/backend/src/kitconcept/core/subscribers/__init__.py @@ -0,0 +1,32 @@ +from collective.person.content.person import Person +from pas.plugins.identity.core.indexers.subscribers import profile_moved +from pas.plugins.identity.core.interfaces import IUserProfile +from zope.interface import alsoProvides +from zope.lifecycleevent.interfaces import IObjectAddedEvent + + +def is_user(person: Person) -> bool: + """Report whether a Person represents a user account. + + :param person: The Person. + :returns: Whether it provides :class:`IUserProfile`. + """ + return IUserProfile.providedBy(person) + + +def person_added(person: Person, event: IObjectAddedEvent) -> None: + """Mark a Person created for a user account as a user profile. + + pas.plugins.identity creates a user's content with a ``login``. Only those + Persons become users; other Persons remain plain content. + + The identity catalog subscribers are bound to the marker, and they were + already looked up for this event, so index the Person here. + + :param person: The Person. + :param event: The added event. + """ + if is_user(person) or not person.login: + return + alsoProvides(person, IUserProfile) + profile_moved(person, event) diff --git a/backend/src/kitconcept/core/subscribers/configure.zcml b/backend/src/kitconcept/core/subscribers/configure.zcml new file mode 100644 index 00000000..fbd311b7 --- /dev/null +++ b/backend/src/kitconcept/core/subscribers/configure.zcml @@ -0,0 +1,10 @@ + + + + + + diff --git a/backend/src/kitconcept/core/testing/distributions/testing/schema.json b/backend/src/kitconcept/core/testing/distributions/testing/schema.json index 076fac45..3306d7a4 100644 --- a/backend/src/kitconcept/core/testing/distributions/testing/schema.json +++ b/backend/src/kitconcept/core/testing/distributions/testing/schema.json @@ -82,7 +82,11 @@ { "const": "keycloak", "title": "Keycloak" }, { "const": "oidc", "title": "OIDC / OAuth2" }, { "const": "authomatic-google", "title": "Social Login Google" }, - { "const": "authomatic-github", "title": "Social Login GitHub" } + { "const": "authomatic-github", "title": "Social Login GitHub" }, + { "const": "identity-keycloak", "title": "Identity: Keycloak" }, + { "const": "identity-oidc", "title": "Identity: OIDC" }, + { "const": "identity-github", "title": "Identity: GitHub" }, + { "const": "identity-google", "title": "Identity: Google" } ] } }, diff --git a/backend/src/kitconcept/core/upgrades/configure.zcml b/backend/src/kitconcept/core/upgrades/configure.zcml index 685f7e2f..63f14ede 100644 --- a/backend/src/kitconcept/core/upgrades/configure.zcml +++ b/backend/src/kitconcept/core/upgrades/configure.zcml @@ -19,5 +19,6 @@ + diff --git a/backend/src/kitconcept/core/upgrades/v20260929001/__init__.py b/backend/src/kitconcept/core/upgrades/v20260929001/__init__.py new file mode 100644 index 00000000..d5a45eab --- /dev/null +++ b/backend/src/kitconcept/core/upgrades/v20260929001/__init__.py @@ -0,0 +1,34 @@ +from kitconcept.core import DEPENDENCIES_PROFILE +from kitconcept.core import logger +from kitconcept.core.utils.users import create_persons_for_users +from Products.GenericSetup.tool import SetupTool + + +IDENTITY_PROFILE = "pas.plugins.identity:default" + +# Import steps of the dependencies profile touched by this upgrade +STEPS = ("typeinfo", "plone.app.registry") + + +def install_identity(tool: SetupTool): + """Install pas.plugins.identity, and use Person as the user content type. + + :param tool: The GenericSetup tool of the site being upgraded. + """ + if tool.getLastVersionForProfile(IDENTITY_PROFILE) == "unknown": + tool.runAllImportStepsFromProfile(f"profile-{IDENTITY_PROFILE}") + logger.info(f"- Installed {IDENTITY_PROFILE}.") + for step in STEPS: + tool.runImportStepFromProfile( + f"profile-{DEPENDENCIES_PROFILE}", step, run_dependencies=False + ) + logger.info(f"- Imported {step} from {DEPENDENCIES_PROFILE}.") + + +def create_persons(tool: SetupTool): + """Create a Person for every user in source_users. + + :param tool: The GenericSetup tool of the site being upgraded. + """ + created = create_persons_for_users() + logger.info(f"- Created {len(created)} Person objects for existing users.") diff --git a/backend/src/kitconcept/core/upgrades/v20260929001/configure.zcml b/backend/src/kitconcept/core/upgrades/v20260929001/configure.zcml new file mode 100644 index 00000000..8297a978 --- /dev/null +++ b/backend/src/kitconcept/core/upgrades/v20260929001/configure.zcml @@ -0,0 +1,21 @@ + + + + + + + + diff --git a/backend/src/kitconcept/core/utils/authentication/__init__.py b/backend/src/kitconcept/core/utils/authentication/__init__.py index 6b1c71c8..92634be2 100644 --- a/backend/src/kitconcept/core/utils/authentication/__init__.py +++ b/backend/src/kitconcept/core/utils/authentication/__init__.py @@ -20,6 +20,7 @@ def _setup_plone_auth(answers: t.AuthAnswers): def get_authentication_provider(provider: str) -> t.AuthenticationProvider: from .authomatic import setup_authomatic + from .identity import setup_identity from .keycloak import setup_keycloak_auth from .oidc import setup_oidc_auth @@ -41,6 +42,10 @@ def get_authentication_provider(provider: str) -> t.AuthenticationProvider: "authomatic-google": t.AuthenticationProvider( setup_authomatic, ("pas.plugins.authomatic:default",) ), + "identity-keycloak": t.AuthenticationProvider(setup_identity, ()), + "identity-oidc": t.AuthenticationProvider(setup_identity, ()), + "identity-github": t.AuthenticationProvider(setup_identity, ()), + "identity-google": t.AuthenticationProvider(setup_identity, ()), } return providers.get(provider, providers["internal"]) diff --git a/backend/src/kitconcept/core/utils/authentication/identity.py b/backend/src/kitconcept/core/utils/authentication/identity.py new file mode 100644 index 00000000..8f80f95d --- /dev/null +++ b/backend/src/kitconcept/core/utils/authentication/identity.py @@ -0,0 +1,89 @@ +from kitconcept.core import _types as t +from kitconcept.core import logger +from plone import api + + +ANSWER_PREFIX = "identity-" + +CALLBACK_PATH = "/login-identity" + +CALLBACK_URL_RECORD = "pas.plugins.identity.callback_url" + +# Answer provider -> (provider id, driver id, title) +PROVIDERS: dict[str, tuple[str, str, str]] = { + "identity-keycloak": ("keycloak", "keycloak", "Keycloak"), + "identity-oidc": ("oidc", "oidc-generic", "OpenID Connect"), + "identity-github": ("github", "github", "GitHub"), + "identity-google": ("google", "google", "Google"), +} + +# Answers that are not driver settings +NON_CONFIG_KEYS = ("site-url", "server_url", "realm_name") + + +def _prepare_answers(raw_answers: t.AnswersIdentity) -> dict: + """Strip the answer prefix, and drop empty values. + + :param raw_answers: Authentication answers. + :returns: Answers keyed by setting name. + """ + return { + key.replace(ANSWER_PREFIX, "", 1): value + for key, value in raw_answers.items() + if key.startswith(ANSWER_PREFIX) and value not in (None, "", []) + } + + +def _prepare_config(provider: str, answers: dict) -> dict: + """Build the driver settings for a provider. + + Settings not answered are left out, so the driver defaults apply. + + :param provider: Answer provider, e.g. ``identity-keycloak``. + :param answers: Answers, as returned by :func:`_prepare_answers`. + :returns: Driver settings. + """ + config = { + key: value for key, value in answers.items() if key not in NON_CONFIG_KEYS + } + if provider == "identity-keycloak" and "issuer" not in config: + server_url = answers["server_url"].rstrip("/") + realm = answers["realm_name"] + config["issuer"] = f"{server_url}/realms/{realm}" + return config + + +def _callback_url(answers: dict) -> str | None: + """Return the callback url for a site url answer, if any. + + :param answers: Answers, as returned by :func:`_prepare_answers`. + :returns: Absolute callback url, or ``None`` to keep the default. + """ + site_url = answers.get("site-url") + if not site_url: + return None + return f"{site_url.rstrip('/')}{CALLBACK_PATH}" + + +def setup_identity(raw_answers: t.AnswersIdentity): + """Configure a pas.plugins.identity provider. + + :param raw_answers: Authentication answers. + """ + from pas.plugins.identity.core.controlpanel import get_providers + from pas.plugins.identity.core.controlpanel import ProviderConfig + from pas.plugins.identity.core.controlpanel import set_providers + + provider = raw_answers["provider"] + provider_id, driver_id, title = PROVIDERS[provider] + answers = _prepare_answers(raw_answers) + config = _prepare_config(provider, answers) + providers = [p for p in get_providers() if p.provider_id != provider_id] + providers.append(ProviderConfig(provider_id, driver_id, title=title, config=config)) + set_providers(providers) + logger.info(f"Authentication: Configured identity provider {provider_id}") + callback_url = _callback_url(answers) + if callback_url: + api.portal.set_registry_record(CALLBACK_URL_RECORD, callback_url) + logger.info(f"Authentication: Set registry value on {CALLBACK_URL_RECORD}") + logger.info("Authentication: Identity authentication setup") diff --git a/backend/src/kitconcept/core/utils/scripts.py b/backend/src/kitconcept/core/utils/scripts.py index 96da37fc..b1be42bc 100644 --- a/backend/src/kitconcept/core/utils/scripts.py +++ b/backend/src/kitconcept/core/utils/scripts.py @@ -104,6 +104,37 @@ def as_list(value: str) -> list[str]: "SITE_AUTHENTICATION_AUTHOMATIC-GOOGLE-SCOPE", as_list, ), + ( + "authentication.identity-server_url", + "SITE_AUTHENTICATION_IDENTITY-SERVER_URL", + None, + ), + ( + "authentication.identity-realm_name", + "SITE_AUTHENTICATION_IDENTITY-REALM_NAME", + None, + ), + ("authentication.identity-issuer", "SITE_AUTHENTICATION_IDENTITY-ISSUER", None), + ( + "authentication.identity-client_id", + "SITE_AUTHENTICATION_IDENTITY-CLIENT_ID", + None, + ), + ( + "authentication.identity-client_secret", + "SITE_AUTHENTICATION_IDENTITY-CLIENT_SECRET", + None, + ), + ( + "authentication.identity-site-url", + "SITE_AUTHENTICATION_IDENTITY-SITE-URL", + None, + ), + ( + "authentication.identity-scope", + "SITE_AUTHENTICATION_IDENTITY-SCOPE", + as_list, + ), ) diff --git a/backend/src/kitconcept/core/utils/users.py b/backend/src/kitconcept/core/utils/users.py new file mode 100644 index 00000000..880781d9 --- /dev/null +++ b/backend/src/kitconcept/core/utils/users.py @@ -0,0 +1,53 @@ +from collective.person.content.person import Person +from kitconcept.core import logger +from kitconcept.core.content.person import split_fullname +from plone import api + + +def create_person_for_user(userid: str) -> Person | None: + """Create the Person representing an existing user account. + + Does nothing when the user already has a profile. + + :param userid: The userid. + :returns: The new Person, or ``None`` when none was created. + """ + from pas.plugins.identity.core.container import get_container + from pas.plugins.identity.core.profiles import get_profile + + if get_profile(userid) is not None: + return None + user = api.user.get(userid=userid) + if user is None: + logger.warning(f"User {userid} not found, no Person created") + return None + login = user.getUserName() + first_name, last_name = split_fullname(user.getProperty("fullname", "") or "") + first_name = first_name or login + email = user.getProperty("email", "") or "" + with api.env.adopt_roles(["Manager"]): + container = get_container(create=True) + person = api.content.create( + container=container, + type="Person", + id=userid, + login=login, + first_name=first_name, + last_name=last_name, + emails=(email,) if email else (), + ) + logger.info(f"Created Person for user {userid}") + return person + + +def create_persons_for_users() -> list[str]: + """Create a Person for every user in ``source_users``. + + :returns: The userids a Person was created for. + """ + portal = api.portal.get() + created = [] + for userid in portal.acl_users.source_users.getUserIds(): + if create_person_for_user(userid) is not None: + created.append(userid) + return created diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 63ac14bf..64e8185d 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -70,7 +70,7 @@ def current_versions() -> CurrentVersions: from kitconcept.core import __version__ return CurrentVersions( - base="20260918001", + base="20260929001", dependencies="1000", package=__version__, ) diff --git a/backend/tests/content_types/test_ct_person.py b/backend/tests/content_types/test_ct_person.py index 558d0b80..acb5f757 100644 --- a/backend/tests/content_types/test_ct_person.py +++ b/backend/tests/content_types/test_ct_person.py @@ -16,6 +16,7 @@ def _setup(self, portal, get_fti): [ ("title", "Person"), ("klass", "collective.person.content.person.Person"), + ("schema", "kitconcept.core.content.person.IPerson"), ("global_allow", True), ], ) @@ -35,6 +36,7 @@ def test_fti(self, attr: str, expected): "kitconcept.core.biography", "collective.contact_behaviors.contact_info", "kitconcept.core.additional_contact_info", + "kitconcept.core.person_emails", "plone.namefromtitle", "plone.shortname", "volto.navtitle", diff --git a/backend/tests/creation/authentication/conftest.py b/backend/tests/creation/authentication/conftest.py index 23eb82eb..327fae2d 100644 --- a/backend/tests/creation/authentication/conftest.py +++ b/backend/tests/creation/authentication/conftest.py @@ -13,3 +13,13 @@ def func() -> dict: return json.loads(api.portal.get_registry_record(KEY)) return func + + +@pytest.fixture(scope="session") +def identity_provider(): + from pas.plugins.identity.api import provider + + def func(provider_id: str): + return provider.get(provider_id) + + return func diff --git a/backend/tests/creation/authentication/identity_github/__init__.py b/backend/tests/creation/authentication/identity_github/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/tests/creation/authentication/identity_github/conftest.py b/backend/tests/creation/authentication/identity_github/conftest.py new file mode 100644 index 00000000..424ce3b6 --- /dev/null +++ b/backend/tests/creation/authentication/identity_github/conftest.py @@ -0,0 +1,19 @@ +import pytest + + +@pytest.fixture(scope="session") +def answers(): + return { + "site_id": "site", + "title": "Site", + "description": "Site created with A CMS solution for public websites. Created by kitconcept.", + "available_languages": ["en"], + "default_language": "en", + "portal_timezone": "Europe/Berlin", + "setup_content": True, + "authentication": { + "provider": "identity-github", + "identity-client_id": "gh-32510011", + "identity-client_secret": "12345678", + }, + } diff --git a/backend/tests/creation/authentication/identity_github/test_auth_identity_github_creation.py b/backend/tests/creation/authentication/identity_github/test_auth_identity_github_creation.py new file mode 100644 index 00000000..ab96a19d --- /dev/null +++ b/backend/tests/creation/authentication/identity_github/test_auth_identity_github_creation.py @@ -0,0 +1,37 @@ +from plone import api + +import pytest + + +class TestSiteCreation: + @pytest.fixture(autouse=True) + def _setup(self, portal, identity_provider): + self.portal = portal + self.provider = identity_provider("github") + + @pytest.mark.parametrize( + "attr,expected", + [ + ["driver_id", "github"], + ["title", "GitHub"], + ["enabled", True], + ], + ) + def test_provider(self, attr, expected): + assert getattr(self.provider, attr) == expected + + @pytest.mark.parametrize( + "key,expected", + [ + ["client_id", "gh-32510011"], + ["client_secret", "12345678"], + # Not answered, so the driver default applies + ["scope", ("read:user", "user:email")], + ], + ) + def test_provider_config(self, key, expected): + assert self.provider.config[key] == expected + + def test_callback_url_default(self): + value = api.portal.get_registry_record("pas.plugins.identity.callback_url") + assert value == "/login-identity" diff --git a/backend/tests/creation/authentication/identity_google/__init__.py b/backend/tests/creation/authentication/identity_google/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/tests/creation/authentication/identity_google/conftest.py b/backend/tests/creation/authentication/identity_google/conftest.py new file mode 100644 index 00000000..4377ecbd --- /dev/null +++ b/backend/tests/creation/authentication/identity_google/conftest.py @@ -0,0 +1,20 @@ +import pytest + + +@pytest.fixture(scope="session") +def answers(): + return { + "site_id": "site", + "title": "Site", + "description": "Site created with A CMS solution for public websites. Created by kitconcept.", + "available_languages": ["en"], + "default_language": "en", + "portal_timezone": "Europe/Berlin", + "setup_content": True, + "authentication": { + "provider": "identity-google", + "identity-client_id": "google-12345", + "identity-client_secret": "12345678", + "identity-scope": ["openid", "email"], + }, + } diff --git a/backend/tests/creation/authentication/identity_google/test_auth_identity_google_creation.py b/backend/tests/creation/authentication/identity_google/test_auth_identity_google_creation.py new file mode 100644 index 00000000..4e71faa5 --- /dev/null +++ b/backend/tests/creation/authentication/identity_google/test_auth_identity_google_creation.py @@ -0,0 +1,30 @@ +import pytest + + +class TestSiteCreation: + @pytest.fixture(autouse=True) + def _setup(self, portal, identity_provider): + self.portal = portal + self.provider = identity_provider("google") + + @pytest.mark.parametrize( + "attr,expected", + [ + ["driver_id", "google"], + ["title", "Google"], + ["enabled", True], + ], + ) + def test_provider(self, attr, expected): + assert getattr(self.provider, attr) == expected + + @pytest.mark.parametrize( + "key,expected", + [ + ["client_id", "google-12345"], + ["client_secret", "12345678"], + ["scope", ("openid", "email")], + ], + ) + def test_provider_config(self, key, expected): + assert self.provider.config[key] == expected diff --git a/backend/tests/creation/authentication/identity_keycloak/__init__.py b/backend/tests/creation/authentication/identity_keycloak/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/tests/creation/authentication/identity_keycloak/conftest.py b/backend/tests/creation/authentication/identity_keycloak/conftest.py new file mode 100644 index 00000000..899360ae --- /dev/null +++ b/backend/tests/creation/authentication/identity_keycloak/conftest.py @@ -0,0 +1,23 @@ +import pytest + + +@pytest.fixture(scope="session") +def answers(): + return { + "site_id": "site", + "title": "Site", + "description": "Site created with A CMS solution for public websites. Created by kitconcept.", + "available_languages": ["en"], + "default_language": "en", + "portal_timezone": "Europe/Berlin", + "setup_content": True, + "authentication": { + "provider": "identity-keycloak", + "identity-server_url": "http://localhost:8180", + "identity-realm_name": "site", + "identity-client_id": "plone", + "identity-client_secret": "12345678", + "identity-site-url": "http://localhost:3000", + "identity-scope": ["openid", "profile", "email"], + }, + } diff --git a/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py b/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py new file mode 100644 index 00000000..954234a5 --- /dev/null +++ b/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py @@ -0,0 +1,57 @@ +from plone import api + +import pytest + + +class TestSiteCreation: + @pytest.fixture(autouse=True) + def _setup(self, portal, identity_provider): + self.portal = portal + self.provider = identity_provider("keycloak") + + def test_profile_installed(self, profile_last_version): + result = profile_last_version("profile-pas.plugins.identity:default") + assert isinstance(result, str) + assert result != "" + + @pytest.mark.parametrize("plugin_id", ["identity", "identity_profile"]) + def test_plugin_created(self, plugin_id): + assert plugin_id in self.portal.acl_users.objectIds() + + @pytest.mark.parametrize( + "attr,expected", + [ + ["driver_id", "keycloak"], + ["title", "Keycloak"], + ["enabled", True], + ], + ) + def test_provider(self, attr, expected): + assert getattr(self.provider, attr) == expected + + @pytest.mark.parametrize( + "key,expected", + [ + ["issuer", "http://localhost:8180/realms/site"], + ["client_id", "plone"], + ["client_secret", "12345678"], + ["scope", ("openid", "profile", "email")], + ], + ) + def test_provider_config(self, key, expected): + assert self.provider.config[key] == expected + + def test_callback_url(self): + value = api.portal.get_registry_record("pas.plugins.identity.callback_url") + assert value == "http://localhost:3000/login-identity" + + @pytest.mark.parametrize( + "profile_id", + [ + "pas.plugins.oidc:default", + "pas.plugins.keycloakgroups:default", + "pas.plugins.authomatic:default", + ], + ) + def test_legacy_profile_not_installed(self, setup_tool, profile_id): + assert setup_tool.getLastVersionForProfile(profile_id) == "unknown" diff --git a/backend/tests/creation/authentication/identity_oidc/__init__.py b/backend/tests/creation/authentication/identity_oidc/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/tests/creation/authentication/identity_oidc/conftest.py b/backend/tests/creation/authentication/identity_oidc/conftest.py new file mode 100644 index 00000000..7a8a1fa7 --- /dev/null +++ b/backend/tests/creation/authentication/identity_oidc/conftest.py @@ -0,0 +1,21 @@ +import pytest + + +@pytest.fixture(scope="session") +def answers(): + return { + "site_id": "site", + "title": "Site", + "description": "Site created with A CMS solution for public websites. Created by kitconcept.", + "available_languages": ["en"], + "default_language": "en", + "portal_timezone": "Europe/Berlin", + "setup_content": True, + "authentication": { + "provider": "identity-oidc", + "identity-issuer": "https://auth.example.com", + "identity-client_id": "plone", + "identity-client_secret": "12345678", + "identity-site-url": "https://www.example.com", + }, + } diff --git a/backend/tests/creation/authentication/identity_oidc/test_auth_identity_oidc_creation.py b/backend/tests/creation/authentication/identity_oidc/test_auth_identity_oidc_creation.py new file mode 100644 index 00000000..35e97fb0 --- /dev/null +++ b/backend/tests/creation/authentication/identity_oidc/test_auth_identity_oidc_creation.py @@ -0,0 +1,36 @@ +from plone import api + +import pytest + + +class TestSiteCreation: + @pytest.fixture(autouse=True) + def _setup(self, portal, identity_provider): + self.portal = portal + self.provider = identity_provider("oidc") + + @pytest.mark.parametrize( + "attr,expected", + [ + ["driver_id", "oidc-generic"], + ["title", "OpenID Connect"], + ["enabled", True], + ], + ) + def test_provider(self, attr, expected): + assert getattr(self.provider, attr) == expected + + @pytest.mark.parametrize( + "key,expected", + [ + ["issuer", "https://auth.example.com"], + ["client_id", "plone"], + ["client_secret", "12345678"], + ], + ) + def test_provider_config(self, key, expected): + assert self.provider.config[key] == expected + + def test_callback_url(self): + value = api.portal.get_registry_record("pas.plugins.identity.callback_url") + assert value == "https://www.example.com/login-identity" diff --git a/backend/tests/identity/__init__.py b/backend/tests/identity/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/tests/identity/test_identity_setup.py b/backend/tests/identity/test_identity_setup.py new file mode 100644 index 00000000..d06fa420 --- /dev/null +++ b/backend/tests/identity/test_identity_setup.py @@ -0,0 +1,73 @@ +from plone import api + +import pytest + + +PREFIX = "pas.plugins.identity" + + +class TestIdentitySetup: + @pytest.fixture(autouse=True) + def _setup(self, portal): + self.portal = portal + + def test_profile_installed(self, profile_last_version): + result = profile_last_version(f"profile-{PREFIX}:default") + assert isinstance(result, str) + assert result != "" + + @pytest.mark.parametrize("plugin_id", ["identity", "identity_profile"]) + def test_plugin_created(self, plugin_id): + assert plugin_id in self.portal.acl_users.objectIds() + + @pytest.mark.parametrize( + "key,expected", + [ + ["user_content_type", "Person"], + ["user_container_path", "users"], + ["group_content_type", ""], + ["profile_container_id", "users"], + ["profile_container_title", "Users"], + ["profile_container_type", "PrincipalsContainer"], + ["profile_enumeration_states", ("private", "pending", "published")], + ["enforce_required_profile_fields", False], + ], + ) + def test_registry(self, key: str, expected): + assert api.portal.get_registry_record(f"{PREFIX}.{key}") == expected + + @pytest.mark.parametrize( + "portal_type,expected", + [ + ["UserProfile", False], + ["UserGroup", False], + ["Person", True], + ], + ) + def test_global_allow(self, get_fti, portal_type: str, expected: bool): + assert get_fti(portal_type).global_allow is expected + + def test_principals_container_allows_person(self, get_fti): + fti = get_fti("PrincipalsContainer") + assert fti.allowed_content_types == ("Person",) + + +class TestIdentityPatch: + @pytest.mark.parametrize( + "module", + [ + "pas.plugins.identity.core.catalog", + "pas.plugins.identity.core.profiles", + "pas.plugins.identity.core.pas.profile", + "pas.plugins.identity.core.subscribers.principals", + ], + ) + def test_profile_portal_type(self, module: str): + from importlib import import_module + + assert import_module(module).PROFILE_PORTAL_TYPE == "Person" + + def test_catalogued_types(self): + from pas.plugins.identity.core import catalog + + assert catalog.CATALOGUED_TYPES == ("Person", "UserGroup") diff --git a/backend/tests/identity/test_identity_users.py b/backend/tests/identity/test_identity_users.py new file mode 100644 index 00000000..c6b91c92 --- /dev/null +++ b/backend/tests/identity/test_identity_users.py @@ -0,0 +1,134 @@ +from kitconcept.core.utils import users as utils +from pas.plugins.identity.core.interfaces import IUserProfile +from plone import api +from plone.app.testing import TEST_USER_PASSWORD + +import pytest + + +@pytest.fixture +def create_user(portal): + def func(username: str = "jdoe", fullname: str = "John Doe"): + with api.env.adopt_roles(["Manager"]): + return api.user.create( + email=f"{username}@example.com", + username=username, + password=TEST_USER_PASSWORD, + properties={"fullname": fullname}, + ) + + return func + + +@pytest.fixture +def create_source_user(portal): + """Create a user in source_users only, as on a site before the upgrade.""" + + def func(userid: str = "legacy", fullname: str = "Ada Lovelace"): + portal.acl_users.source_users.addUser(userid, userid, TEST_USER_PASSWORD) + user = api.user.get(userid=userid) + user.setMemberProperties({ + "fullname": fullname, + "email": f"{userid}@example.com", + }) + return userid + + return func + + +class TestUserAsPerson: + @pytest.fixture(autouse=True) + def _setup(self, portal, create_user): + self.portal = portal + self.user = create_user() + self.person = self.portal["users"]["jdoe"] + + def test_person_created(self): + assert self.person.portal_type == "Person" + + def test_person_is_user(self): + assert IUserProfile.providedBy(self.person) + + @pytest.mark.parametrize( + "attr,expected", + [ + ["userid", "jdoe"], + ["login", "jdoe"], + ["first_name", "John"], + ["last_name", "Doe"], + ["fullname", "John Doe"], + ], + ) + def test_person_attributes(self, attr: str, expected: str): + assert getattr(self.person, attr) == expected + + def test_user_found(self): + user = api.user.get(userid="jdoe") + assert user is not None + assert user.getProperty("fullname") == "John Doe" + + def test_user_in_search(self): + results = api.user.get_users() + assert "jdoe" in [user.getId() for user in results] + + def test_group_membership_in_source_groups(self): + api.group.add_user(groupname="Site Administrators", username="jdoe") + groups = api.group.get_groups(username="jdoe") + assert "Site Administrators" in [group.getId() for group in groups] + + +class TestPersonContent: + @pytest.fixture(autouse=True) + def _setup(self, portal): + self.portal = portal + with api.env.adopt_roles(["Manager"]): + self.person = api.content.create( + container=portal, + type="Person", + id="jane", + first_name="Jane", + last_name="Roe", + ) + + def test_person_is_not_user(self): + assert not IUserProfile.providedBy(self.person) + + def test_no_user_for_person(self): + assert api.user.get(userid="jane") is None + + def test_emails_optional(self): + from kitconcept.core.behaviors.person_emails import IPersonEmails + + assert IPersonEmails["emails"].required is False + assert self.person.emails == () + + +class TestCreatePersonsForUsers: + @pytest.fixture(autouse=True) + def _setup(self, portal, create_source_user): + self.portal = portal + self.userid = create_source_user() + + def test_source_user_has_no_person(self): + assert "legacy" not in self.portal.get("users", {}) + + def test_creates_person(self): + created = utils.create_persons_for_users() + assert self.userid in created + person = self.portal["users"][self.userid] + assert IUserProfile.providedBy(person) + assert person.first_name == "Ada" + assert person.last_name == "Lovelace" + assert person.email == "legacy@example.com" + + def test_idempotent(self): + utils.create_persons_for_users() + assert utils.create_persons_for_users() == [] + + def test_user_still_authenticates_against_source_users(self): + utils.create_persons_for_users() + acl_users = self.portal.acl_users + assert acl_users.source_users.authenticateCredentials({ + "login": "legacy", + "password": TEST_USER_PASSWORD, + }) == ("legacy", "legacy") diff --git a/backend/tests/setup/test_upgrades.py b/backend/tests/setup/test_upgrades.py index b51311d2..18433c83 100644 --- a/backend/tests/setup/test_upgrades.py +++ b/backend/tests/setup/test_upgrades.py @@ -46,6 +46,7 @@ def _setup(self, portal_class, current_versions) -> None: "20260620001", "20260706001", "20260917001", + "20260918001", ], ) def test_upgrade_to_latest(self, list_upgrades, src_version: str) -> None: @@ -74,3 +75,36 @@ def test_update_blocks_config_permission(self) -> None: if role["selected"] } assert selected == {"Manager", "Site Administrator"} + + +class TestUpgradeIdentity: + @pytest.fixture(autouse=True) + def _setup(self, portal) -> None: + self.portal = portal + self.setup_tool: SetupTool = portal.portal_setup + + def test_install_identity(self) -> None: + """Reimporting the settings keeps Person as the user content type.""" + from kitconcept.core.upgrades.v20260929001 import install_identity + from plone import api + + api.portal.set_registry_record("pas.plugins.identity.user_content_type", "") + + install_identity(self.setup_tool) + + assert ( + api.portal.get_registry_record("pas.plugins.identity.user_content_type") + == "Person" + ) + fti = self.portal.portal_types.Person + assert fti.schema == "kitconcept.core.content.person.IPerson" + + def test_create_persons(self) -> None: + """Every user in source_users gets a Person.""" + from kitconcept.core.upgrades.v20260929001 import create_persons + + self.portal.acl_users.source_users.addUser("legacy", "legacy", "secret-1234") + + create_persons(self.setup_tool) + + assert self.portal["users"]["legacy"].portal_type == "Person" diff --git a/backend/tests/utils/authentication/test_identity.py b/backend/tests/utils/authentication/test_identity.py new file mode 100644 index 00000000..35d284d3 --- /dev/null +++ b/backend/tests/utils/authentication/test_identity.py @@ -0,0 +1,82 @@ +from kitconcept.core.utils.authentication import identity as utils + +import pytest + + +@pytest.fixture +def answers() -> dict: + return { + "provider": "identity-keycloak", + "identity-server_url": "http://localhost:8180/", + "identity-realm_name": "site", + "identity-client_id": "plone", + "identity-client_secret": "12345678", + "identity-site-url": "http://localhost:3000/", + "identity-scope": ["openid", "profile", "email"], + } + + +class TestPrepareAnswers: + def test_strips_prefix(self, answers): + result = utils._prepare_answers(answers) + assert result["client_id"] == "plone" + assert "provider" not in result + + @pytest.mark.parametrize("value", [None, "", []]) + def test_drops_empty_values(self, answers, value): + answers["identity-scope"] = value + result = utils._prepare_answers(answers) + assert "scope" not in result + + +class TestPrepareConfig: + @pytest.mark.parametrize( + "key,expected", + [ + ["issuer", "http://localhost:8180/realms/site"], + ["client_id", "plone"], + ["client_secret", "12345678"], + ["scope", ["openid", "profile", "email"]], + ], + ) + def test_keycloak(self, answers, key: str, expected: str | list[str]): + result = utils._prepare_config( + "identity-keycloak", utils._prepare_answers(answers) + ) + assert result[key] == expected + + @pytest.mark.parametrize("key", ["site-url", "server_url", "realm_name"]) + def test_non_config_keys_removed(self, answers, key: str): + result = utils._prepare_config( + "identity-keycloak", utils._prepare_answers(answers) + ) + assert key not in result + + def test_keycloak_explicit_issuer_wins(self, answers): + answers["identity-issuer"] = "https://sso.example.com/realms/other" + result = utils._prepare_config( + "identity-keycloak", utils._prepare_answers(answers) + ) + assert result["issuer"] == "https://sso.example.com/realms/other" + + def test_github_has_no_issuer(self): + answers = { + "provider": "identity-github", + "identity-client_id": "gh-123", + "identity-client_secret": "12345678", + } + result = utils._prepare_config( + "identity-github", utils._prepare_answers(answers) + ) + assert result == {"client_id": "gh-123", "client_secret": "12345678"} + + +class TestCallbackUrl: + def test_from_site_url(self, answers): + result = utils._callback_url(utils._prepare_answers(answers)) + assert result == "http://localhost:3000/login-identity" + + def test_without_site_url(self, answers): + answers.pop("identity-site-url") + result = utils._callback_url(utils._prepare_answers(answers)) + assert result is None diff --git a/backend/tests/utils/test_utils_scripts.py b/backend/tests/utils/test_utils_scripts.py index 84aacbe0..68f4f5f0 100644 --- a/backend/tests/utils/test_utils_scripts.py +++ b/backend/tests/utils/test_utils_scripts.py @@ -144,6 +144,23 @@ def test_nested_authentication_keys(self, monkeypatch): "oidc-scope": ["openid", "email"], } + def test_nested_identity_keys(self, monkeypatch): + monkeypatch.setenv("SITE_AUTHENTICATION_PROVIDER", "identity-keycloak") + monkeypatch.setenv( + "SITE_AUTHENTICATION_IDENTITY-SERVER_URL", "https://auth.example.com" + ) + monkeypatch.setenv("SITE_AUTHENTICATION_IDENTITY-REALM_NAME", "site") + monkeypatch.setenv("SITE_AUTHENTICATION_IDENTITY-CLIENT_SECRET", "secret") + monkeypatch.setenv("SITE_AUTHENTICATION_IDENTITY-SCOPE", "openid, email") + result = scripts.get_environmental_variables(self.options) + assert result["authentication"] == { + "provider": "identity-keycloak", + "identity-server_url": "https://auth.example.com", + "identity-realm_name": "site", + "identity-client_secret": "secret", + "identity-scope": ["openid", "email"], + } + def test_mixed_plain_and_nested(self, monkeypatch): monkeypatch.setenv("SITE_ID", "Plone") monkeypatch.setenv("SITE_AUTHENTICATION_PROVIDER", "oidc") diff --git a/backend/uv.lock b/backend/uv.lock index 318c2027..0b6aa76c 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -553,6 +553,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/65/15/716495cf0759c1dd8e449afb26cda840e3378b7b5afd47f63c8045434d1e/AuthEncoding-6.0-py3-none-any.whl", hash = "sha256:4c1a8c3db8222a9892a947d77fde7db3b4be224bbcf0765762e44abdb4da3a0c", size = 9092, upload-time = "2025-02-26T10:58:33.1Z" }, ] +[[package]] +name = "authlib" +version = "1.8.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cryptography" }, + { name = "joserfc" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/f1/51/bc1729d3cfdc214b4935f4e886e4dd443c3065fd8e1e66423fe84b490f81/authlib-1.8.0.tar.gz", hash = "sha256:f3ecd5f1da737262fb53bf1a4d95c4ea1ad9dd509316587a255c99ab1838a4f0", size = 177759, upload-time = "2026-08-30T12:12:34.833Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b8/c6/6f124bcfbbfb20fba22c939b4e43a06dccfc0e1ca20e5634ca573cb1e271/authlib-1.8.0-py2.py3-none-any.whl", hash = "sha256:88aebbd9af6757e14e912d5dc007ae1dc1f3e27e3b2152ce7c552ee2c3b3c121", size = 260804, upload-time = "2026-08-30T12:12:33.162Z" }, +] + [[package]] name = "authomatic" version = "1.3.0" @@ -1307,6 +1320,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/cb/b1/3846dd7f199d53cb17f49cba7e651e9ce294d8497c8c150530ed11865bb8/iniconfig-2.3.0-py3-none-any.whl", hash = "sha256:f631c04d2c48c52b84d0d0549c99ff3859c98df65b3101406327ecc7d53fbf12", size = 7484, upload-time = "2025-10-18T21:55:41.639Z" }, ] +[[package]] +name = "joserfc" +version = "1.7.5" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cryptography" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/19/94/80fea1514b7c6d7d37804d3fe9ca81455f633347fc98731bd71ffe1faa17/joserfc-1.7.5.tar.gz", hash = "sha256:d5ff536e658e17664f8c1b1ab60dc4aa62aa973fcef1edd33cc44bda45d6f5ea", size = 234990, upload-time = "2026-08-29T13:05:42.057Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/67/c5/82addfd375e5ee6520644e0553e4aadde92d668c4fc99cc716d337fe7bb3/joserfc-1.7.5-py3-none-any.whl", hash = "sha256:add2c2c84e8373b084d526a8b53daba5d7a513a118cd2dcd9fc9f979d0922159", size = 71269, upload-time = "2026-08-29T13:05:40.718Z" }, +] + [[package]] name = "jsonschema" version = "4.26.0" @@ -1356,6 +1381,7 @@ dependencies = [ { name = "collective-volto-formsupport" }, { name = "kitconcept-voltolighttheme" }, { name = "pas-plugins-authomatic" }, + { name = "pas-plugins-identity" }, { name = "pas-plugins-keycloakgroups" }, { name = "pas-plugins-oidc" }, { name = "plone-api" }, @@ -1397,6 +1423,7 @@ requires-dist = [ { name = "horse-with-no-namespace", marker = "extra == 'test'" }, { name = "kitconcept-voltolighttheme", specifier = ">=8.0.0" }, { name = "pas-plugins-authomatic", specifier = "==2.0.0" }, + { name = "pas-plugins-identity", specifier = "==1.0.0a11" }, { name = "pas-plugins-keycloakgroups", specifier = "==1.0.0b1" }, { name = "pas-plugins-oidc", specifier = "==2.0.0" }, { name = "plone-api" }, @@ -1747,6 +1774,26 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/e8/52/4ff1fe71cffc3a9318c314115899172b18b46cf3c684767d0f3e1f204f7e/pas_plugins_authomatic-2.0.0-py3-none-any.whl", hash = "sha256:41ce9149c93dc1ddec5098a1a7a4b8c737662d3454d3499ad237430619a2b853", size = 62564, upload-time = "2025-05-15T17:43:33.759Z" }, ] +[[package]] +name = "pas-plugins-identity" +version = "1.0.0a11" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "authlib" }, + { name = "defusedxml" }, + { name = "joserfc" }, + { name = "plone-api" }, + { name = "plone-app-dexterity" }, + { name = "plone-exportimport" }, + { name = "plone-restapi" }, + { name = "products-cmfplone" }, + { name = "requests" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/18/73/14b81d051e2b222f841de2d59a8e0d07370012b9422bb7f956bd74f29cb3/pas_plugins_identity-1.0.0a11.tar.gz", hash = "sha256:e7fcdfd017293a6142f59aa25fb5c1d80bd9374c646d8015810eb2679676b18a", size = 1072756, upload-time = "2026-09-28T18:56:59.829Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c4/ca/98859e860977fa896ee7cda0a112199e7ac26080a27a8cc9b0e2dbe17e52/pas_plugins_identity-1.0.0a11-py3-none-any.whl", hash = "sha256:2c3c51f64a4777a9c8af43a32c34f0f18525156a135f4fd4245dc4ef6957a005", size = 723316, upload-time = "2026-09-28T18:56:58.083Z" }, +] + [[package]] name = "pas-plugins-keycloakgroups" version = "1.0.0b1" From 511e777e84f310a8afdc797053c361928b6a0945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Wed, 30 Sep 2026 12:08:15 -0300 Subject: [PATCH 2/6] Upgrade pas.plugins.identity to 1.0.0a12 1.0.0a12 reads user_content_type and group_content_type everywhere instead of hardcoding UserProfile and UserGroup (upstream #127), so the patch that rewrote PROFILE_PORTAL_TYPE in its modules is removed. Only the user attributes of Person are still patched. The type records are now seeded once at install and left to the site, so the dependencies profile sets user_content_type to Person and an empty group_content_type, keeping groups in source_groups. - Replace the patch tests with checks on principal_types - Test that new groups stay in source_groups, with no UserGroup content Known gap until upstream #129: ensure_profile only creates the user type when the type provides IUserProfile, so a first login no longer creates a Person. Upstream: https://github.com/collective/pas-plugins-identity/issues/129 Refs: https://gitlab.kitconcept.io/kitconcept/distribution-kitconcept-intranet/-/issues/604 --- backend/pyproject.toml | 2 +- .../src/kitconcept/core/patches/identity.py | 48 ++----------------- ...trolpanel.interfaces.IIdentitySettings.xml | 20 ++++++++ backend/tests/identity/test_identity_setup.py | 27 +++++------ backend/tests/identity/test_identity_users.py | 16 +++++++ backend/uv.lock | 8 ++-- 6 files changed, 56 insertions(+), 65 deletions(-) create mode 100644 backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IIdentitySettings.xml diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 05de6034..0bb6d005 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -34,7 +34,7 @@ dependencies = [ "collective.honeypot", "collective.person==1.0.0b6", "pas.plugins.authomatic==2.0.0", - "pas.plugins.identity==1.0.0a11", + "pas.plugins.identity==1.0.0a12", "pas.plugins.keycloakgroups==1.0.0b1", "pas.plugins.oidc==2.0.0", ] diff --git a/backend/src/kitconcept/core/patches/identity.py b/backend/src/kitconcept/core/patches/identity.py index dee7f772..3612e858 100644 --- a/backend/src/kitconcept/core/patches/identity.py +++ b/backend/src/kitconcept/core/patches/identity.py @@ -1,52 +1,13 @@ -"""Use ``Person`` as the user content type of ``pas.plugins.identity``. +"""Give ``Person`` the user attributes of ``pas.plugins.identity``. -``pas.plugins.identity`` hardcodes ``UserProfile`` as the portal type of a -user in several places (``PROFILE_PORTAL_TYPE``), and resets the -``user_content_type`` and ``group_content_type`` registry records to its own -types whenever the container settings change. Until the add-on reads the type -from the registry, point every copy of those constants at ``Person``, and at no -group type, so groups stay in ``source_groups``. - -The ``Person`` class also gets the attributes the identity catalog and the -PAS plugins read from a ``UserProfile``. +``Person`` is the user content type (``user_content_type``), so the identity +catalog and the PAS plugins read from a ``Person`` the attributes they read +from a ``UserProfile``. The ``collective.person`` class gets them here. """ -from importlib import import_module from logging import Logger -PROFILE_PORTAL_TYPE = "Person" - -# Empty: groups are not content, they stay in source_groups -GROUP_PORTAL_TYPE = "" - -# Modules holding a copy of PROFILE_PORTAL_TYPE -PROFILE_TYPE_MODULES = ( - "pas.plugins.identity.core.catalog", - "pas.plugins.identity.core.completeness", - "pas.plugins.identity.core.doctor", - "pas.plugins.identity.core.profiles", - "pas.plugins.identity.core.subscribers.principals", - "pas.plugins.identity.core.pas.profile", - "pas.plugins.identity.core.services.types", - "pas.plugins.identity.core.services.myprofile", - "pas.plugins.identity.core.services.groups", - "pas.plugins.identity.exportimport.importer", - "pas.plugins.identity.exportimport.exporter", -) - - -def _patch_portal_types(logger: Logger): - for name in PROFILE_TYPE_MODULES: - vars(import_module(name))["PROFILE_PORTAL_TYPE"] = PROFILE_PORTAL_TYPE - catalog = vars(import_module("pas.plugins.identity.core.catalog")) - catalog["CATALOGUED_TYPES"] = (PROFILE_PORTAL_TYPE, catalog["GROUP_PORTAL_TYPE"]) - # Only used by sync_core_records, to fill group_content_type - principals = vars(import_module("pas.plugins.identity.core.subscribers.principals")) - principals["GROUP_PORTAL_TYPE"] = GROUP_PORTAL_TYPE - logger.info(f"Patched pas.plugins.identity to use {PROFILE_PORTAL_TYPE}") - - def _patch_person(logger: Logger): from collective.person.content.person import Person from kitconcept.core.content import person @@ -61,5 +22,4 @@ def apply_patch(logger: Logger): :param logger: Logger used to report the patches. """ - _patch_portal_types(logger) _patch_person(logger) diff --git a/backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IIdentitySettings.xml b/backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IIdentitySettings.xml new file mode 100644 index 00000000..285382ce --- /dev/null +++ b/backend/src/kitconcept/core/profiles/dependencies/registry/pas.plugins.identity.core.controlpanel.interfaces.IIdentitySettings.xml @@ -0,0 +1,20 @@ + + + + + + + Person + + + + + + + diff --git a/backend/tests/identity/test_identity_setup.py b/backend/tests/identity/test_identity_setup.py index d06fa420..011993e9 100644 --- a/backend/tests/identity/test_identity_setup.py +++ b/backend/tests/identity/test_identity_setup.py @@ -25,7 +25,7 @@ def test_plugin_created(self, plugin_id): [ ["user_content_type", "Person"], ["user_container_path", "users"], - ["group_content_type", ""], + ["group_content_type", None], ["profile_container_id", "users"], ["profile_container_title", "Users"], ["profile_container_type", "PrincipalsContainer"], @@ -52,22 +52,17 @@ def test_principals_container_allows_person(self, get_fti): assert fti.allowed_content_types == ("Person",) -class TestIdentityPatch: - @pytest.mark.parametrize( - "module", - [ - "pas.plugins.identity.core.catalog", - "pas.plugins.identity.core.profiles", - "pas.plugins.identity.core.pas.profile", - "pas.plugins.identity.core.subscribers.principals", - ], - ) - def test_profile_portal_type(self, module: str): - from importlib import import_module +class TestIdentityPrincipalTypes: + @pytest.fixture(autouse=True) + def _setup(self, portal): + self.portal = portal + + def test_user_portal_type(self): + from pas.plugins.identity.core.principal_types import user_portal_type - assert import_module(module).PROFILE_PORTAL_TYPE == "Person" + assert user_portal_type() == "Person" def test_catalogued_types(self): - from pas.plugins.identity.core import catalog + from pas.plugins.identity.core.principal_types import catalogued_types - assert catalog.CATALOGUED_TYPES == ("Person", "UserGroup") + assert catalogued_types() == ("Person", "UserGroup") diff --git a/backend/tests/identity/test_identity_users.py b/backend/tests/identity/test_identity_users.py index c6b91c92..a5883a0d 100644 --- a/backend/tests/identity/test_identity_users.py +++ b/backend/tests/identity/test_identity_users.py @@ -77,6 +77,22 @@ def test_group_membership_in_source_groups(self): assert "Site Administrators" in [group.getId() for group in groups] +class TestGroupsInSourceGroups: + @pytest.fixture(autouse=True) + def _setup(self, portal): + self.portal = portal + with api.env.adopt_roles(["Manager"]): + api.group.create(groupname="editors-team", title="Editors team") + + def test_group_in_source_groups(self): + source_groups = self.portal.acl_users.source_groups + assert "editors-team" in source_groups.getGroupIds() + + def test_no_group_content(self): + brains = api.content.find(portal_type="UserGroup") + assert len(brains) == 0 + + class TestPersonContent: @pytest.fixture(autouse=True) def _setup(self, portal): diff --git a/backend/uv.lock b/backend/uv.lock index 0b6aa76c..cd9e7609 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -1423,7 +1423,7 @@ requires-dist = [ { name = "horse-with-no-namespace", marker = "extra == 'test'" }, { name = "kitconcept-voltolighttheme", specifier = ">=8.0.0" }, { name = "pas-plugins-authomatic", specifier = "==2.0.0" }, - { name = "pas-plugins-identity", specifier = "==1.0.0a11" }, + { name = "pas-plugins-identity", specifier = "==1.0.0a12" }, { name = "pas-plugins-keycloakgroups", specifier = "==1.0.0b1" }, { name = "pas-plugins-oidc", specifier = "==2.0.0" }, { name = "plone-api" }, @@ -1776,7 +1776,7 @@ wheels = [ [[package]] name = "pas-plugins-identity" -version = "1.0.0a11" +version = "1.0.0a12" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "authlib" }, @@ -1789,9 +1789,9 @@ dependencies = [ { name = "products-cmfplone" }, { name = "requests" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/18/73/14b81d051e2b222f841de2d59a8e0d07370012b9422bb7f956bd74f29cb3/pas_plugins_identity-1.0.0a11.tar.gz", hash = "sha256:e7fcdfd017293a6142f59aa25fb5c1d80bd9374c646d8015810eb2679676b18a", size = 1072756, upload-time = "2026-09-28T18:56:59.829Z" } +sdist = { url = "https://files.pythonhosted.org/packages/70/a0/5e7090746432e635a91ae617b18715b595e3abeeb8607958604683985902/pas_plugins_identity-1.0.0a12.tar.gz", hash = "sha256:d46050c5b9fba9816320487c6bcdcd3f2f434a8b13efbd95cb0649c004be2586", size = 1084868, upload-time = "2026-09-30T14:45:31.166Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/c4/ca/98859e860977fa896ee7cda0a112199e7ac26080a27a8cc9b0e2dbe17e52/pas_plugins_identity-1.0.0a11-py3-none-any.whl", hash = "sha256:2c3c51f64a4777a9c8af43a32c34f0f18525156a135f4fd4245dc4ef6957a005", size = 723316, upload-time = "2026-09-28T18:56:58.083Z" }, + { url = "https://files.pythonhosted.org/packages/a9/47/bb8e43d2e8c76e0acdb8c460cd8086309b4148d7899b72c34e942d47df3d/pas_plugins_identity-1.0.0a12-py3-none-any.whl", hash = "sha256:411dcfdea80e4a652b2ddd681ffb80a6866fa966ad8928972909413d34f01698", size = 728649, upload-time = "2026-09-30T14:45:29.167Z" }, ] [[package]] From 203de9014c061e5f1eb2f27161e70e8715634b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Wed, 30 Sep 2026 13:28:27 -0300 Subject: [PATCH 3/6] Use only pas.plugins.identity for new sites Site creation now refuses the keycloak, oidc, authomatic-github and authomatic-google providers, naming the identity-* replacement, so any distribution based on kitconcept.core creates sites with pas.plugins.identity or plain Plone authentication only. - Raise UnsupportedAuthenticationProvider for the old providers - Deprecate setup_oidc_auth, setup_keycloak_auth and setup_authomatic - Replace the shared schema definitions with identity-keycloak, identity-oidc, identity-github and identity-google - Ignore the SITE_AUTHENTICATION_OIDC-* and AUTHOMATIC-* environment variables, with a warning - Offer only internal and the identity-* providers in the testing schema, without the "Identity:" title prefix - identity-keycloak installs pas.plugins.keycloakgroups for the realm groups, and mints userids from the Keycloak sub, the user id the groups plugin looks up - Replace the old provider site creation tests, and fix two profile-installed tests that passed with the profile missing Refs: https://gitlab.kitconcept.io/kitconcept/distribution-kitconcept-intranet/-/issues/604 --- backend/news/604.breaking | 1 + backend/news/604.feature | 2 +- .../core/patches/local_definitions.json | 95 ++++++++-------- .../testing/distributions/testing/schema.json | 31 +++--- .../core/utils/authentication/__init__.py | 77 +++++++++---- .../core/utils/authentication/authomatic.py | 10 ++ .../core/utils/authentication/identity.py | 35 +++++- .../core/utils/authentication/keycloak.py | 8 ++ .../core/utils/authentication/oidc.py | 9 ++ backend/src/kitconcept/core/utils/scripts.py | 67 +++++------ .../tests/creation/authentication/conftest.py | 14 --- .../authentication/github/__init__.py | 0 .../authentication/github/conftest.py | 20 ---- .../github/test_auth_github_creation.py | 37 ------ .../authentication/google/__init__.py | 0 .../authentication/google/conftest.py | 20 ---- .../google/test_auth_google_creation.py | 39 ------- .../test_auth_identity_keycloak_creation.py | 41 ++++++- .../authentication/keycloak/__init__.py | 0 .../authentication/keycloak/conftest.py | 23 ---- .../keycloak/test_auth_keycloak_creation.py | 52 --------- .../creation/authentication/oidc/__init__.py | 0 .../creation/authentication/oidc/conftest.py | 22 ---- .../oidc/test_auth_oidc_creation.py | 34 ------ backend/tests/identity/test_identity_setup.py | 7 +- backend/tests/services/sites/test_sites.py | 8 +- .../authentication/test_authentication.py | 105 ++++++++++++++++++ backend/tests/utils/test_utils_scripts.py | 30 ++--- 28 files changed, 371 insertions(+), 416 deletions(-) create mode 100644 backend/news/604.breaking delete mode 100644 backend/tests/creation/authentication/github/__init__.py delete mode 100644 backend/tests/creation/authentication/github/conftest.py delete mode 100644 backend/tests/creation/authentication/github/test_auth_github_creation.py delete mode 100644 backend/tests/creation/authentication/google/__init__.py delete mode 100644 backend/tests/creation/authentication/google/conftest.py delete mode 100644 backend/tests/creation/authentication/google/test_auth_google_creation.py delete mode 100644 backend/tests/creation/authentication/keycloak/__init__.py delete mode 100644 backend/tests/creation/authentication/keycloak/conftest.py delete mode 100644 backend/tests/creation/authentication/keycloak/test_auth_keycloak_creation.py delete mode 100644 backend/tests/creation/authentication/oidc/__init__.py delete mode 100644 backend/tests/creation/authentication/oidc/conftest.py delete mode 100644 backend/tests/creation/authentication/oidc/test_auth_oidc_creation.py create mode 100644 backend/tests/utils/authentication/test_authentication.py diff --git a/backend/news/604.breaking b/backend/news/604.breaking new file mode 100644 index 00000000..aa2994cb --- /dev/null +++ b/backend/news/604.breaking @@ -0,0 +1 @@ +Removed the `keycloak`, `oidc`, `authomatic-github` and `authomatic-google` authentication providers from site creation: new sites use `pas.plugins.identity` through the `identity-keycloak`, `identity-oidc`, `identity-github` and `identity-google` providers, and creating a site with one of the old providers fails with `UnsupportedAuthenticationProvider`, naming its replacement. The shared schema definitions are now `identity-keycloak`, `identity-oidc`, `identity-github` and `identity-google`, so distributions have to update their `schema.json`. The `SITE_AUTHENTICATION_OIDC-*` and `SITE_AUTHENTICATION_AUTHOMATIC-*` environment variables are ignored with a warning, and the `setup_oidc_auth`, `setup_keycloak_auth` and `setup_authomatic` helpers are deprecated. Existing sites keep their `pas.plugins.oidc` and `pas.plugins.authomatic` configuration. @ericof diff --git a/backend/news/604.feature b/backend/news/604.feature index 3284042f..d7cd4e16 100644 --- a/backend/news/604.feature +++ b/backend/news/604.feature @@ -1 +1 @@ -Added `pas.plugins.identity` as an always installed dependency, with `Person` as the user content type: each user account is a `Person` in the new `/users` folder, while other `Person` objects stay plain content. An upgrade step installs the add-on on existing sites and creates a `Person` for each user in `source_users`. Site creation gains the `identity-keycloak`, `identity-oidc`, `identity-github` and `identity-google` authentication providers, configured with `identity-*` answers or `SITE_AUTHENTICATION_IDENTITY-*` environment variables, side by side with the existing `pas.plugins.oidc` and `pas.plugins.authomatic` providers. @ericof +Added `pas.plugins.identity` as an always installed dependency, with `Person` as the user content type: each user account is a `Person` in the new `/users` folder, while other `Person` objects stay plain content. An upgrade step installs the add-on on existing sites and creates a `Person` for each user in `source_users`. Site creation gains the `identity-keycloak`, `identity-oidc`, `identity-github` and `identity-google` authentication providers, configured with `identity-*` answers or `SITE_AUTHENTICATION_IDENTITY-*` environment variables. With `identity-keycloak`, the realm groups come from `pas.plugins.keycloakgroups`, and Keycloak users get their Keycloak user id (`sub`) as Plone userid. @ericof diff --git a/backend/src/kitconcept/core/patches/local_definitions.json b/backend/src/kitconcept/core/patches/local_definitions.json index 3969e507..4517ca96 100644 --- a/backend/src/kitconcept/core/patches/local_definitions.json +++ b/backend/src/kitconcept/core/patches/local_definitions.json @@ -5,31 +5,31 @@ "minItems": 1, "uniqueItems": true }, - "keycloak": { + "identity-keycloak": { "type": "object", "properties": { - "oidc-server_url": { + "identity-server_url": { "type": "string", "title": "Server URL" }, - "oidc-realm_name": { + "identity-realm_name": { "type": "string", "title": "Realm" }, - "oidc-client_id": { + "identity-client_id": { "type": "string", "title": "Client ID" }, - "oidc-client_secret": { + "identity-client_secret": { "type": "string", "title": "Client Secret" }, - "oidc-site-url": { + "identity-site-url": { "type": "string", "title": "Public URL of your site", - "description": "Please inform the address this site will be available." + "description": "Please inform the address this site will be available. Leave it empty to use the address of the request." }, - "oidc-scope": { + "identity-scope": { "$ref": "#/definitions/field-scope", "items": { "type": "string", @@ -42,35 +42,33 @@ } }, "required": [ - "oidc-server_url", - "oidc-realm_name", - "oidc-client_id", - "oidc-client_secret", - "oidc-site-url", - "oidc-scope" + "identity-server_url", + "identity-realm_name", + "identity-client_id", + "identity-client_secret" ] }, - "oidc": { + "identity-oidc": { "type": "object", "properties": { - "oidc-issuer": { + "identity-issuer": { "type": "string", "title": "Issuer (URL)" }, - "oidc-client_id": { + "identity-client_id": { "type": "string", "title": "Client ID" }, - "oidc-client_secret": { + "identity-client_secret": { "type": "string", "title": "Client Secret" }, - "oidc-site-url": { + "identity-site-url": { "type": "string", "title": "Public URL of your site", - "description": "Please inform the address this site will be available." + "description": "Please inform the address this site will be available. Leave it empty to use the address of the request." }, - "oidc-scope": { + "identity-scope": { "$ref": "#/definitions/field-scope", "items": { "type": "string", @@ -83,25 +81,28 @@ } }, "required": [ - "oidc-issuer", - "oidc-client_id", - "oidc-client_secret", - "oidc-site-url", - "oidc-scope" + "identity-issuer", + "identity-client_id", + "identity-client_secret" ] }, - "authomatic-github": { + "identity-github": { "type": "object", "properties": { - "authomatic-github-consumer_key": { + "identity-client_id": { "type": "string", - "title": "Consumer Key" + "title": "Client ID" + }, + "identity-client_secret": { + "type": "string", + "title": "Client Secret" }, - "authomatic-github-consumer_secret": { + "identity-site-url": { "type": "string", - "title": "Consumer Secret" + "title": "Public URL of your site", + "description": "Please inform the address this site will be available. Leave it empty to use the address of the request." }, - "authomatic-github-scope": { + "identity-scope": { "$ref": "#/definitions/field-scope", "items": { "type": "string", @@ -113,27 +114,32 @@ } }, "required": [ - "authomatic-github-consumer_key", - "authomatic-github-consumer_secret", - "authomatic-github-scope" + "identity-client_id", + "identity-client_secret" ] }, - "authomatic-google": { + "identity-google": { "type": "object", "properties": { - "authomatic-google-consumer_key": { + "identity-client_id": { "type": "string", - "title": "Consumer Key" + "title": "Client ID" }, - "authomatic-google-consumer_secret": { + "identity-client_secret": { "type": "string", - "title": "Consumer Secret" + "title": "Client Secret" }, - "authomatic-google-scope": { + "identity-site-url": { + "type": "string", + "title": "Public URL of your site", + "description": "Please inform the address this site will be available. Leave it empty to use the address of the request." + }, + "identity-scope": { "$ref": "#/definitions/field-scope", "items": { "type": "string", "enum": [ + "openid", "profile", "email" ] @@ -141,9 +147,8 @@ } }, "required": [ - "authomatic-google-consumer_key", - "authomatic-google-consumer_secret", - "authomatic-google-scope" + "identity-client_id", + "identity-client_secret" ] } -} \ No newline at end of file +} diff --git a/backend/src/kitconcept/core/testing/distributions/testing/schema.json b/backend/src/kitconcept/core/testing/distributions/testing/schema.json index 3306d7a4..aa367741 100644 --- a/backend/src/kitconcept/core/testing/distributions/testing/schema.json +++ b/backend/src/kitconcept/core/testing/distributions/testing/schema.json @@ -79,14 +79,10 @@ "default": "internal", "oneOf": [ { "const": "internal", "title": "Plone" }, - { "const": "keycloak", "title": "Keycloak" }, - { "const": "oidc", "title": "OIDC / OAuth2" }, - { "const": "authomatic-google", "title": "Social Login Google" }, - { "const": "authomatic-github", "title": "Social Login GitHub" }, - { "const": "identity-keycloak", "title": "Identity: Keycloak" }, - { "const": "identity-oidc", "title": "Identity: OIDC" }, - { "const": "identity-github", "title": "Identity: GitHub" }, - { "const": "identity-google", "title": "Identity: Google" } + { "const": "identity-keycloak", "title": "Keycloak" }, + { "const": "identity-oidc", "title": "OIDC" }, + { "const": "identity-github", "title": "GitHub" }, + { "const": "identity-google", "title": "Google" } ] } }, @@ -95,48 +91,48 @@ "if": { "properties": { "provider": { - "const": "keycloak" + "const": "identity-keycloak" } } }, "then": { - "$ref": "#/definitions/keycloak" + "$ref": "#/definitions/identity-keycloak" } }, { "if": { "properties": { "provider": { - "const": "oidc" + "const": "identity-oidc" } } }, "then": { - "$ref": "#/definitions/oidc" + "$ref": "#/definitions/identity-oidc" } }, { "if": { "properties": { "provider": { - "const": "authomatic-github" + "const": "identity-github" } } }, "then": { - "$ref": "#/definitions/authomatic-github" + "$ref": "#/definitions/identity-github" } }, { "if": { "properties": { "provider": { - "const": "authomatic-google" + "const": "identity-google" } } }, "then": { - "$ref": "#/definitions/authomatic-google" + "$ref": "#/definitions/identity-google" } }, { @@ -162,5 +158,8 @@ } }, "uischema": { + "default_language": { + "ui:widget": "hidden" + } } } \ No newline at end of file diff --git a/backend/src/kitconcept/core/utils/authentication/__init__.py b/backend/src/kitconcept/core/utils/authentication/__init__.py index 92634be2..e31be6f2 100644 --- a/backend/src/kitconcept/core/utils/authentication/__init__.py +++ b/backend/src/kitconcept/core/utils/authentication/__init__.py @@ -3,8 +3,42 @@ from plone import api from Products.GenericSetup.tool import SetupTool +import warnings -__all__ = ["setup_authentication"] + +__all__ = [ + "DEPRECATED_PROVIDERS", + "UnsupportedAuthenticationProvider", + "setup_authentication", +] + + +# Providers based on pas.plugins.oidc and pas.plugins.authomatic -> replacement +DEPRECATED_PROVIDERS: dict[str, str] = { + "keycloak": "identity-keycloak", + "oidc": "identity-oidc", + "authomatic-github": "identity-github", + "authomatic-google": "identity-google", +} + + +class UnsupportedAuthenticationProvider(ValueError): + """An authentication provider new sites can no longer use.""" + + +def warn_deprecated(name: str, provider: str): + """Warn that a setup helper of a deprecated provider was called. + + :param name: Name of the helper. + :param provider: Deprecated provider the helper sets up. + """ + replacement = DEPRECATED_PROVIDERS[provider] + message = ( + f"{name} is deprecated and will be removed in a future version: " + f"use the {replacement!r} authentication provider instead." + ) + warnings.warn(message, DeprecationWarning, stacklevel=3) + logger.warning(f"Authentication: {message}") def _install_profiles(profiles: tuple[str, ...]): @@ -19,30 +53,26 @@ def _setup_plone_auth(answers: t.AuthAnswers): def get_authentication_provider(provider: str) -> t.AuthenticationProvider: - from .authomatic import setup_authomatic + """Return the setup handler and profiles of an authentication provider. + + :param provider: Provider answer, e.g. ``identity-keycloak``. + :returns: The provider; ``internal`` for an unknown provider. + :raises UnsupportedAuthenticationProvider: For a provider based on + ``pas.plugins.oidc`` or ``pas.plugins.authomatic``. + """ from .identity import setup_identity - from .keycloak import setup_keycloak_auth - from .oidc import setup_oidc_auth + if provider in DEPRECATED_PROVIDERS: + replacement = DEPRECATED_PROVIDERS[provider] + raise UnsupportedAuthenticationProvider( + f"Authentication provider {provider!r} is no longer supported, " + f"use {replacement!r} instead." + ) providers = { "internal": t.AuthenticationProvider(_setup_plone_auth, ()), - "keycloak": t.AuthenticationProvider( - setup_keycloak_auth, - ( - "pas.plugins.oidc:default", - "pas.plugins.keycloakgroups:default", - ), - ), - "oidc": t.AuthenticationProvider( - setup_oidc_auth, ("pas.plugins.oidc:default",) + "identity-keycloak": t.AuthenticationProvider( + setup_identity, ("pas.plugins.keycloakgroups:default",) ), - "authomatic-github": t.AuthenticationProvider( - setup_authomatic, ("pas.plugins.authomatic:default",) - ), - "authomatic-google": t.AuthenticationProvider( - setup_authomatic, ("pas.plugins.authomatic:default",) - ), - "identity-keycloak": t.AuthenticationProvider(setup_identity, ()), "identity-oidc": t.AuthenticationProvider(setup_identity, ()), "identity-github": t.AuthenticationProvider(setup_identity, ()), "identity-google": t.AuthenticationProvider(setup_identity, ()), @@ -51,7 +81,12 @@ def get_authentication_provider(provider: str) -> t.AuthenticationProvider: def setup_authentication(answers: t.AuthAnswers): - """Setup authentication based on answers provided by the user.""" + """Setup authentication based on answers provided by the user. + + :param answers: Authentication answers. + :raises UnsupportedAuthenticationProvider: For a provider based on + ``pas.plugins.oidc`` or ``pas.plugins.authomatic``. + """ provider = answers.get("provider", "internal") auth_provider = get_authentication_provider(provider) # First install dependencies diff --git a/backend/src/kitconcept/core/utils/authentication/authomatic.py b/backend/src/kitconcept/core/utils/authentication/authomatic.py index 00b90029..c707416e 100644 --- a/backend/src/kitconcept/core/utils/authentication/authomatic.py +++ b/backend/src/kitconcept/core/utils/authentication/authomatic.py @@ -76,6 +76,16 @@ def _prepare_json_config(raw_answers: t.AnswersAuthomatic) -> dict: def setup_authomatic(raw_answers: t.AnswersAuthomatic): + """Configure pas.plugins.authomatic. + + Deprecated: use the ``identity-github`` or ``identity-google`` + authentication provider. + + :param raw_answers: Authentication answers. + """ + from kitconcept.core.utils.authentication import warn_deprecated + + warn_deprecated("setup_authomatic", raw_answers["provider"]) config = _prepare_json_config(raw_answers) data = json.dumps(config, indent=2) api.portal.set_registry_record(KEY, data) diff --git a/backend/src/kitconcept/core/utils/authentication/identity.py b/backend/src/kitconcept/core/utils/authentication/identity.py index 8f80f95d..825e345f 100644 --- a/backend/src/kitconcept/core/utils/authentication/identity.py +++ b/backend/src/kitconcept/core/utils/authentication/identity.py @@ -20,6 +20,15 @@ # Answers that are not driver settings NON_CONFIG_KEYS = ("site-url", "server_url", "realm_name") +# pas.plugins.keycloakgroups looks groups up by Keycloak user id, which is the +# ``sub`` claim, so Keycloak users get it as their Plone userid +KEYCLOAK_USERID_SOURCE = "subject" + +KEYCLOAK_GROUPS_PREFIX = "keycloak_groups" + +# Answers copied to the pas.plugins.keycloakgroups settings +KEYCLOAK_GROUPS_KEYS = ("server_url", "realm_name", "client_id", "client_secret") + def _prepare_answers(raw_answers: t.AnswersIdentity) -> dict: """Strip the answer prefix, and drop empty values. @@ -46,13 +55,29 @@ def _prepare_config(provider: str, answers: dict) -> dict: config = { key: value for key, value in answers.items() if key not in NON_CONFIG_KEYS } - if provider == "identity-keycloak" and "issuer" not in config: - server_url = answers["server_url"].rstrip("/") - realm = answers["realm_name"] - config["issuer"] = f"{server_url}/realms/{realm}" + if provider == "identity-keycloak": + if "issuer" not in config: + server_url = answers["server_url"].rstrip("/") + realm = answers["realm_name"] + config["issuer"] = f"{server_url}/realms/{realm}" + config.setdefault("userid_source", KEYCLOAK_USERID_SOURCE) return config +def _setup_keycloak_groups(answers: dict): + """Use the groups of the Keycloak realm, with pas.plugins.keycloakgroups. + + :param answers: Answers, as returned by :func:`_prepare_answers`. + """ + settings = {key: answers[key] for key in KEYCLOAK_GROUPS_KEYS if key in answers} + # Last, so the plugin is activated with the connection settings in place + settings["enabled"] = True + for key, value in settings.items(): + name = f"{KEYCLOAK_GROUPS_PREFIX}.{key}" + api.portal.set_registry_record(name, value) + logger.info(f"Authentication: Set registry {name}") + + def _callback_url(answers: dict) -> str | None: """Return the callback url for a site url answer, if any. @@ -86,4 +111,6 @@ def setup_identity(raw_answers: t.AnswersIdentity): if callback_url: api.portal.set_registry_record(CALLBACK_URL_RECORD, callback_url) logger.info(f"Authentication: Set registry value on {CALLBACK_URL_RECORD}") + if provider == "identity-keycloak": + _setup_keycloak_groups(answers) logger.info("Authentication: Identity authentication setup") diff --git a/backend/src/kitconcept/core/utils/authentication/keycloak.py b/backend/src/kitconcept/core/utils/authentication/keycloak.py index 292cb001..25d7beaf 100644 --- a/backend/src/kitconcept/core/utils/authentication/keycloak.py +++ b/backend/src/kitconcept/core/utils/authentication/keycloak.py @@ -37,8 +37,16 @@ def _setup_keycloak_groups(payload_groups: dict): def setup_keycloak_auth(raw_answers: t.AnswersKeycloak): + """Configure pas.plugins.oidc and pas.plugins.keycloakgroups. + + Deprecated: use the ``identity-keycloak`` authentication provider. + + :param raw_answers: Authentication answers. + """ + from kitconcept.core.utils.authentication import warn_deprecated from kitconcept.core.utils.authentication.oidc import _setup_oidc + warn_deprecated("setup_keycloak_auth", "keycloak") # Setup keycloak_groups payload_groups = _answers_to_keycloak_groups(raw_answers) _setup_keycloak_groups(payload_groups) diff --git a/backend/src/kitconcept/core/utils/authentication/oidc.py b/backend/src/kitconcept/core/utils/authentication/oidc.py index 18a82af4..207874b0 100644 --- a/backend/src/kitconcept/core/utils/authentication/oidc.py +++ b/backend/src/kitconcept/core/utils/authentication/oidc.py @@ -31,5 +31,14 @@ def _setup_oidc(raw_answers: t.AnswersOIDC): def setup_oidc_auth(raw_answers: t.AnswersOIDC): + """Configure pas.plugins.oidc. + + Deprecated: use the ``identity-oidc`` authentication provider. + + :param raw_answers: Authentication answers. + """ + from kitconcept.core.utils.authentication import warn_deprecated + + warn_deprecated("setup_oidc_auth", "oidc") _setup_oidc(raw_answers) logger.info("Authentication: OIDC authentication setup") diff --git a/backend/src/kitconcept/core/utils/scripts.py b/backend/src/kitconcept/core/utils/scripts.py index b1be42bc..2f8e3afc 100644 --- a/backend/src/kitconcept/core/utils/scripts.py +++ b/backend/src/kitconcept/core/utils/scripts.py @@ -18,6 +18,7 @@ import logging import os import transaction +import warnings marker = object() # Unique marker object to indicate no value @@ -63,47 +64,6 @@ def as_list(value: str) -> list[str]: ("setup_content", "SITE_SETUP_CONTENT", as_bool), ("demo_content", "SITE_DEMO_CONTENT", as_bool), ("authentication.provider", "SITE_AUTHENTICATION_PROVIDER", None), - ("authentication.oidc-server_url", "SITE_AUTHENTICATION_OIDC-SERVER_URL", None), - ("authentication.oidc-realm_name", "SITE_AUTHENTICATION_OIDC-REALM_NAME", None), - ("authentication.oidc-client_id", "SITE_AUTHENTICATION_OIDC-CLIENT_ID", None), - ( - "authentication.oidc-client_secret", - "SITE_AUTHENTICATION_OIDC-CLIENT_SECRET", - None, - ), - ("authentication.oidc-site-url", "SITE_AUTHENTICATION_OIDC-SITE-URL", None), - ("authentication.oidc-scope", "SITE_AUTHENTICATION_OIDC-SCOPE", as_list), - ("authentication.oidc-issuer", "SITE_AUTHENTICATION_OIDC-ISSUER", None), - ( - "authentication.authomatic-github-consumer_key", - "SITE_AUTHENTICATION_AUTHOMATIC-GITHUB-CONSUMER_KEY", - None, - ), - ( - "authentication.authomatic-github-consumer_secret", - "SITE_AUTHENTICATION_AUTHOMATIC-GITHUB-CONSUMER_SECRET", - None, - ), - ( - "authentication.authomatic-github-scope", - "SITE_AUTHENTICATION_AUTHOMATIC-GITHUB-SCOPE", - as_list, - ), - ( - "authentication.authomatic-google-consumer_key", - "SITE_AUTHENTICATION_AUTHOMATIC-GOOGLE-CONSUMER_KEY", - None, - ), - ( - "authentication.authomatic-google-consumer_secret", - "SITE_AUTHENTICATION_AUTHOMATIC-GOOGLE-CONSUMER_SECRET", - None, - ), - ( - "authentication.authomatic-google-scope", - "SITE_AUTHENTICATION_AUTHOMATIC-GOOGLE-SCOPE", - as_list, - ), ( "authentication.identity-server_url", "SITE_AUTHENTICATION_IDENTITY-SERVER_URL", @@ -137,6 +97,26 @@ def as_list(value: str) -> list[str]: ), ) +# Prefixes of the environment variables of the pas.plugins.oidc and +# pas.plugins.authomatic providers, which are no longer read +DEPRECATED_ENV_PREFIXES: tuple[str, ...] = ( + "SITE_AUTHENTICATION_OIDC-", + "SITE_AUTHENTICATION_AUTHOMATIC-", +) + + +def _warn_deprecated_env_vars() -> None: + """Warn about environment variables of providers no longer supported.""" + for env_var in sorted(os.environ): + if not env_var.startswith(DEPRECATED_ENV_PREFIXES): + continue + message = ( + f"{env_var} is ignored: use the SITE_AUTHENTICATION_IDENTITY-* " + "variables of the identity-* authentication providers instead." + ) + warnings.warn(message, DeprecationWarning, stacklevel=3) + logger.warning(message) + def parse_answers(answers_file: Path, answers_env: dict) -> dict: """Load site-creation answers from a JSON file and override them from the @@ -236,9 +216,14 @@ def get_environmental_variables( containing a ``.`` (e.g. ``authentication.provider``) are expanded into a nested dictionary. + Variables of the ``pas.plugins.oidc`` and ``pas.plugins.authomatic`` + providers (see :data:`DEPRECATED_ENV_PREFIXES`) are ignored, with a + warning. + :returns: Mapping of answer keys to their (optionally transformed) values, ready to be merged into the site-creation answers. """ + _warn_deprecated_env_vars() env_vars: dict[str, Any] = {} for key, env_var, transform in options: value = os.getenv(env_var, marker) diff --git a/backend/tests/creation/authentication/conftest.py b/backend/tests/creation/authentication/conftest.py index 327fae2d..fef14371 100644 --- a/backend/tests/creation/authentication/conftest.py +++ b/backend/tests/creation/authentication/conftest.py @@ -1,20 +1,6 @@ -from plone import api - -import json import pytest -KEY = "pas.plugins.authomatic.interfaces.IPasPluginsAuthomaticSettings.json_config" - - -@pytest.fixture(scope="session") -def authomatic_config(): - def func() -> dict: - return json.loads(api.portal.get_registry_record(KEY)) - - return func - - @pytest.fixture(scope="session") def identity_provider(): from pas.plugins.identity.api import provider diff --git a/backend/tests/creation/authentication/github/__init__.py b/backend/tests/creation/authentication/github/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/backend/tests/creation/authentication/github/conftest.py b/backend/tests/creation/authentication/github/conftest.py deleted file mode 100644 index 8ce4bf03..00000000 --- a/backend/tests/creation/authentication/github/conftest.py +++ /dev/null @@ -1,20 +0,0 @@ -import pytest - - -@pytest.fixture(scope="session") -def answers(): - return { - "site_id": "site", - "title": "Site", - "description": "Site created with A CMS solution for public websites. Created by kitconcept.", - "available_languages": ["en"], - "default_language": "en", - "portal_timezone": "Europe/Berlin", - "setup_content": True, - "authentication": { - "provider": "authomatic-github", - "authomatic-github-consumer_key": "gh-32510011", - "authomatic-github-consumer_secret": "12345678", - "authomatic-github-scope": ["read:user", "user:email"], - }, - } diff --git a/backend/tests/creation/authentication/github/test_auth_github_creation.py b/backend/tests/creation/authentication/github/test_auth_github_creation.py deleted file mode 100644 index 01d7a515..00000000 --- a/backend/tests/creation/authentication/github/test_auth_github_creation.py +++ /dev/null @@ -1,37 +0,0 @@ -import pytest - - -class TestSiteCreation: - @pytest.fixture(autouse=True) - def _setup(self, portal): - self.portal = portal - - @pytest.mark.parametrize( - "profile_id", - [ - "profile-pas.plugins.authomatic:default", - ], - ) - def test_profile_installed(self, profile_last_version, profile_id): - result = profile_last_version(profile_id) - assert isinstance(result, str) - assert result != "" - - @pytest.mark.parametrize( - "path,expected", - [ - ["github/propertymap/email", "email"], - ["github/propertymap/link", "home_page"], - ["github/propertymap/name", "fullname"], - ["github/propertymap/location", "location"], - ["github/propertymap/avatar_url", "portrait"], - ["github/propertymap/username", "github_username"], - ["github/consumer_key", "gh-32510011"], - ["github/consumer_secret", "12345678"], - ["github/scope", ["read:user", "user:email"]], - ["github/access_headers/User-Agent", "Plone (kitconcept.core)"], - ], - ) - def test_authomatic_settings(self, traverse, authomatic_config, path, expected): - config = authomatic_config() - assert traverse(config, path) == expected diff --git a/backend/tests/creation/authentication/google/__init__.py b/backend/tests/creation/authentication/google/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/backend/tests/creation/authentication/google/conftest.py b/backend/tests/creation/authentication/google/conftest.py deleted file mode 100644 index deb5395c..00000000 --- a/backend/tests/creation/authentication/google/conftest.py +++ /dev/null @@ -1,20 +0,0 @@ -import pytest - - -@pytest.fixture(scope="session") -def answers(): - return { - "site_id": "site", - "title": "Site", - "description": "Site created with A CMS solution for public websites. Created by kitconcept.", - "available_languages": ["en"], - "default_language": "en", - "portal_timezone": "Europe/Berlin", - "setup_content": True, - "authentication": { - "provider": "authomatic-google", - "authomatic-google-consumer_key": "google-12345", - "authomatic-google-consumer_secret": "12345678", - "authomatic-google-scope": ["profile", "email"], - }, - } diff --git a/backend/tests/creation/authentication/google/test_auth_google_creation.py b/backend/tests/creation/authentication/google/test_auth_google_creation.py deleted file mode 100644 index b5984831..00000000 --- a/backend/tests/creation/authentication/google/test_auth_google_creation.py +++ /dev/null @@ -1,39 +0,0 @@ -import pytest - - -class TestSiteCreation: - @pytest.fixture(autouse=True) - def _setup(self, portal): - self.portal = portal - - @pytest.mark.parametrize( - "profile_id", - [ - "profile-pas.plugins.authomatic:default", - ], - ) - def test_profile_installed(self, profile_last_version, profile_id): - result = profile_last_version(profile_id) - assert isinstance(result, str) - assert result != "" - - @pytest.mark.parametrize( - "path,expected", - [ - ["google/id", 1], - ["google/propertymap/email", "email"], - ["google/propertymap/link", "home_page"], - ["google/propertymap/name", "fullname"], - ["google/propertymap/picture", "portrait"], - ["google/consumer_key", "google-12345"], - ["google/consumer_secret", "12345678"], - ["google/scope", ["profile", "email"]], - [ - "google/access_headers/User-Agent", - "Plone (kitconcept.core)", - ], - ], - ) - def test_authomatic_settings(self, traverse, authomatic_config, path, expected): - config = authomatic_config() - assert traverse(config, path) == expected diff --git a/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py b/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py index 954234a5..30a13677 100644 --- a/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py +++ b/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py @@ -9,10 +9,10 @@ def _setup(self, portal, identity_provider): self.portal = portal self.provider = identity_provider("keycloak") - def test_profile_installed(self, profile_last_version): - result = profile_last_version("profile-pas.plugins.identity:default") - assert isinstance(result, str) - assert result != "" + def test_profile_installed(self): + setup_tool = self.portal.portal_setup + version = setup_tool.getLastVersionForProfile("pas.plugins.identity:default") + assert version != "unknown" @pytest.mark.parametrize("plugin_id", ["identity", "identity_profile"]) def test_plugin_created(self, plugin_id): @@ -36,11 +36,19 @@ def test_provider(self, attr, expected): ["client_id", "plone"], ["client_secret", "12345678"], ["scope", ("openid", "profile", "email")], + ["userid_source", "subject"], ], ) def test_provider_config(self, key, expected): assert self.provider.config[key] == expected + def test_userid_is_keycloak_user_id(self): + """The Keycloak user id survives userid minting unchanged.""" + from pas.plugins.identity.core.pas.plugin import mint_userid + + subject = "4f0c2b8e-9d2a-4c1e-8f3b-2a6d7e9c1b05" + assert mint_userid("subject", subject=subject) == subject + def test_callback_url(self): value = api.portal.get_registry_record("pas.plugins.identity.callback_url") assert value == "http://localhost:3000/login-identity" @@ -49,9 +57,32 @@ def test_callback_url(self): "profile_id", [ "pas.plugins.oidc:default", - "pas.plugins.keycloakgroups:default", "pas.plugins.authomatic:default", ], ) def test_legacy_profile_not_installed(self, setup_tool, profile_id): assert setup_tool.getLastVersionForProfile(profile_id) == "unknown" + + def test_keycloak_groups_installed(self, setup_tool): + profile_id = "pas.plugins.keycloakgroups:default" + assert setup_tool.getLastVersionForProfile(profile_id) != "unknown" + + @pytest.mark.parametrize( + "key,expected", + [ + ["enabled", True], + ["server_url", "http://localhost:8180"], + ["realm_name", "site"], + ["client_id", "plone"], + ["client_secret", "12345678"], + ], + ) + def test_keycloak_groups_settings(self, key, expected): + value = api.portal.get_registry_record(f"keycloak_groups.{key}") + assert value == expected + + def test_keycloak_groups_plugin_active(self): + from Products.PluggableAuthService.interfaces.plugins import IGroupsPlugin + + active = self.portal.acl_users.plugins.listPluginIds(IGroupsPlugin) + assert "groups_keycloak" in active diff --git a/backend/tests/creation/authentication/keycloak/__init__.py b/backend/tests/creation/authentication/keycloak/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/backend/tests/creation/authentication/keycloak/conftest.py b/backend/tests/creation/authentication/keycloak/conftest.py deleted file mode 100644 index fd6979a3..00000000 --- a/backend/tests/creation/authentication/keycloak/conftest.py +++ /dev/null @@ -1,23 +0,0 @@ -import pytest - - -@pytest.fixture(scope="session") -def answers(): - return { - "site_id": "site", - "title": "Site", - "description": "Site created with A CMS solution for public websites. Created by kitconcept.", - "available_languages": ["en"], - "default_language": "en", - "portal_timezone": "Europe/Berlin", - "setup_content": True, - "authentication": { - "provider": "keycloak", - "oidc-server_url": "http://localhost:8180", - "oidc-realm_name": "site", - "oidc-client_id": "plone", - "oidc-client_secret": "12345678", - "oidc-site-url": "http://localhost:3000", - "oidc-scope": ["openid", "profile", "email"], - }, - } diff --git a/backend/tests/creation/authentication/keycloak/test_auth_keycloak_creation.py b/backend/tests/creation/authentication/keycloak/test_auth_keycloak_creation.py deleted file mode 100644 index 06034050..00000000 --- a/backend/tests/creation/authentication/keycloak/test_auth_keycloak_creation.py +++ /dev/null @@ -1,52 +0,0 @@ -from plone import api - -import pytest - - -class TestSiteCreation: - @pytest.fixture(autouse=True) - def _setup(self, portal): - self.portal = portal - - @pytest.mark.parametrize( - "profile_id", - [ - "profile-pas.plugins.oidc:default", - "profile-pas.plugins.keycloakgroups:default", - ], - ) - def test_profile_installed(self, profile_last_version, profile_id): - result = profile_last_version(profile_id) - assert isinstance(result, str) - assert result != "" - - @pytest.mark.parametrize( - "attr,expected", - [ - ("issuer", "http://localhost:8180/realms/site"), - ("client_id", "plone"), - ("client_secret", "12345678"), - ("scope", ["openid", "profile", "email"]), - ["redirect_uris", ("http://localhost:3000/login-oidc/oidc",)], - ["create_restapi_ticket", True], - ], - ) - def test_oidc_settings(self, attr, expected): - plugin = self.portal.acl_users.oidc - value = getattr(plugin, attr, None) - assert value == expected - - @pytest.mark.parametrize( - "key,expected", - [ - ["enabled", True], - ["server_url", "http://localhost:8180"], - ["realm_name", "site"], - ["client_id", "plone"], - ["client_secret", "12345678"], - ], - ) - def test_keycloak_groups_settings(self, key, expected): - key = f"keycloak_groups.{key}" - value = api.portal.get_registry_record(key) - assert value == expected diff --git a/backend/tests/creation/authentication/oidc/__init__.py b/backend/tests/creation/authentication/oidc/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/backend/tests/creation/authentication/oidc/conftest.py b/backend/tests/creation/authentication/oidc/conftest.py deleted file mode 100644 index 9a4a6941..00000000 --- a/backend/tests/creation/authentication/oidc/conftest.py +++ /dev/null @@ -1,22 +0,0 @@ -import pytest - - -@pytest.fixture(scope="session") -def answers(): - return { - "site_id": "site", - "title": "Site", - "description": "Site created with A CMS solution for public websites. Created by kitconcept.", - "available_languages": ["en"], - "default_language": "en", - "portal_timezone": "Europe/Berlin", - "setup_content": True, - "authentication": { - "provider": "oidc", - "oidc-issuer": "http://localhost:8180/realms/site", - "oidc-client_id": "plone", - "oidc-client_secret": "12345678", - "oidc-site-url": "http://localhost:3000", - "oidc-scope": ["openid", "profile", "email"], - }, - } diff --git a/backend/tests/creation/authentication/oidc/test_auth_oidc_creation.py b/backend/tests/creation/authentication/oidc/test_auth_oidc_creation.py deleted file mode 100644 index 14274496..00000000 --- a/backend/tests/creation/authentication/oidc/test_auth_oidc_creation.py +++ /dev/null @@ -1,34 +0,0 @@ -import pytest - - -class TestSiteCreation: - @pytest.fixture(autouse=True) - def _setup(self, portal): - self.portal = portal - - @pytest.mark.parametrize( - "profile_id", - [ - "profile-pas.plugins.oidc:default", - ], - ) - def test_profile_installed(self, profile_last_version, profile_id): - result = profile_last_version(profile_id) - assert isinstance(result, str) - assert result != "" - - @pytest.mark.parametrize( - "attr,expected", - [ - ("issuer", "http://localhost:8180/realms/site"), - ("client_id", "plone"), - ("client_secret", "12345678"), - ("scope", ["openid", "profile", "email"]), - ["redirect_uris", ("http://localhost:3000/login-oidc/oidc",)], - ["create_restapi_ticket", True], - ], - ) - def test_oidc_settings(self, attr, expected): - plugin = self.portal.acl_users.oidc - value = getattr(plugin, attr, None) - assert value == expected diff --git a/backend/tests/identity/test_identity_setup.py b/backend/tests/identity/test_identity_setup.py index 011993e9..38c57013 100644 --- a/backend/tests/identity/test_identity_setup.py +++ b/backend/tests/identity/test_identity_setup.py @@ -11,10 +11,9 @@ class TestIdentitySetup: def _setup(self, portal): self.portal = portal - def test_profile_installed(self, profile_last_version): - result = profile_last_version(f"profile-{PREFIX}:default") - assert isinstance(result, str) - assert result != "" + def test_profile_installed(self): + setup_tool = self.portal.portal_setup + assert setup_tool.getLastVersionForProfile(f"{PREFIX}:default") != "unknown" @pytest.mark.parametrize("plugin_id", ["identity", "identity_profile"]) def test_plugin_created(self, plugin_id): diff --git a/backend/tests/services/sites/test_sites.py b/backend/tests/services/sites/test_sites.py index 90ee5ce3..472c6061 100644 --- a/backend/tests/services/sites/test_sites.py +++ b/backend/tests/services/sites/test_sites.py @@ -22,10 +22,10 @@ def test_get(self): "languages", "timezones", "field-scope", - "keycloak", - "oidc", - "authomatic-github", - "authomatic-google", + "identity-keycloak", + "identity-oidc", + "identity-github", + "identity-google", ), ) def test_get_add_definitions(self, key: str): diff --git a/backend/tests/utils/authentication/test_authentication.py b/backend/tests/utils/authentication/test_authentication.py new file mode 100644 index 00000000..15d7b77d --- /dev/null +++ b/backend/tests/utils/authentication/test_authentication.py @@ -0,0 +1,105 @@ +from kitconcept.core.utils import authentication as utils +from kitconcept.core.utils.authentication import authomatic +from kitconcept.core.utils.authentication import identity +from kitconcept.core.utils.authentication import keycloak +from kitconcept.core.utils.authentication import oidc + +import pytest + + +DEPRECATED = ( + ("keycloak", "identity-keycloak"), + ("oidc", "identity-oidc"), + ("authomatic-github", "identity-github"), + ("authomatic-google", "identity-google"), +) + + +class TestGetAuthenticationProvider: + @pytest.mark.parametrize( + "provider,profiles", + [ + ("identity-keycloak", ("pas.plugins.keycloakgroups:default",)), + ("identity-oidc", ()), + ("identity-github", ()), + ("identity-google", ()), + ], + ) + def test_identity(self, provider: str, profiles: tuple[str, ...]): + result = utils.get_authentication_provider(provider) + assert result.handler is identity.setup_identity + assert result.profiles == profiles + + @pytest.mark.parametrize("provider", ["internal", "unknown"]) + def test_internal(self, provider: str): + result = utils.get_authentication_provider(provider) + assert result.handler is utils._setup_plone_auth + assert result.profiles == () + + @pytest.mark.parametrize("provider,replacement", DEPRECATED) + def test_deprecated_refused(self, provider: str, replacement: str): + with pytest.raises( + utils.UnsupportedAuthenticationProvider, match=f"use '{replacement}'" + ): + utils.get_authentication_provider(provider) + + @pytest.mark.parametrize("provider,replacement", DEPRECATED) + def test_setup_authentication_refused(self, provider: str, replacement: str): + with pytest.raises(utils.UnsupportedAuthenticationProvider): + utils.setup_authentication({"provider": provider}) + + +class TestDeprecatedHelpers: + def test_setup_oidc_auth(self, monkeypatch): + monkeypatch.setattr(oidc, "_setup_oidc", lambda answers: None) + with pytest.deprecated_call(match="identity-oidc"): + oidc.setup_oidc_auth({"provider": "oidc"}) + + def test_setup_keycloak_auth(self, monkeypatch): + monkeypatch.setattr(oidc, "_setup_oidc", lambda answers: None) + monkeypatch.setattr(keycloak, "_setup_keycloak_groups", lambda payload: None) + answers = { + "provider": "keycloak", + "oidc-server_url": "http://localhost:8180", + "oidc-realm_name": "site", + } + with pytest.deprecated_call(match="identity-keycloak"): + keycloak.setup_keycloak_auth(answers) + + @pytest.mark.parametrize( + "provider,replacement", + [ + ("authomatic-github", "identity-github"), + ("authomatic-google", "identity-google"), + ], + ) + def test_setup_authomatic(self, monkeypatch, provider: str, replacement: str): + monkeypatch.setattr( + authomatic.api.portal, "set_registry_record", lambda key, value: None + ) + with pytest.deprecated_call(match=replacement): + authomatic.setup_authomatic({"provider": provider}) + + +class TestNewSite: + """A new site uses pas.plugins.identity, and none of the old plugins.""" + + @pytest.fixture(autouse=True) + def _setup(self, portal): + self.portal = portal + + @pytest.mark.parametrize( + "profile_id", + [ + "pas.plugins.oidc:default", + "pas.plugins.keycloakgroups:default", + "pas.plugins.authomatic:default", + ], + ) + def test_old_profile_not_installed(self, profile_id: str): + setup_tool = self.portal.portal_setup + assert setup_tool.getLastVersionForProfile(profile_id) == "unknown" + + @pytest.mark.parametrize("plugin_id", ["oidc", "authomatic"]) + def test_old_plugin_not_created(self, plugin_id: str): + assert plugin_id not in self.portal.acl_users.objectIds() diff --git a/backend/tests/utils/test_utils_scripts.py b/backend/tests/utils/test_utils_scripts.py index 68f4f5f0..9bb5f234 100644 --- a/backend/tests/utils/test_utils_scripts.py +++ b/backend/tests/utils/test_utils_scripts.py @@ -131,18 +131,20 @@ def test_as_bool_transform(self, monkeypatch, value: str, expected: bool): is expected ) - def test_nested_authentication_keys(self, monkeypatch): - monkeypatch.setenv("SITE_AUTHENTICATION_PROVIDER", "oidc") - monkeypatch.setenv( - "SITE_AUTHENTICATION_OIDC-SERVER_URL", "https://auth.example.com" - ) - monkeypatch.setenv("SITE_AUTHENTICATION_OIDC-SCOPE", "openid, email") - result = scripts.get_environmental_variables(self.options) - assert result["authentication"] == { - "provider": "oidc", - "oidc-server_url": "https://auth.example.com", - "oidc-scope": ["openid", "email"], - } + @pytest.mark.parametrize( + "env_var", + ( + "SITE_AUTHENTICATION_OIDC-SERVER_URL", + "SITE_AUTHENTICATION_AUTHOMATIC-GITHUB-CONSUMER_KEY", + ), + ) + def test_deprecated_authentication_keys(self, monkeypatch, env_var: str): + """Variables of the old providers are ignored, with a warning.""" + monkeypatch.setenv("SITE_AUTHENTICATION_PROVIDER", "identity-oidc") + monkeypatch.setenv(env_var, "value") + with pytest.deprecated_call(match=env_var): + result = scripts.get_environmental_variables(self.options) + assert result["authentication"] == {"provider": "identity-oidc"} def test_nested_identity_keys(self, monkeypatch): monkeypatch.setenv("SITE_AUTHENTICATION_PROVIDER", "identity-keycloak") @@ -163,10 +165,10 @@ def test_nested_identity_keys(self, monkeypatch): def test_mixed_plain_and_nested(self, monkeypatch): monkeypatch.setenv("SITE_ID", "Plone") - monkeypatch.setenv("SITE_AUTHENTICATION_PROVIDER", "oidc") + monkeypatch.setenv("SITE_AUTHENTICATION_PROVIDER", "identity-oidc") result = scripts.get_environmental_variables(self.options) assert result["site_id"] == "Plone" - assert result["authentication"] == {"provider": "oidc"} + assert result["authentication"] == {"provider": "identity-oidc"} class TestCreateSite: From 7685d187f67ccc0d57c2f1aca81431a11bf9a2bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Wed, 30 Sep 2026 13:47:33 -0300 Subject: [PATCH 4/6] Limit ALLOWED_DISTRIBUTIONS --- backend/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/Makefile b/backend/Makefile index 33381706..c028e46c 100644 --- a/backend/Makefile +++ b/backend/Makefile @@ -37,6 +37,8 @@ BIN_FOLDER=$(VENV_FOLDER)/bin # Environment variables to be exported export PYTHONWARNINGS := ignore export DOCKER_BUILDKIT := 1 +export ALLOWED_DISTRIBUTIONS := testing + all: build From 7060586390baadd522e1f56a353b670127316e0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Wed, 30 Sep 2026 14:44:34 -0300 Subject: [PATCH 5/6] Keep the Person full name a provider wrote pas.plugins.identity writes a name claim only over the value it wrote itself. Person.fullname returned the Person title, which broke that in two ways: - A Person without names has the title "None" (collective.person formats a missing first name that way), so the first login never wrote the name - The title utility does not round-trip the written value (last_first gives "Hopper, Grace", extra spaces collapse), so no later login could update the name The getter now returns exactly the full name last written while the first and last name still split from it, and the names joined once they are edited, so a provider keeps a name it owns and leaves an edited one alone. Refs: https://gitlab.kitconcept.io/kitconcept/distribution-kitconcept-intranet/-/issues/604 --- backend/src/kitconcept/core/content/person.py | 21 +++++- backend/tests/identity/test_identity_users.py | 74 +++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/backend/src/kitconcept/core/content/person.py b/backend/src/kitconcept/core/content/person.py index 7e692312..ed1a50ec 100644 --- a/backend/src/kitconcept/core/content/person.py +++ b/backend/src/kitconcept/core/content/person.py @@ -82,11 +82,25 @@ def userid(self, value: str) -> None: @property def fullname(self) -> str: - """Return the full name, which is the Person title. + """Return the full name. - :returns: The full name. + pas.plugins.identity writes a name claim only over the value it wrote + itself, so this returns exactly the full name last written, while the + first and last name still split from it. After they are edited, it + returns them joined, and the provider no longer overwrites them. + + Not the Person title: the title utility of collective.person may + order the names differently (``last_first``), and formats a missing + first name as ``"None"``. + + :returns: The full name, empty when there are no names. """ - return self.title or "" + first_name = getattr(self, "first_name", None) or "" + last_name = getattr(self, "last_name", None) or "" + written = getattr(self, "_fullname", "") + if written and split_fullname(written) == (first_name, last_name): + return written + return " ".join(name for name in (first_name, last_name) if name) @fullname.setter def fullname(self, value: str) -> None: @@ -99,6 +113,7 @@ def fullname(self, value: str) -> None: return self.first_name = first_name self.last_name = last_name + self._fullname = value @property def emails(self) -> tuple[str, ...]: diff --git a/backend/tests/identity/test_identity_users.py b/backend/tests/identity/test_identity_users.py index a5883a0d..8a285071 100644 --- a/backend/tests/identity/test_identity_users.py +++ b/backend/tests/identity/test_identity_users.py @@ -119,6 +119,80 @@ def test_emails_optional(self): assert self.person.emails == () +class TestPersonFullname: + @pytest.fixture(autouse=True) + def _setup(self, portal): + self.portal = portal + + def _create(self, **names): + from pas.plugins.identity.core.container import get_container + + with api.env.adopt_roles(["Manager"]): + return api.content.create( + container=get_container(create=True), + type="Person", + id="sso-user", + login="sso-user", + **names, + ) + + @pytest.mark.parametrize( + "names,expected", + [ + [{}, ""], + [{"last_name": "Hopper"}, "Hopper"], + [{"first_name": "Grace"}, "Grace"], + [{"first_name": "Grace", "last_name": "Hopper"}, "Grace Hopper"], + ], + ) + def test_fullname(self, names: dict, expected: str): + assert self._create(**names).fullname == expected + + def test_name_claim_synced(self): + """A login writes the name claim onto a Person created without names.""" + from pas.plugins.identity.core.profiles import sync_claims + + person = self._create() + with api.env.adopt_roles(["Manager"]): + changed = sync_claims(person, {"fullname": "Grace Hopper"}, "") + assert changed == ["fullname"] + assert (person.first_name, person.last_name) == ("Grace", "Hopper") + + @pytest.mark.parametrize( + "utility,first,second", + [ + ("first_last", "Grace Hopper", "Grace M. Hopper"), + ("last_first", "Grace Hopper", "Grace M. Hopper"), + ("first_last", "Grace Hopper", "Grace M. Hopper"), + ("first_last", "Grace", "Grace Hopper"), + ], + ) + def test_later_login_updates_name(self, utility: str, first: str, second: str): + """The name last written round-trips, so the provider still owns it.""" + from pas.plugins.identity.core.profiles import sync_claims + + api.portal.set_registry_record("person.title_utility", utility) + person = self._create() + with api.env.adopt_roles(["Manager"]): + sync_claims(person, {"fullname": first}, "") + assert person.fullname == first + changed = sync_claims(person, {"fullname": second}, "") + assert changed == ["fullname"] + assert person.fullname == second + + def test_edited_name_is_kept(self): + """A name edited on the Person is not overwritten by the next login.""" + from pas.plugins.identity.core.profiles import sync_claims + + person = self._create() + with api.env.adopt_roles(["Manager"]): + sync_claims(person, {"fullname": "Grace Hopper"}, "") + person.last_name = "Murray Hopper" + changed = sync_claims(person, {"fullname": "Grace B. Hopper"}, "") + assert changed == [] + assert person.fullname == "Grace Murray Hopper" + + class TestCreatePersonsForUsers: @pytest.fixture(autouse=True) def _setup(self, portal, create_source_user): From f197ef2fc5a0ed09b1fc5f8eccd22d4ac0d76c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Wed, 30 Sep 2026 15:55:07 -0300 Subject: [PATCH 6/6] Upgrade pas.plugins.identity to 1.0.0a13 1.0.0a13 creates the site's user type at login when the site marks its objects IUserProfile one by one (upstream #129), and lists users that only have a profile in api.user.get_users() and listMembers() (upstream #131). With Person as the user type, an SSO first login now creates the Person, and federated users are listed once. - Test that federated and source_users logins create a Person, that the name claim is synced and updated, and that every user is listed once while a Person without login is never listed Upstream: https://github.com/collective/pas-plugins-identity/pull/130 Refs: https://gitlab.kitconcept.io/kitconcept/distribution-kitconcept-intranet/-/issues/604 --- backend/pyproject.toml | 2 +- backend/tests/identity/test_identity_login.py | 147 ++++++++++++++++++ backend/uv.lock | 8 +- 3 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 backend/tests/identity/test_identity_login.py diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 0bb6d005..b5846be8 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -34,7 +34,7 @@ dependencies = [ "collective.honeypot", "collective.person==1.0.0b6", "pas.plugins.authomatic==2.0.0", - "pas.plugins.identity==1.0.0a12", + "pas.plugins.identity==1.0.0a13", "pas.plugins.keycloakgroups==1.0.0b1", "pas.plugins.oidc==2.0.0", ] diff --git a/backend/tests/identity/test_identity_login.py b/backend/tests/identity/test_identity_login.py new file mode 100644 index 00000000..8280bce2 --- /dev/null +++ b/backend/tests/identity/test_identity_login.py @@ -0,0 +1,147 @@ +"""A login creates the Person of a user, and every user is listed once.""" + +from pas.plugins.identity.core.interfaces import IUserProfile +from plone import api +from plone.app.testing import TEST_USER_PASSWORD +from Products.PlonePAS.events import UserLoggedInEvent +from zope.event import notify + +import pytest + + +# Normalized claims, as a driver hands them to the login subscribers +CLAIMS = { + "email": "sso@example.org", + "email_verified": True, + "fullname": "Grace Hopper", + "username": "ghopper", +} + + +@pytest.fixture +def listed(portal): + """Return the userids each way of listing users answers.""" + + def func() -> dict[str, list[str]]: + acl_users = portal.acl_users + return { + "searchUsers": [user["id"] for user in acl_users.searchUsers()], + "get_users": [user.getId() for user in api.user.get_users()], + "listMembers": [ + member.getId() for member in portal.portal_membership.listMembers() + ], + "getUserIds": list(acl_users.getUserIds()), + } + + return func + + +@pytest.fixture +def federated_login(): + """Sign in through a provider, as the identity plugin does.""" + from pas.plugins.identity.core.subscribers import _handle + + def func(userid: str = "sso-user", **claims): + _handle(userid, {**CLAIMS, **claims}, "keycloak", sign_in=True) + + return func + + +class TestFederatedLogin: + @pytest.fixture(autouse=True) + def _setup(self, portal, federated_login, listed): + self.portal = portal + self.listed = listed + self.login = federated_login + federated_login() + self.person = portal["users"]["sso-user"] + + def test_person_created(self): + assert self.person.portal_type == "Person" + assert IUserProfile.providedBy(self.person) + + def test_claims_synced(self): + assert (self.person.first_name, self.person.last_name) == ("Grace", "Hopper") + assert self.person.email == "sso@example.org" + + def test_user_properties(self): + user = api.user.get(userid="sso-user") + assert user.getProperty("fullname") == "Grace Hopper" + + def test_later_login_updates_name(self): + self.login(fullname="Grace M. Hopper") + assert self.person.fullname == "Grace M. Hopper" + + def test_later_login_keeps_one_person(self): + self.login() + assert list(self.portal["users"].objectIds()).count("sso-user") == 1 + + @pytest.mark.parametrize( + "path", ["searchUsers", "get_users", "listMembers", "getUserIds"] + ) + def test_listed_once(self, path: str): + assert self.listed()[path].count("sso-user") == 1 + + +class TestUserInBothStores: + """A user with a source_users credential and a Person.""" + + @pytest.fixture(autouse=True) + def _setup(self, portal, listed): + self.portal = portal + self.listed = listed + with api.env.adopt_roles(["Manager"]): + api.user.create( + email="jdoe@example.com", + username="jdoe", + password=TEST_USER_PASSWORD, + properties={"fullname": "John Doe"}, + ) + + def test_both_stores(self): + assert "jdoe" in self.portal.acl_users.source_users.getUserIds() + assert IUserProfile.providedBy(self.portal["users"]["jdoe"]) + + @pytest.mark.parametrize("path", ["get_users", "listMembers", "getUserIds"]) + def test_listed_once(self, path: str): + assert self.listed()[path].count("jdoe") == 1 + + +class TestSourceUsersLogin: + """A user only in source_users gets a Person when signing in.""" + + @pytest.fixture(autouse=True) + def _setup(self, portal, listed): + self.portal = portal + self.listed = listed + acl_users = portal.acl_users + acl_users.source_users.addUser("legacy", "legacy", TEST_USER_PASSWORD) + notify(UserLoggedInEvent(acl_users.getUserById("legacy"))) + + def test_person_created(self): + assert IUserProfile.providedBy(self.portal["users"]["legacy"]) + + def test_listed_once(self): + assert self.listed()["get_users"].count("legacy") == 1 + + +class TestPersonWithoutLogin: + """A Person that is not an account, like a team page, is not a user.""" + + @pytest.fixture(autouse=True) + def _setup(self, portal, listed): + self.listed = listed + with api.env.adopt_roles(["Manager"]): + self.person = api.content.create( + container=portal, + type="Person", + title="Ada Team", + first_name="Ada", + last_name="Team", + ) + + @pytest.mark.parametrize( + "path", ["searchUsers", "get_users", "listMembers", "getUserIds"] + ) + def test_not_listed(self, path: str): + assert self.person.getId() not in self.listed()[path] diff --git a/backend/uv.lock b/backend/uv.lock index cd9e7609..f2167cac 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -1423,7 +1423,7 @@ requires-dist = [ { name = "horse-with-no-namespace", marker = "extra == 'test'" }, { name = "kitconcept-voltolighttheme", specifier = ">=8.0.0" }, { name = "pas-plugins-authomatic", specifier = "==2.0.0" }, - { name = "pas-plugins-identity", specifier = "==1.0.0a12" }, + { name = "pas-plugins-identity", specifier = "==1.0.0a13" }, { name = "pas-plugins-keycloakgroups", specifier = "==1.0.0b1" }, { name = "pas-plugins-oidc", specifier = "==2.0.0" }, { name = "plone-api" }, @@ -1776,7 +1776,7 @@ wheels = [ [[package]] name = "pas-plugins-identity" -version = "1.0.0a12" +version = "1.0.0a13" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "authlib" }, @@ -1789,9 +1789,9 @@ dependencies = [ { name = "products-cmfplone" }, { name = "requests" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/70/a0/5e7090746432e635a91ae617b18715b595e3abeeb8607958604683985902/pas_plugins_identity-1.0.0a12.tar.gz", hash = "sha256:d46050c5b9fba9816320487c6bcdcd3f2f434a8b13efbd95cb0649c004be2586", size = 1084868, upload-time = "2026-09-30T14:45:31.166Z" } +sdist = { url = "https://files.pythonhosted.org/packages/74/dc/500d4ffedd19299d1cbf59a90268706427db290ed4547874e4dc358a4fb9/pas_plugins_identity-1.0.0a13.tar.gz", hash = "sha256:066a4e752dada947100522d35bea688e943f8feb5916938dc10f4a3cb390291d", size = 1089951, upload-time = "2026-09-30T18:42:53.754Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/a9/47/bb8e43d2e8c76e0acdb8c460cd8086309b4148d7899b72c34e942d47df3d/pas_plugins_identity-1.0.0a12-py3-none-any.whl", hash = "sha256:411dcfdea80e4a652b2ddd681ffb80a6866fa966ad8928972909413d34f01698", size = 728649, upload-time = "2026-09-30T14:45:29.167Z" }, + { url = "https://files.pythonhosted.org/packages/7a/57/5a3d790c23a7ab0803f2053a2b650ddcd0b90ccd5fa8275e30eacf2b3931/pas_plugins_identity-1.0.0a13-py3-none-any.whl", hash = "sha256:4fce3df52e9621898fc64b1a9fc1fd0f0eb37f41f2fade1fca6bef990a55e92b", size = 731301, upload-time = "2026-09-30T18:42:51.583Z" }, ] [[package]]