abpframework / abp

Open Source Web Application Framework for ASP.NET Core. Offers an opinionated architecture to build enterprise software solutions with best practices on top of the .NET and the ASP.NET Core platforms. Provides the fundamental infrastructure, production-ready startup templates, application modules, UI themes, tooling, guides and documentation.
https://abp.io
GNU Lesser General Public License v3.0
12.31k stars 3.32k forks source link

AppService derived from ApplicationService returns HTML when not signed in. #7503

Closed andre-artus closed 2 years ago

andre-artus commented 3 years ago

A service derived from ApplicationService, via the generated AppNameAppService produces HTML instead of JSON when an error occurs, e.g. user not logged in.

[Authorize(AppNamePermissions.Assets.Default)]
public class ThingsAppService : AppNameAppService, IThingsAppService
{
}
public abstract class AppNameAppService : ApplicationService
    {
        protected AppNameAppService()
        {
            LocalizationResource = typeof(AppNameResource);
        }
    }

Is it necessary to use a Controller in Host.Api as a proxy?

One way I saw is to have a controller that implements ICrudAppService act as proxy to the service, is this the recommended way?

davidzwa commented 3 years ago

If I understand the title correctly, I had the same problem. Also interested into fixing this!

Reproduction (for my case): start with default CLI 4.1+ template with IdentityServer and Angular. Any error does not give back the exception from Abp Exception filter, but instead HTML.

davidzwa commented 3 years ago

@andre-artus I have a reproduction below where it is happening in a Controller, so it might not even be related to AppService https://github.com/thor-it/thor-sso/blob/9c92df8176b9036e36403c585ee1cda2d1785c0f/aspnet-core/src/Thor.SSO.HttpApi/Controllers/Random/RandomController.cs#L19

Let me know if it matches your problem, if not I'll split out to a separate issue. If it helps, we can let the authors use that.

davidzwa commented 3 years ago

Related to the RandomController endpoint linked above (with Authorize attribute) image

andre-artus commented 3 years ago

@davidzwa , It looks similar. Have you tried implementing ICrudAppService (or an interface derived from it)?

andre-artus commented 3 years ago

@davidzwa , It looks similar. Have you tried implementing ICrudAppService (or an interface derived from it)?

It seems I have to implement CrudAppService on the service as well. Defeats the idea.

davidzwa commented 3 years ago

I think this is related to Exception Filtering. I honestly abandoned that project for later for this reason.

Btw, https://docs.abp.io/en/abp/latest/Exception-Handling#send-exception-details-to-the-client Does this fix it?

Also, I dont have this on my 4.2 project where I dont have IDS4. So that might also be the culprit. I dont think it is related to MVC/Controllers/Services. Just config or Identity Server re-routing stuff. That would be my last guess of where to look.

Or.... maybe the idea is that Authorization exceptions are re-routed back to the original client?

andre-artus commented 3 years ago

Thanks @davidzwa , I'll check it out. Just want to add that when the service is derived from ICrudAppService and implements CrudAppService then it behaves as expected.

Maybe I don;t understand the purpose of ApplicationService? I [almost] never want a service to return HTML.

davidzwa commented 3 years ago

@maliming or @realLiangshiwei do you have an answer to this problem(s) we're facing?

maliming commented 3 years ago

See https://github.com/abpframework/abp/issues/5235 and https://github.com/abpframework/abp/issues/5235#issuecomment-727700212

andre-artus commented 3 years ago

@maliming ,

Thanks, but when I change AddAuthentication() to

context.Services.AddAuthentication(options =>
{
  options.DefaultAuthenticateScheme = JwtBearerDefaults.AuthenticationScheme;
  options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme;
})

Then all the services, even ones working fine previously, return 401 in the Swagger UI (despite being logged in). except the error message differs:

 content-length: 0 
 date: Fri29 Jan 2021 03:46:44 GMT 
 server: Kestrel 
 www-authenticate: Bearer 
 x-correlation-id: ...

vs

{
  "error": {
    "code": "Volo.Authorization:010002",
    "message": "Authorization failed! Given policy has not granted: [Name]",
    "details": null,
    "data": {
      "PolicyName": "[Name]"
    },
    "validationErrors": null
  }
}

What is the correct approach to take if I want an endpoint that I can use in Angular in the same way as CrudAppService? And if you know, and don;t mind me asking, what is the purpose of ApplicationService derivatives? Are they only meant for MVC?

If I inherit from CrudAppService then my GetListAsync is in conflict with another one.

davidzwa commented 3 years ago

Before I post my intended answer, you stated that it worked below if I'm correct? I believe, although you might've swapped interface and class in your answer, you seem to be on the right path.

Thanks @davidzwa , I'll check it out. Just want to add that when the service is derived from ICrudAppService and implements CrudAppService then it behaves as expected.

Example for the points below

public class NotificationLogService : ApplicationService, INotificationLogService
{

}

And the INotificationLogService interface implements the IApplicationService interface

Maybe I don;t understand the purpose of ApplicationService? I [almost] never want a service to return HTML.

I'm probably explaining a lot of stuff you already know below. Still, those are good questions to ask and the most important of Abp as I see it! 3 aspects we need to get out of the way:

So let's get back to the issue. It's all about the Authorize tag. Not so much just any exception. It's about Authentication failing and redirecting. When I throw the following non-authentication-related exception in a controller (jep, bear with me), the exception is JSON:

[HttpGet("exception")]
public int GetRandomException()
{
      throw new UserFriendlyException("asdasdasd");
}

Json response:

{
  "error": {
    "code": null,
    "message": "asdasdasd",
    "details": null,
    "data": {},
    "validationErrors": null
  }
}

So it's all about how to configure the Authentication middleware. Let's focus on that ok? :) When authentication fails, it's quite usual that login is redirected. And I get where you're coming from, I dont want it as well for Angular. It's just a default for Cookie based auth I believe. So we gotta dive into the Authentication scheme and understand where the redirect is configured.

Quoting maliming's answer below

When you call the controller that requires authentication, the authentication middleware finds that the current user is not authenticated and calls ChallengeAsync (DefaultChallengeScheme is identity cookie). At this point the request has been shorted.

So now let's focus on JWT, Cookies and swapping the auth scheme. I will stop and await your answer on the question whether you want to keep using Cookies or JWT/Access token? Just to be sure, are you using Identity Server?

davidzwa commented 3 years ago

https://stackoverflow.com/a/64266340/2733437 Also a nice way to see the bigger picture on how to configure auth Cookies.

andre-artus commented 3 years ago

I believe, although you might've swapped interface and class in your answer, you seem to be on the right path.

Yes, I copied the names then removed the I from the wrong one :).

andre-artus commented 3 years ago

I'm using the default option where the IS tables are in the same DB. I'm not calling to IS (the service).

I use JWT for my API, but I would like to also use the Swagger UI and that seems to require cookies.

andre-artus commented 3 years ago

@davidzwa , the website uses the default, cookie based, login.

davidzwa commented 3 years ago

Website => you mean MVC/Razor instead of Angular? Could explain why after changing to JWT auth scheme everything is suddenly Unauthenticated.

andre-artus commented 3 years ago

Website => you mean MVC/Razor instead of Angular?

It's Angular, but the Login is Razor Pages (as per default ABP+Angular template).

Could explain why after changing to JWT auth scheme everything is suddenly Unauthenticated.

davidzwa commented 3 years ago

@andre-artus any progress so far?

andre-artus commented 3 years ago

@davidzwa , not really. I'm just inheriting from CrudAppService and giving the actions different names for now. I cannot stay stuck behind this issue.

To be honest I'm not convinced I'll be using this framework again. The first 80% is really easy , almost effortless, then it feels like you're a ball that rolled into a divot, with every direction being uphill from where you are in your local minimum.

I understand that most (if not all) of this is due to me not understanding the framework, but I'm not sure where to get what I need other than dissecting the source code, which is time consuming.

I was able to complete the previous iteration of my, quite simple, project (with own rolled framework) in the time I spent on this and it has been running problem free for years. The two features where ABP was meant to make my life easier, multi-tenancy and auth, have proven to be major sticking points. My home grown versions of these were rudimentary, but auth worked with both cookies and tokens.

I my mind my requirements are run of the mill: I want a Web API with Swagger UI, I want my SPA (Angular) and mobile app to sign in and talk to the API and get reasonable (actionable, i.e HTTP) errors. I'm confident that I'm not the only one with these requirements, so others must be getting it to work, I would just like to know how.

When I add the code suggested in #5235 to ConfigureAuthentication [1] it just breaks seemingly everything authentication related:

IdentityServer4.Extensions.StringExtensions.AddQueryString(string url, string query)


1. ConfigureAuthentication 

context.Services.AddAuthentication(options => { options.DefaultAuthenticateScheme = JwtBearerDefaults.AuthenticationScheme; options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme; })



2. 200 on invalid login
{
  "result": 2,
  "description": "InvalidUserNameOrPassword"
}
davidzwa commented 3 years ago

Hmm, I'd really like to emphasize that this is not really ABP's fault. I run an solution without IdentityServer, so I had to override some config and I never had this problem. It's .NET's (slightly OP) authentication setup which is hiding what is happening. So it's authentication => .NET Core which is the problem here. Please don't blame this awesome framework for that.

Back to the issue. The MVC page by IdentityServer was replaced by ABP's and this uses the api/account/login API as you correctly state. This controller calls SignInManager a manager provided by .NET Core and that returns the sign-in result. So you see? This has basically very little to do with ABP.

https://github.com/abpframework/abp/blob/e36a2c6311e07c1ad302fc0b48cc794fab97949b/modules/account/src/Volo.Abp.Account.Web/Areas/Account/Controllers/AccountController.cs#L58

I hope this can help you point realise what the direction is the problem lies.

What I did in my non-cookie + non-IDS4 solution was the following:

            Configure<AbpIdentityAspNetCoreOptions>(options =>
                options.ConfigureAuthentication = false
            );

This skips the configuration and allows you to override it. (I added this to my .Application project). Not saying this is the solution, as it quite a harsh attempt, but it is a start.

This skips the code

                context.Services
                    .AddAuthentication(o =>
                    {
                        o.DefaultScheme = IdentityConstants.ApplicationScheme;
                        o.DefaultSignInScheme = IdentityConstants.ExternalScheme;
                    })
                    .AddIdentityCookies();

And now you might see what is going on, ABP configures .NET Core for you, but it does not provide an Authentication solution for you. It's all Identity Server with .NET Core.

andre-artus commented 3 years ago

Hmm, I'd really like to emphasize that this is not really ABP's fault. I run an solution without IdentityServer, so I had to override some config and I never had this problem. It's .NET's (slightly OP) authentication setup which is hiding what is happening. So it's authentication => .NET Core which is the problem here. Please don't blame this awesome framework for that.

It was not my intention to put the blame on ABP, I did say that my battles with it is probably down to me not having wrapped my head around it, but I can understand, after re-reading my previous post, how it could come across as such. I apologize as it was not my intention to denigrate a gift horse.

I probably let my frustrations bleed through as I'm trying to finish this project so I can move onto another one. This is my first time using ABP, and my first time using IS and things that to my mental model seemed straightforward (e.g. API endpoints return HTTP status codes) seems to require workarounds (which also don't seem to work for me). I'm not saying ABP cannot, or does not, do what I need, I'm just saying I don't know how to get it to do what I need.

My comment about probably not using ABP again is not meant to slight the framework, but merely a reflection on the idea that if I have to modify, bypass or work around various aspects of the framework just for it to work as I expect it to then it's probably not a good fit for my needs. Just like if I need a small pickup truck then starting with a large Mercedes saloon car and modifying it down is probably not the way to go (clearly not a reflection on Mercedes suitability as a luxury sedan) . I'm sure ABP works as intended, just not how I expected, My home grown auth system has many deficiencies, but it was pretty easy to get either a token or cookie from a login depending on the calling context, but /api/account/login seems only to return cookies and I don't see another sign in endpoint whether by looking at Swagger UI or by searching either the ABP.io site or the web in general.

All I need at the moment is to be able to sign in and get a JWT that I can use to call the rest of my API, without it breaking the existing auth system. I found in one of the comments potentially a way to bypass the redirection to HTML when a certain KV pair has been set in the headers, which I'll try once I get the auth working with JWT.

andre-artus commented 3 years ago

@davidzwa I just want to say thank you for all the effort you have put into helping me so far, I recognize that even as I struggle.

davidzwa commented 3 years ago

Don't worry! It's all really hard and pretty complex, so to be able to solve the mystery only benefits us for later - or that's my belief anyway.

Btw, how bad would it be to follow up on the redirect and let Angular load the login page? Just need to ask. I'm considering this for my project as well. At this point sounds like the 'best' and 'easiest' path as it only happens on authentication errors, so login is definitely required.

Approach:

andre-artus commented 3 years ago

@davidzwa I decided to generate a throwaway project with the ABP React Native template to see how it does it. It seems that they call /connect/token (IS endpoint) directly, this returns a token and so far all the errors I've tested for are HTTP responses. It also seems to be a mechanism employable by an Angular SPA. It does seem to require exposing something called a "client_secret" to the world, which seems dubious but is probably fine.

The comment I saw to bypass redirection said it merely required passing X-Requested-With: XMLHttpRequest in the headers, I have not tried it out yet myself , but thought it may help you if I mentioned it so long.

davidzwa commented 3 years ago

Wow that's really helpful! Sometimes these headers tricks are a bit mystical to me, but hey I am not complaining

andre-artus commented 3 years ago

X-Requested-With: XMLHttpRequest works to return a status code instead of HTML, but it does not return the error message. Not really the solution I need right now.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

davidzwa commented 3 years ago

Still relevant

cocosip commented 3 years ago

I think this is related to Exception Filtering. I honestly abandoned that project for later for this reason.

Btw, https://docs.abp.io/en/abp/latest/Exception-Handling#send-exception-details-to-the-client Does this fix it?

Also, I dont have this on my 4.2 project where I dont have IDS4. So that might also be the culprit. I dont think it is related to MVC/Controllers/Services. Just config or Identity Server re-routing stuff. That would be my last guess of where to look.

Or.... maybe the idea is that Authorization exceptions are re-routed back to the original client?

We have the same problem, we have two projects with different version(4.2 and v4.3). The v4.3 project have this problem.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

andre-artus commented 2 years ago

Unfortunately this problem still persists.

All I want is for the custom services/controllers/actions I add to have the same behaviour as the auto controllers, i.e to give HTTP error codes and a structured error message.

All I want to know is how to ensure consistent behaviour. I cannot have a web api return 200 and HTML on a failure. This is extremely frustrating.

All the solutions proposed thus far ends up breaking something else. I just want the same behaviour for all API endpoints.

maliming commented 2 years ago

I will check it again.

maliming commented 2 years ago

Move to #9926

maliming commented 2 years ago

https://github.com/abpframework/abp/pull/9940