Topics

C#•2y ago

❔ implementing api authentication in asp.net

Any .net developers who would be willing to help me implement an api with token authentication?

44 Replies

HeyBaldur•2y ago

HeyBaldur•2y ago

I can help! I have a repo in Github that runs all that implementation. Please take a look at this https://github.com/HeyBaldur/Company.API and let me know if it is clear enough

GitHub

GitHub - HeyBaldur/Company.API: The project is based on good practi...

The project is based on good practices for developers and demonstrates an optimal way to print data. The main utility of this project is to store company information such as basic information, who ...

Pobiega•2y ago

The token auth implemented there is pretty bare bones, and some other things in that project is either just outdated (.NET 5, startup) or borderline a war crime (Company.API/Helpers/GenericReturnableHelper.cs).

HeyBaldur•2y ago

@pobiega my apologies but 1. This repo is effectively basic, very good eye from your side 2. It is about helping, this is NOT a judgment about technologies, versions and so on 3. If you have better idea to provide help to @ayman56 then provide it blobthumbsup

blobthumbsup

Pobiega•2y ago

Im just clarifying for any potential reader to not try and learn generally from that project, because it has issues. The Auth itself works, if you want a super stripped down non-validating Auth.

The project is based on good practices for developers and demonstrates an optimal way to print data. It can be used both for learning and for real production work.

With statements like this comes the responsibility to actually deliver on those promises.

snowy | switching accounts•2y ago

hmm I think you might hate something I did for my project oh wait I just saw this yeah this is a war crime doing _genericReturnableHelper.GenericReturnableObject(System.Net.HttpStatusCode.OK, result); to prevent doing Ok(result)

HeyBaldur•2y ago

@ayman65 Did somebody help you finally? @pobiega @snowy9228 Where are you seeing this _genericReturnableHelper.GenericReturnableObject(System.Net.HttpStatusCode.OK, result);

Pobiega•2y ago

https://github.com/HeyBaldur/Company.API/blob/main/Company.API/Controllers/BusinessController.cs#L54

ayman23OP•2y ago

Hello thank you for your help. I am still processing the code inside the repo you linked to me I am new to .net development I need to specify that the api is inside a mvc 5 web app and not a separate project. Would this still work for me?

HeyBaldur•2y ago

@pobiega Interesting, but in the code I see this return _genericReturnableHelper.GenericReturnableObject(result.StatusCode, result); That actually is coming from the result in the service public async Task<GenericOperationResponse<bool>> CreateCompanyAsync(List<BusinessDto> businesses, string userId) coming from here https://github.com/HeyBaldur/Company.API/blob/main/Company.Infrastructure/Services/CompanyService.cs#L42 That finally the logic is implemented in this helper https://github.com/HeyBaldur/Company.API/blob/main/Company.API/Helpers/GenericReturnableHelper.cs I don't see where the crime is

GitHub

Company.API/Company.API/Helpers/GenericReturnableHelper.cs at main ...

The project is based on good practices for developers and demonstrates an optimal way to print data. The main utility of this project is to store company information such as basic information, who ...

GitHub

Company.API/Company.Infrastructure/Services/CompanyService.cs at ma...

The project is based on good practices for developers and demonstrates an optimal way to print data. The main utility of this project is to store company information such as basic information, who ...

Pobiega•2y ago

Authorization is applied the same in all forms of ASP projects Remove your embeds The "crime" is this: _genericReturnableHelper.GenericReturnableObject thats literally just a wrapper for Ok and your helper inherits from Controller in order to do it If you just want an easier way to have a single return based on code, use an extension method

HeyBaldur•2y ago

Nope! There is not wrapper for Ok It will return any of the results from these public IActionResult GenericReturnableObject<T>(HttpStatusCode httpStatusCode, T data)

HeyBaldur•2y ago

It will return any of these

Pobiega•2y ago

I'm aware. Note how you inherit from Controller in order to do so which is a crime in itself you are also removing a layer of type safety in order to do this

HeyBaldur•2y ago

If you noticied, this is actually a service that returns an IActionResult interface tha you have to inherit the class Controller in here public class GenericReturnableHelper : Controller, IGenericReturnableHelper coming from using Microsoft.AspNetCore.Mvc;

Pobiega•2y ago

If you want that exact behavior, why not just use an extension method?

public static class ControllerExtensions
{
    public static ActionResult<T> HttpResult<T>(this ControllerBase controller, HttpStatusCode code, T data)
        => code switch
        {
            HttpStatusCode.OK => controller.Ok(data),
            HttpStatusCode.BadRequest => controller.BadRequest(data),
            HttpStatusCode.Forbidden => controller.Forbid(),
            HttpStatusCode.InternalServerError => controller.StatusCode((int)code, data),
            HttpStatusCode.NotFound => controller.NotFound(data),
            _ => controller.Conflict()
        };
}

public static class ControllerExtensions
{
    public static ActionResult<T> HttpResult<T>(this ControllerBase controller, HttpStatusCode code, T data)
        => code switch
        {
            HttpStatusCode.OK => controller.Ok(data),
            HttpStatusCode.BadRequest => controller.BadRequest(data),
            HttpStatusCode.Forbidden => controller.Forbid(),
            HttpStatusCode.InternalServerError => controller.StatusCode((int)code, data),
            HttpStatusCode.NotFound => controller.NotFound(data),
            _ => controller.Conflict()
        };
}

there you go. no more warcrimes no inheriting from controller in a non-controller, no super long type/function names And also note how its no longer returning IActionResult because thats the untyped HTTP response type you want the typed version, if possible.

HeyBaldur•2y ago

It is nice approach actually, but how the controller will call this extension? How should look the controller when it is returning the final result

Pobiega•2y ago

public async Task<ActionResult<TodoList>> Test(int id)
{
    var result = await _company.CreateCompanyAsync(businesses, _tokenValidator.ReturnUserId(Request));
    
    return this.HttpResult<TodoList>(result.Code, result.Data);
}

public async Task<ActionResult<TodoList>> Test(int id)
{
    var result = await _company.CreateCompanyAsync(businesses, _tokenValidator.ReturnUserId(Request));
    
    return this.HttpResult<TodoList>(result.Code, result.Data);
}

it should be able to infer the generic type if your result model is well defined still, I don't think this is the best way to do it. I'd prefer to use a rich result type as the return type from your CreateCompanyAsync method that method shouldnt know about http details like status codes here is an example

[HttpGet]
public async Task<ActionResult<TodoList>> Get(int id)
{
    var query = new GetTodoListQuery()
    {
        Id = id
    };

    var result = await _mediator.Send(query);
    if (!result.IsSuccess)
    {
        return result.Error switch
        {
            NotFoundError notFoundError => NotFound(notFoundError.Message),
            ValidationError validationError => BadRequest(validationError.ValidationFailures),
            _ => Problem(detail: result.Error.Message, title: "Internal Server Error", statusCode: 500)
        };
    }

    // return _mapper.Map<TodoListDto>(result.Entity);

    return result.Entity;
}

[HttpGet]
public async Task<ActionResult<TodoList>> Get(int id)
{
    var query = new GetTodoListQuery()
    {
        Id = id
    };

    var result = await _mediator.Send(query);
    if (!result.IsSuccess)
    {
        return result.Error switch
        {
            NotFoundError notFoundError => NotFound(notFoundError.Message),
            ValidationError validationError => BadRequest(validationError.ValidationFailures),
            _ => Problem(detail: result.Error.Message, title: "Internal Server Error", statusCode: 500)
        };
    }

    // return _mapper.Map<TodoListDto>(result.Entity);

    return result.Entity;
}

the idea being that the controller is in charge of mapping the result to the correct http response, and nothing else my command/service/handler isnt aware of any http concepts

HeyBaldur•2y ago

Here is where I disagree because this kind of logic should not be in controller, the service must handle this And, also, the controller must not have the model like Task<ActionResult<TodoList>> it should be clean an generic

Pobiega•2y ago

No. The service should not be aware of HTTP concepts. Uh no controller actions should avoid having untyped response types with IActionResult swagger has no idea what you are returning with ActionResult<T> it does You are of course free to code as you will, but your way of doing things is an antipattern and definately not best practice. Just to add some credibility to my statements, I'm a senior backend engineer and have been working with ASP.NET for over 10 years. Untyped response models and http logic in services/handlers would not pass a code review at any company I've worked at.

HeyBaldur•2y ago

I partially agree, good one @ayman65 Did you catch anything? haha I think it is great discussion

Pobiega•2y ago

I think its a bit off-topic from his original question sadly 😛

HeyBaldur•2y ago

haha I think so! Anyway for learning purposes, and understand briefly how a token could be implemented, the repo might give you an idea @ayman65

Florian Voß•2y ago

@pobiega is returning IActionResult<T> fine rather than IActionResult? I think that this will allow Swagger to understand the type T, just like when returning ActionResult<T> right? if that's true, wouldn't it then be better to use IActionResult<T> rather than ActionResult<T>? Because we have a strongly typed response either way but using abstraction gives more flexiblity over concretion

Pobiega•2y ago

There is no such thing as IActionResult<T> and ActionResult<T> has implicit conversions for T => ActionResult<T> What "flexiblity over concretion" do you need in a controller?

snowy | switching accounts•2y ago

I'm pretty sure it was on the "CashierController" or something. But your returnable helper is just an useless piece of code that has no actual value i'm sorry but its true Also it is a war crime the way you use HttpStatus code to map to IActionResults why do that if you get to the point of doing that, either use a Result and map it in your controller/appservice or just straight up return the objectresult lol I didn't see this lmao thanks im not the one that have an issue with the word definitely It's not bad to use IActionResult tho, it has it's use cases. If you want to keep using IActionResult and make sure swagger knows the return type, you can always just use the ProducesResponse and pass on the Type property. Although using an ActionResult<T> is easier in most scenarios

Pobiega•2y ago

Genuinely curious, what legitimate cases do you have for using IActionResult? I can only think of having multiple return models (error model, sucess model) (And in that case, I personally prefer a composite return object to make consumption easier)

snowy | switching accounts•2y ago

It's not really an use case, it's just that due to the flow of my application there is no way any other response type can come from that controller. And I simply don't care for swagger being aware or not of the return type.

Florian Voß•2y ago

oops xD I could've sworn I always write Task<IActionResult<SomeDTO>> into my apis but you are totally right that doesn't exist. Forget what I said, I am using Task<ActionResult<SomeDTO>>

snowy | switching accounts•2y ago

also back then I just didn't want to have duplicated code to handle generic results

public interface IResultErrorHandler
{
    bool CanHandle(IResultError error);
    IActionResult Handle(IResultError error);
    ActionResult<T> Handle<T>(IResultError error);
}

public interface IResultErrorHandler
{
    bool CanHandle(IResultError error);
    IActionResult Handle(IResultError error);
    ActionResult<T> Handle<T>(IResultError error);
}

There is nothing inherently wrong in using IActionResult, it can lead to inconsistence but just because the controller isn't strongly typed, doesn't mean the rest of the application isn't. Also being approved in a code review at your company or not is more a question of company policies regarding their product's code @ayman65 Are you using identity? Is it possible for you to post your api to github? implementing authentication and authorization isn't really that complicated The content around it kinda sucks tho

HeyBaldur•2y ago

@pobiega gave great points, however can you explain why @snowy9228 why do you think it is a useless piece of code?

snowy | switching accounts•2y ago

You're literally adding more complexity to your code by having a switch case that receives an http status code and maps it to a helper method in the controller base class When you could simply return Ok() or return new OkObjectResult(result);

HeyBaldur•2y ago

Nope, because that means we have 200 HTTP response the UI will always treat it like 200 response, what happens if the records were not found in the DB and there are another logic in the UI that maps 401, 500, 200, 201 and so on? You cannot always return Ok

snowy | switching accounts•2y ago

so you'd return NotFound() or return new NotFoundObjectResult() or return new NotFoundResult()? It's the same thing I said Ok but I am not telling you to return Ok everywhere The same way you can return Ok, you can return different response types

HeyBaldur•2y ago

ok, mean that in the controller, I should have all of them, based on the response from the service even if the in the service there was a problem in the catch

snowy | switching accounts•2y ago

Your GenericReturnableObject literally does that, since it already knows what to return based on the HttpStatusCode it receives, it is literally running around in circles brb gotta take a shower

HeyBaldur•2y ago

Let's imagine this scenario, in this service https://github.com/HeyBaldur/Company.API/blob/main/Company.Infrastructure/Services/UserService.cs#L51 Before anything it should be NotFound, instead BadRequest but the service is already telling the controller what should return, why should I need to create a repetitve logic in every controller, if(HTTP_Reponse) then return NotFound() in every controller all the time, this is the reason of having the GenericReturnableHelper because think in a big scale, X controllers, with Y amount of the methods, writting all the validations for each one of them, it will make this enormous.

GitHub

Company.API/Company.Infrastructure/Services/UserService.cs at main ...

The project is based on good practices for developers and demonstrates an optimal way to print data. The main utility of this project is to store company information such as basic information, who ...

Pobiega•2y ago

There is already a generic response method. its called StatusCode you use it in one of your branches

snowy | switching accounts•2y ago

@HeyBaldur And honestly if you get to the point of doing this, just use a Result type already it is super useful I’d like to hear ur opinion on this https://discord.com/channels/143867839282020352/1126646147197436005 ignore the fact that it uses IActionResult

HeyBaldur•2y ago

I agree! Me too

snowy | switching accounts•2y ago

he already opinionated in there

HeyBaldur•2y ago

@ayman65 just in case, if you are getting involved in Software Developement, normally these kind of conversations are very normal! Do not get afraid of them, actually if you ask something, I am prettty sure if any of us will kindly answer you! 🙂 I will check it out!

snowy | switching accounts•2y ago

also why the hell are you using a semaphore to perform database operations

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.

We are a programming server aimed at coders discussing everything related to C# (CSharp) and .NET.

54KMembers

View on Discord

Want results from more Discord servers?

Add your server