show error message to user for invalid login (with @) #2229

[muhammadbilal19]

Another small issue for showing error message to user in case of invalid login name. It will look like that

[deruelle]

Thanks @muhammadbilal19. You're on PR fire ;)

@ammendonca can you please help review this ?

[muhammadbilal19]

@ammendonca did you managed to review this PR?

[muhammadbilal19]

Thanks @ammendonca Yes i agreed with you, i changed the ui see the screenshot

As per my info only restriction which i can see is '@' character in loginName feel free to comment if you want me to add any other restrictions on loginName.

[ammendonca]

Hi @muhammadbilal19,

I'm not sure what are all the forbidden (or the allowed) characters for the login name, but I think it cannot have spaces as well, it cannot have colon :, etc...

Typically, for security reasons it's best to allow what we want and forbid eveything else, instead of trying to block only a certain characters and allow everything else. Maybe checking how it is validated in the backend and we do the same for the frontend, should be the best way.

[muhammadbilal19]

Thanks @ammendonca validation in the backend only checking for '@' symbol.

[ammendonca]

@muhammadbilal19 I just saw that... Well, I think the backend might need some improvements then as well, as you can effectively register new clients with such characters, but I'm not sure it's valid, some of them don't work. @gvagenas any hints on what should be valid ?

[muhammadbilal19]

@ammendonca i am happy to fix backend as well. Lets wait for @gvagenas or @deruelle comments on it.

[deruelle]

@ammendonca @muhammadbilal19 @gvagenas let's make the backend consistent with what we allow in the front end. I would keep only alphanumeric characters as a first iteration and assess if others are required later.

[muhammadbilal19]

@ammendonca sorry for being late i was bit busy these days, i changed the validation for login name. Following are the changes which i did

• Now only alphanumeric characters allowed as Login Name.
• I also fixed one more validation if used creates clients using api and send login name as null system creates clients successfully. See the attached images

I know it is best practice to create new issue and then create new PR but i thought that can be checked along with this PR because you don't have to do any extra steps for it. Feel free to share your comments if you have any.

[ammendonca]

Thanks @muhammadbilal19.

Still a few comments regarding this:

• When the login is invalid, the input form should be highlighted in red and the Register button should be disabled, as it happens when the field is empty or has an @. Otherwise it seems everything is OK, and clicking Register in that case, doesn't even show any visible error.
• Personally I think we should allow periods (.) in the login, it's something common in emails for instance and if we ever want to map the clients to an email account, that should be useful;
• The popup message should be a bit more clearer for the general audience, instead of Must be alphanumeric something like Letters, numbers and periods can be used. (also, please use proper spacing and punctuation);
• I wouldn't use the com.google.common.base.Strings.isNullOrEmpty(text) for such a simple test, but this is more of a personal preference, to not rely on 3rd party libs unless necessary;

I think that's all :)