Sign-In Is Impossible If You Capitalise

Normalisation bugs are the quietest class of defect there is, because both halves of the code look correct in isolation. The write side lower-cases, or trims, or strips diacritics, for reasons that are obviously right. The read side does the same, for reasons that are obviously right. The only thing that matters is whether they do the same thing, and nothing in the language, the ORM or the database will tell you they do not. Part 7 was about work the service does too much of. This one is about work it does exactly once, in one of two places.

Two lines, one word apart

Trill.Services.Users.Core/Domain/Entities/User.cs, inside the constructor, lines 43 and 44:

Email = email.ToLowerInvariant();
Name = name.Trim();

Trill.Services.Users.Core/Mongo/Repositories/UserRepository.cs, lines 32 and 43:

public async Task<User> GetByEmailAsync(string email)
{
    ...
    var document = await _repository.GetAsync(x => x.Email == email.ToLowerInvariant());
    return document?.ToEntity();
}

public async Task<User> GetByNameAsync(string name)
{
    ...
    var document = await _repository.GetAsync(x => x.Name == name.ToLowerInvariant());
    return document?.ToEntity();
}

Two methods, identical in shape, and only one of them is symmetric with the write path.

• Email is stored lower-cased and queried lower-cased. Correct, and it round-trips for any input.
• Name is stored trimmed and queried lower-cased. For an account created as Kumar, Mongo holds "Kumar". GetByNameAsync("Kumar") filters on Name == "kumar". The document does not match. GetAsync returns null. GetByNameAsync returns default.

Nothing in between rescues it. UserDocument's constructor is a straight member copy — Name = user.Name; — and ToEntity() copies it back. There is no serialisation-time normalisation, no case-insensitive collation on the collection, no Regex in the filter. The value that goes into Mongo is exactly what the constructor produced, and the predicate that comes back is exactly what the repository wrote.

Any account whose name contains a single capital letter is unreachable through GetByNameAsync.

Which means sign-in

Commands/Handlers/SignInHandler.cs, the first line of HandleAsync:

public async Task HandleAsync(SignIn command)
{
    var user = await _userRepository.GetByNameAsync(command.Name);
    if (user is null || !_passwordService.IsValid(user.Password, command.Password))
    {
        _logger.LogError($"User with name: {command.Name} was not found.");
        throw new InvalidCredentialsException(command.Name);
    }
    ...
}

GetByNameAsync is the only lookup. There is no fallback to email, no second attempt, no case-insensitive retry. A user who signs up as Kumar receives a 201, has a document in Mongo, has funds credited, and can never authenticate. Every attempt short-circuits on user is null, throws InvalidCredentialsException, and returns 400 {"code":"invalid_credentials"} — the same response as a wrong password. The account exists and the system will tell you, forever, that your credentials are wrong.

The log line makes it worse in a different direction. It fires on both branches of that ||, so a wrong-password attempt against a perfectly valid lowercase account is also recorded as “User with name: … was not found.” At Error level, with the username interpolated. An operator reading that log stream cannot distinguish “no such user” from “bad password”, and gets a stream of usernames either way.

And which means uniqueness, too

GetByNameAsync has a second caller, four lines above the point where the account is created. Commands/Handlers/CreateUserHandler.cs:

user = await _userRepository.GetByNameAsync(command.Name);
if (user is {})
{
    _logger.LogError($"Name already in use: {command.Name}");
    throw new NameInUseException(command.Name);
}

Sign up as Kumar; a second person signs up as Kumar; the check queries for "kumar", finds nothing, and lets it through. The domain's NameInUseException — a well-named DomainException that would surface as 400 {"code":"name_in_use","reason":"Name Kumar is already in use."} — never fires for a mixed-case name.

What catches it instead is the unique Mongo index created at startup on users.Name. That surfaces a raw MongoWriteException out of AddAsync, which is not a DomainException and not an AppException, so ExceptionToResponseMapper's final switch arm takes it:

_ => new ExceptionResponse(new {code = "error", reason = "There was an error."},
    HttpStatusCode.BadRequest)

400 {"code":"error","reason":"There was an error."}. A precise, translatable domain error degrades into the estate's generic fallback, purely because the guard in front of it could not see the row the index could.

There is a third-order effect too. Because the index is case-sensitive, Kumar and kumar are two distinct documents. The service therefore permits two accounts whose names differ only in case, of which exactly one — the lowercase one — can sign in.

The two fields that got it right

What makes this a design problem rather than a typo is that the same constructor normalises two other fields correctly, and does it in two different places.

Email = email.ToLowerInvariant();
Name  = name.Trim();
Role  = role.ToLowerInvariant();

Email and Role are both lower-cased on the way in. And Role gets a second layer: Role.IsValid, called four lines earlier in the same constructor, lower-cases its argument before comparing it against the two constants.

public static bool IsValid(string role)
{
    if (string.IsNullOrWhiteSpace(role))
    {
        return false;
    }

    role = role.ToLowerInvariant();

    return role == User || role == Admin;
}

CreateUserHandler then lower-cases it a third time before passing it in — command.Role.ToLowerInvariant(). So the rule for Role is applied by the handler, by the validator and by the constructor, three times for one field, and the rule for Name is applied by nobody on the write path and by the repository on the read path.

The same author, in the same class, applied the same normalisation to three fields with three different levels of redundancy, and the field with zero is the one used for authentication. That is what an invariant enforced by discipline rather than by types looks like at the end of a working week. Trim() and ToLowerInvariant() are not competing techniques here; one is a whitespace rule and one is a case rule, and the field needed both.

Why nobody noticed

The .rest file is the only executable specification in the repository, and it opens with:

@email = trill-user1@mailinator.com
@name = user1

user1. Every request in the file — sign-up, sign-in, browse — uses that variable. Lower-case names round-trip perfectly, ToLowerInvariant() on an already-lowercase string is a no-op, and the entire happy path works. The fixture selects, by accident, the one input class the bug cannot reach.

This is the shape of the problem in general. Normalisation bugs are found by adversarial inputs, not representative ones, and a hand-written fixture is representative by construction. A property test — “for any non-empty name, sign-up then sign-in succeeds” — finds this on the first generated string with a capital in it. The repository has four test projects and zero test files, which part 15 takes up properly.

Where the rule belongs

The fix is one word, and the wrong fix is tempting. You could change the repository to stop lower-casing:

var document = await _repository.GetAsync(x => x.Name == name.Trim());

That makes the two halves agree today and leaves the same trap set for tomorrow, because the agreement lives in two files that have no reason to change together.

The right fix puts the rule where it cannot be disagreed with. A Username value object that normalises in its constructor, used as the parameter type of both User's constructor and GetByNameAsync, makes the two call sites physically incapable of applying different rules — the normalisation happens once, on the way into the type, and neither caller can opt out. The Stories service in this same estate already has that habit, with StoryText, Rate, Author and Visibility in Core/ValueObjects; the Users service, as part 1 noted, has no value objects at all and models every field as a primitive. This bug is the cost of that decision, expressed in one word.

Two supporting moves are worth having regardless:

• Store the normalised form in a separate field. Name for display, NormalisedName for lookup and for the unique index, written once by the mapper. ASP.NET Core Identity does exactly this, with NormalizedUserName, and it exists for exactly this failure.
• Let the database enforce what the guard checks. If uniqueness is case-insensitive, the index must be case-insensitive too — a Mongo collation with strength: 2 on that collection — otherwise the guard and the constraint disagree about what a duplicate is.

The durable rule: normalisation is a property of the field, not of the call site. Any time you find yourself writing .ToLowerInvariant() inside a query predicate, the question to ask is not “is this correct?” but “where else is this written, and what happens the day one of the two changes?” In Trill, one of the two was never written at all, and the answer was that nobody with a capital letter in their name could log in.

Next, the abstraction that was supposed to make the caller's identity trustworthy in the first place — built, registered, and injected by nobody: the identity abstraction nobody injected.