Question

In my login method I used this code to login user:

FormsAuthentication.SetAuthCookie(model.UserName, model.RememberMe);

Since I wanted to avoid database call always when I need UserId and some other data I made something and I am not very sure that I did it right.
So this is why I need that someone check my code, is it secure and not stupid :)
Am I breaking some rule with doing this.
This is the final stage in my phase of site security since I am not using Membership/Role any more.

So I changed above SetAuth... code with:

CustomPrincipalSerializeModel serializeModel = new CustomPrincipalSerializeModel();
                    var usr = userRepository.GetUser(model.UserName);

                    serializeModel.UserId = usr.UserId;
                    serializeModel.Username = usr.UserName;

                    JavaScriptSerializer serializer = new JavaScriptSerializer();

                    string userData = serializer.Serialize(serializeModel);

                    FormsAuthenticationTicket authTicket = new FormsAuthenticationTicket(
                             1,
                             usr.UserName,
                             DateTime.Now,
                             DateTime.Now.AddMinutes(30),
                             model.RememberMe,
                             userData);

                    string encTicket = FormsAuthentication.Encrypt(authTicket);
                    HttpCookie faCookie = new HttpCookie(FormsAuthentication.FormsCookieName, encTicket);
                    Response.Cookies.Add(faCookie);

In my Global.asax I have added:

protected void Application_PostAuthenticateRequest(Object sender, EventArgs e)
        {
            HttpCookie authCookie = Request.Cookies[FormsAuthentication.FormsCookieName];

            if (authCookie != null)
            {
                FormsAuthenticationTicket authTicket = FormsAuthentication.Decrypt(authCookie.Value);

                JavaScriptSerializer serializer = new JavaScriptSerializer();

                CustomPrincipalSerializeModel serializeModel = serializer.Deserialize<CustomPrincipalSerializeModel>(authTicket.UserData);

                CustomPrincipal newUser = new CustomPrincipal(authTicket.Name);
                newUser.UserId = serializeModel.UserId;
                newUser.Username = serializeModel.Username;
                newUser.FirstName = serializeModel.FirstName;
                newUser.LastName = serializeModel.LastName;

                HttpContext.Current.User = newUser;
            }
        }

Custom principal code - probably not important for main goal of the question:

public interface ICustomPrincipal : IPrincipal
    {
        int UserId { get; set; }
        string Username { get; set; }
        string FirstName { get; set; }
        string LastName { get; set; }        
    }
public class CustomPrincipal : ICustomPrincipal
    {
        public IIdentity Identity { get; private set; }
        public bool IsInRole(string role) { return false; }

        public CustomPrincipal(string email)
        {
            this.Identity = new GenericIdentity(email);
        }

        public int UserId { get; set; }
        public string Username { get; set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
    }
public class CustomPrincipalSerializeModel
    {
        public int UserId { get; set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public string Username { get; set; }
    }
Was it helpful?

Solution

The first problem with your code that I can see is that you should use the forms authentication settings from web.config (such as timeout, domain, path, requireSSL, ...) to set those values for the forms authentication ticket and cookie. Right now you have hardcoded those values. For example you have hardcoded 30 minutes timeout for the ticket which might be different than the timeout set in your web.config for the cookie life. The default is 20 minutes. But if you change this value in your web.config to increase it, your cookie will live longer than the forms authentication ticket. As a consequence the user will always get logged out after 30 minutes and not after the timeout you specified.

Also I would have used a custom Authorize attribute to parse the forms authentication ticket and set the principal instead of using the global Application_PostAuthenticateRequest event. The first being more MVCish way to achieve that. But this is just a recommendation, no problems from security or behavior point of view.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top