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.

Either get the Salt value your application uses for hashing and the Encryption Algorithm used so that you can get the byte arrays for `PasswordSalt` and `PasswordHash`, or don't call `VerifyPasswordHash()`. Without seeing what `VerifyPasswordHash` does, it is difficult to help with much more than what I commented. This post shows how salting and hashing works (https://stackoverflow.com/questions/2138429/hash-and-salt-passwords-in-c-sharp) — Ryan Wilson, Feb 07 '20 at 14:43
Well, how do `passwordSalt` and `passwordHash` get populated _outside_ of the unit tests? I'm assuming that given a password and a (random) salt, a hash is computed, and the salt and hash are stored. Why can;t you do that in the unit test? — D Stanley, Feb 07 '20 at 14:48
Hello, thank you. I added VerifyPasswordHash method. I was thinking about adding the method which creates Hash and Salt into database when user registers, but really didn't want to make this question seem too complicated. — mm_archeris, Feb 07 '20 at 14:51
Congratulations. This is the first question I've seen in a **LOOOONG** time to actually demonstrate an understanding of correct password handling. +1 just for that. — Joel Coehoorn, Feb 07 '20 at 14:53
JoelCoehoorn - not bad for a beginner, eh? (obviously it's not mine, all credits to https://jasonwatmore.com/post/2019/10/14/aspnet-core-3-simple-api-for-authentication-registration-and-user-management) @MarcoSalerno the picture makes question look more appealing :D — mm_archeris, Feb 07 '20 at 15:14
It's very common for us to need to copy/paste code from the question into our answers. If you post an image, we have to re-type. That's often enough extra work to make someone skip the question completely and move on to the next. It's also less work for you to paste the code text instead of making an image. Finally, a certain very small set of users can't even see the images. — Joel Coehoorn, Feb 07 '20 at 16:41

Joel Coehoorn · Accepted Answer · 2020-02-07T15:24:26.447

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();
}

Hardcode C# byte property into a model

1 Answers1