WebAPI Authentication

Question

Do you see any security issues with this authentication approach or have any suggestions?

WebAPI is REST-based. The user logs in by calling the Authentication Controller and expects a 401 if unauthenticated or a 200 with a session token in the header.

GitHub

Session session = null;
try
{
    session = Models.User.Login(user);
}
catch (Exception ex)
{
    var errResponse = new HttpResponseMessage(System.Net.HttpStatusCode.Unauthorized)
    {
        Content = new StringContent("Invalid Username or Password"),
        ReasonPhrase = ex.InnerException.ToString()
    };
    throw new HttpResponseException(errResponse);
}

On every call except Authentication the request is intercepted and expects a session header. If a session exists in the database then the request is authenticated.

protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
    try
    {
        //Unauthorized
        if (HttpContext.Current.Request.Headers[SessionHeader] == null)
        {
            var errResponse = new HttpResponseMessage(System.Net.HttpStatusCode.Forbidden)
            {
                Content = new StringContent(SessionError),
                ReasonPhrase = SessionError
            };
            return errResponse;

        }

        string sessionHeader = HttpContext.Current.Request.Headers[SessionHeader];


        Session session = Session.Lookup(sessionHeader);

        if (session == null)
        {
            throw new SecurityException("No session record exists");
        }
        else if (session.Expired)
        {
            throw new SecurityException("Session Expired");
        }
        else
        {
            session.LastAccess = DateTime.Now;
            session.Save();
            var identity = new SecurityPrincipal(session);
            Thread.CurrentPrincipal = identity;
            HttpContext.Current.User = identity;

        }

The session lookup is done with Entity Framework

public static Session Lookup(string sessionId) {
    using (var context = new SingleAppContextCustom())
    {
        var session = (from s in context.Sessions.IncludeChildren()
                        where s.SessionId == sessionId
                        select s).FirstOrDefault();

        return session;
    }
}

Answer 1

In general, you want to give as little information as possible for why access was denied. So I would change that section of code to:

if (session == null || session.Expired)
{
    throw new SecurityException("Access Denied");
}

Presumably this exception gets converted to an appropriate HTTP status code.

SendAsyc is an odd name for a function that enforces security, but maybe you have a reason for that. That said, it seems like there's something missing since I don't see where it is async or returns a Task.