Revamp authentication events
Open
@LukeTowers is already working on this.
Since Sep 1, 2021.
help wanted
- Dominant language
- PHP
- Stars
- 1.5k
- Forks
- 246
- Avg merge
- 19h 2m
- Merged PRs (30d)
- 7
Description
This is a proposal to revamp the authentication events system within Winter CMS. Feedback and suggestions very much welcome. Related: https://github.com/wintercms/wn-user-plugin/pull/11 & https://github.com/wintercms/storm/pull/48
The current events are as follows:
backend.user.login-> Global event for backend users logging inmodel.auth.afterImpersonate-> Local event triggered after impersonation is stoppedmodel.auth.beforeImpersonate-> Local event triggered before impersonation is startedcanBeImpersonated()-> Model method used if impersonation events don't return anythingwinter.user.activate-> Global event triggered after Winter.User user's activate for the first timewinter.user.reactivate-> Global event triggered after Winter.User user's activate after being deactivatedwinter.user.login-> Global event triggered after Winter.User user's loginwinter.user.deactivate-> Global event triggered after Winter.User user's deactivatewinter.user.activate-> Global event triggered after Winter.User user's activatewinter.user.getNotificationVars-> Not technically part of the auth system, used for extending the variables made available to the Winter.Notify notificationswinter.user.logout-> Global event triggered after Winter.User user's logoutwinter.user.beforeAuthenticate-> Global event triggered immediately before Winter.User user's credentials are checkedwinter.user.beforeRegister-> Global event triggered before Winter.User user's registration is processedwinter.user.register-> Global event triggered after Winter.User user's registration is processed
The revised system would remove any direct event logic from the Auth system and instead move it to model event methods in the form of the following:
Model events:
beforeRegister()afterRegister()beforeLogin()(already exists)afterLogin()(already exists)beforeLogout()afterLogout()beforeActivate($activationCode)afterActivate($activationCode)beforeResetPassword($resetCode, $newPassword)afterResetPassword()beforeImpersonate($impersonator)afterImpersonate($impersonator)beforeEndImpersonation($impersonator)afterEndImpersonation($impersonator)beforeSuspend()afterSuspend()beforeUnsuspend()afterUnsuspend()beforeBan()afterBan()beforeUnban()afterUnban()
The Backend module & Winter.User plugin would then use those methods to fire the externally accessible events:
*.user.beforeRegister*.user.register*.user.beforeLogin*.user.login*.user.beforeLogout*.user.logout*.user.beforeImpersonate*.user.impersonate*.user.beforeEndImpersonate*.user.endImpersonate*.user.beforeResetPassword*.user.resetPassword*.user.beforeActivate*.user.activate*.user.beforeDeactivate*.user.deactivate*.user.beforeReactivate*.user.reactivate*.user.beforeSuspend*.user.suspend*.user.beforeUnsuspend*.user.unsuspend*.user.beforeBan*.user.ban*.user.beforeUnban*.user.unban
Some questions to consider:
- What needs to be done with the existing events to retain BC as much as possible? The impersonation events are unlikely to be in wide spread use at the moment, but everything else existing is fair game.
- How many of these newly proposed events are redundant in that developers can relatively easily use already existing more generic model local events in order to accomplish the same end results? Should we avoid adding new events for those already addressable use-cases and instead publish a guide for using those existing events to achieve the same results?
- Which of these events should be halting vs non-halting? What return values should be supported if any? What arguments will be provided if any? Should each event be global or local to the user instance?
- Are any of these events likely to be utilized within high traffic workflows? I.e. what are the performance implications of adding these additional events; are there any concerns about that?
- What extra use cases (beyond just easier targeting for logging entries into an audit log) will the addition of each new event support?
- Should we also add an authentication log that tracks these events for every user and display it in the backend as a system level view and a contextual view? Should we be logging extra data for these events if so or leave it to a plugin to worry about that? (extra data like user agent and IP address).
- The previous developer guidelines recommended never using "after" terminology for events, however this was inconsistently applied even within the core (model events for example for the most part have after versions, although some don't instead using the unmodified verb as the "after" event name); should we retain that recommendation or revert it to being it more in line with the model events? How does that effect our existing collection of events?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.