2

I manage all app erros in my Application_Error() in Global.asax:

protected void Application_Error(object sender, EventArgs e)
{
    Exception exception = Server.GetLastError();

    Log.LogException(exception);

    Response.Clear();

    HttpException httpException = exception as HttpException;

    RouteData routeData = new RouteData();
    routeData.Values.Add("controller", "Erro");

    if (httpException == null)
    {
        routeData.Values.Add("action", "Index");
    }
    else //It's an Http Exception
    {
        switch (httpException.GetHttpCode())
        {
            case 404:
                //Page not found
                routeData.Values.Add("action", "HttpError404");
                break;
            case 500:
                //Server error
                routeData.Values.Add("action", "HttpError500");
                break;

            // Here you can handle Views to other error codes.
            // I choose a General error template  
            default:
                routeData.Values.Add("action", "General");
                break;
        }
    }

    //Pass exception details to the target error View.
    routeData.Values.Add("error", exception);

    //Clear the error on server.
    Server.ClearError();

    //Avoid IIS7 getting in the middle
    Response.TrySkipIisCustomErrors = true;

    //Call target Controller and pass the routeData.
    IController errorController = new ErroController();
    errorController.Execute(new RequestContext(new HttpContextWrapper(Context), routeData));
}

So, I have a Custom Authorize Attribute in my app that handle Unauthorized Request and I want to redirect to Application_Error() manipulate that instead.

So, I do that:

protected override void HandleUnauthorizedRequest(AuthorizationContext context)
{
    if (context.HttpContext.Request.IsAuthenticated)
    {
        throw new HttpException(403, "Forbidden Access.");
    }
    else
    {
        base.HandleUnauthorizedRequest(context);
    }
}

In that way the Application_Error() is called, but it seems ugly to me to call a exception so directly, exist another way? What you think guys?

Acaz Souza
  • 8,311
  • 11
  • 54
  • 97

3 Answers3

2

You should not raise an exception inside of AuthorizeAttribute because that could lead to performance problems for code that is relying on AuthorizeAttribute to do an authorization check. AuthorizeAttribute is meant for checking whether or not Authorization is valid, not for taking action based on this information. That is why the original code doesn't throw an exception directly - it delegates the task to the HttpUnauthorizedResult class.

Instead, you should create a custom handler (similar to the HttpUnauthorizedResult) to raise the exception. This will cleanly separate the logic of checking authorization and taking an action based on being unauthorized into 2 different classes.

public class HttpForbiddenResult : HttpStatusCodeResult
{
    public HttpForbiddenResult()
        : this(null)
    {
    }

    // Forbidden is equivalent to HTTP status 403, the status code for forbidden
    // access. Other code might intercept this and perform some special logic. For
    // example, the FormsAuthenticationModule looks for 401 responses and instead
    // redirects the user to the login page.
    public HttpForbiddenResult(string statusDescription)
        : base(HttpStatusCode.Forbidden, statusDescription)
    {
    }
}

And then in your custom AuthorizeAttribute, you just need to set the new handler in HandleUnauthorizedRequest.

protected override void HandleUnauthorizedRequest(AuthorizationContext context)
{
    if (context.HttpContext.Request.IsAuthenticated)
    {
        // Returns HTTP 403 - see comment in HttpForbiddenResult.cs.
        filterContext.Result = new HttpForbiddenResult("Forbidden Access.");
    }
    else
    {
        base.HandleUnauthorizedRequest(context);
    }
}

If you need to execute a different action than throwing an HttpException, you should subclass ActionResult and implement the action in the ExecuteResult method or use one of the built-in classes that inherits ActionResult.

NightOwl888
  • 55,572
  • 24
  • 139
  • 212
2

Because the Unauthorized is not an error by default!!! Just add this method to global.asax

    protected void Application_EndRequest(object sender, EventArgs e) {
        if (Context.Response.StatusCode == 401 || Context.Response.StatusCode == 403) {
        // this is important, because the 401 is not an error by default!!!
            throw new HttpException(401, "You are not authorised");
        }
    }
amiry jd
  • 27,021
  • 30
  • 116
  • 215
1

Your code is fine. By default if you call the base.HandleUnauthorizedRequest it throws a 401 exception which is intercepted by the forms authentication module and you get redirected to the login page (which might not be the desired behavior). So your approach is correct.

Another possibility is to directly render the corresponding error view if you don't want to go through the Application_Error:

protected override void HandleUnauthorizedRequest(AuthorizationContext context)
{
    if (context.HttpContext.Request.IsAuthenticated)
    {
        context.Result = new ViewResult
        {
            ViewName = "~/Views/Shared/Forbidden.cshtml"
        };
    }
    else
    {
        base.HandleUnauthorizedRequest(context);
    }
}
Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
  • Usage name of Views, Controllers and others in that class is what i'm avoiding. Pass the buck to the Application_Error() is better, because keep all centralized in one place. You aggree? – Acaz Souza Sep 21 '11 at 14:55