Activities of "hra"

Hi @hra

Thanks. Is it possible for you to create an issue on https://github.com/aspnetzero/aspnet-zero-core ? We can re-evaluate this.

Thanks, done: https://github.com/aspnetzero/aspnet-zero-core/issues/5084

Thanks @ismcagdas,

Just to be clear - please note that I changed my example to return a POCO instead of IActionResult - just to prove it actually makes no difference what the return type is - the problem remains.

Thanks for helping out here. What I am concerned about, is that while I've been building our product on ANZ, I have inadvertently caused the authentication behavior to deviate between ControllerBase and ApplicationServer. If you find that baseline ANZ correctly returns HTTP 402 for both ControllerBase and ApplicationService, then I'm going to need to figure out how I've broken it. If, however, you find that ControllerBase fails to return the same HTTP error code as an ApplicationService, when you have an invalid token - then I guess we can work together to fix both our code bases.

Of course, I'm quietly rooting for the latter :)

Note: A real easy way to test this without mucking around with tokens, is in ProductJwtSecurityTokenHandler.cs on line 45, simply throw exception.

Hi - thanks for the quick response,

Please note that my "hack" was just one way to reproduce this behavior. It could have just as easily occurred by a user account becoming unavailable (deletion?) during token refresh - or possibly there is another way for the user to not be found - the original author of TokenAuthController appears to have thought so when they wrote it.

Here's my thinking...

• whomever wrote TokenAuthController.Refresh was clearly intending to handle the case where a user could not be found for the user-id claim - because they are testing for "user == null"
• when "user == null" they are throwing a UserFriendlyException - which is great - it provides some insight to the end user - instead of just HTTP500.
• unfortunately, the code checking for null being returned from GetUserAsync will never evaluate to true - because GetUserAsync cannot return null (it throws an exception internally if null is detected).
• the ACTUAL result of a user not being found, is GetUserAsync throws an Exception, which is caught by the Refresh method catch-all handler, which then rethrows as a ValidationException, which then gets mapped to HTTP500 - a very unhelpful error.

Hopefully I've described my thinking clearly - I agree with the original intent of the code, to deliver useful information about the user not being found, to the end user - however the way it's implemented prevents that intent from being realised - with code that can never be reached by the instruction pointer...

Thanks @ismcagdas

Hi,

Does it make any difference if you use AbpAuthorize instead of AbpMvcAuthorize ?

No difference. Incidentally, I did originally use AbpAuthorize, I only recently switched it to AbpMvcAuthorize while I was trying to overcome this issue.

What is your answer to

1. I am expecting that an expired token should trigger the same HTTP result code (401) if the client is calling a ControllerBase method, or an ApplicationService method. Currently that is not the case. Should I be expecting that?

I am testing authentication failure to ensure our smart client (a mobile application) is resilient.

During this testing, I will obtain an authentication token from one environment, and then use that token against another environment (yes, a crazy use-case, but this is just a byproduct of trying to debug a collateral issue).

The token is successfully decoded by the different environment, however, the user id does not exist on that environment, so it throws an exception with the relevant information. I found that, instead of returning useful information, such as "invalid refresh token", the server just returns an unhelpful HTTP 500.

I dug a little deeper, an I found that any time the server receives an invalid token, it will 500 instead of giving an appropriate response.

The reason for this is:

https://github.com/aspnetzero/aspnet-zero-core/blob/c3941a4248476419e91071448956468e7e985f16/aspnet-core/src/MyCompanyName.AbpZeroTemplate.Web.Core/Controllers/TokenAuthController.cs#L249 When an invalid token is received, either because the token payload is invalid (line 256) or because the user id within the payload is not found (264) the method throws an ValidationError. (note: line 268 checks user for null, but this code never executes because UserManager.GetUserAsync explicitly throws an Exception when no user is found - hence line 268 is redundant (this in itself is a bug)

The AbpExceptionFilter then wraps this exception, and because the type "ValidationException" does not match any of the mappable types in the abp DefaultErrorInfoConverter, the error is simply mapped to HTTP 500.

So, there are 2 bugs here. The following line is redundant because GetUserAsync cannot return null: https://github.com/aspnetzero/aspnet-zero-core/blob/c3941a4248476419e91071448956468e7e985f16/aspnet-core/src/MyCompanyName.AbpZeroTemplate.Web.Core/Controllers/TokenAuthController.cs#L268

And "ValidationException" is probably the wrong exception to throw - I think it should be something like AbpAuthorizationException - allowing the client to explain to the user, or at least figure out that token refresh is not going to work, and tell the user that they need to provide their credentials again - as opposed to retrying the same operation.

Of course, I dont know how this change will affect the rest of the application - so it would need to be assessed.

Hi @ismcagdas,

The authorization filter never fires if I'm calling a ControllerBase method with an expired token - neither my customised one you recommended, nor the built-in ABP one - which is not surprising, because this specifically relates to an expired token that is being passed - so the MS auth middleware would be rejecting the request pretty early.

Calling an ApplicationService function with an expired token DOES execute the authorization filter however.

So, clearly the logic of the Authorization Filter is not the cause, because it never gets a chance to run.

When I disable the below code - the auth filter does fire, and I get the correct result back - but I already knew that. The "UseStatusCodePages" middleware appears to be the cause - but I didnt add that - it's part of ANZ.

            app.UseStatusCodePagesWithRedirects("~/Error?statusCode={0}");

So, a couple of core questions

1. I am expecting that an expired token should trigger the same HTTP result code (401) if the client is calling a ControllerBase method, or an ApplicationService method. Currently that is not the case. Should I be expecting that?
2. Assuming it IS a bug - where to from here. Clearly, the authorizationfilter log is good, because when it runs I get the correct result, when it never fires I get a bad result.

Thanks!

Hi, I get a failure with this example

namespace HRA.Portal.Web.Controllers
{
    [AbpMvcAuthorize]
    [Route("Sync/[action]")]
    [DisableAuditing]
    public class SyncController : PortalControllerBase
    {
        public SyncController()
        {
        }

        [HttpPost]
        public Task<MyResult> UploadFile()
        {
            return Task.FromResult(new MyResult());
        }
    }

    public class MyResult
    {

    }
}

I have a similar issue to here: https://github.com/aspnetboilerplate/aspnetboilerplate/issues/5164

However, I need to return IActionResult, because my method will return a FileStreamResult in the case of success, but ObjectResults for detailed errors:

    var result = new ObjectResult(new AjaxResponse(
        ErrorInfoBuilder.BuildForException(vex),
        false
    ))
{
    StatusCode = (int)HttpStatusCode.BadRequest
};
return result;

When the customers mobile application makes a request with an expired token, they get a 302 instead of a 401 - not what the app is expecting.

How do I resolve this?

UPDATE: I have noticed that this is happening on the Azure hosted application, but when running on local machine, it correctly produces a 302. Why does it behave differently in Azure?

**Note: ** Yes I am setting the ajax-request header

      options.headers = {
        ...options.headers, 
        'Authorization': 'Bearer $_token',
        'X-Requested-With': 'XMLHttpRequest'};
        

Another note: If I call methods in the Application services layer, instead of my custom Controllers, then the server produces the correct 401. It's just this 1 method in a Controller which will not return 401 :(

Another Update I have found the cause of the difference between Azure and running locally... The default ANZ code for turning on redirects... of course, I dont want to just go blindly removing ANZ default template code...

if (env.IsDevelopment())
{
    app.UseDeveloperExceptionPage();
}
else
{
    app.UseStatusCodePagesWithRedirects("~/Error?statusCode={0}");
    app.UseExceptionHandler("/Error");
}

I've also remote debugged app in Azure and stepped into CookieAuthenticationEvents.IsAjaxRequest - and it definitely returns True

public Func<RedirectContext<CookieAuthenticationOptions>, Task> OnRedirectToLogin { get; set; } = context =>
{
    if (IsAjaxRequest(context.Request))
    {
        context.Response.Headers[HeaderNames.Location] = context.RedirectUri;
        context.Response.StatusCode = 401;
    }
    else
    {
        context.Response.Redirect(context.RedirectUri);
    }
    return Task.CompletedTask;
};

Tracking this through to the StatusCodePagesMiddleware, I can see that the response code is 401 (as I want), but then the StatusCodePagesMiddleware overwrites it, returning the 302.

I also debugged an ajax call to a method that resides in the application service layer (not a pure controller) and that looks like during the authentication, the response gets closed early, before the StatusCodePagesMiddleware has a chance to execute (i.e, the client has already received the 401 before the server middleware finishes evaluating, hence the "bad" behavior of the StatusCodePagesMiddleware doesnt matter - it's too late to mess things up).

I've simplified my code all the way back to a very basic method in the controller, declared to return a POCO instead of IActionResult - and it still returns 302 instead of 401. It looks like controllers just have this problem - only the application services correctly return 401 when an auth error occurs. Where do I go from here?

Hi,

Probably my biggest loss of productivity is when I am writing new angular HTML template code, and I accidentally save some malformed HTML, or syntactically incorrect template binding code in the html. The angular hot reload attempts to compile, and fails (yes it does identify the bad code), bit it also dumps a lot of "No pipe found with name 'localize'" errors.

The problem, is that even if I fix my incorrect syntax, the hot reload is now completely broken, the "No pipe found with name 'localize'" errors remain, I have to kill my angular server, and re-run it - meaning a full recompile which wastes a couple of minutes.

As you can imagine, this is quite a productivity killer.

This has always happened in my AspNetZero code base, so I dont believe I introduced anything to cause this brittle behavior.

Is this just me? Can this be made more resilient?

Thanks,

We have customers who have existing user accounts, and they have now decided to enable Microsoft Authentication on their tenant.

What is the recommended procedure for converting their user accounts to be associated with an Azure A/D login with the same email address?

Note: If the user clicks "sign in with Microsoft", they receive an error similar to "cannot create user account, email already exists". This makes sense - but is clearly not desirable.

Thanks,