feat(harvester): use dedicated user with inspire-harvester role by TahaKhan998 · Pull Request #899 · CERNDocumentServer/cds-rdm

TahaKhan998 · 2026-08-05T15:01:39Z

Closes #881
Moves the INSPIRE harvester off system_identity onto a dedicated service user with the inspire-harvester role, so harvested records have a clear owner. Harvester create/update/publish/search paths now use that identity.

kpsherva · 2026-08-10T07:08:32Z

+            "Harvester user is missing the inspire-harvester role "
+            "(role id must equal name)."
+        )
+    g.cds_harvester_identity = identity


could you explain why did you choose to go with a global? it is an approach opposite to what we have normally in invenio

I cached the harvester user here because get harvester identity is called many times in a harvest run to create, publish, handle files, and search. If it is not cached then get harvester identity is called many times, which I thought could be slow. i removed it now to follow the invenio conventions

kpsherva · 2026-08-10T13:27:38Z


-    can_create = [AuthenticatedRegularUser(), SystemProcess()]
+    # Harvester role can manage records it does not own (legacy system-owned).
+    can_manage = RDMRecordPermissionPolicy.can_manage + [InspireHarvester()]


why should it be able to manage records?

so we need to add the inspire harvester generator to the can_manage permission policy because previously many records were harvested by system and, in production, there are records owned by other users and not the inspire harvester user, so for it to be able to update those records with new info from inspire, create new versions of those records, or assign them to the right communities, we need can_manage.

what about can_curate? isn't it enough? by assigning Harvester to manage you give it also permission to manage the access rights to the record. Doesn't it sound as too much for what the hervester does?

kpsherva · 2026-08-10T13:31:11Z

+def get_harvester_user():
+    """Get the configured harvester user."""
+    email = current_app.config.get("CDS_HARVESTER_USER_EMAIL")
+    if not email:
+        raise HarvesterUserError("CDS_HARVESTER_USER_EMAIL is not configured.")
+
+    user = current_datastore.get_user_by_email(email)
+    if user is None:
+        raise HarvesterUserError(f"Harvester user '{email}' was not found.")
+    if not user.active:
+        raise HarvesterUserError(f"Harvester user '{email}' is inactive.")
+    return user
+
+
+def get_harvester_user_id():
+    """Get the configured harvester user id."""
+    return get_harvester_user().id
+
+
+def get_harvester_identity():
+    """Get an identity for harvester operations."""
+    identity = get_identity(get_harvester_user())
+    if inspire_harvester_role not in identity.provides:
+        raise HarvesterUserError(
+            "Harvester user is missing the inspire-harvester role "
+            "(role id must equal name)."
+        )
+    return identity


how about exploring an alternative implementation of these functions, in a form of a class with specific interface that allows us to retrieve id and identity?

done i changed it to a class based implementation instead of separate helper functions so now one place handles getting the harvester user id and identity and i instantiate it once and reuse it in the flow so it stays cleaner and easier to maintain

kpsherva · 2026-08-11T13:29:37Z

+    def user(self):
+        """Get the configured harvester user."""
+        email = current_app.config.get("CDS_HARVESTER_USER_EMAIL")
+        if not email:
+            raise RuntimeError("CDS_HARVESTER_USER_EMAIL is not configured.")
+
+        user = current_datastore.get_user_by_email(email)
+        if user is None:
+            raise RuntimeError(f"Harvester user '{email}' was not found.")
+        if not user.active:
+            raise RuntimeError(f"Harvester user '{email}' is inactive.")
+        return user


implementing the class like this will cause the DB query to be fired up every time you call the method - it is not very optimised approach, also considering that you call this method multiple times.

kpsherva · 2026-08-11T13:40:52Z

+from cds_rdm.generators import inspire_harvester_role
+
+
+class HarvesterUserService:


normally this name is a bit misleading, since this is not really a service in invenio understanding

kpsherva · 2026-08-11T13:43:22Z

+class HarvesterUserService:
+    """Service for resolving configured harvester user data."""
+
+    def user(self):


now the class starts to resemble something we already have existing which allows us to retrieve users - the users service, check https://github.com/inveniosoftware/invenio-users-resources/blob/master/invenio_users_resources/services/users/service.py

once you retrieve the user object, both id and the identity are reachable from this user object, so we don't need a wrapper class to help us retrieve it again. The only thing which we need is id parameter to be able to read the specific user

kpsherva · 2026-08-11T13:46:55Z

+        self.harvester_user_service = HarvesterUserService()
+        self.identity = self.harvester_user_service.identity()


if you use user object mentioned in comments above, then this can be simplified

Suggested change

self.harvester_user_service = HarvesterUserService()

self.identity = self.harvester_user_service.identity()

self.harvester_user = HarvesterUser().get()

and later on you call (needs verification on your side)

self.harvester_user.identity

which is also more descriptive in the code, because we get to know of which user identity we are trying to call

kpsherva · 2026-08-11T13:48:48Z

+    def identity(self):
+        """Get an identity for harvester operations."""
+        identity = get_identity(self.user())
+        if inspire_harvester_role not in identity.provides:


nit: this check is redundant - we should be informed when harvester tries to do something which it does not have permissions for by PermissionsError

TahaKhan998 force-pushed the feat/harvester-dedicated-user branch 4 times, most recently from a765b9a to 7474668 Compare August 6, 2026 08:32