? ? ? Success

User tests: Successful: Unsuccessful:

avatar mxkmp29
mxkmp29
8 Aug 2016

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
avatar joomla-cms-bot joomla-cms-bot - change - 8 Aug 2016
Category Administration Components Language & Strings Templates (admin) Front End
avatar mxkmp29 mxkmp29 - open - 8 Aug 2016
avatar mxkmp29 mxkmp29 - change - 8 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Aug 2016
Labels Added: ? ?
avatar brianteeman
brianteeman - comment - 8 Aug 2016

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

avatar brianteeman brianteeman - change - 8 Aug 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-08-08 13:17:05
Closed_By brianteeman
Labels
avatar brianteeman
brianteeman - comment - 8 Aug 2016

Closing for now until the branch is fixed


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

avatar brianteeman brianteeman - change - 8 Aug 2016
Labels
avatar brianteeman brianteeman - close - 8 Aug 2016
avatar roland-d roland-d - change - 9 Aug 2016
Status Closed New
Closed_Date 2016-08-08 13:17:05
Closed_By brianteeman
avatar roland-d roland-d - reopen - 9 Aug 2016
avatar roland-d roland-d - change - 9 Aug 2016
Status New Pending
avatar mxkmp29 mxkmp29 - close - 9 Aug 2016
avatar mxkmp29 mxkmp29 - change - 9 Aug 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-08-09 08:20:54
Closed_By mxkmp29
avatar mxkmp29 mxkmp29 - change - 9 Aug 2016
Status Closed Pending
avatar roland-d roland-d - reopen - 9 Aug 2016
avatar brianteeman brianteeman - test_item - 11 Aug 2016 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 11 Aug 2016

I have tested this item 🔴 unsuccessfully on 53e1a77

Before this PR there is ONE tab called password options
After this PR there are TWO tabs


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

avatar Schmidie64 Schmidie64 - test_item - 12 Aug 2016 - Tested successfully
avatar Schmidie64
Schmidie64 - comment - 12 Aug 2016

I have tested this item ✅ successfully on 809f68a

@icampus
I have tested this Pull Request Successfully. I installed the Pull Request and followed the Test Instructions.
All domains i set on the Blacklist were blocked.
Everything work well.


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

avatar brianteeman brianteeman - test_item - 12 Aug 2016 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 12 Aug 2016

I have tested this item 🔴 unsuccessfully on 809f68a

The default setting for the number of characters in a password is 0 which means there is no minimum or maximum characters
BUT
Try to save a username with just one character and you will get the following error
Save failed with the following error: Please enter a valid username. No space at beginning or end, at least 2 characters and must not contain the following characters: < > \ " ' % ; ( ) &.


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

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Entering an email from the blocked list gives a different error in the admin and the frontend

admin

Warning
Invalid field: Email

frontend

Warning
The email address you entered is already in use or invalid. Please enter another email address.

It would be preferable if they were the same and then we can work on some more appropriate text for the message


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

avatar brianteeman
brianteeman - comment - 12 Aug 2016

####AJAX
Maybe its me misunderstanding but i am not seeing any ajax validation. Can you please record a small screen capture of what I am supposed to be seeing


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

avatar mxkmp29
mxkmp29 - comment - 12 Aug 2016

username
If a username is entered and the field is leaved the ajax check is exectuted.
Thanks for the testing I'll fix the issue with the username length and the backend message.

avatar mxkmp29 mxkmp29 - change - 12 Aug 2016
Status Pending Closed
Closed_Date 2016-08-09 08:20:54 2016-08-12 09:25:12
avatar mxkmp29 mxkmp29 - close - 12 Aug 2016
avatar mxkmp29 mxkmp29 - change - 12 Aug 2016
Status Closed New
Closed_Date 2016-08-12 09:25:12
Closed_By mxkmp29
avatar mxkmp29 mxkmp29 - change - 12 Aug 2016
Status New Pending
avatar mxkmp29 mxkmp29 - reopen - 12 Aug 2016
avatar mxkmp29
mxkmp29 - comment - 12 Aug 2016

sorry, I clicked the wrong button

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Ah I see now that the AJAX stuff only works on the frontend - it doesnt work on the admin at all

avatar roland-d
roland-d - comment - 12 Aug 2016

@brianteeman The reason you see 2 different messages is because the back-end form uses Javascript validation and the front-end does not. The Javascript validation has no option for using language strings at this moment. It simply reads the invalid field label and concatenates that to the Invalid field: message.

To make this consistent Max would have to rewrite the whole back-end form validation. Even though I agree the messages should be the same, this may be out of scope for this pull request and would warrant another pull request.

In general I believe we should have a more flexible and consistent validation system for forms without duplicating checks.

Your thoughts?

avatar brianteeman
brianteeman - comment - 12 Aug 2016

yes I see now that the ajax stuff is not present on the admin side at all.

  • yes it probably is out of scope

On 12 August 2016 at 11:01, RolandD notifications@github.com wrote:

@brianteeman https://github.com/brianteeman The reason you see 2
different messages is because the back-end form uses Javascript validation
and the front-end does not. The Javascript validation has no option for
using language strings at this moment. It simply reads the invalid field
label and concatenates that to the *Invalid field: * message.

To make this consistent Max would have to rewrite the whole back-end form
validation. Even though I agree the messages should be the same, this may
be out of scope for this pull request and would warrant another pull
request.

In general I believe we should have a more flexible and consistent
validation system for forms without duplicating checks.

Your thoughts?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11516 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8fNngwWlGZnPeG6IevptfpH8Q0Kpks5qfERxgaJpZM4Je_OM
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar mxkmp29 mxkmp29 - change - 12 Aug 2016
The description was changed
avatar roland-d
roland-d - comment - 12 Aug 2016

Thanks for the input, we will keep it to the front-end for this PR. Max already fixed the username length, so it's ready for testing again.

avatar brianteeman
brianteeman - comment - 12 Aug 2016

thanks for fixing the username length - that works

On 12 August 2016 at 11:10, RolandD notifications@github.com wrote:

Thanks for the input, we will keep it to the front-end for this PR. Max
already fixed the username length, so it's ready for testing again.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11516 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8dmov8RTArR4-N36STvEosjLB_aeks5qfEaggaJpZM4Je_OM
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Is there supposed to be an ajax check on the email?


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

avatar ggppdk
ggppdk - comment - 12 Aug 2016

This PR needs a "New feature" label ?

avatar mxkmp29
mxkmp29 - comment - 12 Aug 2016
avatar brianteeman
brianteeman - comment - 12 Aug 2016

Max already fixed the username length, so it's ready for testing again.

The language string needs updating now as there isnt a Zero indicates no limit

avatar brianteeman brianteeman - change - 12 Aug 2016
Labels Added: ?
avatar brianteeman
brianteeman - comment - 12 Aug 2016

Its possible to set the maximum number of characters in the username to less than the minimum

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Set the minimum number of characters to 3 and you can still create a 2 character username in the admin.
In the front end you are restricted but the error messages are wrong
You get these two

Warning
The username field should contain a minimum of 3 characters. You only give 2.
The username you entered is not available. Please pick another username.

You dont need the two messages - and I would be frustrated if I then chose a username that was longer than the maximum

The username should contain a minimum of 3 characters and a maximum of 5. You only gave 2. Please enter another username.

avatar ggppdk
ggppdk - comment - 12 Aug 2016

Some comments,

Using a comparison tool, i see that : media/system/js/validate-uncompressed.js
only has fixed indentationand some new lines, i guess it is ok

Also i see that this can be usable in 3rd party code too

A question should the number of requests to:

$ajaxUri = JRoute::_('index.php?option=com_users&task=registration.validate&format=json&' . JSession::getFormToken() . '=1');

be limited to somehow ?
or is it ok to allow unlimited number ?
someone can grab the above url, from the HTML and make unlimited requests ?

avatar roland-d
roland-d - comment - 12 Aug 2016

@brianteeman We looked at this and it actually works as designed at the moment. I agree with you that we should just show one error message. The problem is the following. To be able to make a customized error message based on the settings of the site, we need to pass it the configuration values. This is impossible with the current way Joomla works. By adding the message to the rule which contains these settings we can customize the message.

Now the XML definition contains an element called message which holds the language string to show when there is an error. This is why we get two messages, the customized one and the default one. Now the idea was to remove the default one but that resulted in Invalid field: Username. So that is not the way to go either.

The only solution I see for one message is to modify the existing default ones and forget about the customizing of the message. The default one should be very generic in that case.

@ggppdk The requests are limited by the session token which I believe is good enough. Yes, you can grab that URL and make unlimited requests just like you can take the homepage URL and make unlimited requests. I guess I don't see this as an issue.

avatar brianteeman
brianteeman - comment - 12 Aug 2016

If it tells me that I have failed the minimum and try again
and then I try again and it tells me the maximum I will not be a happy user

avatar roland-d
roland-d - comment - 12 Aug 2016

The current default message is:

The username you entered is not available. Please pick another username.

This we can change to:

The username does not meet the requirements.

It just won't tell you which requirements because we can't specify them.

avatar ggppdk
ggppdk - comment - 12 Aug 2016

@roland-d

The requests are limited by the session token which I believe is good enough. Yes, you can grab that URL and make unlimited requests just like you can take the homepage URL and make unlimited requests. I guess I don't see this as an issue.

ok, i see
i was just wondering if this makes much easier, to find a sub-set of usernames and emails that are used in a website,

JSON requests will also run much faster and also it is much easier to check the response
i guess, related plugins at JED will be updated to handle this case too,

avatar roland-d
roland-d - comment - 12 Aug 2016

@ggppdk As for finding out existing usernames/emails that will depend on the message given I guess. You can also write a script that mimics sending the user registration and parse the output of that. Indeed JSON responses are easier but does it make a real difference with todays computing powers?

avatar ggppdk
ggppdk - comment - 12 Aug 2016

@roland-d
indeed you are right, not too much difference,

and adding built-in brute-force prevention is off-topic for this PR (and i know that there are good plugins at JED for this)

avatar roland-d
roland-d - comment - 12 Aug 2016

@ggppdk You have a valid point but we may want to have something generic then in Joomla. Perhaps with webservices ;)

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Just wondering why the maximum number of characters for a username is 99 when the field is varchar(150)

avatar mxkmp29
mxkmp29 - comment - 12 Aug 2016

because the minimum length have to be 3 characters

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Maybe I am being thick but why does an absolute minimum of three prevent you from setting a minimum of 150

avatar mxkmp29
mxkmp29 - comment - 12 Aug 2016

sorry I was a bit confused. Should the maximum 149 because of the 0 it would be 150 Characters or I'm wrong ? @brianteeman @roland-d

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Yes you are confused. You are confusing the value of an integer with the nu!best of characters in a field

avatar roland-d roland-d - change - 16 Aug 2016
Milestone Added:
avatar roland-d
roland-d - comment - 16 Aug 2016

Since the DB size of the field is 255 characters, we can set the maximum to 255. The minimum is 3 and up as shown in the message. Those are the correct numbers I think.

avatar brianteeman
brianteeman - comment - 12 Sep 2016

What is the status of this?


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

avatar mxkmp29
mxkmp29 - comment - 13 Sep 2016

its ready to test @brianteeman

avatar JoshuaLewis JoshuaLewis - test_item - 22 Sep 2016 - Tested successfully
avatar JoshuaLewis
JoshuaLewis - comment - 22 Sep 2016

I have tested this item ✅ successfully on 37a7ec8

The checks seem to work pretty well, however I do suggest making the warning message go away once you pass validation. For example if the name "Josh" was taken and I change it to "JoshLewis", the message above remains which gives the vibe that both are taken.


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

avatar framontb
framontb - comment - 24 Sep 2016

Happy to see this almost done, @mxkmp29 . An impressive work by the whole team !
It was harder than it seemed at first sight.
Perhaps it should be tested specifically the UNICODE Security MECHANISMS IMPLEMENTATION of Highly Restrictive level (libraries/joomla/form/rule/username.php). I did some tests when I implemented it, but it's worthy a review. The Default Identifier Syntax code (UAX31-D1) is commented because I thought it's too restrictive. Test and insight required too.
Regards. Good job !

avatar mxkmp29
mxkmp29 - comment - 30 Sep 2016

@JoshuaLewis thanks for the feedback, I changed it.

avatar JoshuaLewis JoshuaLewis - test_item - 1 Oct 2016 - Tested successfully
avatar JoshuaLewis
JoshuaLewis - comment - 1 Oct 2016

I have tested this item ✅ successfully on a8f6ae3

Awesome, love the new addition! :-D Tested successfully, works great.


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

avatar JoshuaLewis
JoshuaLewis - comment - 1 Oct 2016

I have tested this item ✅ successfully on a8f6ae3

Awesome, love the new addition! :-D Tested successfully, works great.


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

avatar JoshuaLewis
JoshuaLewis - comment - 1 Oct 2016

Looking for more testers. @brianteeman @wilsonge Can you test this as well?

avatar brianteeman brianteeman - change - 2 Oct 2016
Category Administration Components Language & Strings Templates (admin) Front End Administration Components Feature Request Front End Language & Strings Templates (admin)
avatar brianteeman brianteeman - change - 2 Oct 2016
Labels
avatar brianteeman
brianteeman - comment - 2 Oct 2016

@mxkmp29 please address the comments in the code by @roland-d @mbabker and myself

avatar mxkmp29
mxkmp29 - comment - 4 Oct 2016

@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.

avatar JoshuaLewis
JoshuaLewis - comment - 4 Oct 2016

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

avatar JoshuaLewis JoshuaLewis - test_item - 5 Oct 2016 - Tested successfully
avatar JoshuaLewis
JoshuaLewis - comment - 5 Oct 2016

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.

avatar JoshuaLewis
JoshuaLewis - comment - 5 Oct 2016

@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.

avatar brianteeman
brianteeman - comment - 6 Oct 2016

@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?

avatar brianteeman
brianteeman - comment - 6 Oct 2016

Surely it is not correct that I can set the minimum number of characters to be greater than the maximum number of characters
screen shot 2016-10-06 at 02 32 07


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

avatar brianteeman
brianteeman - comment - 6 Oct 2016

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

ajax

avatar mxkmp29
mxkmp29 - comment - 10 Oct 2016

@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.

avatar brianteeman
brianteeman - comment - 10 Oct 2016

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

avatar brianteeman
brianteeman - comment - 10 Oct 2016

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.

avatar framontb
framontb - comment - 16 Oct 2016

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

avatar brianteeman
brianteeman - comment - 16 Oct 2016

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

avatar brianteeman
brianteeman - comment - 10 Nov 2016

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.

avatar SniperSister
SniperSister - comment - 20 Nov 2016

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.

avatar conconnl
conconnl - comment - 2 Dec 2016

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

screen shot 2016-12-02 at 06 42 15


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

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 10 Jan 2017

@mxkmp29 is this PR for testing?

avatar jeckodevelopment
jeckodevelopment - comment - 10 Jan 2017

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

avatar mbabker
mbabker - comment - 16 Jan 2017

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.

avatar mxkmp29
mxkmp29 - comment - 16 Jan 2017

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

avatar mxkmp29 mxkmp29 - change - 16 Jan 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-01-16 13:53:45
Closed_By mxkmp29
avatar mxkmp29 mxkmp29 - close - 16 Jan 2017
avatar joomla-cms-bot joomla-cms-bot - change - 16 Jan 2017
Category Administration Components Language & Strings Templates (admin) Front End Feature Request Administration com_users Language & Strings Front End Libraries JavaScript Components Feature Request

Add a Comment

Login with GitHub to post a comment