Testable authentication handler for web API

Question

I need to unit test my authentication handler. I don't really want do an assert against the text message returned by the handler. How could this be improved ?

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
 // Check for API key
 if (!request.Headers.Contains(AuthConfig.ApiKeyHeader))
 {
 return SendResponseText("ApiKey is required");
 }
 // Check for timestamp
 if (request.GetQueryNameValuePairs().Where(k => k.Key.ToLower() == "timestamp").Count() != 1)
 {
 return SendResponseText("Timestamp is required");
 }
 // Validate timestamp
 if (IsValidTimestamp(request) == false)
 {
 return SendResponseText("Invalid timeStamp");
 }
 // Check for signature
 if (request.Headers.Authorization == null || request.Headers.Authorization.Scheme != AuthConfig.AuthenticationScheme)
 {
 return SendResponseText("Signature is required");
 }
 // Validate API Key
 string apikey = request.Headers.GetValues(AuthConfig.ApiKeyHeader).First();
 UserDTO user = _userRepo.GetUserAuthInfo(apikey);
 if (user == null)
 {
 return SendResponseText("Invalid API key");
 }
 // Validate signature
 string signature = _signatureCalculator.GetSignature(user.Secret, request.RequestUri.OriginalString);
 if (request.Headers.Authorization.Parameter != signature)
 {
 return SendResponseText("Invalid signature");
 }
 return base.SendAsync(request, cancellationToken);
}
private Task<HttpResponseMessage> SendResponseText(string text)
{
 Logger.Error(text);
 var response = new HttpResponseMessage(HttpStatusCode.Unauthorized);
 response.ReasonPhrase = text;
 response.Content = new StringContent(text);
 var tcs = new TaskCompletionSource<HttpResponseMessage>();
 tcs.SetResult(response);
 return tcs.Task;
}

So far, I have put the custom reason phrases in a constant

public sealed class AuthError
{
 public const string ApiKeyMissing = "ApiKey is required";
 ...
}

And my tests look like this

[TestMethod]
public void SecurityHandler_ApiKeyMissingFromHeader_ReturnsUnauthorizedHttpStatus()
{
 var handler = new SecurityHandler(new Mock<IClientRepository>().Object, new Mock<ISignatureCalculator>().Object);
 var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "http://localhost");
 var client = new HttpClient(handler);
 var result = client.SendAsync(httpRequestMessage).Result;
 Assert.AreEqual(result.StatusCode, HttpStatusCode.Unauthorized);
 Assert.AreEqual(result.ReasonPhrase, SecurityHandler.AuthError.ApiKeyMissing);
}

I'm not too happy with this, and I'm doing 2 asserts which is bad practice.. Any ideas ?

Answer 1

Basically you need somehow confirm that

• StatusCode == HttpStatusCode.Unauthorized
• ReasonPhrase == someDefinedConstant

If you only Assert the first, you can't be sure that the logic is flawless. If you only test the second you can't be sure that StatusCode == HttpStatusCode.Unauthorized.

But again, proper unit tests should fail exactly for one reason.

As others had this problem also, Omer Rauchwerger has created a NUnit addin which can be found here: http://rauchy.net/oapt/

Comments

Comments should be used to describe why something is done. The code itself should describe what is done.
So comments like // Check for API key should be removed and the code following should be extracted to a separate method.

timestamp validation

As the message will be "Timestamp is required" for a failed validation, I guess that you need to check if at least one key is found. There it is better to use Any() over Count().

Style

You should be consistent in your coding style. Here you one time use if (!condition) and another time you use if (condition == false).

Refactoring

Extracted validation methods

private bool ContainsApiKey(HttpRequestMessage request)
{
 return request.Headers.Contains(AuthConfig.ApiKeyHeader);
}
private bool ContainsTimeStamp(HttpRequestMessage request)
{
 return request.GetQueryNameValuePairs().Any(k => k.Key.ToLower() == "timestamp");
}
private bool ContainsSignature(HttpRequestMessage request)
{
 return (request.Headers.Authorization != null && request.Headers.Authorization.Scheme == AuthConfig.AuthenticationScheme);
}
private UserDTO GetUser(HttpRequestMessage request)
{
 string apikey = request.Headers.GetValues(AuthConfig.ApiKeyHeader).First();
 return _userRepo.GetUserAuthInfo(apikey);
}
private bool IsValidUser(UserDTO user)
{
 return user != null;
}
private bool IsValidSignature(UserDTO user, HttpRequestMessage request)
{
 string signature = _signatureCalculator.GetSignature(user.Secret, request.RequestUri.OriginalString);
 return request.Headers.Authorization.Parameter == signature
}

and the cleaned up former method

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
 if (!ContainsApiKey(request))
 {
 return SendResponseText(SecurityHandler.AuthError.ApiKeyMissing);
 }
 if (!ContainsTimeStamp(request))
 {
 return SendResponseText(SecurityHandler.AuthError.TimeStampMissing);
 }
 if (!IsValidTimestamp(request))
 {
 return SendResponseText(SecurityHandler.AuthError.TimeStampInvalid);
 }
 if (!ContainsSignature(request))
 {
 return SendResponseText(SecurityHandler.AuthError.SignatureMissing);
 }
 UserDTO user = GetUser(request);
 if (!IsValidUser(user))
 {
 return SendResponseText(SecurityHandler.AuthError.ApiKeyInvalid);
 }
 if (!IsValidSignature(user,request))
 {
 return SendResponseText(SecurityHandler.AuthError.SignatureInvalid);
 }
 return base.SendAsync(request, cancellationToken);
}

• ThunderDev: Thanks for your time, I forgot to mention I'm using MsTest so I can't use that plugin. I don't totally agree on your refactoring as is seems to add a bit of complexity to extract the validation into several one liner methods, but I agree about your other points.