Problem/Motivation
A programmer could apply the AlternativeUserEmailsValidator to a non-user entity. This will have unexpected results because the validator does not check to see if it is operating on a user entity. (It used to, but this check was mistakenly removed. The check is necessary because the validator uses the entity id as the user ID to exclude the current user from the query, so if the entity is not a user entity, the wrong ID will be used.)
I propose not only restoring the check but renaming the variables to make it clear this is a $user, not a generic $entity, and throwing exceptions if the fundamental criteria for running the validator are not met. Silently failing is dangerous if a programmer expects validation and no error is returned, and these exceptions can only be triggered by programmers applying the validator to new code.
Another approach would be to inject the current user and then get the UID that way.
Issue fork alternative_user_emails-3426894
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
ptmkenny commentedComment #4
ptmkenny commentedComment #5
ptmkenny commentedAfter thinking about this some more, I think it makes more sense to inject the current user and then get the UID that way. With that approach, it doesn't matter whether the entity is a user entity or not.
Comment #6
ptmkenny commentedComment #7
ptmkenny commentedIn fact, injecting the current user is the wrong approach because it assumes that the user saving the user entity is always the same, which is definitely not the case (for example, when an admin edits another user's account). So I again propose the fix in the first post.
Comment #8
ptmkenny commented(duplicate post)
Comment #9
ptmkenny commentedComment #10
ptmkenny commentedComment #11
jonathanshawYou're right. This validator would be very hard to use sensibly on a non-user entity, however we tweaked it, and we don't do anyone a favour by allowing it.
I like that you used an exception here.
I've added a few nits on the MR, feel free to ignore.
Comment #13
ptmkenny commentedComment #20
ptmkenny commentedComment #21
jonathanshawAwesome working with you @ptmkenny :)