Is it considered bad practice to reference the Microsoft.AspNet.Identity in the service layer of a multi layered web application?

Question

In my MVC application I am currently setting the Thread.CurrentPrincipal = HttpContext.Current.User in the Application_PostAuthenticateRequest() method e.g.

    protected void Application_PostAuthenticateRequest()
    {
        Thread.CurrentPrincipal = HttpContext.Current.User;
    }

This allows me to use the Thread.CurrentPrincipal in other assemblies i.e. the service layer. For example:

using System.Security;
using System.Security.Permissions;
using System.Threading;
using Microsoft.AspNet.Identity;

namespace ServiceLayer
{
public class FinancialAccount
{
    public decimal Balance { get; set; }
    public string Owner { get; set; }
}

public class FinancialAccountRepository
{
    public FinancialAccount GetById(int id)
    {
        if (id == 1)
            return new FinancialAccount {Owner = "ac40fe16-1971-4b0d-b4d5-af850d0c2c05", Balance = 40324234};

        return new FinancialAccount {Owner = "3e2d1b43-1c63-4263-8c52-44d050279596", Balance = 100};
    }
}

public class FinancialService
{
    private readonly FinancialAccountRepository _financialAccountRepository;

    public FinancialService()
    {
        _financialAccountRepository = new FinancialAccountRepository();
    }

    [PrincipalPermission(SecurityAction.Demand, Role = Constants.RoleNames.AccountHolder)]
    [PrincipalPermission(SecurityAction.Demand, Role = Constants.RoleNames.BankManager)]
    public string GetFinancialAccountDetails(int accountId)
    {
        FinancialAccount financialAccount = _financialAccountRepository.GetById(accountId);
        ThrowExceptionIfUnauthorized(financialAccount);
        return "The account balance of account: " + accountId + " is " + financialAccount.Balance.ToString("C");
    }

    private void ThrowExceptionIfUnauthorized(FinancialAccount financialAccount)
    {
        if (financialAccount.Owner != Thread.CurrentPrincipal.Identity.GetUserId() && !Thread.CurrentPrincipal.IsInRole(Constants.RoleNames.BankManager))
            throw new SecurityException();
    }
}
}

This all seems to work perfectly although I have two concerns:

1. Is it okay to set the Thread.CurrentPrincipal in the PostAuthenticationRequest method?
2. Is it okay to reference the using Microsoft.AspNet.Identity in my service layer?

The reason I need to reference Microsoft.AspNet.IDentity is because the IPrincipal does not contain the userId and it only contains the username.

If any of this is considered bad practice how do I get around my current issues?

Answer 1

1. Is it okay to set the Thread.CurrentPrincipal in the PostAuthenticationRequest method?

Yes it is ok to assign Principal object (HttpContext.Current.User) to current thread.

2. Is it okay to reference the using Microsoft.AspNet.Identity in my service layer?

It is not a good practice, although you can access it.

The reasons are -

1. Service Layer should not be tightly couple with Presentation Layer.
2. It is hard to unit-test the Service Layer.

Instead, you want to pass UserId as a parameter if you want UserId in service layer.

In Your Scenario

You want to return FinancialAccount instead of string value, and let presentation layer creates the text using string.Format().

The reason is you want to maintain Single Responsibility Principle. In other words, if you want to change the text later which happens very often, you do want to touch the Service Layer again.

public FinancialAccount GetFinancialAccountDetails(int accountId)
{
   return _financialAccountRepository.GetById(accountId);        
}