Hardcode C# byte property into a model

Question

I'm writing xunit to test Authenticate method. It's pretty straightforward:

public User Authenticate(string username, string password)
{
        if (string.IsNullOrEmpty(username) || string.IsNullOrEmpty(password))
            return null;

        var user = _context.Users.SingleOrDefault(x => x.Username == username);

        // check if username exists
        if (user == null)
            return null;

        // check if password is correct
        if (!VerifyPasswordHash(password, user.PasswordHash, user.PasswordSalt))
            return null;

        // authentication successful
        return user;
}

VerifyPasswordHash method:

 private static bool VerifyPasswordHash(string password, byte[] storedHash, byte[] storedSalt)
    {
        if (password == null) throw new ArgumentNullException("password");
        if (string.IsNullOrWhiteSpace(password)) throw new ArgumentException("Value cannot be empty or whitespace only string.", "password");
        if (storedHash.Length != 64) throw new ArgumentException("Invalid length of password hash (64 bytes expected).", "passwordHash");
        if (storedSalt.Length != 128) throw new ArgumentException("Invalid length of password salt (128 bytes expected).", "passwordHash");

        using (var hmac = new System.Security.Cryptography.HMACSHA512(storedSalt))
        {
            var computedHash = hmac.ComputeHash(System.Text.Encoding.UTF8.GetBytes(password));
            for (int i = 0; i < computedHash.Length; i++)
            {
                if (computedHash[i] != storedHash[i]) return false;
            }
        }

        return true;
    }

But to test that, I need to seed my DB with some User entities.

This is what I was trying to do:

public void TestAuthenticate()
{
        //Arrange
        var options = new DbContextOptionsBuilder<DataContext>() //instead of mocking we use inMemoryDatabase.
            .UseInMemoryDatabase(databaseName: "TestAuthenticate")
            .Options;

        var config = new MapperConfiguration(cfg =>
            cfg.AddProfile<AutoMapperProfile>());

        var mapper = config.CreateMapper();
        var fakeUser = new User()
        {
            Username = "anon1", FirstName = "fakename", LastName = "fakelastname", Role = "admin", PasswordHash = null, PasswordSalt = null
        };

        using (var context = new DataContext(options))
        {
            context.Users.Add(fakeUser);
            context.SaveChanges();
        }

        // Act
        using (var context = new DataContext(options))
        {
            var service = new UserService(context, mapper);
            var result = service.Authenticate(fakeUser.Username, "somepassword");

            // Assert
            Assert.IsType<User>(result);
        }
}

I made PasswordHash and PasswordSalt null here, but they are supposed to be byte[], this is how they are stored in the database:

public class User
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Username { get; set; }
    public byte[] PasswordHash { get; set; }
    public byte[] PasswordSalt { get; set; }
    public string Role { get; set; }
}

Please let me know how to make this test work and leave some feedback if you find overall test logic weird. It's my first attempt at writing unit tests.

Answer 1

I would factor out the code to create the hash values into it's own method you can unit test separately.

So this:

private static bool VerifyPasswordHash(string password, byte[] storedHash, byte[] storedSalt)
{
    if (password == null) throw new ArgumentNullException("password");
    if (string.IsNullOrWhiteSpace(password)) throw new ArgumentException("Value cannot be empty or whitespace only string.", "password");
    if (storedHash.Length != 64) throw new ArgumentException("Invalid length of password hash (64 bytes expected).", "passwordHash");
    if (storedSalt.Length != 128) throw new ArgumentException("Invalid length of password salt (128 bytes expected).", "passwordHash");

    using (var hmac = new System.Security.Cryptography.HMACSHA512(storedSalt))
    {
        var computedHash = hmac.ComputeHash(System.Text.Encoding.UTF8.GetBytes(password));
        for (int i = 0; i < computedHash.Length; i++)
        {
            if (computedHash[i] != storedHash[i]) return false;
        }
    }

    return true;
}

Becomes this:

private static byte[] ComputeHash(string data, byte[] salt)
{
    using (var hmac = new System.Security.Cryptography.HMACSHA512(salt))
    {
        return hmac.ComputeHash(System.Text.Encoding.UTF8.GetBytes(data));
    }
}

private static bool VerifyPasswordHash(string password, byte[] storedHash, byte[] storedSalt)
{
    if (password == null) throw new ArgumentNullException("password");
    if (string.IsNullOrWhiteSpace(password)) throw new ArgumentException("Value cannot be empty or whitespace only string.", "password");
    if (storedHash.Length != 64) throw new ArgumentException("Invalid length of password hash (64 bytes expected).", "passwordHash");
    if (storedSalt.Length != 128) throw new ArgumentException("Invalid length of password salt (128 bytes expected).", "passwordHash");

    var computedHash = ComputeHash(password, storedSalt);  
    for (int i = 0; i < computedHash.Length; i++)
    {
        if (computedHash[i] != storedHash[i]) return false;
    }
    return true;
}

There are several purposes for this: it lets you share this method with the code to generate password hashes at creation, change, and reset, making sure the code to initially hash the passwords is using the same process as the code to verify the hashes; it lets you isolate the hash generation for a separate unit test; and it makes it a little safer and easier to adjust the hashing algorithm should sha512 cease to be viable. There are other reasons for this, too.

While I'm here, I might also add an authType field to the user, which will make it easier and safer to adjust this algorithm should sha512 ever cease to be viable, and even have two different process active at the same time. For example, you might want a separate process if you ever need to integrate with outside OAuth or SAML services.

Once you have a ComputeHash() function, you should do something similar to create a GenerateRandomSalt() function to invoke when creating new users. With both of these in hand, creating the reference data for your unit test of the full authentication process is much easier:

var fakeUser = new User()
{
    Username = "anon1", FirstName = "fakename", LastName = "fakelastname",
    Role = "admin", PasswordHash = null, PasswordSalt = GenerateRandomSalt()
};
fakeUser.PasswordHash = ComputeHash("somepassword", fakeUser.PasswordSalt);

using (var context = new DataContext(options))
{
    context.Users.Add(fakeUser);
    context.SaveChanges();
}