[Solved] Setting up a user redirect

[eluser]riwakawd[/eluser]
I would like to know best way to set up my user redirect. So if user if not logged on and try's to access a page it will redirect to my 'admin'

I have autoloaded the user library and sessions

On the parent construct part of my dashboard controller I have set up my sessions redirect but stops me from logging on. If I remove it I can login fine, very strange. Should let me login while using sessions as a redirect.

Dashboard Controller

Code:

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

class Dashboard extends MX_Controller {

   public function __construct()
   {
      parent::__construct();
      $this->lang->load('admin/common/dashboard', 'english');
      
     if ($this->session->userdata('islogged') && $this->session->userdata('username')) {
       return true;
      } else {
       redirect('admin');
      }

   }

}

Login Controller

Code:

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

class Login extends MX_Controller {
private $error = array();

public function __construct()
   {
      parent::__construct();
      $this->lang->load('admin/common/login', 'english');
   }

public function index()
{

  $this->document->setTitle($this->lang->line('heading_title'));

  if (($this->input->server('REQUEST_METHOD') == 'POST') && $this->validate()) {

   $data_session = array(
    'isLogged' => true,
    'user_id' => $this->user->isLogged(),
    'username' => $this->user->getUserName(),
    //'email' => $this->user->getEmail(),
    'last_logged' => $this->user->last_logged()
   );

   $this->session->set_userdata($data_session);

   redirect('admin/dashboard');
  }

  $data['heading_title'] = $this->lang->line('heading_title');

  $data['text_login'] = $this->lang->line('text_login');
  $data['text_forgotten'] = $this->lang->line('text_forgotten');

  $data['entry_username'] = $this->lang->line('entry_username');
  $data['entry_password'] = $this->lang->line('entry_password');

  $data['button_login'] = $this->lang->line('button_login');

  if (array_key_exists('warning', $this->error)) {
   $data['error_warning'] = $this->error['warning'];
  } else {
   $data['error_warning'] = '';
  }

  $data['action'] = site_url('admin');

  if (trim($this->input->post('username'))) {
   $data['username'] = $this->input->post('username');
  } else {
   $data['username'] = '';
  }

  if (trim($this->input->post('password'))) {
   $data['password'] = $this->input->post('password');
  } else {
   $data['password'] = '';
  }


  return $this->load->view('common/login', $data);
}

protected function validate() {
  if (!trim($this->input->post('username')) || !trim($this->input->post('password')) || !$this->user->login($this->input->post('username'), $this->input->post('password'))) {
   $this->error['warning'] = $this->lang->line('error_login');
  }

  return !$this->error;
}
}

Library

Code:

<?php

class User {
private $user_id;
private $username;
private $last_logged;
//private $getEmail;
private $permission = array();

public function __construct() {
  $this->CI =& get_instance();

  if (trim($this->CI->session->userdata('user_id'))) {
   $user_query = $this->CI->db->query("SELECT * FROM " . $this->CI->db->dbprefix . "user WHERE user_id = '" . (int)$this->CI->session->userdata('user_id') . "' AND status = '1'");

   if ($user_query->num_rows) {
    $this->user_id = $user_query->row('user_id');
    $this->username = $user_query->row('username');
    $this->last_logged = $user_query->row('last_logged');
    //$this->getEmail = $user_query->row('email');

    $this->CI->db->query("UPDATE " . $this->CI->db->dbprefix . "user SET ip = " . $this->CI->db->escape($this->CI->input->ip_address()) . " WHERE user_id = '" . (int)$this->CI->session->userdata('user_id') . "'");

    $now = date("Y-m-d H:i:s");

    $data = array(
               'last_logged' => $now
            );

    $this->CI->db->where('user_id', $this->CI->session->userdata('user_id'));
    $this->CI->db->update('user', $data);
   } else {
    $this->logout();
   }
  }
}

public function login() {
  $username = $this->CI->input->post('username');
  $password = $this->CI->input->post('password');

  $user_query = $this->CI->db->query("SELECT * FROM " . $this->CI->db->dbprefix . "user WHERE username = " . $this->CI->db->escape($username) . " AND (password = SHA1(CONCAT(salt, SHA1(CONCAT(salt, SHA1(" . $this->CI->db->escape($password) . "))))) OR password = " . $this->CI->db->escape(md5($password)) . ") AND status = '1'");

  if ($user_query->num_rows() == 1) {

   $this->user_id = $user_query->row('user_id');
   $this->username = $user_query->row('username');
   $this->last_logged = $user_query->row('last_logged');
   //$this->getEmail = $user_query->row('email');

   return true;
  } else {
   return false;
  }
}

public function logout() {
  //$this->CI->session->sess_destroy();
  $this->CI->session->unset_userdata($this->CI->session->userdata('user_id'));
  $this->user_id = '';
  $this->username = '';
}

public function isLogged() {
  return $this->user_id;
}

public function last_logged() {
  return $this->last_logged;
}

public function getId() {
  return $this->user_id;
}

public function getEmail() {
  return $this->email;
}

public function getUserName() {
  return $this->username;
}
}

[eluser]riwakawd[/eluser]
All working now

I after thinking about it over night. I found what was issue. I Removed the trim from my parent construct area

User lib

Old

Code:

if (trim($this->CI->session->userdata('user_id'))) {

New

Code:

if ($this->CI->session->userdata('user_id')) {

And on my parent construct area on my dashboard controller i put.

Code:

if ($this->session->userdata('isLogged')) {
return true;
} else {
redirect('admin');
}

[eluser]CroNiX[/eluser]
because if the session variable doesn't exist, session returns boolean false, and trim(false) is an empty string. So you should be using === comparisons when checking session to see if value exists so it checks the variables data TYPE in addition to its value.

Code:

if ($this->CI->session->userdata('user_id')) {

would not pass your check if the user id is 0, even though the user exists in session, so that's not a good check. It might not matter here with this example, but it will with others.

Code:

if ($this->CI->session->userdata('user_id') !== FALSE) {

is much more accurate to see if the value is set in session or not. There are plenty of instances where 0 is a valid value in session but it wouldn't work the way you are checking.

Also, if you allow

Code:

$this->CI->db->escape(md5($password))

then its next to useless. md5 should NOT be allowed on its own. There are plenty of rainbow tables out there that will bypass that in NO TIME. You better hope none of your users, or the public who can access your app, are hackers. Your site, or more likely your clients site, would become theirs. I guess you haven't been paying attention to the news lately of all of the hacking going on. It's because of stuff like you're doing using md5(), which is totally insecure.
https://crackstation.net/
https://www.freerainbowtables.com/tables/
http://project-rainbowcrack.com/table.htm

[eluser]InsiteFX[/eluser]
salt password salt

[eluser]riwakawd[/eluser]
[quote author="InsiteFX" date="1409439648"]salt password salt
[/quote]

I had solved it before I posted what I did to fix it.

[eluser]riwakawd[/eluser]
[quote author="CroNiX" date="1409416117"]because if the session variable doesn't exist, session returns boolean false, and trim(false) is an empty string. So you should be using === comparisons when checking session to see if value exists so it checks the variables data TYPE in addition to its value.

Code:

if ($this->CI->session->userdata('user_id')) {

would not pass your check if the user id is 0, even though the user exists in session, so that's not a good check. It might not matter here with this example, but it will with others.

Code:

if ($this->CI->session->userdata('user_id') !== FALSE) {

is much more accurate to see if the value is set in session or not. There are plenty of instances where 0 is a valid value in session but it wouldn't work the way you are checking.

Also, if you allow

Code:

$this->CI->db->escape(md5($password))

then its next to useless. md5 should NOT be allowed on its own. There are plenty of rainbow tables out there that will bypass that in NO TIME. You better hope none of your users, or the public who can access your app, are hackers. Your site, or more likely your clients site, would become theirs. I guess you haven't been paying attention to the news lately of all of the hacking going on. It's because of stuff like you're doing using md5(), which is totally insecure.
https://crackstation.net/
https://www.freerainbowtables.com/tables/
http://project-rainbowcrack.com/table.htm[/quote]

I had solved it before I posted what I did to fix it.

[eluser]Tim Brownlaw[/eluser]
Well you made it "Work" without really understanding why!

Both CroNiX and InsiteFX know you got it "Working" but make some really good suggestions you shouldn't just ignore!

I was about to go into a more detailed explanation of how "if" works and how PHP allows for potential disasters as has been explained above!

Things like...

Code:

if ($this->CI->session->userdata('user_id')) {

should make you shiver in horror! ( See CroNiX's take on it )
Does that definitely return a TRUE or a FALSE?????

Please don't go replying that you had already fixed it, without acknowledging what has been suggested. If you need more explanation, please ask!

[eluser]riwakawd[/eluser]
[quote author="Tim Brownlaw" date="1409466616"]Well you made it "Work" without really understanding why!

Both CroNiX and InsiteFX know you got it "Working" but make some really good suggestions you shouldn't just ignore!

I was about to go into a more detailed explanation of how "if" works and how PHP allows for potential disasters as has been explained above!

Things like...

Code:

if ($this->CI->session->userdata('user_id')) {

should make you shiver in horror! ( See CroNiX's take on it )
Does that definitely return a TRUE or a FALSE?????

Please don't go replying that you had already fixed it, without acknowledging what has been suggested. If you need more explanation, please ask!
[/quote]

I know all about the md5 issues and that it is only temporary. Until I find a suitable security login system. Most of the ones that I have tried are over the top and sometimes does not work with what I am after.