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

Is there a way to change the status code of the authorization extension from 400 to 401? #986

Closed
nichovski opened this issue Jan 9, 2023 · 11 comments · Fixed by #1141
Closed
Labels
question Further information is requested

Comments

@nichovski
Copy link

Hi guys,

It is very strange when my HTTP response codes are not corresponding to the error I want to return to the front end. How can I change that?

I've tried overriding the GraphQLMiddleware, but it is triggered only when my query doesn't have errors.

Thank you!

@Shane32
Copy link
Member

Shane32 commented Jan 9, 2023

Override the WriteJsonResponseAsync method, inspect to see if there are any AccessDeniedError instances within the Errors property of the response, and if so call the base method with the response code of 401 instead of the statusCode passed to the method.

@Shane32
Copy link
Member

Shane32 commented Jan 9, 2023

This answer is applicable when using GraphQL.NET Server 7 to host your endpoint.

@Shane32
Copy link
Member

Shane32 commented Jan 9, 2023

At the bottom of this section there is a sample demonstrating how to override methods of GraphQLHttpMiddleware and use the derived instance

https://github.com/graphql-dotnet/server#graphqlhttpmiddleware

@Shane32
Copy link
Member

Shane32 commented Jan 9, 2023

The same technique can be used to return any status code for any validation error.

Note that for transport-level errors, including authentication errors, the status code will already be set properly by the GraphQLHttpMiddleware. But validation errors, such as those raised by the authentication validation rule, always return by default either 200 or 400 based on the ValidationErrorsReturnBadRequest option.

@nichovski
Copy link
Author

Thank you, I will try that out.

@OpenSpacesAndPlaces
Copy link

Other option you have is to add your own ExecutionError to the IResolveFieldContext and there is a "Data" column that is a dictionary you can use to stuff additional error information like that. Then pull from the ExecuteAsync result "Errors" field.

@sungam3r
Copy link
Member

sungam3r commented Jan 12, 2023

@nichovski Could you provide expected response and actual response to demonstrate where you want 400 instead of 401?

@nichovski
Copy link
Author

@Shane32 thanks for the help. I've tried multiple ways to solve that, but nothing worked as I wanted. Instead of returning 401 on the GraphQL level, I wrote GraphQLHTTPMiddleware for catching errors.

I'm sharing my solution here for everyone interested.

  public class GraphQLHTTPMiddleware
    {
        private readonly RequestDelegate _next;
        private readonly ILogger _logger;
        public GraphQLHTTPMiddleware(RequestDelegate next,
                                                ILoggerFactory loggerFactory)
        {
            _next = next;
            _logger = loggerFactory
                      .CreateLogger<GraphQLHTTPMiddleware>();
        }
        public async Task Invoke(HttpContext context)
        {
           
            Stream originalBody = context.Response.Body;

            try
            {
                using var memStream = new MemoryStream();
                context.Response.Body = memStream;

                await _next.Invoke(context);

                if (context.Request.Path.StartsWithSegments("/graphql"))
                {
                    if (context.Request.Method == "POST")
                    {
                        if (context.Response.StatusCode == 400)
                        {
                            memStream.Position = 0;
                            string responseBody = new StreamReader(memStream).ReadToEnd();
                            context.Response.StatusCode = GetHttpErrorCode(responseBody, context);
                        }
                    }
                }

                memStream.Position = 0;
                await memStream.CopyToAsync(originalBody);

            }
            finally
            {
                context.Response.Body = originalBody;
            }
        }

        private int GetHttpErrorCode(string jsonString, HttpContext context)
        {
            var error = JsonSerializer.Deserialize<GraphQLErrorResult>(jsonString);
            if (error.Errors.Any(x => x.Extensions.Code == "ACCESS_DENIED"))
            {
                return 401;
            }
            return context.Response.StatusCode;


        }
    }

@Shane32
Copy link
Member

Shane32 commented Jan 23, 2023

Did you try the code designed as I suggested? It should perform identically without the need to copy the stream or deserialize the response. It does require GraphQL.NET Server 7.0 or later. See below:

// within Program.cs:
app.UseGraphQL<MyGraphQLHttpMiddleware<ISchema>>("/graphql", new GraphQLHttpMiddlewareOptions());

// custom middleware
public class MyGraphQLHttpMiddleware<T> : GraphQLHttpMiddleware<T>
    where T : ISchema
{
    public MyGraphQLHttpMiddleware(RequestDelegate next, IGraphQLTextSerializer serializer,
        IDocumentExecuter<T> documentExecuter, IServiceScopeFactory serviceScopeFactory,
        GraphQLHttpMiddlewareOptions options, IHostApplicationLifetime hostApplicationLifetime)
        : base(next, serializer, documentExecuter, serviceScopeFactory, options, hostApplicationLifetime)
    {
    }

    protected override Task WriteJsonResponseAsync<TResult>(HttpContext context, HttpStatusCode httpStatusCode, TResult result)
    {
        // place code here to change the status code as desired
        if ((result as ExecutionResult)?.Errors?.Any(x => x.Code == "ACCESS_DENIED") ?? false)
            httpStatusCode = HttpStatusCode.Unauthorized;

        return base.WriteJsonResponseAsync(context, httpStatusCode, result);
    }
}

@Shane32 Shane32 transferred this issue from graphql-dotnet/graphql-dotnet Jan 23, 2023
@Shane32 Shane32 added the question Further information is requested label Jan 23, 2023
@nichovski
Copy link
Author

Yeah, I've tried and it works great. Thanks a lot for your solution. We are still deciding what solution we should choose.

@Shane32
Copy link
Member

Shane32 commented Aug 9, 2024

Version 8 will return more appropriate status codes from the authorization validation rule in certain cases

@Shane32 Shane32 closed this as completed Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants