NancyFx / Nancy

Lightweight, low-ceremony, framework for building HTTP based services on .Net and Mono
http://nancyfx.org
MIT License
7.15k stars 1.47k forks source link

CSRF cannot be enabled on 500 error responses #2691

Open sberney opened 7 years ago

sberney commented 7 years ago

Prerequisites

Description

I'm developing an application (SPA) that returns a fully functional webpage on errors (eg 404, 403, and notably: 500). I have implemented my own custom error handler. If I try to handle status 500 errors in this way, all unhandled exceptions when Csrf is ostensibly enabled are obsfucated.

The error message, regardless of the specific unhandled exception, always reads: "CSRF is not enabled on this request". This is because CSRF handling is performed in the AfterRequest pipeline, and is not invoked when we drop into the OnError pipeline. I am using razor templates to embed the CSRF token on the [error] page: on errors, the token has not been attached to the request, and attempts to access it result in the error message.

This could be solved by introducing an AfterError pipeline, so that CSRF can be enabled even when we enter the error state.

The simplest solution is to not use custom error responses requiring CSRF (or any post-response processing) for status 500 pages, which is the approach I will take for now. However, Internal Server Errors are not always unrecoverable in my web application. And the need to modify error responses seems like something which may have more general use cases.

Steps to Reproduce

Link to functional example

System Configuration

Steinblock commented 5 years ago

Thanks for pointing me in the right direction, I was searching for half a day why nancy would not render my custom error page for status code 500.

I have a Form with an

<form id="logoutForm" action="~/Account/Logout" method="post">
  @Html.AntiForgeryToken()
   <a href="javascript:document.getElementById('logoutForm').submit()">Abmelden</a>
</form>

in my code which causes the error. On My Error page I now do not include this code but I agree a OnError hook in https://github.com/NancyFx/Nancy/blob/master/src/Nancy/Security/Csrf.cs would solve this issue.

Steinblock commented 5 years ago

I found a nice workaround, that will enable me to continue using the CsrfToken in my viewstart (even with validation).

This is my statuscode handler.

I inject a responseNegotiator for content negotiation (because this way I already get a json response for json requests.

I added WithCookies(...) and WithHeaders(...) and for error I also set the cookie and csrf item myself. The only downside is I have to get the token via reflection and assign a CryptographyConfiguration because my bootstrapper uses his own CryptographyConfiguration for Csrf.Enable(...) etc.

    public class CustomStatusCodeHandler : global::Nancy.ErrorHandling.IStatusCodeHandler
    {

            private readonly IResponseNegotiator responseNegotiator;
            private readonly IViewResolver viewResolver;
            internal static CryptographyConfiguration CryptographyConfiguration { get; set; }

            public CustomStatusCodeHandler(IResponseNegotiator responseNegotiator, IViewResolver viewResolver, IViewRenderer viewRenderer)
            {
                this.responseNegotiator = responseNegotiator;
                this.viewResolver = viewResolver;
            }

            public bool HandlesStatusCode(HttpStatusCode statusCode, NancyContext context)
            {
                return null != viewResolver.GetViewLocation($"/status/{statusCode:d}", null, new ViewLocationContext { 
            }

            public void Handle(HttpStatusCode statusCode, NancyContext context)
            {

                var model = new DefaultStatusCodeHandler.DefaultStatusCodeHandlerResult
                {
                    StatusCode = statusCode,
                    Message = "...",
                };

                if (context.Items.TryGetValue(NancyEngine.ERROR_EXCEPTION, out object errorObject) && errorObject is Exception exception)
                {

                    model.Message = exception.Message;
                    if (!StaticConfiguration.DisableErrorTraces && exception is RequestExecutionException && exception.InnerException != null)
                    {
                        // Overwrite "Oh noes!" with inner exception message
                        model.Message = exception.InnerException.Message;
                        model.Details = exception.InnerException.ToString();
                    }

                    // Csrf is enabled in AfterRequest pipeline which does not run
                    // if an exception occures so we add it ourselfs
                    var config = CustomStatusCodeHandler.CryptographyConfiguration;
                    var tokenObject = (string)typeof(Csrf).GetMethod("GenerateTokenString", BindingFlags.NonPublic | BindingFlags.Static)?.Invoke(null, new object[] { config });
                    context.Items.Add(CsrfToken.DEFAULT_CSRF_KEY, tokenObject);

                    context.Response.Cookies.Add(NancyCookie(
                        CsrfToken.DEFAULT_CSRF_KEY,
                        tokenObject,
                        true, 
                        false)
                    );

                }

                Negotiator negotiator = new Negotiator(context)
                    .WithStatusCode(statusCode)
                    .WithCookies(context.Response.Cookies)
                    .WithHeaders(context.Response.Headers.Select(h => Tuple.Create(h.Key, h.Value)).ToArray())
                    .WithView($"/status/{statusCode:d}")
                    .WithModel(model);

                context.Response = responseNegotiator.NegotiateResponse(negotiator, context);
         }
    }