using a Singleton to pass credentials in a multi-tenant application a code smell?

Posted by Hans Gruber on Stack Overflow See other posts from Stack Overflow or by Hans Gruber
Published on 2010-05-28T23:54:39Z Indexed on 2010/05/29 0:02 UTC
Read the original article Hit count: 236

Currently working on a multi-tenant application that employs Shared DB/Shared Schema approach. IOW, we enforce tenant data segregation by defining a TenantID column on all tables. By convention, all SQL reads/writes must include a Where TenantID = '?' clause. Not an ideal solution, but hindsight is 20/20.

Anyway, since virtually every page/workflow in our app must display tenant specific data, I made the (poor) decision at the project's outset to employ a Singleton to encapsulate the current user credentials (i.e. TenantID and UserID). My thinking at the time was that I didn't want to add a TenantID parameter to each and every method signature in my Data layer.

Here's what the basic pseudo-code looks like:

public class UserIdentity
    {
        public UserIdentity(int tenantID, int userID)
        {
            TenantID = tenantID;
            UserID = userID;
        }

        public int TenantID { get; private set; }
        public int UserID { get; private set; }
    }

    public class AuthenticationModule : IHttpModule
    {

        public void Init(HttpApplication context)
        {
            context.AuthenticateRequest +=
                new EventHandler(context_AuthenticateRequest);
        }

        private void context_AuthenticateRequest(object sender, EventArgs e)
        {
            var userIdentity = _authenticationService.AuthenticateUser(sender);
            if (userIdentity == null)
            {
                //authentication failed, so redirect to login page, etc
            }
            else
            {
                //put the userIdentity into the HttpContext object so that
                //its only valid for the lifetime of a single request
                HttpContext.Current.Items["UserIdentity"] = userIdentity;
            }
        }
    }


    public static class CurrentUser
    {
        public static UserIdentity Instance
        {
            get { return HttpContext.Current.Items["UserIdentity"]; }
        }
    }

  public class WidgetRepository: IWidgetRepository{

    public IEnumerable<Widget> ListWidgets(){
         var tenantId = CurrentUser.Instance.TenantID;
         //call sproc with tenantId parameter
    }
  }

As you can see, there are several code smells here. This is a singleton, so it's already not unit test friendly. On top of that you have a very tight-coupling between CurrentUser and the HttpContext object. By extension, this also means that I have a reference to System.Web in my Data layer (shudder).

I want to pay down some technical debt this sprint by getting rid of this singleton for the reasons mentioned above. I have a few thoughts on what an better implementation might be, but if anyone has any guidance or lessons learned they could share, I would be much obliged.

© Stack Overflow or respective owner

Related posts about unit-testing

Related posts about singleton