ASP.NET membership activate user - user id as querystring

Question

i have web site with ASP membership default provider. i want to activte new users by email with link to "activate" page. in email html i added link to "activate" page with query string of user id in order to active it. is it safe to do it? it seams sensitive data but i can find any alternatives.code for "activate" page:

protected void Page_Load(object sender, EventArgs e)
{
    //request for signed user UserId
    Guid newUserId = new Guid(Request.QueryString["UserId"]);
    //get signed user
    MembershipUser newUser = Membership.GetUser(newUserId);
    //activate signed user
    newUser.IsApproved = true;
    Membership.UpdateUser(newUser);
}

---

i have the folowing code in activation page after the user activate the account with email link. i am getting "object reference not set to an instance of an object" error for the line " newUser.IsApproved = true;". i check userId string and it gets the user Id string and the string is not empty.why do i get this error?

    //request for signed user UserId
    Guid activationCode = new Guid(Request.QueryString["ActivationCode"]);
    string userId = "";

    string ConnectionString = ConfigurationManager.AppSettings["myConnectionString"];
    try
    {
        using (SqlConnection sqlConnection = new SqlConnection(ConnectionString))
        {
            SqlCommand sqlCommand = new SqlCommand("SELECT user_id FROM ActivationCode WHERE activation_code LIKE '%" + activationCode + "%'", sqlConnection);
            sqlConnection.Open();
            SqlDataReader userIdRdr;
            userIdRdr = sqlCommand.ExecuteReader();
            while (userIdRdr.Read())
            {
                userId = userIdRdr["user_id"].ToString();
            }
            sqlConnection.Close();

            MembershipUser newUser = Membership.GetUser(userId);
            //activate signed user
            newUser.IsApproved = true;
            Membership.UpdateUser(newUser);
        }
    }

Answer 1

Try this

private void SendActivationEmail(string userId)
{
 string connectionString = ConfigurationManager.ConnectionStrings["connectionString"].ConnectionString;
 string activationCode = Guid.NewGuid().ToString();
 try
 {
   using (SqlConnection sqlConnection = new SqlConnection(connectionString))
   {
     SqlCommand sqlCommand = new SqlCommand("spInsertActivationCode", sqlConnection);
     sqlCommand.CommandType = CommandType.StoredProcedure;
     sqlCommand.Parameters.AddWithValue("@UserId", userId);
     sqlCommand.Parameters.AddWithValue("@ActivationCode", activationCode);
     sqlConnection.Open();
     sqlCommand.ExecuteNonQuery();
     sqlConnection.Close();

     //Afer successfull inserion of Activation Code You can send mail 

     using (MailMessage message = new MailMessage("[email protected]", txtEmail.Text))
     {
       message.Subject = "Account Activation";
       string body = "Hello " + txtUsername.Text.Trim() + ",";
       body += "<br /><br />Please click the following link to activate your account";
       body += "<br /><a href = '" + Request.Url.AbsoluteUri.Replace("User.aspx", "User_Activation.aspx?ActivationCode=" + activationCode) + "'>Click here to activate your account.</a>";
       body += "<br /><br />Thanks";
       message.Body = body;
       message.IsBodyHtml = true;
       SmtpClient smtpClient = new SmtpClient();
       smtpClient.Host = "smtp.gmail.com";
       smtpClient.EnableSsl = true;
       NetworkCredential networkCredentials = new NetworkCredential("[email protected]", "<password>");
       smtpClient.UseDefaultCredentials = false;
       smtpClient.Credentials = networkCredentials;
       smtpClient.Port = 587;
       smtpClient.Send(message);
     }
   }
 }
 catch (Exception ex)
 {
   //error
 }
}

------------Stored Procedure-------

CREATE PROCEDURE spInsertActivationCode
@UserId VARCHAR(30),
@ActivationCode VARCHAR(200)

AS
BEGIN
  INSERT INTO tblActivationCode VALUES(@UserId,@ActivationCode)
END

Hope this will help you.

Answer 2

The problem you might run into is that someone can activate his account by simply generating the activation link e.g.:

1. User registered and given id, id probably visible somewhere on the webpage (or url/cookie)
2. User knowing his id generates "activate" link
3. User account activated without actually verifying an email.

I would rather generate one off unique string/guid and store it with user id in a database and use it to generate activation link. So the flow would be something like this:

1. User register and given id, unique activation guid/string (token) generated and stored in db
2. User link generated with activation token
3. User clicks the link, activation token retrieved from database with user id
4. User account activated, user token removed.

And activation page should probably requires logging in.