C#•2y ago

❔ What is the appropriate way to confirm User ID for API

Hi folks, I'm creating a WebApi to go alongside my front-end. Each call to my controller and related service has the

[Authorize]

[Authorize]

attribute so I know that a user has to be authorized before accessing the data. My query is regarding the retrieval of the UserID to get the user-specific data from my database. This is what I have currently:

cs 
[Authorize]
public class CategoryService : ICategoryService
{
    private readonly DataDbContext        _context;
    private readonly IHttpContextAccessor _httpContextAccessor;
    
    private readonly string? _userId;

    public CategoryService(DataDbContext context, IHttpContextAccessor httpContextAccessor)
    {
        _context = context;
        _httpContextAccessor = httpContextAccessor;
        
        _userId = _httpContextAccessor.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier);
    }
    
    public async Task<List<CategoryDTO>> GetAll()
    {
        return await _context.Categories.Where( c => c.UserId == _userId ).Select( c => new CategoryDTO()
            {
            Id = c.Id,
            Name = c.Name
            } ).ToListAsync();
    }

cs 
[Authorize]
public class CategoryService : ICategoryService
{
    private readonly DataDbContext        _context;
    private readonly IHttpContextAccessor _httpContextAccessor;
    
    private readonly string? _userId;

    public CategoryService(DataDbContext context, IHttpContextAccessor httpContextAccessor)
    {
        _context = context;
        _httpContextAccessor = httpContextAccessor;
        
        _userId = _httpContextAccessor.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier);
    }
    
    public async Task<List<CategoryDTO>> GetAll()
    {
        return await _context.Categories.Where( c => c.UserId == _userId ).Select( c => new CategoryDTO()
            {
            Id = c.Id,
            Name = c.Name
            } ).ToListAsync();
    }

Is this an acceptable and importantly safe way to do it? I'm fairly new to Authorization/Authentication so trying to create a portflio worthy project without any glaringly obvious security flaws. TIA

12 Replies

Yawnder•2y ago

Never seen [Authorize] outside of a controller tbh. Also, I wouldn't have my DAL straight up used in the API layer. That business logic should go elsewhere.

JakenVeina•2y ago

is this, like a gRPC Service? That definitely supports parsing the [Authorize] attribute otherwise, yeah, the [Authorize] attribute does literally nothing it's purely metadata it's only as useful as whatever frameworks you're using that GIVE it meaning by actively looking for it and using its presence to decide to do something and there's really nothing wrong with putting all your logic in the API layer having separate business and DAL layers is an organizational pattern, and there's nothing fundamentally wrong with deciding you don't need that depends on the size and scope of your app and your development team

Pascal•2y ago

No this can not be a gRPC service, that would at least need to accept a ServerCallContext as one of the parameters. Yes something may or may not be wrong with this service depending on your DI registration. If CategoryService is registered as a Singleton (which I assume you are from your use of IHttpContextAccessor ) then this code breaks, _userId will always have the same value after initial initialization. HttpContext is only present in an Http scope, so if you initialize CategoryService outside of an Http scope, _httpContextAccessor.HttpContext will return null. Also the [Authorize] attribute here does nothing, you need to add it to your endpoint: Http endpoint, controller endpoint, signalR or gRPC. If your CategoryService is a singleton something like this could work.

public class CategoryService : ICategoryService
{
    private readonly DataDbContext        _context;
    private readonly IHttpContextAccessor _httpContextAccessor;

    public CategoryService(DataDbContext context, IHttpContextAccessor httpContextAccessor)
    {
        _context = context;
        _httpContextAccessor = httpContextAccessor;
    }
    
    public async Task<List<CategoryDTO>> GetAll()
    {
// this still breaks outside an http scope
        string? _userId = _httpContextAccessor.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier);
        return await _context.Categories.Where( c => c.UserId == _userId ).Select( c => new CategoryDTO()
            {
            Id = c.Id,
            Name = c.Name
            } ).ToListAsync();
    }
}

public class CategoryService : ICategoryService
{
    private readonly DataDbContext        _context;
    private readonly IHttpContextAccessor _httpContextAccessor;

    public CategoryService(DataDbContext context, IHttpContextAccessor httpContextAccessor)
    {
        _context = context;
        _httpContextAccessor = httpContextAccessor;
    }
    
    public async Task<List<CategoryDTO>> GetAll()
    {
// this still breaks outside an http scope
        string? _userId = _httpContextAccessor.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier);
        return await _context.Categories.Where( c => c.UserId == _userId ).Select( c => new CategoryDTO()
            {
            Id = c.Id,
            Name = c.Name
            } ).ToListAsync();
    }
}

But I suggest you redesign ICategoryService.GetAll to accept userId as a parameter. Here is an example below.

public interface ICategoryService
{
  Task<List<CategoryDTO>> GetAll(string userId);
}

public class CategoryService : ICategoryService
{
    private readonly DataDbContext        _context;

    public CategoryService(DataDbContext context)
    {
        _context = context;
    }
    
    public async Task<List<CategoryDTO>> GetAll(string userId)
    {
        return await _context.Categories.Where( c => c.UserId == userId ).Select( c => new CategoryDTO()
            {
            Id = c.Id,
            Name = c.Name
            } ).ToListAsync();
    }
}

public interface ICategoryService
{
  Task<List<CategoryDTO>> GetAll(string userId);
}

public class CategoryService : ICategoryService
{
    private readonly DataDbContext        _context;

    public CategoryService(DataDbContext context)
    {
        _context = context;
    }
    
    public async Task<List<CategoryDTO>> GetAll(string userId)
    {
        return await _context.Categories.Where( c => c.UserId == userId ).Select( c => new CategoryDTO()
            {
            Id = c.Id,
            Name = c.Name
            } ).ToListAsync();
    }
}

JakenVeina•2y ago

not true

Pascal•2y ago

can you show an example (C# example) where this is not the case?

JakenVeina•2y ago

https://github.com/discord-csharp/MODiX/blob/2.0-prerelease/MODiX.Web.Protocol/Diagnostics/IDiagnosticsContract.cs

GitHub

MODiX/IDiagnosticsContract.cs at 2.0-prerelease · discord-csharp/MO...

Discord Bot handling basic moderation needs, soon implements statistics. - MODiX/IDiagnosticsContract.cs at 2.0-prerelease · discord-csharp/MODiX

Pascal•2y ago

You are correct, I had no idea you could define the contract as code first and the tooling would be able to understand it.

JakenVeina•2y ago

the code-first stuff is a whole separate package built on top of the gRPC core, but yeah, very cool

WillowBearOP•2y ago

Hi everyone, thank you for respnding! So the Controller does infact have the

[Authorize]

[Authorize]

attribute. I put it on the Service also as I wasn't sure if it had to be everywhere that I wanted only authorised users to have access. I'll remove it from the service! The service is registered Scoped. I'm not sure where i saw it but i thought that was the default way but appreciate I need to look more into DI. Not really sure what gRPC means, so definetly not that. Just a "normal" service which performs CRUD actions to the DB when called by my Controller. What would be a better way to retrieve the current userId? Should that be a seperate service?

Pascal•2y ago

in your controller endpoint you have access to the User object. Which you can use to pass down the logged in user id User.FindFirstValue(ClaimTypes.NameIdentifier). If your service should only be used in an http scope, then you may go with one of the suggestions above.

JakenVeina•2y ago

so, yeah, if the Service class is just something you've defined for yourself, putting [Authorize] on it accomplishes nothing there's a variety of ways to retrieve the current user ID, the most direct one being what Pascal said you can also retrieve the HttpContext itself through DI, and the User object will be there

Accord•2y ago

Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.

Gaming

Programming

❔ What is the appropriate way to confirm User ID for API