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 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 new file mode 100644 index 00000000..d7cd4e16 --- /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. 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/pyproject.toml b/backend/pyproject.toml index 7ce5fc80..b5846be8 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.0a13", "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. + + 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. + """ + 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: + """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 + self._fullname = value + + @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..3612e858 --- /dev/null +++ b/backend/src/kitconcept/core/patches/identity.py @@ -0,0 +1,25 @@ +"""Give ``Person`` the user attributes of ``pas.plugins.identity``. + +``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 logging import Logger + + +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_person(logger) 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/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.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/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..aa367741 100644 --- a/backend/src/kitconcept/core/testing/distributions/testing/schema.json +++ b/backend/src/kitconcept/core/testing/distributions/testing/schema.json @@ -79,10 +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": "Keycloak" }, + { "const": "identity-oidc", "title": "OIDC" }, + { "const": "identity-github", "title": "GitHub" }, + { "const": "identity-google", "title": "Google" } ] } }, @@ -91,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" } }, { @@ -158,5 +158,8 @@ } }, "uischema": { + "default_language": { + "ui:widget": "hidden" + } } } \ No newline at end of file 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..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,34 +53,40 @@ def _setup_plone_auth(answers: t.AuthAnswers): def get_authentication_provider(provider: str) -> t.AuthenticationProvider: - from .authomatic import setup_authomatic - from .keycloak import setup_keycloak_auth - from .oidc import setup_oidc_auth + """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 + 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",) - ), - "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, ("pas.plugins.keycloakgroups:default",) ), + "identity-oidc": t.AuthenticationProvider(setup_identity, ()), + "identity-github": t.AuthenticationProvider(setup_identity, ()), + "identity-google": t.AuthenticationProvider(setup_identity, ()), } return providers.get(provider, providers["internal"]) 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 new file mode 100644 index 00000000..825e345f --- /dev/null +++ b/backend/src/kitconcept/core/utils/authentication/identity.py @@ -0,0 +1,116 @@ +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") + +# 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. + + :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": + 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. + + :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}") + 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 96da37fc..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,49 +64,59 @@ 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", + "authentication.identity-server_url", + "SITE_AUTHENTICATION_IDENTITY-SERVER_URL", 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", + "authentication.identity-realm_name", + "SITE_AUTHENTICATION_IDENTITY-REALM_NAME", None, ), + ("authentication.identity-issuer", "SITE_AUTHENTICATION_IDENTITY-ISSUER", None), ( - "authentication.authomatic-github-consumer_secret", - "SITE_AUTHENTICATION_AUTHOMATIC-GITHUB-CONSUMER_SECRET", + "authentication.identity-client_id", + "SITE_AUTHENTICATION_IDENTITY-CLIENT_ID", None, ), ( - "authentication.authomatic-github-scope", - "SITE_AUTHENTICATION_AUTHOMATIC-GITHUB-SCOPE", - as_list, - ), - ( - "authentication.authomatic-google-consumer_key", - "SITE_AUTHENTICATION_AUTHOMATIC-GOOGLE-CONSUMER_KEY", + "authentication.identity-client_secret", + "SITE_AUTHENTICATION_IDENTITY-CLIENT_SECRET", None, ), ( - "authentication.authomatic-google-consumer_secret", - "SITE_AUTHENTICATION_AUTHOMATIC-GOOGLE-CONSUMER_SECRET", + "authentication.identity-site-url", + "SITE_AUTHENTICATION_IDENTITY-SITE-URL", None, ), ( - "authentication.authomatic-google-scope", - "SITE_AUTHENTICATION_AUTHOMATIC-GOOGLE-SCOPE", + "authentication.identity-scope", + "SITE_AUTHENTICATION_IDENTITY-SCOPE", as_list, ), ) +# 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 @@ -205,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/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..fef14371 100644 --- a/backend/tests/creation/authentication/conftest.py +++ b/backend/tests/creation/authentication/conftest.py @@ -1,15 +1,11 @@ -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)) +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/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/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/github/__init__.py b/backend/tests/creation/authentication/identity_github/__init__.py similarity index 100% rename from backend/tests/creation/authentication/github/__init__.py rename to backend/tests/creation/authentication/identity_github/__init__.py diff --git a/backend/tests/creation/authentication/google/conftest.py b/backend/tests/creation/authentication/identity_github/conftest.py similarity index 64% rename from backend/tests/creation/authentication/google/conftest.py rename to backend/tests/creation/authentication/identity_github/conftest.py index deb5395c..424ce3b6 100644 --- a/backend/tests/creation/authentication/google/conftest.py +++ b/backend/tests/creation/authentication/identity_github/conftest.py @@ -12,9 +12,8 @@ def answers(): "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"], + "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/google/__init__.py b/backend/tests/creation/authentication/identity_google/__init__.py similarity index 100% rename from backend/tests/creation/authentication/google/__init__.py rename to backend/tests/creation/authentication/identity_google/__init__.py diff --git a/backend/tests/creation/authentication/github/conftest.py b/backend/tests/creation/authentication/identity_google/conftest.py similarity index 64% rename from backend/tests/creation/authentication/github/conftest.py rename to backend/tests/creation/authentication/identity_google/conftest.py index 8ce4bf03..4377ecbd 100644 --- a/backend/tests/creation/authentication/github/conftest.py +++ b/backend/tests/creation/authentication/identity_google/conftest.py @@ -12,9 +12,9 @@ def answers(): "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"], + "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/keycloak/__init__.py b/backend/tests/creation/authentication/identity_keycloak/__init__.py similarity index 100% rename from backend/tests/creation/authentication/keycloak/__init__.py rename to backend/tests/creation/authentication/identity_keycloak/__init__.py diff --git a/backend/tests/creation/authentication/keycloak/conftest.py b/backend/tests/creation/authentication/identity_keycloak/conftest.py similarity index 54% rename from backend/tests/creation/authentication/keycloak/conftest.py rename to backend/tests/creation/authentication/identity_keycloak/conftest.py index fd6979a3..899360ae 100644 --- a/backend/tests/creation/authentication/keycloak/conftest.py +++ b/backend/tests/creation/authentication/identity_keycloak/conftest.py @@ -12,12 +12,12 @@ def answers(): "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"], + "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..30a13677 --- /dev/null +++ b/backend/tests/creation/authentication/identity_keycloak/test_auth_identity_keycloak_creation.py @@ -0,0 +1,88 @@ +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): + 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): + 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")], + ["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" + + @pytest.mark.parametrize( + "profile_id", + [ + "pas.plugins.oidc: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/oidc/__init__.py b/backend/tests/creation/authentication/identity_oidc/__init__.py similarity index 100% rename from backend/tests/creation/authentication/oidc/__init__.py rename to backend/tests/creation/authentication/identity_oidc/__init__.py diff --git a/backend/tests/creation/authentication/oidc/conftest.py b/backend/tests/creation/authentication/identity_oidc/conftest.py similarity index 59% rename from backend/tests/creation/authentication/oidc/conftest.py rename to backend/tests/creation/authentication/identity_oidc/conftest.py index 9a4a6941..7a8a1fa7 100644 --- a/backend/tests/creation/authentication/oidc/conftest.py +++ b/backend/tests/creation/authentication/identity_oidc/conftest.py @@ -12,11 +12,10 @@ def answers(): "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"], + "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/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/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/__init__.py b/backend/tests/identity/__init__.py new file mode 100644 index 00000000..e69de29b 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/tests/identity/test_identity_setup.py b/backend/tests/identity/test_identity_setup.py new file mode 100644 index 00000000..38c57013 --- /dev/null +++ b/backend/tests/identity/test_identity_setup.py @@ -0,0 +1,67 @@ +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): + 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): + 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", None], + ["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 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 user_portal_type() == "Person" + + def test_catalogued_types(self): + from pas.plugins.identity.core.principal_types import catalogued_types + + assert 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..8a285071 --- /dev/null +++ b/backend/tests/identity/test_identity_users.py @@ -0,0 +1,224 @@ +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 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): + 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 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): + 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/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/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_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/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..9bb5f234 100644 --- a/backend/tests/utils/test_utils_scripts.py +++ b/backend/tests/utils/test_utils_scripts.py @@ -131,25 +131,44 @@ 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") + @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") monkeypatch.setenv( - "SITE_AUTHENTICATION_OIDC-SERVER_URL", "https://auth.example.com" + "SITE_AUTHENTICATION_IDENTITY-SERVER_URL", "https://auth.example.com" ) - monkeypatch.setenv("SITE_AUTHENTICATION_OIDC-SCOPE", "openid, email") + 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": "oidc", - "oidc-server_url": "https://auth.example.com", - "oidc-scope": ["openid", "email"], + "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") + 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: diff --git a/backend/uv.lock b/backend/uv.lock index 318c2027..f2167cac 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.0a13" }, { 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.0a13" +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/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/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]] name = "pas-plugins-keycloakgroups" version = "1.0.0b1"