I have a problem with controllers authorization in C#. I use JWT authentication with token and role-based authorization in controllers.
Let’s examine this endpoint.
[HttpDelete("{userId}/post/{postId}")]
[Authorize(Roles = "User")]
public async Task<IActionResult> DeleteUserPost(int userId, int postId, CancellationToken ct)
{
_mediator.Send(new DeletePostCommand(userId, postId), ct);
return Ok("Post deleted.");
}
Does it mean that every person who has “User” roles can delete sombody else post?
For example authorized user with Id = 1 and role = “User” can call this endpoint with passing values like userId = 5 and postId = 20 (which are values for another user). This will cause deleting somebody’s post by somebody else.
Is it a good solution to get authorized user Id from HttpContext and change the endpoint to something like this?
[HttpDelete("post/{postId}")]
[Authorize(Roles = "User")]
public async Task<IActionResult> DeleteUserPost(int postId, CancellationToken ct)
{
_mediator.Send(new DeletePostCommand(postId), ct);
return Ok("Post deleted.");
}
UserId would be set in command-handler based on HttpContext and current authorized user instead of endpoint’s parameter. Should it prevent user from deleting somebody’s else post?
JacobDev is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
Yes, using the userId parameter directly in the endpoint allows any user with the User role to delete any other user’s post.
you can retrieve the authenticated user’s Id from the HttpContextand use it to ensure that users can only delete their own posts.
This is just an example:
[HttpDelete("post/{postId}")]
[Authorize(Roles = "User")]
public async Task<IActionResult> DeleteUserPost(int postId, CancellationToken ct)
{
var userId = int.Parse(User.FindFirst(ClaimTypes.NameIdentifier)?.Value);
if (userId == 0)
{
return Unauthorized("Invalid user ID.");
}
_mediator.Send(new DeletePostCommand(userId, postId), ct);
return Ok("Post deleted.");
}
Edit
It is a good practice to add such rule into your Custom Repository (DAL Layer if you’re adhering to a similar n-layer architecture), So that your application validates the request with the rule being Post.UserId == Request.UserId for example.
IT helps keep your service and controller layers clean and focused on their primary responsibilities. (First principle of the SOLIDs)
2
Yes. At the moment everyone with the Role ‘User’ can delete any post.
If you want to prevent that users can delete post, they didnt created you should probably store the UserId in the Post and check it in the CommandHandler or directly in the Controller (Depending on what you prefer).
Something like:
if(post.UserId != userId){
// Throw exception or return a result indicating the deletion failed because of authorization
...
}
//
Handle your deletion