New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Email Domain Restrictions #20383

Merged
merged 39 commits into from Sep 8, 2018

Conversation

Projects
None yet
@SharkyKZ
Copy link
Contributor

SharkyKZ commented May 13, 2018

Pull Request for Issue # .

Summary of Changes

This PR adds user email domain restrictions. This is an alternative to #20164 with more flexibility added.

Testing Instructions

Apply patch.
Go to Users configuration.
In Email Domain Options add some rules.

a) Try to register with disallowed email.
b) Try to register with an email that isn't disallowed.
c) Try to change email in user profile to a disallowed email.
d) Try to change email in user profile to an allowed email.
e) Remove all rules and try to register again.

Expected result

a) Registration fails.
b) Registration successful.
c) Changes not saved.
d) Changes saved.
e) Registration successful.

Documentation Changes Required

Document the new option.
Document validDomains attribute usage in EmailRule.
Document message property in FormRule.

SharkyKZ added some commits Apr 27, 2018

@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented May 13, 2018

On the backend, text not translated JGLOBAL_EMAIL_DOMAIN_NOT_ALLOWED.

@SharkyKZ

This comment has been minimized.

Copy link
Contributor

SharkyKZ commented May 13, 2018

Added strings. Thanks.

@brianteeman
Copy link
Contributor

brianteeman left a comment

Surely we have a string for this already

@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented May 13, 2018

For the Rule, how about making it a radio type with the red/green switcher instead of a list?

{
if ($domain->rule == 0)
{
$allowed = false;

This comment has been minimized.

@Quy

Quy May 13, 2018

Contributor

Put a break here since there is no point in processing additional rules if any.

This comment has been minimized.

@SharkyKZ

SharkyKZ May 13, 2018

Contributor

No, this would prevent other rules from overwriting the first matched rule.

COM_USERS_CONFIG_FIELD_ALLOWREGISTRATION_DESC="If set to Yes, new Users are allowed to self-register."
COM_USERS_CONFIG_FIELD_ALLOWREGISTRATION_LABEL="Allow User Registration"
COM_USERS_CONFIG_FIELD_CAPTCHA_DESC="Select the captcha plugin that will be used in the registration, password and username reminder forms. You may need to enter required information for your captcha plugin in the Plugin Manager.<br />If 'Use Global' is selected, make sure a captcha plugin is selected in Global Configuration."
COM_USERS_CONFIG_FIELD_CAPTCHA_LABEL="Captcha"
COM_USERS_CONFIG_FIELD_CHANGEUSERNAME_DESC="Allow users to change their Username when editing their profile."
COM_USERS_CONFIG_FIELD_CHANGEUSERNAME_LABEL="Change Username"
COM_USERS_CONFIG_FIELD_DOMAIN_NAME_DESC="Enter a domain name. Wildcards (*) are supported. For example:<br /><b>*</b> allows or disallows all domains;<br /><b>*.com</b> allows or disallows all .com domains;<br /><b>*.joomla.org</b> allows or disallows all joomla.org subdomains."

This comment has been minimized.

@infograf768

infograf768 May 13, 2018

Member

please use strong instead of b

@@ -343,6 +343,9 @@ JGLOBAL_DOWN="Down"
JGLOBAL_EDIT_ITEM="Edit item"
JGLOBAL_EDIT_PREFERENCES="Edit Preferences"
JGLOBAL_EMAIL="Email"
JGLOBAL_EMAIL_DOMAIN_NOT_ALLOWED="The email domain <b>%s</b> is not allowed. Please enter another email address."

This comment has been minimized.

@infograf768

infograf768 May 13, 2018

Member

same comment

@@ -199,6 +199,9 @@ JGLOBAL_DISPLAY_NUM="Display #"
JGLOBAL_EDIT="Edit"
JGLOBAL_EDIT_TITLE="Edit article"
JGLOBAL_EMAIL="Email"
JGLOBAL_EMAIL_DOMAIN_NOT_ALLOWED="The email domain <b>%s</b> is not allowed. Please enter another email address."

This comment has been minimized.

@infograf768

infograf768 May 13, 2018

Member

same comment

SharkyKZ added some commits May 13, 2018

@ReLater

This comment has been minimized.

Copy link
Contributor

ReLater commented Jul 3, 2018

Tested also unsuccessfully without dot, just *example.org

@SharkyKZ

This comment has been minimized.

Copy link
Contributor

SharkyKZ commented Jul 3, 2018

Yes, wildcards currently work on full segments only as per comments #20383 (comment). But this can be improved. Any suggestions welcome (especially for fixing tests).

@ReLater

This comment has been minimized.

Copy link
Contributor

ReLater commented Jul 3, 2018

To clarify: I could not allow tester@example.org. All tests unsuccessful. Makes no difference if my rule is with (#20383 (comment)) or without dot.

Joomla 3.8.11-dev nightly of yesterday.

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 3, 2018

@ReLater
I do not confirm your findings here. For me it works as intended. (Latest staging).

@ReLater

This comment has been minimized.

Copy link
Contributor

ReLater commented Jul 4, 2018

@infograf768

I do not confirm your findings here. For me it works as intended. (Latest staging).

Thank you. Maybe I have to install a new staging. I'm using the Joomla upload/update feature for a longer time now with nightly update packages for 3.8.x. Or maybe a conflict with an installed 3rd extension. I will see.

@ReLater

This comment has been minimized.

Copy link
Contributor

ReLater commented Jul 4, 2018

Forget this comment:

After a new installation of staging nightly build of today I have the same unsuccessful results like in comment #20383 (comment)
Could you please have a look on the settings there?

I've tried it also with other rules/domains than *.example.org.

* : Disallowed 
*.example.org : Allowed

PHP 7.1 and 7.2

PHP Built On | Linux dd1620 3.13.0-151-generic #201-Ubuntu SMP Wed May 30 14:22:13 UTC 2018 x86_64
Database Type | mysql
Database Version | 5.5.58-nmm1-log
Database Collation | utf8_general_ci
Database Connection Collation | utf8mb4_general_ci
PHP Version | 7.2.1
Web Server | Apache
WebServer to PHP Interface | fpm-fcgi
Joomla! Version | Joomla! 3.8.11-dev Development [ Amani ] 26-June-2018 15:45 GMT
Joomla! Platform Version | Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT
User Agent | Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0

@ReLater

This comment has been minimized.

Copy link
Contributor

ReLater commented Jul 4, 2018

After 2 coffees => "Stupid me!"

For allowed email relater@example.org one have to set

* : Disallowed 
example.org : Allowed
@ReLater

This comment has been minimized.

Copy link
Contributor

ReLater commented Jul 4, 2018

I have tested this item successfully on c7a8282


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20383.

@ka3media

This comment has been minimized.

Copy link

ka3media commented Sep 8, 2018

I have tested this item successfully on c7a8282

Tested on J3.9 alpha

Applied the patch and checked various rulesets and rule sorting to check if sorting has the expected behavior.

Works for me


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20383.

@franz-wohlkoenig

This comment has been minimized.

Copy link

franz-wohlkoenig commented Sep 8, 2018

Ready to Commit after 4 successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label Sep 8, 2018

@hamby

This comment has been minimized.

Copy link

hamby commented Sep 8, 2018

"Testing Instructions" from above a) - e) was tested successfully

But i found a other problem: Ording Dependence Problem

image

--> Registration of example.us is possible

image

--> Registration of example.us is NOT possible

@ka3media

This comment has been minimized.

Copy link

ka3media commented Sep 8, 2018

But that's the expected behavior... if the last action is "disallow all" than all is disallowed

@mbabker mbabker added this to the Joomla 3.9.0 milestone Sep 8, 2018

@mbabker mbabker merged commit 00f06f9 into joomla:staging Sep 8, 2018

3 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
Hound No violations found. Woof!
JTracker/HumanTestResults Human Test Results: 4 Successful 0 Failed.
Details
continuous-integration/drone/pr the build was successful
Details

@joomla-cms-bot joomla-cms-bot removed the RTC label Sep 8, 2018

@PhilETaylor

This comment has been minimized.

Copy link
Contributor

PhilETaylor commented Sep 9, 2018

Are we just ignoring the fact this breaks the build and unit tests are now failing?

eg: https://travis-ci.org/joomla/joomla-cms/jobs/426125442

@mbabker

This comment has been minimized.

Copy link
Member

mbabker commented Sep 9, 2018

Fixed via 1ddb06d

@SharkyKZ SharkyKZ deleted the SharkyKZ:com_users_3 branch Sep 24, 2018

@810

This comment has been minimized.

Copy link
Contributor

810 commented Oct 31, 2018

Any option to disable this feature?

Because I can't edit a user option now:
image

@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented Oct 31, 2018

Try deleting the row.

@810

This comment has been minimized.

Copy link
Contributor

810 commented Oct 31, 2018

nothing happend when I click on -
IE11.1001.18267.0

@810

This comment has been minimized.

Copy link
Contributor

810 commented Oct 31, 2018

ie11
image

Edge:
image

Looks like html issue

@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented Oct 31, 2018

Strange. I am able to delete so there are no entries.

email-domain-options

@810

This comment has been minimized.

Copy link
Contributor

810 commented Oct 31, 2018

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment