Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I just started working in a project that wasn't developed by me, I'm worried about some of the functionalities and I would like some suggestions.

After the login validations, the login procedure is made like this:

var _ticket = new FormsAuthenticationTicket(1, user.ID, DateTime.Now, DateTime.Now.AddDays(30), true, user.ID);

string encTicket = FormsAuthentication.Encrypt(_ticket);
HttpContext.Current.Response.Cookies.Add(new HttpCookie(FormsAuthentication.FormsCookieName, encTicket));

As we can see, there is a user object which stores the details of the logged in user (Id, name, e-mail...)

The first problem that I see is that on the FormsAuthenticationTicket the user ID is passed as the name. And every time I want to get any info from the user I have to do this:

Item user = Framework.Business.Item.Load(HttpContext.Current.User.Identity.Name);

The project uses master page, and on every page I have to do this to get the ID/name/picture of the logged user

By the way, at least the load method gets the user date from a collection, BUT, this collection stores not only the users data but all the data that needs to be cached (since ID's are GUIDs) ids won't be duplicated and I think because of this reason, there is only one Collection for everything.

I would like to know if this is right, or what should I do to make it better

share|improve this question

1 Answer

up vote 2 down vote

I think you can simply create a method (or even a property) in your master page, and use it in your pages to follow and adhere to the principal of DRY (Do not repeat yourself). Also in master page function (or property), you can use caching by a hidden class field, to make the performance even better. For example in your master page:

private User user;
public User User
{
   get
   {
       if (user == null)
       {
          // Load user and assign it to the user field
          user = LoadUser(HttpContext.Current.User.Identity.Name);
       }
       return user;
   }
}

Then in your pages, you can refer to your master page, by casting it to the type of your master page. For example, if the name of the master page class is GeneralMaterPage, then in a page using that master page, you can write:

public Page_Load(Object sender, EventArts e)
{
    User user = (MasterPage as GeneralMasterPage).User;
}

This way, you've implemented user extraction and loading code in one place. However, there is a better method for that.

You can write an HTTP Module and bind to AuthenticateRequest event, and do the authentication mechanism yourself. Then, if the user is valid, you can simply create a GeneralPrincipal including the real user name and populate HttpContext.Current.User with that principal.

Take a look at this link.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.