1

So this is an example endpoint to fetch one user by its username

[HttpGet("{username}")]
public async Task<ActionResult<object>> GetUser([FromRoute] string username)
{
    // ...
}

Regardless of the value of username the database will either find and return a user or null. But you know that a username has validation constraints like

  • max length of x
  • no special characters
  • ...

So you could also add a validation route constraint e.g.

[HttpGet("{username:maxlength(20)}")]

or make use of data annotations like

[FromRoute][MaxLength(20)] string username

but then you have to do it for every endpoint. The advantage is that bad parameter values will never touch the database so you might increase the performance because the request gets denied. The bad thing is that you will have to maintain all these endpoints with extra validations instead of saying "I don't care for the value, the database will find it or not.".

Are there any best practises which approach should be considered?

Improve this question
1
  • In terms of best practice, I would say you want to pass queries, to a Service Layer - in your specific case, you may want a UserQueryService; which then has a GetUserByUsername(UserByUsernameQuery userQuery). Then within the service, inject a UserQueryValidator, which includes a Validate(UserByUsernameQuery userQuery) method that takes that specific UserByUsernameQuery model - and that's where you layout your validation criteria - which returns true / false (or something more specific). Check that and return, then in your controller method, create your QueryModel using the input params & call
    – mrdnk
    Jul 18, 2020 at 23:45

4 Answers 4

Reset to default
5

If you want to return a meaningful error message you'll probably want to add validation. Every application I've worked on has done some request validation and returned an error message before attempting to process the request. This helps client developers or users retry with correct data instead of calling first line support complaining something is broken.

In this specific case I'd be careful about what you return. You don't want to allow an attacker to enumerate your users collection by returning too much info. For example, returning "invalid username" when the validation rules are not met but "user not found" when they are may allow an attacker to reduce the total possible number of usernames they have to brute force to enumerate your users. This is probably more important when validating passwords but something to consider.

Improve this answer
1
  • I agree with you here. In a straight API example. We use a generic base ServiceResponse class, with several options. Firstly a validation failure is returns from a service, with return UserQueryServiceResponse.BadRequest(), which is then checked at the controller layer - and returns http BadRequest - without error messages. If something is not found, we return UserQueryServiceResponse.NotFound() - controller picks that up. Then on state or other errors; we return UserQueryServiceResponse.Conflict().AddError(); etc. Success - btw, is UserQueryServiceResponse.Success().AddModel(model).
    – mrdnk
    Jul 18, 2020 at 23:56
2

If you are considering to add such a route level validation then you should be aware of the following:

If the constraint is not met then your action won't be called. Other action might be called depending on your routing table. If no match has been found then ASP.NET Core will handle that case.

This means ASP.NET's error format might differ from your error response's structure. From the consumer perspective I would not want to prepare for several different error response format. So, having a single well-defined error response structure is the preferable approach.

Improve this answer
1

If there are well-defined business constraints on a username regarding its formatting, the best option you have is to introduce an actual value object representing the data.

class Username
{
    public string Value { get; }

    public Username(string value)
    {
        if (value.Length > 20)
        {
            throw new ArgumentException("The length of a username must not exceed 20 characters.");
        }
        
        // add other necessary validations
        
        this.Value = value;
    }
}

then in places where you would search by a username, instead of accepting a string, you start accepting an instance of Username, where the instance itself guarantees that the used username is valid.

You then add a centralised handler, which automatically translates (in my case) the thrown ArgumentException to a 400 Bad Request.

Improve this answer
0

I would suggest having a custom set of validations for endpoint based on specific criteria such as return types, etc and using them consistently throughout your system. You want to prevent your system from making calls to the backend and wasting time processing an exception which could have been caught earlier in the process. It makes your system seem more responsive and allows you to use system resources more effectively.

Improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.