2
\$\begingroup\$
public partial class CreateAdmin : System.Web.UI.Page
    {
        #region Events.

        protected void Page_Load(object sender, EventArgs e)
        {
            if (!IsPostBack)
            {
                LoadOrganizations();
            }
        }

        // Save Admin and Organization Admin in db.
        protected void btnSave_Click(object sender, ImageClickEventArgs e)
        {
            try
            {
                // Save user info in aspnet_Users and aspnet_Membership
                MembershipUser user = SaveMemberShipUser();

                // If user is saved successfully.
                if (user != null)
                {
                    using (TransactionScope transaction = new TransactionScope())
                    {
                        using (var context = new RideshareEntities())
                        {
                            Save(context, (Guid)user.ProviderUserKey);

                            // Commit transaction and save all changes in db.
                            transaction.Complete();
                            context.AcceptAllChanges();
                        }
                    }
                }
            }
            catch (Exception ex)
            {
                if (ex.Message.Equals("The username is already in use."))
                {
                    lblMessage.Text = "The username(Email) is alrady in use.";
                    return;
                }

                // TODO: Remove throw and handle exception(Log error in file)
                throw;
            }
        }

        #endregion

        #region Class Methods.

        // Load organizations in CheckboxList
        private void LoadOrganizations()
        {
            chklstOrganizations.DataTextField = "Name";
            chklstOrganizations.DataValueField = "OrganizationId";

            // More than 2 will break formatting if user add long organization name
            chklstOrganizations.RepeatColumns = 2;

            chklstOrganizations.DataSource = RidersService.GetOrganizations();
            chklstOrganizations.DataBind();
        }

        // Save user info in aspnet_Users and aspnet_Membership
        private MembershipUser SaveMemberShipUser()
        {
            MembershipUser user = null;

            //Adds a new user.
            user = Membership.CreateUser(txtEmail.Text, txtPassword.Text, txtEmail.Text);
            user.IsApproved = true;
            user.Comment = Utility.RandomString(15);

            Membership.UpdateUser(user);

            return user;
        }

        // Save Admin and Organizations.
        private void Save(RideshareEntities context, Guid userId)
        {
            // Get admin object with data.
            Admin admin = GetNewAdminWithData();

            // Assign userId to admin.
            admin.UserId = userId;

            // Add in context and save.
            context.Admins.AddObject(admin);
            context.SaveChanges();

            // Save selected organizations.
            SaveOrganizations(context, admin.AdminId);
        }

        // Save Organizations.
        private void SaveOrganizations(RideshareEntities context, int adminId)
        {
            // Loop through each items in checked list bxo.
            foreach (ListItem item in chklstOrganizations.Items)
            {
                if (item.Selected)
                {
                    int organizationid = 0;

                    // If item successfully parsed in int then save data in OrganizationAdmin table.
                    if (int.TryParse(item.Value, out organizationid))
                    {
                        OrganizationAdmin organizationAdmin = new OrganizationAdmin();
                        organizationAdmin.AdminId = adminId;
                        organizationAdmin.OrganizationId = organizationid;

                        context.OrganizationAdmins.AddObject(organizationAdmin);
                        context.SaveChanges();
                    }
                }
            }
        }

        // Get New Admin object with data populated..
        private Admin GetNewAdminWithData()
        {
            Admin admin = new Admin();

            admin.Email = txtEmail.Text.Trim();
            admin.FirstName = txtFirstName.Text.Trim();
            admin.LastName = txtLastName.Text.Trim();
            admin.ContactNo = !string.IsNullOrEmpty(txtContNumber.Text.Trim()) ? txtContNumber.Text.Trim() : null;
            admin.Address = !string.IsNullOrEmpty(txtAddress.Text.Trim()) ? txtAddress.Text.Trim() : null;
            admin.City = !string.IsNullOrEmpty(txtCity.Text.Trim()) ? txtCity.Text.Trim() : null;
            admin.State = !string.IsNullOrEmpty(txtState.Text.Trim()) ? txtState.Text.Trim() : null;
            admin.Zip = !string.IsNullOrEmpty(txtZip.Text.Trim()) ? txtZip.Text.Trim() : null;

            return admin;
        }

        #endregion
    }
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I don't like that you are using exceptions to determine if the email address is already in use. Exceptions are for exceptional behaviour, not something that is to be (reasonably) expected. It's not that unlikely that someone will try to use the same email address to register on a website (they may have forgotten that they had registered). Checks like that should be done in a separate method, eg:

private bool IsValid()
{
    //check if username is valid by querying the database
    //display an error message if it is and return false.  
    //return true otherwise.
}

protected void btnSave_Click(object sender, ImageClickEventArgs e)
{
    if (!IsValid())
    {
        return
    }

    //perform your save logic.
}

The following method should have a lowercase S for the "ship" as that is how the class is named.

private MembershipUser SaveMemberShipUser()

This line has got SQL injection written all over it:

user = Membership.CreateUser(txtEmail.Text, txtPassword.Text, txtEmail.Text);

Never use user inputted text directly, use a prepared statement instead.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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