graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

AUTH_NOT_AUTHORIZED returned when expecting AUTH_NOT_AUTHENTICATED

Open epbensimpson opened this issue 1 year ago • 8 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Product

Hot Chocolate

Describe the bug

When a user is not authenticated and attempts to access a type/field protected by [Authorize], they receive an AUTH_NOT_AUTHORIZED error instead of AUTH_NOT_AUTHENTICATED

This is in contradiction to the documentation which indicates this should result in AUTH_NOT_AUTHENTICATED.

We need to be able to differentiate between not authenticated and not authorized so we can have the client re-authenticate if necessary.

Steps to reproduce

  1. Set up server with a basic query with [Authorize] on a type:
using HotChocolate.Authorization;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddAuthentication();
builder.Services.AddAuthorization();

builder.Services
    .AddGraphQLServer()
    .AddAuthorization()
    .AddQueryType<Query>();

var app = builder.Build();

app.UseRouting();
app.UseAuthentication();
app.UseAuthorization();
app.MapGraphQL();
app.Run();

public class Query
{
    public User GetUser => new();
}

[Authorize]
public class User
{
    public string Name => "Foo";
}
  1. Execute query:
query {
  getUser {
    name
  }
}
  1. Observe response:
{
  "errors": [
    {
      "message": "The current user is not authorized to access this resource.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "getUser"
      ],
      "extensions": {
        "code": "AUTH_NOT_AUTHORIZED"
      }
    }
  ]
}

Relevant log output

No response

Additional Context?

No response

Version

13.1.0

epbensimpson avatar Jun 01 '23 01:06 epbensimpson

The documentation is outdated. Out of the box you only have AUTH_NOT_AUTHORIZED. While the internal policy provider still could differentiate between those we cannot get that information from the policy itself. You can implement you own auth handler and add that on top.

michaelstaib avatar Jun 01 '23 16:06 michaelstaib

Would you consider unsealing DefaultAuthorizationHandler and making it public, and making the interface methods virtual so I can extend the default handler instead of having to write one from scratch?

Edit to add: curious as to why this isn't implemented in the first place? The logic from PolicyEvaluator is exactly what I'm after here - when the user is unauthenticated and the result from the auth service is a failure, a challenge is returned instead of a forbid.

epbensimpson avatar Jun 01 '23 22:06 epbensimpson

@epbensimpson I ran into same problem. Do you have any suggestion on how to implement new CustomAuthorizationHandler, so not to cause any other issue?

hundreder avatar Sep 13 '23 10:09 hundreder

You could workaround the problem by implementing an IErrorFilter that translates AUTH_NOT_AUTHORIZED error codes to AUTH_NOT_AUTHENTICATED errors in case the user is not authenticated. But I think it isn't the right place to solve the problem.

It would be great if we could extend Hot Chocolate's DefaultAuthorizationHandler instead of implementing our own IAuthorizationHandler from scratch.

doeringp avatar Oct 05 '23 10:10 doeringp

@hundreder Here's a link to a gist with the changes: https://gist.github.com/epbensimpson/401c2418b8f59ed55461d5d001397fff

Changes are at line 77-88 and line 113-134

epbensimpson avatar Oct 24 '23 02:10 epbensimpson

@michaelstaib Just saw that the bug-tag was removed. Anyway, I've stumbled across the same issue and since the AuthorizeMiddleware supports this state, I think it's logical that the AspNetCore implementation correctly returns NotAuthenticated when appropriate.

huysentruitw avatar Feb 16 '24 13:02 huysentruitw

This is how I currently solve this problem.

internal class ErrorFilter(IServiceProvider serviceProvider) : IErrorFilter
{
  public IError OnError(IError error)
  {
    if (error.Code == "AUTH_NOT_AUTHORIZED")
    {
      // if error code is AUTH_NOT_AUTHORIZED then check if user is authenticated
      // if he isn't then swap error code to AUTH_NOT_AUTHENTICATED
      IHttpContextAccessor httpContextAccessor = serviceProvider.GetRequiredService<IHttpContextAccessor>();
      HttpContext context = httpContextAccessor.HttpContext ?? throw new InvalidOperationException();
      if (context.User.Identity?.IsAuthenticated != true)
      {
        return error.WithCode("AUTH_NOT_AUTHENTICATED");
      }
    }

    return error;
  }
}

Don't forget to register the ErrorFilter.

services
  .AddGraphQLServer()
  .AddAuthorization()
  .AddErrorFilter<ErrorFilter>()
  ...

dcby avatar Apr 18 '24 07:04 dcby

While that works, it feels "hacky" to resolve the HttpContext inside the error-filter IMO. Our use-case is to convert it to an HTTP status code. @michaelstaib @PascalSenn any chance to take a look at my PR to fix this? This is a really important issue as a client (front-end) must know the difference in order to know if the JWT needs to be refreshed, the user needs to re-login or if it's just a permission-thing.

private static IError? TransformErrorForAuthn(IError error)
{
    // TODO Below distinction currently doesn't work, because HotChocolate returns NotAuthorized in both cases.
    // Keep an eye on https://github.com/ChilliCream/graphql-platform/issues/6228 
    return error.Code switch
    {
        ErrorCodes.Authentication.NotAuthorized => error
            .SetExtension("statusCode", (int)HttpStatusCode.Forbidden),
        ErrorCodes.Authentication.NotAuthenticated => error
            .SetExtension("statusCode", (int)HttpStatusCode.Unauthorized),
        _ => null,
    };
}

huysentruitw avatar Apr 18 '24 15:04 huysentruitw