Problem/Motivation
Currently, the verification form is always available, meaning anonymous users will always be able to create users with it. This may not always be desirable, so it would be good to make it permission based.
Additionally, logged in users always skip the verification form at present. Allowing this to be controlled by permissions would make the module more flexible.
Steps to reproduce
Create a verification form and log out. Navigate to the form and verify it is available. Next, log in and navigate to the verification, and observe the user is redirected to the form's destination.
Proposed resolution
Create two permissions "Access verification form _______" and "Skip verification form ______". A follow on task will limit access using them.
Remaining tasks
- Add a new class to the
/srccalledVerifyEmailPermissions. - Use the
Drupal\Core\DependencyInjection\AutowireTraitin the class (I think this should work - if I'm wrong, acreatefunction will be needed instead). - Add a constructor to that takes a parameter of type
Drupal\Core\Entity\EntityTypeManagerInterfaceand promotes to a property. - Add a public method of the class called "permissions" that should do the following tasks:
- Initialize permissions as an empty array.
- Load the storage for
verify_email. - Query the verify email entities, and for each one add two elements to the permission array, one with a key of "access verify $id" and a translatable value of "Access verification form $tag", and the second with a key of "skip verify $id" and a translatable value of "Skip verification form $tag".
- Return the array of permissions.
Add an entry toverify_email.services.yml, defined as the fully qualified class name. Autowiring will take care of the properties, but the definition will need the following tag:
tags: - { name: 'user.permission_provider', method: 'createPermissions', provider: 'verify_email' }
Add apermission_callbacksentry toverify_email.permissions.yml, specifying the path to the fully qualified class name and thepermissionfunction- In the setup of test
VerifyEmailSetupTest, add "administer permissions" to the permissions of the created user. - In
testVerifyEmailConfig, add a check after the form creation that loads the/admin/people/permissionspage, and checks for the presence of "Access verification form Test verify email" and "Skip verification form Test verify email".
User interface changes
Only the new permissions on the permissions page.
API changes
None.
Data model changes
None.
Issue fork verify_email-3556368
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 #2
nidhi27Hii @lostcarpark,
As per https://www.drupal.org/node/3421580 change record changes are currently in draft state. May be we need to use permission_callback. Let me know your suggestion! Thanks!
Comment #3
lostcarpark commentedYou're right, I jumped the gun on the service tag.
I have updated the Remaining tasks to specify permission_callbacks instead.
Comment #4
nidhi27Comment #6
nidhi27Hii @lostcarpark,
I have created MR !50 for this. Let me know if any changes are needed. Thanks!
Comment #7
lostcarpark commentedThis is almost perfect. The only minor issue is that when using AutowireTrait, you don't need a
create()function. Autowiring generates one automatically. Just deletecreate()and we should be ready to merge.Thanks!
Comment #9
ritarshi_chakraborty commentedHey @lostcarpark, please review the changes.
Comment #10
lostcarpark commentedSorry, I missed one small thing on my previous review. The
$entityTypeManagercurrently uses a separate declaration. In PHP 8 this can be replace by property promotion from the constructor.Comment #11
lostcarpark commentedAccidentally selected RTBC before I noticed the property promotion issue.
Comment #12
lostcarpark commentedYou are almost there, but you need to change the name of the constructor parameter from
$entity_type_managerto the property name$entityTypeManager.Thanks for your work on this @ritarshi_chakraborty.
Comment #13
lostcarpark commentedAh, so close!
Just need to update the docblock comment with the new parameter name.
Comment #14
lostcarpark commentedI have reviewed the changes, and run the tests locally. I also ran the "test only changes" and verified it failed. I did some manual testing to verify the permissions are created for each verification form.
Comment #16
lostcarpark commentedMerged change and verified merge train passed successfully.
Moving to fixed and awarding contribution credit.
Thank you @nidhi27 and @ritarshi_chakraborty for working on this. I think it's a nice improvement to the module.