Is this a good way to track current site being reported on? I'm trying to find an elegant and secure way to track the currently reported site. I originally just used a cookie but realized this was not very secure. I have since switched to combine the session and the cookie. It still feels a bit clumsy. Am I missing something here as far as best practices? Can this be improved on?

public static int SiteID
        {
            get
            {
                if (HttpContext.Current.Session[SITE_ID_KEY] == null)
                {
                    if (Web.CookieUtil.CookieExists(SITE_ID_KEY))
                    {
                        Guid userId = (Guid)Membership.GetUser().ProviderUserKey;
                        int siteId = Convert.ToInt32(Web.CookieUtil.GetCookieValue(SITE_ID_KEY));
                        if (Sites.ValidateSite(StateManager.AccountID.Value, siteId, userId))
                        {
                            HttpContext.Current.Session[SITE_ID_KEY] = siteId;
                        }
                        else
                        {
                            Web.CookieUtil.DeleteCookie(SITE_ID_KEY);
                            return -1;
                        }
                    }
                    else
                    {
                        return -1;
                    }
                }

                return (int)HttpContext.Current.Session[SITE_ID_KEY];
            }
            set
            {
                HttpContext.Current.Session[SITE_ID_KEY] = value;
                Web.CookieUtil.CreateCookie(SITE_ID_KEY, value.ToString(), null);
            }
        }
As you pointed out, relying on cookies means that the user can choose someone else's site. That means you should not use cookies at all, not even in addition to the session. If you choose to use cookies anyway, you have to validate that the current user actually has access to the selected site before returning any data (or even worse, before performing some action), which your code already does. You also have to be very careful that there isn't any code path that doesn't perform the validation.

Because of this, I think you should store the data in session only. If you need more persistent storage, use your database, not cookies.



Some points about your code:

return -1;


You shouldn't use “magic” values like -1 to indicate “no value”. Instead, change the return type to int? and return null of no site is selected.

Convert.ToInt32(…)
value.ToString()


This may not work well unless you're 100 % sure that all servers running this code will be on the same culture. You should probably use InvariantCulture here.

Also, if the cookie contains garbage, I think your code should return “no value” not throw an exception.