I am working on a basic blog application in Codeigniter 3.1.8 and Bootstrap 4.
The application allows Registration and Login. I have concerns about the security level of the Registration system I have put together.
The Register controller:
class Register extends CI_Controller {
public function __construct()
{
parent::__construct();
}
public function index() {
$data = $this->Static_model->get_static_data();
$data['pages'] = $this->Pages_model->get_pages();
$data['tagline'] = 'Want to write for ' . $data['site_title'] . '? Create an account.';
$data['categories'] = $this->Categories_model->get_categories();
$this->form_validation->set_rules('first_name', 'First name', 'required');
$this->form_validation->set_rules('last_name', 'Last name', 'required');
$this->form_validation->set_rules('email', 'Email', 'required|trim|valid_email');
$this->form_validation->set_rules('password', 'Password', 'required|min_length[6]');
$this->form_validation->set_rules('cpassword', 'Confirm password', 'required|matches[password]');
$this->form_validation->set_rules('terms', 'Terms and Conditions', 'required', array('required' => 'You have to accept the Terms and Conditions'));
$this->form_validation->set_error_delimiters('<p class="error-message">', '</p>');
// If validation fails
if ($this->form_validation->run() === FALSE) {
$this->load->view('partials/header', $data);
$this->load->view('auth/register');
$this->load->view('partials/footer');
} else {
// If the provided email does not already
// exist in the authors table, register user
if (!$this->Usermodel->email_exists()) {
// Encrypt the password
$enc_password = md5($this->input->post('password'));
// Give the first author admin privileges
if ($this->Usermodel->get_num_rows() < 1) {
$active = 1;
$is_admin = 1;
} else {
$active = 0;
$is_admin = 0;
}
// Register user
$this->Usermodel->register_user($enc_password, $active, $is_admin);
if ($this->Usermodel->get_num_rows() == 1) {
$this->session->set_flashdata('user_registered', "You are now registered as an admin. You can sign in");
} else {
$this->session->set_flashdata('user_registered', "You are now registered. Your account needs the admin's aproval before you can sign in.");
}
redirect('login');
} else {
// The user is already registered
$this->session->set_flashdata('already_registered', "The email you provided already exists in our database. Please login.");
redirect('login');
}
}
}
}
The Usermodel model:
class Usermodel extends CI_Model {
public function email_exists() {
$query = $this->db->get_where('authors', ['email' => $this->input->post('email')]);
return $query->num_rows() > 0;
}
public function get_num_rows() {
$query = $this->db->get('authors');
return $query->num_rows();
}
public function getAuthors(){
$query = $this->db->get('authors');
return $query->result();
}
public function deleteAuthor($id) {
return $this->db->delete('authors', array('id' => $id));
}
public function activateAuthor($id) {
$author = null;
$updateQuery = $this->db->where(['id' => $id, 'is_admin' => 0])->update('authors', array('active' => 1));
if ($updateQuery !== false) {
$authorQuery = $this->db->get_where('authors', array('id' => $id));
$author = $authorQuery->row();
}
return $author;
}
public function deactivateAuthor($id) {
$author = null;
$updateQuery = $this->db->where(['id' => $id, 'is_admin' => 0])->update('authors', array('active' => 0));
if ($updateQuery !== false) {
$authorQuery = $this->db->get_where('authors', array('id' => $id));
$author = $authorQuery->row();
}
return $author;
}
public function register_user($enc_password, $active, $is_admin) {
// User data
$data = [
'first_name' => $this->input->post('first_name'),
'last_name' => $this->input->post('last_name'),
'email' => $this->input->post('email'),
'password' => $enc_password,
'register_date' => date('Y-m-d H:i:s'),
'active' => $active,
'is_admin' => $is_admin
];
return $this->db->insert('authors', $data);
}
public function user_login($email, $password)
{
$query = $this->db->get_where('authors', ['email' => $email, 'password' => md5($password)]);
return $query->row();
}
}
UPDATE:
I have decided to post the login()
method, from the Login controller, as changing the Register
class would require changing the login accordingly:
public function login() {
$this->form_validation->set_rules('email', 'Email', 'required|trim|valid_email');
$this->form_validation->set_rules('password', 'Password', 'required|trim');
$this->form_validation->set_error_delimiters('<p class="error-message">', '</p>');
if ($this->form_validation->run()) {
$email = $this->input->post('email');
$password = $this->input->post('password');
$this->load->model('Usermodel');
$current_user = $this->Usermodel->user_login($email, $password);
// If we find a user
if ($current_user) {
// If the user found is active
if ($current_user->active == 1) {
$this->session->set_userdata(
array(
'user_id' => $current_user->id,
'user_email' => $current_user->email,
'user_first_name' => $current_user->first_name,
'user_is_admin' => $current_user->is_admin,
'user_active' => $current_user->active,
'is_logged_in' => TRUE
)
);
// After login, display flash message
$this->session->set_flashdata('user_signin', 'You have signed in');
//and redirect to the posts page
redirect('/dashboard');
} else {
// If the user found is NOT active
$this->session->set_flashdata("login_failure_activation", "Your account has not been activated yet.");
redirect('login');
}
} else {
// If we do NOT find a user
$this->session->set_flashdata("login_failure_incorrect", "Incorrect email or password.");
redirect('login');
}
}
else {
$this->index();
}
}
Looking for feedback and improvement ideas.
email_exists()
you can add this ruleis_unique[authors.email]
in your validation. You should not usemd5
for encrypting password. Usepassword_hash()
. \$\endgroup\$ – Malik Bilal Aug 31 '19 at 23:49Login
controller could you please add theindex()
function for that class too? \$\endgroup\$ – DFriend Sep 1 '19 at 14:07