.NET Performance Blog

April 5, 2011

OpenFaq (Part 3): Custom Membership Provider in Entity Framework Code First

Filed under: ASP.NET MVC,BDD,DDD,General,TDD — Eric P @ 4:33 am

Previous posts in OpenFaq series
OpenFaq (Part 1): The beginning
OpenFaq (Part 2): Business Requirements in Code
Source code in Codeplex
http://openfaq.codeplex.com/SourceControl/list/changesets

Why Custom Membership Provider

When creating new MVC web site – it is automatically configured to use SqlMembershipProvider. Before it can actually be used, the default tables need to be created in DB using spnet_regsql.exe tool here.
http://www.asp.net/security/tutorials/creating-the-membership-schema-in-sql-server-vb.
That creates a whole bunch of tables, views and stored procs. See link above.

I would like to be pretty consistent in my approach to how the database is accessed (through Entity Framework) and how my tables are named and structured. So I am going to use a flexible Provider model supplied my mine and your friends at Microsoft to implement a custom membership provider that will be driven by Code First Entity Framework.

YAGNI says only to implement things I need and worry about “things I don’t need” when I need them. So at this point in my application I just want to be able to register users, log them in and possibly change their password.

The acceptance tests written in part 2 of this series should still work.

Getting Samples from the Holy Book of MS (MSDN)

I used several examples provided by Microsoft for guidance:
http://msdn.microsoft.com/en-us/library/6tc47t75.aspx – simple example of ODBC Membership provider (C# after VB.NET code)
http://msdn.microsoft.com/en-us/library/aa478948.aspx – toolkit that includes fully featured example of Sql Membership/Role/Profile providers

A couple of interesting discoveries during implementation.

1. Some business logic code that should be re-usable is not provided in base MembershipProvider class

Two major cases of this were:
– Password Validation – like checking that password must be greater then 6 chars
– Password hashing – using Clear/Encrypted/Hashed

Both samples from Microsoft implement their own versions of validation and hashing.
In my case I had to implement my own (mostly by copying the toolkit sample).

I was hoping that provider model would allow to override these scenarios, but provide basic implementation which would be applicable in 90% of the cases.
Unfortunately that’s not the case.

2. Bad coding practices in sample

In Tooklit example there were a couple of things, that would make some developers punch the wall with their head.

The “Code Duplication” is the only major one, but since many developers are looking to microsoft for guidelines on how to write good code, I think MS should really make sure that their code has been properly reviewed and updated whenever there are new .NET features and guidelines. For ex. ODBC Membership provider sample is still using Hungarian notation which MS has been saying not to use for a while now.

Here are some examples of bad coding practices from Toolkit sample:

Code Duplication

Password validation logic is duplicated in CreateUser and ChangePassword

-- In CreateUser

           if( password.Length < MinRequiredPasswordLength )
            {
                status = MembershipCreateStatus.InvalidPassword;
                return null;
            }

            int count = 0;

            for( int i = 0; i < password.Length; i++ )
            {
                if( !char.IsLetterOrDigit( password, i ) )
                {
                    count++;
                }
            }

            if( count < MinRequiredNonAlphanumericCharacters )
            {
                status = MembershipCreateStatus.InvalidPassword;
                return null;
            }

            if( PasswordStrengthRegularExpression.Length > 0 )
            {
                if( !Regex.IsMatch( password, PasswordStrengthRegularExpression ) )
                {
                    status = MembershipCreateStatus.InvalidPassword;
                    return null;
                }
            }

            string salt = GenerateSalt();
            string pass = EncodePassword(password, (int)_PasswordFormat, salt);
            if ( pass.Length > 128 )
            {
                status = MembershipCreateStatus.InvalidPassword;
                return null;
            }


-- In ChangePassword
  

            if( newPassword.Length < MinRequiredPasswordLength )
            {
                throw new ArgumentException(SR.GetString(
                              SR.Password_too_short,
                              "newPassword",
                              MinRequiredPasswordLength.ToString(CultureInfo.InvariantCulture)));
            }

            int count = 0;

            for( int i = 0; i < newPassword.Length; i++ )
            {
                if( !char.IsLetterOrDigit( newPassword, i ) )
                {
                    count++;
                }
            }

            if( count < MinRequiredNonAlphanumericCharacters )
            {
                throw new ArgumentException(SR.GetString(
                              SR.Password_need_more_non_alpha_numeric_chars,
                              "newPassword",
                              MinRequiredNonAlphanumericCharacters.ToString(CultureInfo.InvariantCulture)));
            }

            if( PasswordStrengthRegularExpression.Length > 0 )
            {
                if( !Regex.IsMatch( newPassword, PasswordStrengthRegularExpression ) )
                {
                    throw new ArgumentException(SR.GetString(SR.Password_does_not_match_regular_expression,
                                                             "newPassword"));
                }
            }

            string pass = EncodePassword(newPassword, (int)passwordFormat, salt);
            if ( pass.Length > 128 )
            {
                throw new ArgumentException(SR.GetString(SR.Membership_password_too_long), "newPassword");
            }
            ...

The logic from both functions should be combined into one method like ValidatePassword, which returns enough information to generate proper exception or return error code in calling function.

Using try/catch with no logic in catch

Throughout the toolkit sample there is code like

try {
               //some code
} catch {
                throw;
            }

If you ever stumble over code like above, I would recommend rewriting it like this and then sending it to original coder for review:

DoNothing();
//some code
DoNothing();
DoNothing();
DoNothing();


private void DoNothing()
{
     //Do nothing
}  

On the bright side, it is not as bad as

try {
               //some code
} catch {
              //do nothing, to make sure that no exceptions are ever visible to user or the poor poor developer who will be maintaining this code
}

but nothing really is…

functions that have 10 input and 10 out parameters

   private void GetPasswordWithFormat( string       username,
                                            bool         updateLastLoginActivityDate,
                                            out int      status,
                                            out string   password,
                                            out int      passwordFormat,
                                            out string   passwordSalt,
                                            out int      failedPasswordAttemptCount,
                                            out int      failedPasswordAnswerAttemptCount,
                                            out bool     isApproved,
                                            out DateTime lastLoginDate,
                                            out DateTime lastActivityDate)

Uncle Bob just started spontaneously crying…

Not sure when this sample was written, but unless it was written on punch cards, they must have heard about passing in and returning structures, instead of 10+ args.
Also things like single responsibility principle – why does function called GetPasswordWithFormat return lastLoginDate.

Implementing the beast

From business requirements perspective, at this point I would like to support:
Register
Login
Change password

So I implemented the following functions in new CustomMembershipProvider class:
CreateUser
ValidateUser
ChangePassword

Naming Unit Tests

For each function that I implemented in CustomMembershipProvider I created unit tests first.
I used format
{Function}_With{Description}_Should{Result}
for ex…
CreateUser_With_Valid_Data_Returns_User()

Then I found a nicer format:
[MethodName_StateUnderTest_ExpectedBehavior] from:
http://stackoverflow.com/questions/155436/unit-test-naming-best-practices

So the previous example is now:
CreateUser_WithValidData_ReturnsUser()

I also wanted to include “given” conditions (used in BDD), so it would be:
CreateUser_GivenDefaultMembershipProvideSettings_WithValidData_ReturnsUser()

At the end I changed “With” to “When”, so that we would use the same language for both BDD and TDD and I ended up with:
{Function}_Given{Precondition1}_Given{Precondition2}_When{Action}_{Result}
so example would be:
CreateUser_GivenDefaultMembershipProviderSettings_WhenValidData_ReturnsUser

Here are all the unit tests that should demonstrate what is currently implemented, for your viewing pleasure:

		[TestMethod]
		public void CreateUser_GivenDefaultMembershipProviderSettings_WhenValidData_ReturnsUser();

		[TestMethod]
		public void CreateUser_GivenPasswordRequiresMin6Chars_WhenPasswordHasLessThen6Chars_ReturnsNullAndStatusInvalidPassword();

		[TestMethod]
		public void CreateUser_GivenPasswordRequiresMin6Chars_WhenPasswordHasMoreThen6Chars_ReturnsUser();

		[TestMethod]
		public void CreateUser_GivenPasswordRequiresOneNonAphaNumericCharacter_WhenPasswordHasNoNonAlphaNumericCharacters_ReturnsNullAndStatusInvalidPassword();

		[TestMethod]
		public void CreateUser_GivenPasswordRequiresOneAphaNumericCharacter_WhenPasswordHasOneAlphaNumericCharacter_ReturnsUser();

		[TestMethod]
		public void CreateUser_GivenPasswordRequiresMatchRegularExpressions_WhenPassswordNotMatchingRegularExpreation_ReturnsNullAndStatusInvalidPassword();

		[TestMethod]
		public void CreateUser_GivenPasswordRequiresMatchRegularExpressions_WhenPassswordMatchingRegularExpreation_ReturnsNullAndStatusInvalidPassword();

		[TestMethod]
		public void CreateUser_WhenUsernameNotUnique_ReturnsNullAndStatusDuplicateUserName();

		[TestMethod]
		public void CreateUser_GivenPasswordFormatHashed_HashesPassword();

		[TestMethod]
		public void ChangePassword_WhenValidArguements_ChangesPassword();

		[TestMethod]
		public void ValidateUser_WhenUserWithUsernameAndPasswordExists_ReturnsTrue();

		[TestMethod]
		public void ValidateUser_WhenUserWithUsernameAndPasswordDoesNotExist_ReturnsFalse();

Deep Thoughts

In this episode of OpenFaq (The Series), we looked at how to create a simple Custom Membership Provider that uses EF Code First for back-end.
Coding crimes committed by MS where brought to light with appropriate punishment being administered as we speak.
TDD and BDD is still being followed.

For now you can get all the code discussed in this post from here:
http://openfaq.codeplex.com/releases/view/63833

Coming up next…

Now that we have the basic framework down, we are gonna implement us some FAQ goodness…

Current project road map is here:
http://openfaq.codeplex.com/wikipage?title=Road%20Map%20%26%20Progress

Advertisements

Leave a Comment »

No comments yet.

RSS feed for comments on this post. TrackBack URI

Leave a Reply

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Blog at WordPress.com.

%d bloggers like this: