Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

added event for creating a user #54

Closed
wants to merge 1 commit into from
Closed

added event for creating a user #54

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/pas/plugins/oidc/plugins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from plone.protect.utils import safeWrite
from Products.CMFCore.utils import getToolByName
from Products.PageTemplates.PageTemplateFile import PageTemplateFile
from Products.PluggableAuthService.events import PrincipalCreated
from Products.PluggableAuthService.interfaces.plugins import IAuthenticationPlugin
from Products.PluggableAuthService.interfaces.plugins import IChallengePlugin
from Products.PluggableAuthService.interfaces.plugins import IUserAdderPlugin
Expand All @@ -18,6 +19,7 @@
from secrets import choice
from typing import List
from ZODB.POSException import ConflictError
from zope.event import notify
from zope.interface import implementer
from zope.interface import Interface

Expand Down Expand Up @@ -215,6 +217,8 @@ def rememberIdentity(self, userinfo):
# https://bandit.readthedocs.io/en/1.7.4/plugins/b110_try_except_pass.html
pass
self._updateUserProperties(user, userinfo)
if user is not None:
notify(PrincipalCreated(user))
Copy link
Member

@erral erral May 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is notifying this enough? For instance, when a user logs in a Plone site and Plone fires portal_membership's loginUser, this tool notifies either UserInitialLoginEvent or UserLoggedInEvent.

see: https://github.com/plone/Products.PlonePAS/blob/master/src/Products/PlonePAS/tools/membership.py#L657-L660

I think that we should be notifying those two events too.

Copy link
Author

Choose a reason for hiding this comment

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

If I see it correctly, it can also happen that member properties have to be updated with every new login. From this point of view, it definitely makes sense to notifying more events.

Copy link
Member

Choose a reason for hiding this comment

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

Nice catch indeed.

break
else:
# if time.time() > user.getProperty(LAST_UPDATE_USER_PROPERTY_KEY) + config.get(autoUpdateUserPropertiesIntervalKey, 0):
Expand Down
Loading