My goal is to make a very simple library with the authentications basics: create a user and save its hashed password, change its password, have a method to check if the password is correct for login.
The usage I thought (you find everything in my Github repository), is this:
var cs = "Data Source=localhost\\SQLExpress; Integrated Security=SSPI; Initial Catalog=PLAYGROUND;";
var usersRepository = new SQLServerUsersRepository(cs);
var hasher = new Pbkdf2PasswordHasher();
var users = new Users(usersRepository, hasher);
await users.CreateUser(
username: "user",
plainTextPassword: "someVeryNiceRandomPasswordDifferentFromTheOthers",
firstName: "user's name",
lastName: "user's surname",
userType: UserTypes.SimpleUser);
// If the login fails, an AuthenticationFailedException is thrown
var loggedUser = users.Login("user", "wrongPassword");
To do this, I coded a User
class which depends on IUsersRepository
and IPasswordHasher
.
public async Task CreateUser(
string username,
string plainTextPassword,
string firstName,
string lastName,
UserTypes userType)
{
if (string.IsNullOrWhiteSpace(username))
throw new EmptyFieldException(nameof(username));
if (string.IsNullOrWhiteSpace(plainTextPassword))
throw new EmptyFieldException(nameof(plainTextPassword));
if (string.IsNullOrWhiteSpace(firstName))
throw new EmptyFieldException(nameof(firstName));
if (string.IsNullOrWhiteSpace(lastName))
throw new EmptyFieldException(nameof(lastName));
var salt = _hasher.GenerateSalt();
var hashedPassword = _hasher.Hash(plainTextPassword, salt);
await _usersRepository.Create(new User()
{
FirstName = firstName,
LastName = lastName,
PasswordHash = hashedPassword,
PasswordSalt = salt,
Username = username,
UserStateId = UserStates.Active,
UserTypeId = userType,
RegistrationDate = DateTimeOffset.Now
});
}
The User
table is defined as this:
CREATE TABLE [Users].[Users]
(
[UserId] INT IDENTITY(1, 1) NOT NULL,
[UserTypeId] TINYINT NOT NULL,
[UserStateId] TINYINT NOT NULL,
[Username] VARCHAR(64) NOT NULL,
[PasswordHash] BINARY(32) NOT NULL,
[PasswordSalt] BINARY(16) NOT NULL,
[FirstName] VARCHAR(64) NOT NULL,
[LastName] VARCHAR(64) NOT NULL,
[RegistrationDate] DATETIMEOFFSET(0) NOT NULL
CONSTRAINT [PK_Users_Users_UserId] PRIMARY KEY CLUSTERED (UserId ASC),
CONSTRAINT [UK_Users_Users_Username] UNIQUE (Username),
CONSTRAINT [UK_Users_Users_FirstLastName] UNIQUE (FirstName, LastName),
CONSTRAINT [FK_Users_UserTypes_UserTypeId] FOREIGN KEY (UserTypeId) REFERENCES Users.UserTypes(UserTypeId),
CONSTRAINT [FK_Users_UserStates_UserStateId] FOREIGN KEY (UserStateId) REFERENCES Users.UserStates(UserStateId)
);
I choose DateTimeOffset
because I want to not depend on the server time zone.
Then, the security part. What do you think about my choice? I choosen Pbkdf2
algorithm with HMACSHA1
with salt. To check if the two password hashe (the first from database, the other from the just hashed password during login) I used ReadOnlySpan<byte>.SequenceEqual
:
public class Pbkdf2PasswordHasher : IPasswordHasher
{
public byte[] Hash(string plaintextPassword, byte[] salt)
{
return KeyDerivation.Pbkdf2(
password: plaintextPassword,
salt: salt,
prf: KeyDerivationPrf.HMACSHA1,
iterationCount: 20_000,
numBytesRequested: 256 / 8);
}
public byte[] GenerateSalt()
{
byte[] salt = new byte[128 / 8];
using (var rng = RandomNumberGenerator.Create())
{
rng.GetBytes(salt);
}
return salt;
}
public bool AreEquals(ReadOnlySpan<byte> a1, ReadOnlySpan<byte> a2)
{
return a1.SequenceEqual(a2);
}
}
What do you think? Can my code be improved? Do you see any bugs or potential security issues?
Thanks!
(FirstName, LastName)
is going to cause problems ;-) \$\endgroup\$