Added ajax Username & Email checks #11516

mxkmp · 2016-08-08T12:21:47Z

Pull Request for Issue #9943
I continued the great work of @framontb at @icampus

Summary of Changes

Added ajax username and email checks, which compares the entries of the database (only in Frontend)
Live checks of the equality of the passwords and emails (Frontend)
added wildcard/blacklist for email domains(Works on Front & Backend)

Testing Instructions

(backend) Allow User Registration in Front End (Users -> Options -> Allow User Registration)
Configure Username options in backend
Enter an username which is already in use
Enter an email which is already in use
Enter different passwords or emails in the both fields
Enter a mail from a blacklisted email domain

brianteeman · 2016-08-08T13:07:42Z

administrator/components/com_cache/views/purge/tmpl/default.php

@@ -10,7 +10,7 @@
 defined('_JEXEC') or die;
 ?>

-<form action="<?php echo JRoute::_('index.php?option=com_cache&view=purge'); ?>" method="post" name="adminForm" id="adminForm">
+<form action="<?php echo JRoute::_('index.php?option=com_cache'); ?>" method="post" name="adminForm" id="adminForm">


what does this change have to to with username and email checks

I pulled the Issue 9943 and this was maybe in there. I didn't changed anything in this .php File. Shall I remove this?

@roland-d said he will help you tomorrow to resolve this

brianteeman · 2016-08-08T13:08:26Z

@mxkmp29 there are a lot of unrelated changes in this PR

brianteeman · 2016-08-08T13:09:45Z

administrator/language/en-GB/en-GB.com_users.ini

@@ -34,6 +34,8 @@ COM_USERS_CONFIG_FIELD_CAPTCHA_DESC="Select the captcha plugin that will be used
 COM_USERS_CONFIG_FIELD_CAPTCHA_LABEL="Captcha"
 COM_USERS_CONFIG_FIELD_CHANGEUSERNAME_DESC="Allow users to change their Login name when editing their profile."
 COM_USERS_CONFIG_FIELD_CHANGEUSERNAME_LABEL="Change Login Name"
+COM_USERS_CONFIG_FIELD_EMAIL_REGEX_LABEL="Forbidden email endings"


Do you mean forbidden domains?

yes, sorry I formulate it false.

brianteeman · 2016-08-08T13:17:13Z

Closing for now until the branch is fixed

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11516.}

brianteeman · 2016-08-09T08:37:53Z

administrator/language/en-GB/en-GB.com_users.ini

@@ -34,6 +34,8 @@ COM_USERS_CONFIG_FIELD_CAPTCHA_DESC="Select the captcha plugin that will be used
 COM_USERS_CONFIG_FIELD_CAPTCHA_LABEL="Captcha"
 COM_USERS_CONFIG_FIELD_CHANGEUSERNAME_DESC="Allow users to change their Login name when editing their profile."
 COM_USERS_CONFIG_FIELD_CHANGEUSERNAME_LABEL="Change Login Name"
+COM_USERS_CONFIG_FIELD_EMAIL_REGEX_LABEL="Forbidden domains"
+COM_USERS_CONFIG_FIELD_EMAIL_REGEX_DESC="Expression for forbidden email adress endings. Example: Forbid all email addresses from example.com. Input:<br /> example.com<br />For many entries add a new line after every entry. <br />Input: <br />example.com<br />example2.com"


Can we simplify this to

List of forbidden domain names. Place each domain name on a new line

Fedik · 2016-08-09T08:47:58Z

media/system/js/validate-uncompressed.js

+                return regex.test(value);
+            });
+            //check if username is already in database
+            jQuery('#jform_username').change(function(){


it is com_users specific code it should not be here in validate.js,
and I do not see a reason why you need change the validate.js at all

I thought this should be placed there where the validation of the fields is. Where should I place it that I can integrate it perfectly ?

you can make separated file and place it under /media/com_users/js/validate-user.js
or in the layout where the form is rendered

can we try to use vanilla js please and not jquery specific if at all possible.

@Fedik @dgt41 Can we have some help here on how to turn an AJAX call to vanilla JS and also the .change() functions like jQuery('#jform_username').change(function(){ I don't know how to do that and we need to make sure it supports IE8.

i have used one here... https://github.com/joomla/joomla-cms/pull/8545/files#diff-5a22fd5c5d43e73f7858a435aa497b18R30
but may be @Fedik @dgt41 have a better way? (add vannila js ajax calls in core.js?)

I think https://github.com/joomla-projects/media-manager-improvement/blob/b60da9fb9aa9a5893852c4e0926191f86a9479a8/media/plg_media-editor_imagefilters/js/caman-init.js
is a good starting point. There is a polyfill for IE8/9 that might needed as well here.
Check this article for references: http://danielstrunk.me/blog/2015/03/18/polyfills-for-ie8-support/
ATM I haven't check the current code, will do when I'll get some free time

then we need to think about general AJAX method in core.js,

Joomla.request = function(options){ ... } // do request Joomla.request({ url: '/some/url', onSuccess: function(){alert('works!')}, onError: function(){alert('not works!')}, })

Also we need the events handling.
About IE8, there change does not work, but there can try to use onpropertychange instead change
This require a bit more coding 😉

so for now I would leave it with jQuery. Just move the code from validate.js.

ps. when I will get some time, later will try to prepare the pull request for Joomla ajax and events.

@Fedik I had a Skype chat with @dgt41 today and we came to the exact same conclusion. So I guess we are all on the same page. Also the student has very limited time, so rather have this complete as it is and we can refactor to native JS once we have all our "own" methods in place. Rather I use jQuery now and later move everything to the J! way than everybody copy-pasting possible solutions from all over the net.

brianteeman · 2016-08-09T10:23:48Z

administrator/components/com_users/config.xml

+			cols="50"
+			filter="string"
+			>
+			</field>


Can we add a showon param here so this is only shown when custom is selected - then you can also simplify the language string

eg
showon="allowed_chars_username_preset:1,2,3"

and then the string you can delete "If other than 'CUSTOM' is selected, this text box is not operational."

Fedik · 2016-08-10T10:07:47Z

components/com_users/views/registration/view.html.php

@@ -43,6 +42,10 @@ public function display($tpl = null)
 		$this->state  = $this->get('State');
 		$this->params = $this->state->get('params');

+		JText::script('COM_USERS_PROFILE_EMAIL2_MESSAGE');
+		JText::script('COM_USERS_FIELD_RESET_PASSWORD1_MESSAGE');
+		JHtml::_('script', 'com_users/validate-user.js', false, true);


Please move it to the layout components/com_users/views/registration/tmpl/default.php
Avoid adding a script/style in the view class.

And as it depend from jQuery you also need to call JHtml::_('jquery.framework');

brianteeman · 2016-10-02T14:31:21Z

administrator/language/en-GB/en-GB.com_users.ini

@@ -67,10 +69,28 @@ COM_USERS_CONFIG_FIELD_USERACTIVATION_DESC="If set to None the user will be regi
 COM_USERS_CONFIG_FIELD_USERACTIVATION_LABEL="New User Account Activation"
 COM_USERS_CONFIG_FIELD_USERACTIVATION_OPTION_ADMINACTIVATION="Administrator"
 COM_USERS_CONFIG_FIELD_USERACTIVATION_OPTION_SELFACTIVATION="Self"
+COM_USERS_CONFIG_FIELD_USERNAME_CHARSPRESET_LABEL="Character Preset"
+COM_USERS_CONFIG_FIELD_USERNAME_CHARSPRESET_DESC="Specify from the select list if you want disable this check or allow/forbid CUSTOM character filling the CUSTOM text box. You can also select some of the specified character presettings: alphanumeric, latin, email,..."
+COM_USERS_CONFIG_FIELD_USERNAME_MINNUMCHARS_LABEL="Min. number of character"


Please change to
Min. number of characters

brianteeman · 2016-10-02T14:32:09Z

administrator/language/en-GB/en-GB.com_users.ini

+COM_USERS_CONFIG_FIELD_USERNAME_CHARSPRESET_LABEL="Character Preset"
+COM_USERS_CONFIG_FIELD_USERNAME_CHARSPRESET_DESC="Specify from the select list if you want disable this check or allow/forbid CUSTOM character filling the CUSTOM text box. You can also select some of the specified character presettings: alphanumeric, latin, email,..."
+COM_USERS_CONFIG_FIELD_USERNAME_MINNUMCHARS_LABEL="Min. number of character"
+COM_USERS_CONFIG_FIELD_USERNAME_MINNUMCHARS_DESC="Minimum number of character required for username field."


Please change to
Minimum number of characters required for the username.

brianteeman · 2016-10-02T14:32:22Z

administrator/language/en-GB/en-GB.com_users.ini

+COM_USERS_CONFIG_FIELD_USERNAME_CHARSPRESET_DESC="Specify from the select list if you want disable this check or allow/forbid CUSTOM character filling the CUSTOM text box. You can also select some of the specified character presettings: alphanumeric, latin, email,..."
+COM_USERS_CONFIG_FIELD_USERNAME_MINNUMCHARS_LABEL="Min. number of character"
+COM_USERS_CONFIG_FIELD_USERNAME_MINNUMCHARS_DESC="Minimum number of character required for username field."
+COM_USERS_CONFIG_FIELD_USERNAME_MAXNUMCHARS_LABEL="Max. number of character"


Please change to
Max. number of characters

brianteeman · 2016-10-02T14:32:55Z

administrator/language/en-GB/en-GB.com_users.ini

+COM_USERS_CONFIG_FIELD_USERNAME_MINNUMCHARS_LABEL="Min. number of character"
+COM_USERS_CONFIG_FIELD_USERNAME_MINNUMCHARS_DESC="Minimum number of character required for username field."
+COM_USERS_CONFIG_FIELD_USERNAME_MAXNUMCHARS_LABEL="Max. number of character"
+COM_USERS_CONFIG_FIELD_USERNAME_MAXNUMCHARS_DESC="Maximum number of character required for username field. Zero indicates no limit."


Please change to
Maximum number of characters required for the username. Zero indicates no limit."

brianteeman · 2016-10-02T14:44:50Z

administrator/language/en-GB/en-GB.com_users.ini

 COM_USERS_CONFIG_IMPORT_FAILED="An error was encountered while importing the configuration: %s."
 COM_USERS_CONFIG_PASSWORD_OPTIONS="Password Options"
 COM_USERS_CONFIG_SAVE_FAILED="An error was encountered while saving the configuration: %s."
+COM_USERS_CONFIG_PASSWORD_OPTIONS="Password Options"


Why are you adding this string. It already exists on line 89

mxkmp · 2016-10-04T13:29:17Z

@brianteeman the comments from @roland-d & @mbabker are corrected. The comment was on the wrong fields in the code. Under "Username Options" the changes are visible and working, which were discussed in this comment.

zero-24 · 2016-10-04T14:11:22Z

libraries/joomla/form/rule/username.php

+						$result = false;
+					}
+
+					/*


why do we have commented code here? maybe it should be removed?

This commented code ist form the PR before me. I think I can remove this.

Is the syntax for an Identifier stated in UAX31-D1. I thought it is too restrictive for Joomla username, and because of that I comment the implementation, but left it just in case smart people think it is needed.

JoshJourney · 2016-10-04T19:40:58Z

I'm willing to retest once the comment zero-24 mentioned is either addressed or explained.

JoshJourney · 2016-10-05T22:49:40Z

I have tested this item ✅ successfully on 6046009

Works as expected. :-)

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11516.}

JoshJourney · 2016-10-05T22:52:21Z

@brianteeman How does it look now (in terms of the requirements you've mentioned)? After the recent change it's working great on my end.

brianteeman · 2016-10-06T07:25:58Z

@brianteeman the comments from @roland-d & @mbabker are corrected. The comment was on the wrong fields in the code. Under "Username Options" the changes are visible and working, which were discussed in this comment.

I dont see the correction that @mbabker says to make in https://github.com/joomla/joomla-cms/pull/11516/files#r75477191 are you saying thats not correct?

brianteeman · 2016-10-06T07:32:10Z

Surely it is not correct that I can set the minimum number of characters to be greater than the maximum number of characters

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11516.}

brianteeman · 2016-10-06T07:42:34Z

I am testing this on current 3.7 branch and I am getting no ajax live checking at all - although the rules are working - see the movie below

mxkmp · 2016-10-10T16:22:08Z

@brianteeman

Summary of Changes

Added ajax username and email checks, which compares the entries of the database (only in Frontend)

This happens because the ajax calls only checks usernames or emails in the database, not the validation. The validation was implemented by @framontb in the username.php and contains only checks via php. There is no ajax validation, which checks the the rules from the admin-backend.

brianteeman · 2016-10-10T16:24:56Z

Seems daft to me to only do ajax on "if exists" and not on "if valid" kind of defeats the object of the code to me

brianteeman · 2016-10-10T16:30:06Z

Also if this is the case that it only checks "if exists" and not "if valid" then the error message is incorrect

The email address you entered is already in use or invalid.

framontb · 2016-10-16T19:11:16Z

@brianteeman
what you suggest is translate all the validation to ajax?

brianteeman · 2016-10-16T19:59:48Z

Well I dont see any point in only having half the job as ajax

brianteeman · 2016-11-10T11:22:39Z

There is a possible issue that needs to be considered in that with the ajax checking of existing usernames and email addresses i can quite quickly gather a list of all the users on the site

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11516.}

SniperSister · 2016-11-20T13:55:59Z

The username and email usage check is a user enumeration attack vector from a security perspective. Yes, we have this check during the current registration process too, but this one can at least be protected using a captcha, that's not possible for the AJAX request.

So, I would suggest to remove the "is username or email" already used check in this PR.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11516.}

conconnl · 2016-12-02T12:45:14Z

You can enter and save a lower number of "Max. number of characters" then "Min. number of characters"
This can' be correct. As you can see in my screenshot I can configure to have a username with at least 3 characters but maximum 1 character

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11516.}

ghost · 2017-01-10T14:58:41Z

@mxkmp29 is this PR for testing?

jeckodevelopment · 2017-01-10T21:09:57Z

@mbabker can I ask the JSST decision/opinion about this PR that may include potential security issues?

mbabker · 2017-01-16T13:18:32Z

This is not ready for a review, it still has outstanding change requests from several months ago and just through scanning a couple of files there are miscellaneous whitespace changes littered in the PR.

From a security perspective, the concept is "controversial" and this is introducing a "convenience over security" workflow by making it easier to ascertain whether a username exists on a website. Considering this is triggered by JavaScript based on typing into a field, the implementation needs to ensure that there isn't a way to turn this into a DoS attack on a site.

mxkmp · 2017-01-16T13:53:22Z

Hey,
Thanks for the big discussion under this PR, but I don't have the time and maybe not the knowledge about joomla and how to fix the security issues to finish this PR. Maybe someone else can continue this work, if some of these features are needed in a future version of joomla.

best regards

joomla-cms-bot added Language Change This is for Translators PR-staging labels Aug 8, 2016

brianteeman reviewed Aug 8, 2016
View reviewed changes

brianteeman closed this Aug 8, 2016

roland-d reopened this Aug 9, 2016

mxkmp closed this Aug 9, 2016

mxkmp force-pushed the username branch from de589d4 to ff7e512 Compare August 9, 2016 08:21

Implement AJAX user changes

17bbe6a

roland-d reopened this Aug 9, 2016

brianteeman reviewed Aug 9, 2016
View reviewed changes

added lost backend option

365a130

Fedik reviewed Aug 9, 2016
View reviewed changes

fixxed errors & typos

cc283cd

brianteeman reviewed Aug 9, 2016
View reviewed changes

fixed backend options, moved js to a seperate file, added session tokens

3461949

Fedik reviewed Aug 10, 2016
View reviewed changes

brianteeman reviewed Oct 2, 2016

View reviewed changes

corrected some textstrings

6046009

zero-24 reviewed Oct 4, 2016

View reviewed changes

Merge remote-tracking branch 'remotes/upstream/staging' into username

f2e77fd

jeckodevelopment requested a review from mbabker January 10, 2017 21:09

mxkmp closed this Jan 16, 2017

rdeutz removed this from the Joomla 3.7.0 milestone Jan 19, 2017

Added ajax Username & Email checks #11516

Added ajax Username & Email checks #11516

Conversation

mxkmp commented Aug 8, 2016 • edited by joomla-cms-bot

Summary of Changes

Testing Instructions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianteeman Aug 8, 2016 via email

Choose a reason for hiding this comment

brianteeman commented Aug 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianteeman commented Aug 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgrammatiko Aug 9, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fedik Aug 10, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxkmp commented Oct 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshJourney commented Oct 4, 2016

JoshJourney commented Oct 5, 2016

JoshJourney commented Oct 5, 2016

brianteeman commented Oct 6, 2016

brianteeman commented Oct 6, 2016

brianteeman commented Oct 6, 2016

mxkmp commented Oct 10, 2016

Summary of Changes

brianteeman commented Oct 10, 2016

brianteeman commented Oct 10, 2016

framontb commented Oct 16, 2016

brianteeman commented Oct 16, 2016

brianteeman commented Nov 10, 2016

SniperSister commented Nov 20, 2016

conconnl commented Dec 2, 2016

ghost commented Jan 10, 2017

jeckodevelopment commented Jan 10, 2017

mbabker commented Jan 16, 2017

mxkmp commented Jan 16, 2017

mxkmp commented Aug 8, 2016 •

edited by joomla-cms-bot

dgrammatiko Aug 9, 2016 •

edited

Fedik Aug 10, 2016 •

edited