Moving from Action Filters to Message Handlers

The updated code for the Web API project can be found here In the last post on Making Your ASP.NET Web API’s Secure, I received some very useful comments. Thanks to Pedro Reys for pointing out my non-use of HttpMessageHandlers. Pedro is absolutely correct in his observation that through the use of handlers, we can detect issues BEFORE processing gets into the controller context. If you are concerned about performance (and we all should be!!) – Http Message Handling is where you want to be. It turned out to be a pretty easier conversion. As you may recall, there were 3 actions filters that enforced:

  1. HTTPS
  2. A valid authorization token
  3. A valid IP Host source

Here are the 3 new handlers:

using System;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;

namespace WebAPI
{
 public class HttpsHandler : DelegatingHandler
 {
  protected override Task SendAsync(HttpRequestMessage request,
   CancellationToken cancellationToken)
  {
   if (!String.Equals(request.RequestUri.Scheme, "https", StringComparison.OrdinalIgnoreCase))
   {
    return Task.Factory.StartNew(() =>
    {
     return new HttpResponseMessage(HttpStatusCode.BadRequest)
     {
      Content = new StringContent("HTTPS Required")
     };
    });
   }
   return base.SendAsync(request, cancellationToken);
  }
 }
}
using System;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using WebAPI.Models;

namespace WebAPI
{
 public class TokenValidationHandler : DelegatingHandler
 {
  protected override Task SendAsync(HttpRequestMessage request,
   CancellationToken cancellationToken)
  {
   string token;

   try
   {
    token = request.Headers.GetValues("Authorization-Token").FirstOrDefault();
   }
   catch (System.InvalidOperationException)
   {
    return Task.Factory.StartNew(() =>
    {
     return new HttpResponseMessage(HttpStatusCode.BadRequest)
     {
      Content = new StringContent("Missing Authorization-Token")
     };
    });
   }

   try
   {
    var foundUser = AuthorizedUserRepository.GetUsers().FirstOrDefault(x => x.Name == RSAClass.Decrypt(token));
    if (foundUser == null)
     return Task.Factory.StartNew(() =>
     {
      return new HttpResponseMessage(HttpStatusCode.Forbidden)
      {
       Content = new StringContent("Unauthorized User")
      };
     });
   }
   catch (RSAClass.RSAException)
   {
    return Task.Factory.StartNew(() =>
    {
     return new HttpResponseMessage(HttpStatusCode.InternalServerError)
     {
      Content = new StringContent("Error encountered while attempting to process authorization token")
     };
    });
   }
   return base.SendAsync(request, cancellationToken);
  }
 }
}
using System;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using WebAPI.Models;

namespace WebAPI
{
 public class IPHostValidationHandler : DelegatingHandler
 {
   protected override Task SendAsync(HttpRequestMessage request,
   CancellationToken cancellationToken) {

    var context = request.Properties["MS_HttpContext"] as System.Web.HttpContextBase;
    string userIP = context.Request.UserHostAddress;

    var foundIP = AuthorizedIPRepository.GetAuthorizedIPs().FirstOrDefault(x => x == userIP);
    if (foundIP == null)
     return Task.Factory.StartNew(() =>
     {
      return new HttpResponseMessage(HttpStatusCode.Forbidden)
      {
       Content = new StringContent("Unauthorized IP Address")
      };
     });

    return base.SendAsync(request, cancellationToken);

   }
 }
}

I went ahead and created a specific exception to address situations when the RSA encryption/description process throws the generic bad data exception. Here is the revised RSAClass:

using System;
using System.Linq;
using System.Security.Cryptography;
using System.Text;

namespace WebAPI
{
 public class RSAClass
 {
  private static string _privateKey = "s6lpjspk+3o2GOK5TM7JySARhhxE5gB96e9XLSSRuWY2W9F951MfistKRzVtg0cjJTdSk5mnWAVHLfKOEqp8PszpJx9z4IaRCwQ937KJmn2/2VyjcUsCsor+fdbIHOiJpaxBlsuI9N++4MgF/jb0tOVudiUutDqqDut7rhrB/oc=AQAB</pre>
3J2+VWMVWcuLjjnLULe5TmSN7ts0n/TPJqe+bg9avuewu1rDsz+OBfP66/+rpYMs5+JolDceZSiOT+ACW2Neuw==
<pre><q>0HogL5BnWjj9BlfpILQt8ajJnBHYrCiPaJ4npghdD5n/JYV8BNOiOP1T7u1xmvtr2U4mMObE17rZjNOTa1rQpQ==</q>jbXh2dVQlKJznUMwf0PUiy96IDC8R/cnzQu4/ddtEe2fj2lJBe3QG7DRwCA1sJZnFPhQ9svFAXOgnlwlB3D4Gw==evrP6b8BeNONTySkvUoMoDW1WH+elVAH6OsC8IqWexGY1YV8t0wwsfWegZ9IGOifojzbgpVfIPN0SgK1P+r+kQ==LeEoFGI+IOY/J+9SjCPKAKduP280epOTeSKxs115gW1b9CP4glavkUcfQTzkTPe2t21kl1OrnvXEe5Wrzkk8rA==HD0rn0sGtlROPnkcgQsbwmYs+vRki/ZV1DhPboQJ96cuMh5qeLqjAZDUev7V2MWMq6PXceW73OTvfDRcymhLoNvobE4Ekiwc87+TwzS3811mOmt5DJya9SliqU/ro+iEicjO4v3nC+HujdpDh9CVXfUAWebKnd7Vo5p6LwC9nIk=";
  private static string _publicKey = "s6lpjspk+3o2GOK5TM7JySARhhxE5gB96e9XLSSRuWY2W9F951MfistKRzVtg0cjJTdSk5mnWAVHLfKOEqp8PszpJx9z4IaRCwQ937KJmn2/2VyjcUsCsor+fdbIHOiJpaxBlsuI9N++4MgF/jb0tOVudiUutDqqDut7rhrB/oc=AQAB";
  private static UnicodeEncoding _encoder = new UnicodeEncoding();

  public static string Decrypt(string data)
  {

   try
   {
    var rsa = new RSACryptoServiceProvider();
    var dataArray = data.Split(new char[] { ',' });

    byte[] dataByte = new byte[dataArray.Length];
    for (int i = 0; i < dataArray.Length; i++)
    {
     dataByte[i] = Convert.ToByte(dataArray[i]);
    }

    rsa.FromXmlString(_privateKey);
    var decryptedByte = rsa.Decrypt(dataByte, false);
    return _encoder.GetString(decryptedByte);

   }
   catch (Exception)
   {
    throw new RSAException();
   }

  }

  public static string Encrypt(string data)
  {

   try
   {
    var rsa = new RSACryptoServiceProvider();
    rsa.FromXmlString(_publicKey);
    var dataToEncrypt = _encoder.GetBytes(data);
    var encryptedByteArray = rsa.Encrypt(dataToEncrypt, false).ToArray();
    var length = encryptedByteArray.Count();
    var item = 0;
    var sb = new StringBuilder();
    foreach (var x in encryptedByteArray)
    {
     item++;
     sb.Append(x);

     if (item < length)
      sb.Append(",");
    }

    return sb.ToString();

   }
   catch (Exception)
   {
    throw new RSAException();
   }
  }

  public class RSAException : Exception {

   public RSAException() : base("RSA Encryption Error") {}

  }
 }
}

The one difference I did notice between the action filter and handler was in the way unhandled errors were passed back to the client. For example, with an action filter, a missing authorization token would have the following rendered to the client:

On the other and, the message handler would render the same error the client this way:

You may have noticed I changed the linq calls from First to FirstOrDefault. I got flamed a bit for using just First – and then letting the exception deal with the results in the event a value could not be found. I still contend that using an exception or checking the nullity of a value doesn’t make that much of a difference. In the case of checking for the existence of a header, it’s a moot point because if you invoke code like this:

var token =
  request.Headers.GetValues("Authorization-Token").FirstOrDefault();

and the Authorization-Token header does not exist, you never get to the FirstOrDefault() Linq Query…:-) As it turns out, you have to wrap that call in a try catch anyway – at least if you wish to present to the client a predictable and useful message. Always take with a grain of salt when somebody espouses “best practice” or something being a “bad practice” or “bad form”. No question, there are established good/best practices out there. Those practices however, are quantifiable and verifiable through empirical evidence. Anything else is simply a matter of opinion and preference. AND – context means everything. What may be a good/best practice in one scenario may not be so in another scenario. And often, technology has little to nothing to do with the equation. For example, Dependency Injection is a good practice and is often, a best practice. BUT – in a shop where the concept is new, foreign or not understood, if it presents a bar to shipping software, it may not be a best practice for that shop – at that time. One of the many reasons why I quickly brush off notions of what best practices are and are not when asked.. :-) A bit of a rant on my part. I definitely appreciate the comments – but rest assured, I’ll push back a bit when I think conclusions are offered absent a premise. Sorry..that’s the lawyer in me that is now hard wired!!

About johnvpetersen

I've been developing software for 20 years, starting with dBase, Clipper and FoxBase + thereafter, migrating to FoxPro and Visual FoxPro and Visual Basic. Other areas of concentration include Oracle and SQL Server - versions 6-2008. From 1995 to 2001, I was a Microsoft Visual FoxPro MVP. Today, my emphasis is on ASP MVC .NET applications. I am a current Microsoft ASP .NET MVP. Publishing In 1999, I wrote the definitive whitepaper on ADO for VFP Developers. In 2002, I wrote the Absolute Beginner’s Guide to Databases for Que Publishing. I was a co-author of Visual FoxPro Enterprise Development from Prima Publishing with Rod Paddock, Ron Talmadge and Eric Ranft. I was also a co-author of Visual Basic Web Development from Prima Publishing with Rod Paddock and Richard Campbell. Education - B.S Business Administration – Mansfield University - M.B.A. – Information Systems – Saint Joseph’s University - J.D. – Rutgers University School of Law (Camden) In 2004, I graduated from the Rutgers University School of Law with a Juris Doctor Degree. I passed the Pennsylvania and New Jersey Bar exams and was in private practice for several years – concentrating transactional and general business law (contracts, copyrights, trademarks, independent contractor agreements, NDA’s, intellectual property and mergers and acquisitions.).
This entry was posted in ASP.NET MVC 4, ASP.NET Web API. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • Pingback: calling my web api from jquery securely - How-To Video

  • Anonymous

    Indeed…what is best/good for one may often not be best/good for another. And as youi say, get 10 “experts” in the room and you will get 10 different answers. Folks need to understand that while there are cases when there are hard facts to support one approach being better than another, most of what we ecounter in this business are opinions that fall within the spectrum of good/best practice.

  • Anonymous

    If you are getting bad request, most likely, there is a problem with the token you are sending in. Or, you may be using an invalid host name. I’ve tested this on a remote box and it works fine. You have to make sure you are good with the IP address. One suggestion – try disabling the https and ip address handlers – and then start working from there.

  • Anonymous

    Yep – definitely makes sense. When I want to get a status back to the client, why would I want to spin up a background thread? With that, here is an example of modified code:

    if (String.IsNullOrEmpty(token))
    {
    tcs = new TaskCompletionSource();
    tcs.SetResult(new HttpResponseMessage(HttpStatusCode.Forbidden)
    {
    Content = new StringContent(“Missing Authorization Token”)
    });
    return tcs.Task;
    }

    Again, thanks for the tip Ward!!

  • Anonymous

    Hi Ward..

    I’m going to check this out this morning. Thanks for the tip!!

    John

  • Ward Bell

    Nice post, John

    At the risk of another premature optimization, I offer an alternative to your use of Task.Factory.StartNew as in this extract:

    return Task.Factory.StartNew(() =>
    {
    return new HttpResponseMessage(HttpStatusCode.Forbidden)
    {
    Content = new StringContent(“Unauthorized IP Address”)
    };
    });

    Task.Factory.StartNew puts a synchronous task (the return of the exception-bearing message) on a background thread. I don’t think you want to do that. I think you want to use TaskCompletionSource. Here is that extract rewritten for TaskCompletionSource

    var msg = new HttpResponseMessage(HttpStatusCode.Forbidden)
    {
    Content = new StringContent(“Unauthorized IP Address”)
    };
    var tcs = new TaskCompletionSource();
    tcs.SetResult(msg);
    return tcs.Task;

    I haven’t tested it so think of it as pseudo-code.

    All the best

  • Gary Chan

    Sorry – just saw my type :)

    I meant to say ‘The message content is only returned when accessing the service locally’.

  • Gary Chan

    I liked the way you were returning more meaningful error messages from within the message handler. I did find a problem when I was testing. The message content is only returned to the client if the client is accessing service is on a different computer.

    For example in your TokenValidatorHandler, I only get the ‘Missing Authorization-Token’ message when accessing my service from my development system. When I make the identical call from another system, all I get is ‘Bad Request’.

    Can you shed any light on this?

  • Anonymous

    You are making the case to simply fork the entire stack and go from there. I am wondering how long it will be before the community simply co-opts the stack in the way the community overtook Hudson and created Jenkins.

  • http://pedroreys.com/ Pedro Reys

    I believe so, I don’t think we will see pull requests being accepted on aspnetwebstack for Media Type formatters using alternative serializers, for example. Also, there are stuff that might not be in the short term roadmap of the Web API framework but that is really nice to have, like API docs generation.

    But yes, for some stuff that in the past had to be done via a contrib, I expect it to be done via pull requests for the main project.

  • Anonymous

    Is a contrib project necessary anymore given that the Web API project itself is taking pull requests?

  • Anonymous

    Nice!! I like that. Thanks.

  • Craig Cavalier

    Rather than wrapping in a try catch, you may be better off checking if the header exists before trying access it:

    if(!response.Headers.Contains(“Authorization-Token”))

  • Pingback: The Morning Brew - Chris Alcock » The Morning Brew #1080

  • Kingcrim

    I think James Bach has the definitive comment about ‘best practices’:

    http://www.satisfice.com/blog/archives/27

    ““This practice represents a consensus among industry leaders.”No it doesn’t. You’re just making that up. Industry leaders aren’t able to agree on anything much of substance. I know this because I am an industry leader (based on the fact that some people have said so), and other leaders more often disagree with me instead of obeying my commands”

  • http://pedroreys.com/ Pedro Reys

    Nice post, John.

    Do you want to make a pull request to the WebApiContrib project? ;-)

    https://github.com/WebApiContrib/WebAPIContrib