Is this good login process in CodeIgniter?

Question

I'm creating my first login process in CodeIgniter. I'm using the simpleloginsecure library for actual session management but I wrote the controller and model myself and I was hoping if you could see any flaws in it.

My User_model Class

<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');

class User_model extends CI_Model {
    public function __construct() {
        parent::__construct();
    }

    function login($email, $password) {
        if($this->simpleloginsecure->login($email, $password)) {
            return true;
        }
        return false;
    }
} 

?>

My User Controller class

<?php if(!defined('BASEPATH')) exit('No Direct script access allowed');

Class User extends CI_Controller {

    public function __construct() {
        parent::__construct();
        $this->load->model('user_model');
    }

    public function index() {
        if($this->session->userdata('logged_in')) {
            redirect('/user/dashboard/', 'location');
        } else {
            $data['message'] = '<p class="error">You need to be logged in to view the administration area</p>';
            $this->load->view('user/login', $data);
        }
    }

    public function dashboard() {
        if($this->session->userdata('logged_in')) {
            $data['title'] = 'Welcome';
            $this->load->view('user/dashboard', $data);
        } else {
            $data['message'] = '<p class="error">You need to be logged in to view the administration area</p>';
            redirect('/user/login/', 'location');
        }

    }

    public function login() {

        if($this->session->userdata('logged_in')) {
            redirect('/user/dashboard/', 'location');
        }

        $this->form_validation->set_rules('email', 'E-mail', 'trim|required|valid_email');
        $this->form_validation->set_rules('password', 'Wachtwoord', 'trim|required|min_length[4]|max_length[32]');

        if($this->form_validation->run() == FALSE) {
            $this->index();
        } else {
            if($this->user_model->login($this->input->post('email'), $this->input->post('password'))) {
                redirect('/user/dashboard/', 'location');
            } else {
                $this->index();
            }
        }
    }

    public function logout() {
        $this->simpleloginsecure->logout();
        redirect('/user/login/', 'location');
    }



}

Answer 1

1. Simply use return $this->simpleloginsecure->login($email, $password) in the login method.
2. redirect('/user/xxx') is enough, since location is the default redirect type.
3. Since you're doing a redirect(), $data['message']is probably "dead code" here. Use CodeIgniter's Flashdata instead.
4. <p class="error"> should be handled in the view, the controller only needs to say there's an error. For example: $data['message'] = array('error' => 'This is my error text');
5. Trimming passwords is dangerous: what if my password starts with a space? Also, a space will look like a real character in an HTML password form, it makes no sense to trim it.
6. When the validation fails, redirect to the login form and use flashdata to explain what got wrong.