Create marked-per-object user types, and list users who only have a profile

backend/news/129.bugfix

@@ -0,0 +1 @@
+Created the site's own user type again at a first login when the type is a user and a subscriber of the site marks each account `IUserProfile` as it is added, as kitconcept-core does for `collective.person`'s `Person`. Since 1.0.0a12 a login asked the type for `IUserProfile` and refused such a type, so federated and `source_users` logins created no profile and synced no claims. A login now asks the type for `IUserContent`, as the user adder does, and asks the object it created for `IUserProfile`. An object left unmarked is removed again, so the next login does not collide with its id, and a user type that the principals container does not allow is declined instead of failing inside the login, and without creating the container first. @ericof

backend/news/131.bugfix

@@ -0,0 +1 @@
+Listed users who have a profile and no `source_users` account, such as everyone who signed in through a provider, in `api.user.get_users()`, `portal_membership.listMembers()` and `acl_users.getUserIds()`, `getUserNames()` and `getUsers()`. Searches found them, but listings did not, because the `identity_profile` plugin did not implement `IUserIntrospection`. It now does, for the users in an enumeration-active state that no other introspection plugin lists, so a user with both a profile and a `source_users` account is still listed once. Upgrade step 1010 activates the interface on existing sites. @ericof

backend/src/pas/plugins/identity/core/pas/profile.py

@@ -47,6 +47,7 @@
 from plone import api
 from Products.PlonePAS.interfaces.capabilities import IDeleteCapability
 from Products.PlonePAS.interfaces.group import IGroupIntrospection
+from Products.PlonePAS.interfaces.plugins import IUserIntrospection
 from Products.PlonePAS.interfaces.plugins import IUserManagement
 from Products.PlonePAS.plugins.group import PloneGroup
 from Products.PlonePAS.sheet import MutablePropertySheet
@@ -143,6 +144,7 @@ def _matches(candidate: str | None, terms: list[str], exact: bool) -> bool:
     IGroupsPlugin,
     IGroupEnumerationPlugin,
     IGroupIntrospection,
+    IUserIntrospection,
 )
 class IdentityProfilePlugin(BasePlugin):
     """Serves properties, user enumeration and groups from the catalog."""
@@ -508,6 +510,55 @@ def _brain_matches(
             for attribute, terms in criteria
         )
 
+    # -- IUserIntrospection ----------------------------------------------
+    #
+    # "Every user", as opposed to "the users matching this". PAS answers
+    # ``getUserIds``, ``getUserNames`` and ``getUsers`` -- and through them
+    # ``listMembers`` and ``api.user.get_users`` -- by concatenating what each
+    # introspector says, and never removes a duplicate. A user added through
+    # ``api.user.create`` has a Profile and a ``source_users`` credential, so
+    # listing every Profile would list that user twice. These list the users
+    # no other introspector holds, which is every user exactly once whichever
+    # order the plugins are in.
+
+    def _introspected_brains(self) -> list[AbstractCatalogBrain]:
+        """Return the active Profile brains no other introspector lists.
+
+        :returns: Brains, in catalog order.
+        """
+        held: set[str] = set()
+        plugins = self._getPAS()._getOb("plugins")
+        for introspector_id, introspector in plugins.listPlugins(IUserIntrospection):
+            if introspector_id != self.getId():
+                held.update(introspector.getUserIds())
+        return [brain for brain in self._active_brains() if brain.userid not in held]
+
+    def getUserIds(self) -> list[str]:
+        """Return the ids of the users only this plugin holds.
+
+        :returns: Userids.
+        """
+        return [brain.userid for brain in self._introspected_brains()]
+
+    def getUserNames(self) -> list[str]:
+        """Return the login names of the users only this plugin holds.
+
+        :returns: Login names.
+        """
+        return [brain.login for brain in self._introspected_brains()]
+
+    def getUsers(self) -> list[PropertiedUser]:
+        """Return the users only this plugin holds, resolved by PAS.
+
+        Through ``getUserById``, as ``source_users`` does, so each one carries
+        the property sheets, groups and roles every plugin gives it.
+
+        :returns: User objects.
+        """
+        pas = self._getPAS()
+        users = (pas.getUserById(userid) for userid in self.getUserIds())
+        return [user for user in users if user is not None]
+
     # -- groups -------------------------------------------------
 
     def _group_states(self) -> tuple[str, ...]:
@@ -1041,6 +1092,7 @@ def allowDeletePrincipal(self, principal_id: str) -> bool:
     IGroupsPlugin,
     IGroupEnumerationPlugin,
     IGroupIntrospection,
+    IUserIntrospection,
     IUserManagement,
     IDeleteCapability,
 )

backend/src/pas/plugins/identity/core/profiles.py

@@ -60,8 +60,10 @@
 from pas.plugins.identity import logger
 from pas.plugins.identity.core.catalog import query_catalog
 from pas.plugins.identity.core.container import get_container
+from pas.plugins.identity.core.container import settings as container_settings
 from pas.plugins.identity.core.contents.profile import UserProfile
 from pas.plugins.identity.core.interfaces import Claims
+from pas.plugins.identity.core.interfaces import IUserContent
 from pas.plugins.identity.core.interfaces import IUserProfile
 from pas.plugins.identity.core.principal_types import type_provides
 from pas.plugins.identity.core.principal_types import user_portal_type
@@ -166,6 +168,28 @@ def get_profile(userid: str) -> UserProfile | None:
     return brain._unrestrictedGetObject() if brain is not None else None
 
 
+def _container_allows(portal_type: str) -> bool:
+    """Report whether the Profile container will take a user type.
+
+    Asked before the container is created, so that a login which is going to
+    decline leaves no folder behind. The container's own FTI when it exists,
+    which may be a type this package did not create; otherwise the FTI of the
+    type :func:`~pas.plugins.identity.core.container.get_container` would
+    create it as.
+
+    :param portal_type: The user type.
+    :returns: Whether an object of that type may be added there.
+    """
+    container = get_container()
+    if container is not None:
+        fti = container.getTypeInfo()
+    else:
+        fti = api.portal.get_tool("portal_types").getTypeInfo(
+            container_settings()["type"]
+        )
+    return fti is not None and bool(fti.allowType(portal_type))
+
+
 def ensure_profile(userid: str, login: str, claims: Claims) -> UserProfile | None:
     """Return the user's Profile, creating it on first login.
 
@@ -176,8 +200,9 @@ def ensure_profile(userid: str, login: str, claims: Claims) -> UserProfile | Non
     :param userid: Canonical Plone userid.
     :param login: Login name to record.
     :param claims: Normalized claims to seed from.
-    :returns: The Profile, or ``None`` when the layer is not installed here or
-        the site's user type is not one it can catalogue.
+    :returns: The Profile, or ``None`` when the layer is not installed here,
+        the site's user type is not a user, or the object created is not one
+        it can catalogue.
     """
     if query_catalog() is None:
         return None
@@ -187,13 +212,22 @@ def ensure_profile(userid: str, login: str, claims: Claims) -> UserProfile | Non
         return profile
 
     portal_type = user_portal_type()
-    if not type_provides(portal_type, IUserProfile):
-        # The site keeps its users as a type this layer cannot catalogue, so
-        # the object is somebody else's to create. Creating one anyway would
-        # be a second record of the same person that nothing here could find,
-        # and falling back to ``UserProfile`` was exactly that duplicate.
+    if not type_provides(portal_type, IUserContent):
+        # The question ``doAddUser`` asks. A record naming a type that is not
+        # a user at all is a misconfiguration, and the object is somebody
+        # else's to create.
+        logger.info(
+            "Not creating a profile for %s: %r does not provide IUserContent",
+            userid,
+            portal_type,
+        )
+        return None
+
+    if not _container_allows(portal_type):
+        # A user type the site files somewhere else, and creates itself.
+        # Creating it here would fail inside a login rather than decline.
         logger.info(
-            "Not creating a profile for %s: %r does not provide IUserProfile",
+            "Not creating a profile for %s: %r is not allowed in the Profile container",
             userid,
             portal_type,
         )
@@ -208,6 +242,21 @@ def ensure_profile(userid: str, login: str, claims: Claims) -> UserProfile | Non
             userid=userid,
             login=login,
         )
+        # Asked of the instance, not the FTI. A site may keep users and
+        # non-users in one type and mark only the accounts, from a subscriber
+        # to the add event that has just run. An object left unmarked is one
+        # the identity catalog never files, so nothing here could find it
+        # again -- and leaving it in place would make the next login's create
+        # collide with its id.
+        if not IUserProfile.providedBy(profile):
+            api.content.delete(obj=profile, check_linkintegrity=False)
+            logger.info(
+                "Not creating a profile for %s: the %r created does not provide "
+                "IUserProfile",
+                userid,
+                portal_type,
+            )
+            return None
     logger.info("Created profile for %s", userid)
     note_profile_created(profile)
     return profile

backend/src/pas/plugins/identity/profiles/default/metadata.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <metadata>
-  <version>1009</version>
+  <version>1010</version>
   <dependencies>
     <dependency>profile-plone.restapi:default</dependency>
     <dependency>profile-plone.app.dexterity:default</dependency>

backend/src/pas/plugins/identity/setuphandlers/plugins.py

@@ -19,6 +19,7 @@
 from pas.plugins.identity.core.pas.profile import PLUGIN_TITLE as PROFILE_PLUGIN_TITLE
 from Products.PlonePAS.interfaces.group import IGroupIntrospection
 from Products.PlonePAS.interfaces.group import IGroupManagement
+from Products.PlonePAS.interfaces.plugins import IUserIntrospection
 from Products.PlonePAS.interfaces.plugins import IUserManagement
 from Products.PluggableAuthService.interfaces.plugins import IAuthenticationPlugin
 from Products.PluggableAuthService.interfaces.plugins import ICredentialsResetPlugin
@@ -68,12 +69,18 @@
 #: ``source_users`` held -- nothing, for a user who signed in through a
 #: provider -- and left the Profile answering enumeration and serving
 #: properties, so the user was not deleted and nothing said so.
+#:
+#: ``IUserIntrospection`` is PlonePAS's too, for the same kind of reason:
+#: without it ``api.user.get_users()`` and ``listMembers()`` left out every
+#: user who has a Profile and no ``source_users`` row, while searches found
+#: them.
 PROFILE_ACTIVATED_INTERFACES = (
     IPropertiesPlugin,
     IUserEnumerationPlugin,
     IGroupsPlugin,
     IGroupEnumerationPlugin,
     IGroupIntrospection,
+    IUserIntrospection,
     IUserManagement,
 )
 

backend/src/pas/plugins/identity/upgrades/configure.zcml

@@ -21,5 +21,6 @@
   <include package=".v1007" />
   <include package=".v1008" />
   <include package=".v1009" />
+  <include package=".v1010" />
 
 </configure>

backend/src/pas/plugins/identity/upgrades/v1010/__init__.py

@@ -0,0 +1,34 @@
+"""Take a site to profile version 1010.
+
+``identity_profile`` now answers ``IUserIntrospection``, the question PAS asks
+to list every user rather than to search for some. Without it
+``api.user.get_users()``, ``portal_membership.listMembers()`` and
+``acl_users.getUserIds()`` left out every user who has a Profile and no
+``source_users`` row, which is every user who signed in through a provider,
+while searches found them.
+
+The class implements the interface on every site as soon as the code is
+deployed. What a site installed earlier lacks is the activation, which lives
+in ``acl_users/plugins`` and is written once, at install. Only that interface
+is activated here: re-running the whole install handler would also move the
+plugin to the top of ``IPropertiesPlugin``, undoing an order a site may have
+chosen since.
+"""
+
+from pas.plugins.identity import logger
+from pas.plugins.identity.core.pas.profile import PLUGIN_ID
+from plone import api
+from Products.GenericSetup.tool import SetupTool
+from Products.PlonePAS.interfaces.plugins import IUserIntrospection
+
+
+def activate_user_introspection(context: SetupTool) -> None:
+    """Activate ``identity_profile`` for ``IUserIntrospection``.
+
+    :param context: The setup tool running the upgrade.
+    """
+    plugins = api.portal.get_tool("acl_users").plugins
+    if PLUGIN_ID in plugins.listPluginIds(IUserIntrospection):
+        return
+    plugins.activatePlugin(IUserIntrospection, PLUGIN_ID)
+    logger.info("Activated %s for IUserIntrospection", PLUGIN_ID)

backend/src/pas/plugins/identity/upgrades/v1010/configure.zcml

@@ -0,0 +1,20 @@
+<configure
+    xmlns="http://namespaces.zope.org/zope"
+    xmlns:genericsetup="http://namespaces.zope.org/genericsetup"
+    >
+
+  <!-- Listing every user. The class answers the question from the deploy on;
+       an existing site's acl_users has to be told to ask it. -->
+  <genericsetup:upgradeSteps
+      profile="pas.plugins.identity:default"
+      source="1009"
+      destination="1010"
+      >
+    <genericsetup:upgradeStep
+        title="List users who only have a profile"
+        description="Activates the identity_profile plugin for IUserIntrospection, so that listing every user includes those who signed in through a provider and have no source_users account."
+        handler=".activate_user_introspection"
+        />
+  </genericsetup:upgradeSteps>
+
+</configure>

backend/tests/core/pas/test_external_user_record.py

@@ -28,6 +28,7 @@
 from .stubs import NOT_A_USER
 from .stubs import USER_TYPE
 from .stubs import USERS
+from pas.plugins.identity.core.container import get_container
 from pas.plugins.identity.core.events import ExternalIdentityAuthenticated
 from pas.plugins.identity.core.pas import EXTRACTOR
 from pas.plugins.identity.core.pas import PLUGIN_ID
@@ -185,6 +186,17 @@ def test_it_is_reported(self, caplog):
 
         assert "as an identity and as nothing else" in caplog.text
 
+    def test_no_profile_container_is_created(self):
+        """The login declines to create a Profile of this type, so it has no
+        reason to create the folder one would have gone in."""
+        # The layer's site already has one; a site that has not yet filed a
+        # Profile does not.
+        api.content.delete(obj=get_container(), check_linkintegrity=False)
+
+        self.plugin.authenticateCredentials(self.credentials)
+
+        assert get_container() is None
+
     def test_the_warning_names_the_type_that_was_not_created(self, caplog):
         """A site reading this has to know which record to look at."""
         self.plugin.authenticateCredentials(self.credentials)