Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AUTH_NOT_AUTHORIZED returned when expecting AUTH_NOT_AUTHENTICATED #6228

Open
1 task done
epbensimpson opened this issue Jun 1, 2023 · 8 comments · May be fixed by #6908
Open
1 task done

AUTH_NOT_AUTHORIZED returned when expecting AUTH_NOT_AUTHENTICATED #6228

epbensimpson opened this issue Jun 1, 2023 · 8 comments · May be fixed by #6908
Labels
Area: Authorization Issue is related to authorization 🌶️ hot chocolate ❓ question This issue is a question about feature of Hot Chocolate.

Comments

@epbensimpson
Copy link

Is there an existing issue for this?

  • 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 epbensimpson added the 🐛 bug Something isn't working label Jun 1, 2023
@epbensimpson epbensimpson changed the title AUTH_NOT_AUTHENTICATED is never returned AUTH_NOT_AUTHORIZED returned when expecting AUTH_NOT_AUTHENTICATED Jun 1, 2023
@michaelstaib michaelstaib added ❓ question This issue is a question about feature of Hot Chocolate. 🌶️ hot chocolate labels Jun 1, 2023
@michaelstaib
Copy link
Member

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 michaelstaib removed the 🐛 bug Something isn't working label Jun 1, 2023
@epbensimpson
Copy link
Author

epbensimpson commented Jun 1, 2023

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.

@hundreder
Copy link
Contributor

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

@doeringp
Copy link
Contributor

doeringp commented Oct 5, 2023

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.

@epbensimpson
Copy link
Author

@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

huysentruitw added a commit to huysentruitw/hotchocolate that referenced this issue Feb 16, 2024
@huysentruitw
Copy link
Contributor

huysentruitw commented Feb 16, 2024

@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.

@dcby
Copy link

dcby commented Apr 18, 2024

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>()
  ...

@huysentruitw
Copy link
Contributor

huysentruitw commented Apr 18, 2024

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,
    };
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Authorization Issue is related to authorization 🌶️ hot chocolate ❓ question This issue is a question about feature of Hot Chocolate.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants