Broadcast events for logging, email notifications, etc #526
Conversation
| * The only exception to that is a `WP_User` object, which will automatically be | ||
| * reduced to the `user_login`. |
There was a problem hiding this comment.
| * The only exception to that is a `WP_User` object, which will automatically be | |
| * reduced to the `user_login`. | |
| * The only exception to that is a `WP_User` object in the 'user' key, which will | |
| * automatically be reduced to the `user_login`. |
| @@ -886,6 +886,7 @@ public static function verify_login_nonce( $user_id, $nonce ) { | |||
| $login_nonce = get_user_meta( $user_id, self::USER_META_NONCE_KEY, true ); | |||
|
|
|||
| if ( ! $login_nonce || empty( $login_nonce['key'] ) || empty( $login_nonce['expiration'] ) ) { | |||
| self::broadcast( 'login_nonce_missing', compact( 'user_id' ) ); | |||
There was a problem hiding this comment.
I'm concerned that adding events for low-level validation is a bit over the top, yes, they could be used to pick up someone fuzzing the login forms, but they're not scenario's where a logger needs/should be aware of IMHO.
There was a problem hiding this comment.
I was thinking it'd be useful to have a detailed audit trail when investigating a security incident, but at the same time I agree that we also don't wanna clutter everyday logs with standard pentesting.
Broadcasting low-level events doesn't mean they'll necessarily be used, though. #459 might only log high-level stuff by default, but log everything if a user changes a filter. I imagine #476 would hardcode a very short list of events that it chooses to send emails about.
The specifics will vary based on the caller, and the site, though. So IMO it's best to leave that choice up to them. What do you think?
I don't feel strongly, though. Broadcasting high-level stuff would be better than nothing, and we can always add more in the future if we change our minds.
This is a rough sketch of a potential way to provide details on 2FA events, so that they can be logged (#459), emailed (#476), added to Stream (xwp/stream#1386), etc.
This takes a more generic approach than #462, and doesn't do any logging/etc itself, it just publishes the events so that other code can subscribe to them and do whatever is desired. It also allows for more granular events, rather than only failures in the providers.
I'd like to get some early feedback before adding anything else. What do y'all think of this approach?