Fixes issue #115 (logging in with invalid character results in creating a new character)

[sergkr]

The HELLO message was replaced with two new message types, CREATE and LOGIN, which are now handled separately on the server. Quite a few changes, but fairly straightforward.

As of this pull request, there are at least four error conditions that could occur during the login / new character creation process:

• Attempting to log in with a valid user but an incorrect password
• Attempting to log in with a non-existent user
• Attempting to create a new user, but the username is already taken
• Attempting to log in as a user who is already logged in

Let me know if you can think of any other wonky cases we might not be handling at the moment.

There are two gray areas I wasn't too sure about:

• Guilds: It looks like we have partial support for guilds, but I'm not sure of the status, as there were swaths of code related to guilds that were either commented out or otherwise appeared to be non-functional. In particular, see my changes in gameclient.js. There were two versions of sendHello in there, and the one that appears to have support for guilds was commented out, so it appears logging in as a guild member did not work previously. As such, I didn't bother with guild support when making my changes. If guilds were meant to be functional, perhaps we should make this a separate issue?
• Banned players: From the code, it looks like it might be possible to ban players by IP address (see checkBan and banPlayer functions in server/js/db_providers/redis.js). However, I'm not sure how this functionality works and how to go about testing it. I left in the call to checkBan in server/js/player.js, but it's only there for the login case, and not the create new user case. As such, it would probably be possible for a banned player to keep creating new characters to circumvent the ban. If we're actually supporting this functionality, this should probably be fixed, but I couldn't find anything that actually calls banPlayer anywhere within the project code, so I don't know how this works.

If anyone is familiar with any of the above functionality and its current status, please let me know!

[justinclift]

Tried this out and it's working, though there is a security problem with the general approach that needs to be fixed before it's merged.

With the four states:

• Attempting to log in with a valid user but an incorrect password
• Attempting to log in with a non-existent user
• Attempting to create a new user, but the username is already taken
• Attempting to log in as a user who is already logged in

In general good security practise, we shouldn't distinguish between "valid user but an incorrect password" and "non-existent user" when we communicate back to the user.

So, if a user tries to log in with their account but gets the password wrong, we should issue an error message like "Unknown username / password combination". If a user tries with a non-existent user, they get the same response.

It's done this way so evil hacker types can't sent thousands/millions of different login messages to a server to "guess" a valid username, then when they get one they could brute force the password much easier.

Does that make sense? If not, I can dig up references to pages that explain it much better. 😄

[sergkr]

I fixed it so we don't differentiate between invalid usernames and invalid passwords.

I also noticed a couple other minor bugs along the way (respawn didn't work properly after some refactoring I did, and I also noticed funny things happen if you hit Enter multiple times at the login form when the game it already loading - it basically tries to start the game multiple times). I fixed these in the last two commits.

[justinclift]

Excellent @sergkr. Just tried it out here, and that works well. 😄

This means our Login form now operates much more in line with other web apps, using a well known/familiar approach for people. Good stuff!

[justinclift]

As a data point, I've just tagged this commit in git with "v1.4.0":

    https://github.com/browserquest/BrowserQuest/releases/tag/v1.4.0

I think this commit is where we first break backwards compatibility with the original BQ server codebase.

From here onwards, we likely can't use our BQ client with the original Mozilla BQ server.