BP Signup - spaces in username issue

[hnla]

Following on from #4939

The issue of BP converting spaces in usernames silently to hyphens without informing user and the possible issue of the user then not being able to login with what they thought they had used as a username - remains one to resolve.

I ran across this solution run up by Brajesh ​http://buddydev.com/buddypress/prevent-spaces-in-wordpressbuddypress-usernames/ and skimming over thought that in fact the possible solution in returning an error on spaces might not be too hard to effect?

Haven't time to run up a test case at the moment but leaving this link/ticket as a memory jogger.

[boonebgorges]

Thanks for the ticket, hnla. This behavior in BP trackes back to #471 r1061.

My suggestion is that we remove this behavior altogether, and replace it with a registration warning. This will fix the issue described in #471 for all cases where users go through normal registration, ie the vast majority of cases. The big change will be that plugins etc will be able to create new users programatically where the user_login contains spaces. But IMO this is an edge case; and in any case, the current behavior - of swapping the space with a hyphen without any warning or feedback - is at least as bad as the original problem described in #471.

There is a larger question of why we have to enforce this at all. If WP allows for spaces in user_login, then maybe we should too. As per #471, this will cause problems in some places in BP, but this is what user_nicename is for; yet there will be complications with username_compatibility_mode, etc etc etc. So, I'm going to suggest that, for the time being, we keep the behavior the same as it is now - no spaces in usernames - but simply change how that's enforced.

Patch attached.

[hnla]

My suggestion is that we remove this behavior altogether,

Is that possible (no haven't looked at patch yet ) Thought the hyphen was because BP grabbed this value/string for url

WP allows spaces as this is properly a string and spaces matter not; if using username in uri or paths spaces are evil, forbidden, only allowed by feckless Windows.

My point was retain the behaviour but throw up a warning if a space is received asking that user transpose space to a hyphen ( guess that is changing behaviour though in that BP now doesn't need to perform that strreplace itself )

I thought that Brajesh's little scriptlet did just that which is why I opened the ticket.

I'll try and run patch when I have a minute.

[boonebgorges]

My patch does pretty much the same thing as Brajesh's. I just do it inline in the registration function, which is where we do other, similar checks.

My point was retain the behaviour but throw up a warning if a space is received asking that user transpose space to a hyphen

The current behavior is to surreptitiously change the space to a hyphen. I'm suggesting that we stop doing this, and instead throw a warning. This is pretty much what you have suggested.

[henrywright]

Just a quick point to add if I may - I've found that anything that isn't an a - z letter or number in a username is confusing to site viewers.

Take for example john_smith, johnsmith and john-smith.

If somebody said to me "my username is John Smith" - and I wanted to find that person on the website, I'd do a search for John Smith and then look for @johnsmith. Underscores, hyphens and spaces all just get in the way. Does allowing these characters add any value?

I think Boone's approach to disallow these characters at the point of registration works well as everything is done in one simple validation check. This way the user gets a clear message "Get thinking of a new username because you can't have that one!"

[hnla]

@Boone Tested the patch manually and it seems fine, returns error on space used to allow the user to decide and more importantly know what their username is. We could expand the error text to add ? "Please replace spaces with hyphen or no space" but it's gilding the lilly and users can no doubt work out what to do.

@henrywright it's not so much about disallowing characters (BP never did allow spaces) as the terrible UX of silent hyphen being added if spaces encountered without the user being aware.

Where possible BP needs to be in tandem with what WP does / allows after all BP is subordinate to many WP functions & behaviours, gets confusing when/if BP diverges from the norm.

It's why in #4939 I was trying to establish and highlight what characters were allowed matched to what WP allowed, got that slightly wrong though :)

[henrywright]

@hnla agreed BP and WP should be kept consistent - users grown used to WP customs will find BP sites easier to use. By "disallowing characters", I mean't a) characters a user may enter and b) characters the system may generate (e.g the dreaded hyphen).

[r-a-y]

I say let's commit this so we can close #4622 as well.

It's the most sensible approach to this issue instead of the suggestion I made to enforce WP's method of allowing spaces for usernames in my patch for 4622.

It just doesn't make sense to use spaces for usernames in a social network, so we have to deviate from WP here.

[boonebgorges]

I've done some more thinking and hacking related to this issue today. Essentially, I'm going to reverse my opinion stated here:

So, I'm going to suggest that, for the time being, we keep the behavior the same as it is now - no spaces in usernames - but simply change how that's enforced.

and I'm going to endorse r-a-y's original idea on #4622, which was to (a) allow spaces in usernames when WP does, and (b) fix BuddyPress as necessary so that usernames with spaces work.

5185.02.patch does a number of things. I would normally try to split these up, but they're kinda interdependent, thus the single patch.

• In bp_core_validate_user_signup(), defer to wpmu_validate_user_signup() if the latter function exists. This removes all user validation responsibility from BuddyPress when using WordPress Multisite (hooray!). In the case of non-Multisite, we continue to run our own logic, which parallels WordPress's. (Non-MS WP does not have a handy signup validation function like MS does. This is an oversight that should be fixed upstream. Until then, we have to use our own function.)
• Remove bp_core_strip_username_spaces() - in other words, allow spaces in usernames. Note that this only allows spaces if WP's rules allow. MS is still very restrictive here, so if the user runs MS, spaces will still not be allowed.
• In most cases, the change to allow spaces in user_login doesn't affect BP. The exception is BP_ENABLE_USERNAME_COMPATIBILITY_MODE, which forces us to use user_login instead of (the heavily-sanitized) user_nicename in a number of places. The rest of my patch cleans up areas where this needed fixing, to wit:
  • The @-mention gloss in member header templates. @-mentions now always use user_nicename, even with compatibility mode.
  • The 'mentions' scope for bp_has_activities(), which now falls back on user_nicename in all cases.
  • The 'Send Public Message' button.
  • Autocomplete on the Messages compose screen. Here we must use urlencode() to remain compatible with the existing JS.

[hnla]

Go for it then oh capricious one.

Only thing that bothers me is the variance in behaviour between regular & MS really would prefer that it was either or spaces or no spaces; but the fundamental notion of BP falling into line with how WP handles things good or bad ( too a certain degree) has always been my preferred action.

[henrywright]

If I may add my thoughts - I'm a big fan of retaining consistency with WordPress where appropriate. However, on this occasion I think it would be worth WP following BP's lead and adopting the current BP approach to usernames - i.e don't allow spaces.

Not sure if allowing spaces will add any value? Why is WP's approach the right one in this instance?

[boonebgorges]

hnla and henrywright - Thanks for chiming in.

There are a number of incidental reasons why we want to avoid username restrictions in BuddyPress. It adds extra logic to BP that requires maintenance and debugging. And the fact that BP imposes restrictions causes problems for developers and site owners who want to debug signup issues. Along the same lines, when we interfere with stuff in this way, we have the potential to break plugins that modify the signup process for WP but don't explicitly take BP into account.

But the fundamental reason is this: it's not our job. BP does not handle authentication; WP does it for us. The purpose of user_login is authentication. So we really have *no business* messing with it.

I think the reason why people in this thread (including me) feel a bit uncomfortable about spaces in usernames has to do with URLs and other public stuff. But it's important to remember that BuddyPress does *not* use user_login in public places default. It's only when BP_ENABLE_USERNAME_COMPATIBILITY_MODE is enabled that we do. If we could get rid of this mode, I would, but we are stuck with it :)

==

Side note that 5185.02.patch needs testing with more kinds of funky user_login values with username_compatibility. I realized after posting the patch that the switch to user_nicename for activity @-mentions might cause problems for existing sites where @-mentions are already in use (and have users with logins like "foo.bar"). Also, some of the autocomplete fixes for message composing might cause clashes if a site has a user "foo bar" as well as a user "foo-bar". This latter bit is a pretty extreme edge case, but could use some eyes.

[henrywright]

Hi boonebgorges

Thanks for the explanation. I suppose the reason why I think along those lines is I feel usernames in the context of an online comminity are more than just a means to authenticate. Why do we have usernames at all when authentication can be done via an email address or even a numerical ID? I think the @-mentions feature and having usernames in the URL are powerful features of BP - features that aren't necessarily critical to WordPress if you consider WP to be a CMS or blogging tool and not a platform to build an online community.

[boonebgorges]

usernames in the context of an online comminity are more than just a means to authenticate

I'm speaking from a technical point of view here. In a normal BP installation, the *only thing* we use user_login for is authentication. We use user_nicename for everything else - including URLs and @-mentions. user_nicename cannot contain spaces. So, allowing spaces in user_login will only affect people who are using BP_ENABLE_USERNAME_COMPATIBILITY_MODE.

[henrywright]

I see! my apologies and thanks for clarifying.

[boonebgorges]

In 7570:

Remove BuddyPress's restriction spaces in user_login

BuddyPress has historically enforced a no-spaces rule on user_login during
user registration. Originally this was rooted in WPMU's own peculiar character
restrictions, and when the MU requirement was dropped, the same restrictions
were carried over to WordPress Single.

However, these restrictions have caused various problems. BP enforced the "no
spaces" rule during registration by simply swapping out spaces with hyphens and
not telling users. This caused immense confusion. Moreover, the restriction
caused problems when bypassing BP's native user registration, as when
integrating with an external authentication service; these external usernames
*can* sometimes have spaces, and certain areas of BuddyPress were not equipped
to deal with them.

This changeset removes the no-spaces restriction from BuddyPress, and hands
off user_login validation to WordPress Multisite when possible (meaning that on
MS, spaces will still not be allowed during native registration). It also
makes the necessary adjustments throughout BuddyPress to ensure that spaces
in user_login will not break related functionality. On a normal setup, BP (and
WP) only use user_login for authentication, but several changes were necessary
to account for "username compatibility mode", where the user_login is displayed
publicly:

• Refactor the way that activity @-mentions work in username compatibility mode. We now have functions for converting user IDs to "mentionname" (and vice versa) which will produce @-mention-safe versions of user_nicename or user_login, as appropriate.
• Use proper URL encoding when building and parsing URLs that contain usernames when compatibility mode is enabled.
• Fix private messaging autocomplete to work with spaces.

See #4622

Fixes #5185

[hnla]

Ran a few test signups on bp trunk, single install / MS - WP 3.6/3.7

All seems pretty safe. MS doesn't seem to present any issues I could spot, but then in handing off to MS and following it's lead we should be ok?, biggest gripe there is that on failure MS error return lacks detail just saying lower case or numerals please nothing about the fact the actual problem was trying to register a space in user name, which is a shame as that was the original intent of ticket to attempt to make UX better but that's not our validation though!

Only real problem encountered is as mentioned above:

So, allowing spaces in user_login will only affect people who are using BP_ENABLE_USERNAME_COMPATIBILITY_MODE.

If a name registered with a space all is well and fine, however define 'BP_ENABLE_USERNAME_COMPATIBILITY_MODE' and we have the minor issue that now we display '@joe blogs' and that can't be used as a @mention it must be '@joe-blogs' so users might be confused but this is surely an edge case? I would be happier if define didn't exist really :)

Other than that I couldn't see any problems, not an exhaustive series of tests but a few different variations on both types on install.

[boonebgorges]

we have the minor issue that now we display '@joe blogs'

We should not be having that issue. Can you clarify where you're seeing that string? In member headers, it should say @joe-blogs. Make sure you're not using a theme with a custom member-header.php.

[hnla]

Ah ok.

Yes testing on a child theme, yes had overloaded templates, but those were unedited/unchanged templates.
Changed to twentythirteen and @mention is hyphenated in screen display when running 'BP_ENABLE_USERNAME_COMPATIBILITY_MODE' so all is good.

Problem is though that we have gone and swapped out a template tag for a new one 'bp_displayed_user_mentionname()' so that was why I saw the issue, I guess this is a good example of where if possible template tags need to be very generic e.g. 'bp_displayed_at_mention()' and changes fed into that from the core to prevent users having to edit their overloaded template files.

On a sidenote: noticed that checking the 5185-02 patch showed a previous version of the members-header template before that new template tag was introduced so guess that was committed after patch put up?

[boonebgorges]

I guess this is a good example of where if possible template tags need to be very generic e.g. 'bp_displayed_at_mention()' and changes fed into that from the core to prevent users having to edit their overloaded template files.

Yes. And generally we try to do this. But the problem in this case was that the existing template tag in use referred directly to the nicename, which was *too specific*. So it needed to be changed.

The problem you saw will only arise in the wild on an installation that (a) is running username compatibility mode *and* (b) is overriding the member-header.php template. This is a very small number of installations. And even in this case, it's only a display error - the "Public Message" button should still work correctly. So I don't think it's much to worry about.

On a sidenote: noticed that checking the 5185-02 patch showed a previous version of the members-header template before that new template tag was introduced so guess that was committed after patch put up?

A couple of commits happened to that line of the template in the interim. Check the logs for more details.

[hnla]

The problem you saw will only arise in the wild on an installation that (a) is running username compatibility mode *and* (b) is overriding the member-header.php template. This is a very small number of installations. And even in this case, it's only a display error - the "Public Message" button should still work correctly. So I don't think it's much to worry about.

Absolutely agree, it's a edge case, that is unlikely to surface and if does fairly easily rectified.

However they may already be a few sites that have overloaded the template files and who will miss out on the template tag change.

Tested messaging and didn't see any issues.

Unless anyone else has run any tests to report, think this is fairly solid.