19

Since documentation on this process is very vague and confusing (or old), I wanted to verify that I was doing it correctly and not missing any steps.

I am trying to create a secure login system, that expires on browser-close.

-- in my web.config I have the following --

<authentication mode="Forms">
      <forms loginUrl="~/Login.aspx" defaultUrl="Index.aspx" name=".ASPXFORMSAUTH" timeout="100" />
    </authentication>
    <authorization>
      <allow users="?" />
    </authorization>
    <machineKey decryption="AES" validation="SHA1" validationKey.......... />

So I have a login form with username/password textbox and this button:

<asp:Button ID="LoginButton" runat="Server" OnClick="Login_Authenticate" Text="Sign in" />

Inside Login_Authenticate I do the following:

protected void Login_Authenticate(object sender, EventArgs e){
string userName = UserName.Text;
string password = Password.Text;

bool Authenticated = false;

// Here's code that makes sure that Username and Password is CORRECT
if(AuthClass.Authenticate(userName, password)){
 Authenticated = true;
}
// error checking does happen here.

if (Authenticated)
{
  FormsAuthenticationTicket ticket = new FormsAuthenticationTicket(1, userName, DateTime.Now, DateTime.Now.AddMinutes(30), rememberUserName, String.Empty, FormsAuthentication.FormsCookiePath);
  string encryptedCookie = FormsAuthentication.Encrypt(ticket);
  HttpCookie cookie = new HttpCookie(FormsAuthentication.FormsCookieName, encryptedCookie);
  cookie.Expires = DateTime.Now.AddMinutes(30);
  Response.Cookies.Add(cookie);
  //FormsAuthentication.RedirectFromLoginPage(userName, false);

  Response.Redirect("MainPage.aspx");
}
}

--- in the MasterPage.master.cs I have the following check in Page_Init() ---

if (Context.User.Identity.IsAuthenticated)
    {
      int userid = (int)Session["userid"];
      if (userid == null)
      {
        userid = GetUserID(Context.User.Identity.Name);
        if (userid != null)
        {
          Session["userid"] = userid;
        }
      }
    }

EDIT: --- GLOBAL.ASAX ; some code that I am not quite sure is correct or know what it does

protected void Application_AuthenticateRequest(object sender, EventArgs e)
    {
        // look if any security information exists for this request
        if (HttpContext.Current.User != null)
        {
            // see if this user is authenticated, any authenticated cookie (ticket) exists for this user
            if (HttpContext.Current.User.Identity.IsAuthenticated)
            {
                // see if the authentication is done using FormsAuthentication
                if (HttpContext.Current.User.Identity is FormsIdentity)
                {
                    // Get the roles stored for this request from the ticket
                    // get the identity of the user
                    FormsIdentity identity = (FormsIdentity)HttpContext.Current.User.Identity;
                    //Get the form authentication ticket of the user
                    FormsAuthenticationTicket ticket = identity.Ticket;
                    //Get the roles stored as UserData into ticket
                    string[] roles = { };
                    //Create general prrincipal and assign it to current request

                    HttpContext.Current.User = new System.Security.Principal.GenericPrincipal(identity, roles);
                }
            }
        }
    }

--- from then on, on every page, I use the Session userid to gather the user information and content and make sure the user has proper authentication and group-role permissions.

Is this all correct? Or do I have to Decrypt anything somewhere?

Is this enough to make a secure user login? Or should I not bother with forms authentication and find my own way to make my own cookies and manage it myself?

John Saunders
  • 160,644
  • 26
  • 247
  • 397
Dexter
  • 6,170
  • 18
  • 74
  • 101
  • what if the code fails Authenticated always returns true bool Authenticated = false; // Here's code that makes sure that Username and Password is CORRECT Authenticated = true; this is what you have at the top.. but if code fails it never sets Authenticated back to false.. – MethodMan Jan 10 '12 at 21:11
  • @DJKRAZE the code does set it to false if there are errors. I just didn't include the lengthy code for checking username/password. Authenticated = true only happens if everything is successful. – Dexter Jan 10 '12 at 21:13
  • Added some more code for clarification and included my Global.asax code I am using. I don't know if it's working and am pretty confused. – Dexter Jan 10 '12 at 21:25

4 Answers4

14

The way your code is written logins will persist across browser sessions. It might help to understand the basics of what is going on.

For cookie based authentication methods, there are really three actions:

1) Login - validates user's credentials and creates and stores a cookie on their browser.

2) Logout - simply removes the cookie from the browser (by expiring the cookie or deleting it)

3) Per Request Validation (the part that is is your Application_AuthenticateRequest) - check to see if a cookie exists, and if so, get the user's Identity and Roles and set HttpContext.Current.User.

Typically, the FormsAuthentication module hides most of this from you. It looks like your code is trying to use some of the elements of FormAuthentication (like the FormsAuthenticationTicket and FormsIdentity. This is fine as long as you get what you want.

Your Login_Authenticate method looks fine EXCEPT you are setting an expiration on the cookie. This will make the cookie persist even if you close and reopen the browser. Since this is not the behavior you want, I would not set a cookie expiration. Setting this is like checking the "remember me" checkbox.

The code in Application_AuthenticateRequest gets run every time a page is served from your application. It's primary job is to set HttpContext.Current.User. Typically, if no user is logged in, User is either null or an Anonymous user. If a user is logged in, this should represent your user.

If you are doing these three things, then anywhere in your code you can reference HttpContext.Current.User to decide what level of information you want to display. For instance, if you want to restrict a page to administrators only, you could call HttpContext.Current.Users.IsInRole("Administrators"), and redirect them away from the page if the call returns false.

Hope this helps.

Joe Enzminger
  • 11,110
  • 3
  • 50
  • 75
  • I see. I was in conflict with myself on whether to use cookies and persist--or whether to end session when browser closes. I still can't decide if I should go for security or convenience. --- So does the code in Application_AuthenticateRequest, does that work with everything? I don't have to decrypt or anything the cookie anywhere in my code? What do you mean by FormsAuthentication hides, what does it hide exactly and why shouldn't I use FormAuthentication tickets? I don't want to use the Roles in Current.Users because I query database for a very specific role system anyway. – Dexter Jan 11 '12 at 06:30
  • Since you are setting a cookie using the FormsCookieName, and you have FormsAuthentication enabled, Forms Authentication is decrypting the cookie and setting the HttpContext.Current.User for you before Application_AuthenticateRequest gets called. Using FormAuthentication tickets is perfectly acceptable. – Joe Enzminger Jan 11 '12 at 15:49
  • For the last part, using Roles - remember that HttpContext.Current.User is available just about anywhere in your request handling. In order to avoid hitting the database for every "roles" query, you can set all the roles for the user in Application_AuthenticateRequest with one database call. Consider it a "per request role cache". – Joe Enzminger Jan 11 '12 at 15:51
  • Ok if Current.User gets set before authenticaterequest, why bother having that code at all? What is its use? Yeah, I agree about the roles thing but my role system is different, someone can have like 20 roles and memberships to many groups like in facebook. – Dexter Jan 11 '12 at 16:27
  • The code is there so that you can do any application specific modifications to the User that you need to do - most people set User properties and roles here. In your application, if there isn't anything application specific you need to do, you should be able to remove it. – Joe Enzminger Jan 11 '12 at 20:36
  • I guess that is something to consider for the future, right now it would be too complicated for me to replace code to instead interpret User Data in the cookie rather than just querying based on user ID. – Dexter Jan 11 '12 at 20:47
  • You can do that, but generally it is bad practice to put anything but a key into a cookie. The reason is that anything put into the cookie can potentially be hacked/stolen. For instance, if you were storing the user's password in the cookie (which you absolutely should not do), it would be pretty easy for a third party to intercept the cookie, get the users password, and gain access to their account. You can encrypt the cookie, as FormsAuthentication does, but that also doesn't 100% guarantee that it won't be compromised. – Joe Enzminger Jan 11 '12 at 22:36
  • Yeah, that is why I only want to store user ID or username in the cookie. – Dexter Jan 12 '12 at 06:30
  • ***Application_AuthenticateRequest gets run every time a page*** . For get roles a once time, using Cache (`per request role cache`) ? more alternatives about get ***Roles*** of a User Authenticated ***only once time*** in the _Login page_ ? – Kiquenet May 10 '18 at 08:35
7

I am a bit late on the subject, but for those trying to implement forms authentication while keeping things simple (like I was trying to), here is the relevant most current documentation I have found from Microsoft: https://learn.microsoft.com/en-us/previous-versions/aspnet/xdt4thhy(v=vs.100)

In short, do not mess up with setting cookies, checking them, instantiating tickets or principal, ... Leave-it to FormsAuthentication class.

On log on, when your code has check credentials and if they are valid, just call:

FormsAuthentication.RedirectFromLoginPage(yourUserId, false);

It does set the authentication cookie for you, which, combined with the redirect, is enough. The "false" is for not persisting the authorization: it will be lost on browser close (or authorization timeout).

On already authenticated request, there is nothing to check by code for ensuring your authentication is valid. Use Context.User.Identity.Name to know who is connected (would be the string yourUserId above).

On explicit logout, call

FormsAuthentication.SignOut();
FormsAuthentication.RedirectToLoginPage();

And have forms authentication configured in web.config.

<system.web>
  <authentication mode="Forms">
    <forms loginUrl="yourLoginPage" defaultUrl="yourDefaultPageAfterLogin">
    </forms>
  </authentication>
  <authorization>
    <deny users="?" />
  </authorization>
</system.web>

Note that for MVC applications the authorization part should be removed from configuration and handled with AuthorizeAttribute registered as a global filter attribute, with usage of AllowAnonymousAttribute on controllers or actions needing it. (MVC 4; prior to this one, it was required to create its own attributes for handling that.)

Frédéric
  • 9,364
  • 3
  • 62
  • 112
2

The full workflow for Remember Me requires: 1. Write custom data to the cookie. 2. Read that custom data.

Even if you can authenticate a request via cookie, it does not mean an HttpSession object is resumable for that request.

http://www.codeproject.com/Articles/779844/Remember-Me

enter image description here

Believe2014
  • 3,894
  • 2
  • 26
  • 46
2

There's a problem in your authorization tag, should be:

<authorization>
  <deny users="?" />
  <allow users="*" />
</authorization>

because you want to deny anonymous users. If you fix this, you can safely remove all the stuff from the master page and global.asax - you don't have to remap the forms identity to your own custom identity stored in session. It's the waste of resources and I don't think it raises the security of your solution in a significant way. You can rely on the forms cookie.

Wiktor Zychla
  • 47,367
  • 6
  • 74
  • 106
  • Ah I see. But I do want anonymous users to see, like the main page to select a login. And I do want them to see certain other pages. I also need to use the MasterPage stuff because I need the user ID on every page, I have to check for authentication on the MasterPage or else how will ASP.net know what pages to not allow anonymous users? I have to make sure the user is logged in on many pages. (the if context.identity.IsAuthenticated is part of a big if/else tree, so there are other cases). – Dexter Jan 11 '12 at 06:36
  • So if I replace Redirect() with just a message saying "success" that works. But if I replace it with the Response.Redirect, it refuses to go to that page. Apparently, redirects don't work (maybe the cookie header conflicts?). Also I can't use your authorization tag, it goes into a browser error that says "tried to redirect in a way that could not be completed" I have to use – Dexter Jan 11 '12 at 20:44