Problem/Motivation
We needed to rebuilt a site built with Drupal 6 to Drupal 8 and while trying to migrate the user restrictions I noticed the type "Hostname/IP" was missing entirely in version 8. So I had a look on how to add this with custom hooks and it would be really painful :/
Proposed resolution
Switch from hardcoded restriction types to plugins.
Bonus (to be discussed): rename the entity from "User restrictions" (plural) to "User restriction" (singular) including the entity type name itself because the entity itself represents a single restriction.
Remaining tasks
Add patch.
User interface changes
None
API changes
Well, a lot :) After applying the patch, developers can add various custom restriction types using plugins instead of altering the form/submit/validation.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#18 | user_restrictions-refactoring_plugins-2936400-final.patch | 69.03 KB | adammalone |
#16 | user_restrictions-refactoring_plugins-2936400-16.patch | 64.82 KB | adammalone |
| |||
#4 | user_restrictions-refactoring_plugins-2936400-4.patch | 59.48 KB | stBorchert |
#4 | interdiff-2936400-3-4.txt | 6.32 KB | stBorchert |
Comments
Comment #2
stBorchertComment #3
stBorchertFixed some errors in the patch.
Comment #4
stBorchertAdded more fixes (including default expiry date when adding rules).
To maintain backwards compatibility with old restriction masks (using "%" as wildcard) we have to replace all "%" with ".*" in the patterns. Unfortunately you cannot use "LIKE" to query config entities so the pattern have to be a real regular expression ...
Comment #5
adammaloneThis is fantastic thank you for your time and effort from this. I'm uploading a new patch that addresses a couple of very minor issues I found.
One of the things I noticed was that this doesn't work as expected for email addresses due to the '.' character and how preg_quote() changes a rule like ".*@com.au" to "\.\*@com\.au" so the preg_match() doesn't work. I'm wondering if we need to remove the preg_quote here?
The block in question is in src/Plugin/UserRestrictionType/UserRestrictionTypeBase.php
Comment #6
adammaloneAlso noting here I think we need to look at the usecase where we might want to blacklist all email addresses apart from one mask. Currently the functionality introduced in the patch exits as soon as it finds a match so won't allow us to have black/white list combos.
Comment #7
adammaloneAdding new patch to address issue in #6 and interdiff between 4 and 7.
Comment #8
adammaloneFurther work here to add:
Comment #9
adammaloneUploaded wrong file - trying again.
Comment #10
adammaloneComment #11
adammaloneAmended tests further and fixed coding standards violations.
Comment #12
adammaloneFurther work to replace old D7 test code with that of D8
Comment #13
adammaloneFixed an issue where permissions checking for users able to bypass user permissions occurred before they had logged in and therefore had said permissions. Also further work on tests.
Comment #14
adammaloneComment #15
adammaloneComment #16
adammaloneComment #18
adammaloneThanks heaps @stBorchert I've committed this to the 8.1.x dev branch with some minor changes and tests that now work. This is a great base to move forward from. The final patch is attached here.