Ruminations of J.net idle rants and ramblings of a code monkey

New Account Email Validation (Part II)

.NET Stuff | Security | Web (and ASP.NET) Stuff

In my previous post, I discussed the things to keep in mind with new account validation. Well, as promised, I've done a sample of one way to do this.

Certainly step 1 to to do as much as possible without writing any code, following the KISS principle. Since I am using the CreateUserWizard Control, I set the DisableCreatedUser property to true and LoginCreatedUser to false. Easy enough. But that's not the whole story. We need to generate the actual validation key. There are a lot of ways that one can do this. Personally, I wanted, as much as possible, to not have any dependency on storing the validation code in the database anywhere. This, of course, ensures that, should our database be penetrated, the validation codes cannot be determined. With that, then, the validation code should come from data that is supplied by the user and then generated in a deterministic way on the server. Non-deterministic, of course, won't work too well.

I started down (and really, almost completed) a path that took the UserName and Email, concatenated them, generating the bytes (using System.Security.Cryptography.Rfs2898DeriveBytes) to create a 32-byte salt from this. I again concatenated the UserName and email, then hashing it with SHA1. This certainly satisfied my conditions ... the values for this would come from the user and so the validation code didn't need to be stored. And it was certainly convoluted enough that a validation code would be highly difficult to guess, even by brute force. In the email to the user, I also included a helpful link that passed the validation code in the query string. Still, this code was some 28 characters in length. Truly, not an ideal scenario. And definitely complex. It was certainly fun to get the regular expression to validate this correct ... more because I'm just not all that good at regular expressions then anything else. If you are interested, the expression is ^\w{27}=$, just in case you were wondering.

Thinking about this, I really didn't like the complexity. It seems that I fell into that trap that often ensnares developers: loving the idea of a complex solution. Yes, it's true ... sometime developers are absolutely drawn to create things complex solutions to what should be a simple problem because they can. I guess is a sort of intellectual ego coming out ... we seem to like to show off how smart we are. And all developers can be smitten by it. Developing software can be complex enough on its own ... there really is no good reason to add to that complexity when you don't need to. There are 3 key reasons that come to mind for this. 1) The code is harder to maintain. Digging through the convolutions of overly complicated code can make the brain hurt. I've done it and didn't like it at all. 2) The more complex the code, the more likely you are to have bugs or issues. There's more room for error and the fact that it's complicated and convoluted make it easier to introduce these errors and then miss them later. It also makes thorough testing harder, so many bugs may not be caught until it's too late.

So, I wound up re-writing the validation code generation. How did I do it? It's actually very simple. First, I convert the user name, email address and create date into byte arrays. I then loop over all of the values, adding them together. Finally, I subtract the sum of the lengths of the user name, password and creation date and subtract from the previous value. This then becomes the validation code. Typically, it's a 4 digit number. This method has several things going for it. First, it sticks to the KISS principle. It is simple. There are very few lines of code in the procedure and these lines are pretty simple to follow. There are other values that can be used ... for example, the MembershipUser's ProviderKey ... when you are using the Sql Membership provider, this is a GUID. But not depending on it gives you less dependence on this. Second, it is generated from a combination of values supplied by the user and values that are kept in the database. There is nothing that indicates what is being used in the code generation ... it's just a field that happened to be there. This value is not as random as the previous, I know. It's a relatively small number and a bad guy could likely get it pretty quickly with a brute-force attack if they knew it was all numbers. To mitigate against this, one could keep track of attempted validations with the MembershipUser using the comments property, locking the account when there are too many attempts within a certain time period. No, I did not do this. Considering what I was going to use the for (yes, I am actually going to use it), the potential damage was pretty low and I felt that it was an acceptable risk. Overall, it's a pretty simple way to come up with a relatively good validation code. And it's also very user-friendly. Here's the code:

public static string CreateValidationCode(System.Web.Security.MembershipUser user)
{
byte[] userNameBytes = System.Text.Encoding.UTF32.GetBytes(user.UserName);byte[] emailBytes = System.Text.Encoding.UTF32.GetBytes(user.Email);byte[] createDateBytes = System.Text.Encoding.UTF32.GetBytes(user.CreationDate.ToString());int validationcode = 0;foreach (byte value in userNameBytes)   { validationcode += value; }
foreach (byte value in emailBytes)      { validationcode += value; }
foreach (byte value in createDateBytes) { validationcode += value; }
validationcode -= (user.UserName.Length + user.Email.Length + user.CreationDate.ToString().Length);return validationcode.ToString(); 
}

Architecturally, all of the code related to this is in a single class called MailValidation. Everything related to the validation codes is done in that class, so moving from the overly-complex method to my simpler method was easy as pie. All I had to do was change the internal implementation. Now that I think of it, there's no reason why it can't be done using a provider model so that different implementations are plug-able.

Once the user is created, we generate the validation code. It is never stored on the server, but is sent to the user in an email. This email comes from the MailDefinition specified with the CreateUserWizard ... this little property points to a file that the wizard will automatically send to the new user. It will put the user name and password in there (with the proper formatting), but you'll need to trap the SendingMail event to modify it before it gets sent in order to put the URL and validation code in the email.

//This event fires when the control sends an email to the new user.protected void CreateUserWizard1_SendingMail(object sender, MailMessageEventArgs e)
{
//Get the MembershipUser that we just created.MembershipUser newUser = Membership.GetUser(CreateUserWizard1.UserName);//Create the validation codestring validationCode = MailValidation.CreateValidationCode(newUser);//And build the url for the validation page.UriBuilder builder = new UriBuilder("http",
Request.Url.DnsSafeHost,
Request.Url.Port, Page.ResolveUrl("ValidateLogin.aspx"), "C=" + validationCode);//Add the values to the mail message.e.Message.Body = e.Message.Body.Replace("<%validationurl%>", builder.Uri.ToString());
e.Message.Body = e.Message.Body.Replace("<%validationcode%>", validationCode);
}

One thing that I want to point out here ... I'm using the UriBuilder class to create the link back tot he validation page. Why don't I just take the full URL of the page and replace "CreateAccount.aspx" with the new page? Well, I would be concerned about canonicalization issue. I'm not saying that there would be any, but it's better to be safe. The UriBuilder will give us a good, clean url. The port is added in there so that it works even if it's running under the VS development web server, which puts the site on random ports. I do see a lot of developers using things like String.Replace() and parsing to get urls in these kinds of scenarios. I really wish they wouldn't.

Things do get a little more complicated, however, when actually validating the code. There is a separate form, of course, that does this. Basically, it collects the data from the user, regenerated the validation key and then compares them. It also checks the user's password by calling Membership.ValidateUser. If either of these fails, the user is not validated. Seems simple, right? Well, there is a monkey wrench in here. If the MembershipUser's IsValidated property is false, ValidateUser will always fail. So we can't fully validate the user until they are validated. But ... we need the password to validate their user account. See the problem? If I just check the validation code and the password is incorrect, you shouldn't be able to validate. What I had to wind up doing was this: once the validation code was validated, I had to then set IsApproved to true. Then I'd called ValidateUser. If this failed, I'd then set it back.

protected void Validate_Click(object sender, EventArgs e)
{
//Get the membership user.MembershipUser user = Membership.GetUser(UserName.Text);bool validatedUser = false;if (user != null)
{
if (MailValidation.CheckValidationCode(user, ValidationCode.Text))
{
//Have to set the user to approved to validate the passworduser.IsApproved = true;Membership.UpdateUser(user);if (Membership.ValidateUser(UserName.Text, Password.Text))
{
validatedUser = true;
}
}
}
//Set the validity for the user.SetUserValidity(user, validatedUser);
}

You do see, of course, where I had to Approve the user and then check. Not ideal, not what I wanted, but there was really no other way to to it. There are a couple of things, however, that I want to point out. Note that I do the actual, final work at the very end of the function. Nowhere am I called that SetUserValidity method until the end after I've explored all of code branches necessary. Again, I've seen developers embed this stuff directly in the If blocks. Ewww. And that makes it a lot harder if someone needs to alter the process later. Note that I also initialize the validatedUser variable to false. Assume the failure. Only when I know it's gone through all of the tests and is good do I set that validatedUser flag to true. It both helps keep the code simpler and ensure that if something was missed, it would fail.

Well, that's it for now. You can download the code at http://code.msdn.microsoft.com/jdotnet.