Witam

napisałem sobie prosty skrypt logowania i starałem się zabezpieczyć sesję póki co się sprawdza lecz nwm czy to jest w pełni bezpieczne rozwiązanie:

[PHP] pobierz, plaintext 
<?php
class user{
	public $pdo;
	public $timeout; public $timeout_active;
	public $last_visit_time;
	public $prefix_mysql;
 
	public function pdo($pdo, $time, $time_activ, $prefix_mysql)
	{
		$this->pdo = $pdo;
		$this->timeout = $time;
		$this->timeout_active = $time_activ;
		$this->prefix_mysql = $prefix_mysql;
	} 
 
	public function login($name, $password)
	{
			$pdo = $this->pdo;
		try{
			$password_sel = set_password($password);
			$stmt = $pdo -> prepare('SELECT id, password FROM `'.$this->prefix_mysql.users.'` WHERE username = :user OR email = :user  LIMIT 1');
			$stmt -> bindValue(':user', $name, PDO::PARAM_STR); 
			$stmt -> bindValue(':pass', $password_sel, PDO::PARAM_STR); 
			$stmt -> execute(); 
			$count = $stmt->rowCount();
		}catch(PDOException $e){
			$display = 'Błąd zapytania:<br> ' . $e->getMessage();
		}
			$row = $stmt -> fetch();
			$user_id = $row['id'];
			if($count == 1){
				if(check_password($password, $row['password'])){
					$display = 'Zalogowano';
					$seskey = md5($_SERVER['REMOTE_ADDR'].$_SERVER['HTTP_USER_AGENT']);
					$this->last_visit_time = time()+$this->timeout_active;
					$stmt = $pdo -> prepare('UPDATE `'.$this->prefix_mysql.users.'` SET  `session_key` = :seskey, `time_activ` = :date WHERE `id` = :user_id');
					$stmt -> bindValue(':date', $this->last_visit_time, PDO::PARAM_INT); 
					$stmt -> bindValue(':seskey', $seskey, PDO::PARAM_STR); 
					$stmt -> bindValue(':user_id', $user_id, PDO::PARAM_INT); 
					$stmt->execute(); 
					$_SESSION['last_active'] = time();
					$_SESSION['owner_ses'] = $seskey;
					$_SESSION['user_id'] = $user_id;
				}else{
				$display = 'Dane nie prawidłowe.';
				}
			}else{
				$display = 'Takie konto nie istnieje.';
			}
	  return $display;
	} 
 
	public function check($ost_activ)
	{
		if(!empty($_SESSION['user_id']) && !empty($_SESSION['owner_ses'])){
			$timeout = $this->timeout; 
			$owner_ses = md5($_SERVER['REMOTE_ADDR'].$_SERVER['HTTP_USER_AGENT']);
 
			if ((isset($_SESSION['last_active']) && $_SESSION['last_active']<(time()-$timeout))
				|| (isset($_SESSION['owner_ses']) && $_SESSION['owner_ses']!=$owner_ses))
			{
				setcookie(session_name(), '', time()-3600, '/');
				session_destroy();
				header('Location: index.html');
			}else{
				session_regenerate_id(); 
				$_SESSION['last_active'] = time();
				$_SESSION['owner_ses'] = $owner_ses;
				if($ost_activ < time()){
					$this->last_visit_time = time()+$this->timeout_active;
					$pdo = $this->pdo;
					$stmt = $pdo -> prepare('UPDATE `'.$this->prefix_mysql.users.'` SET `time_activ` = :date WHERE `id` = :user_id AND `session_key` = :seskey');
					$stmt -> bindValue(':date', $this->last_visit_time, PDO::PARAM_INT); 
					$stmt -> bindValue(':seskey', $owner_ses, PDO::PARAM_STR); 
					$stmt -> bindValue(':user_id', $_SESSION['user_id'], PDO::PARAM_INT); 
					$stmt->execute(); 		
				}
			}
		}
	}
}
 
$user_class = new user;
$user_class->pdo($pdo,$config['login_time'],$config['login_active_time'], $prefix_mysql);
 
$login = $user_class->login($_POST['email'], $_POST['password']);
 
 
?>
[PHP] pobierz, plaintext 

Jak jeszcze mogę zabezpieczyć logowanie? czy posiada jakieś błędy których nie zauważyłem? chciałbym być pewny przed wydaniem publicznie stronki.

mam nadzieje że to dobry dział, jak nie to przepraszam i prosze o poprawkę

W pełni bezpieczne to nie będzie nigdy. Ale wygląda wszystko ok.

[PHP] pobierz, plaintext 
$user_class->check($get_data['time_activ']); //Sprawdzam sesje
[PHP] pobierz, plaintext 

zapomniałem dodać...


a mam pytanie czy takie tworzenie dostępu jest poprawne?:

[PHP] pobierz, plaintext 
	public function get_permission($id)
	{
		$pdo = $this->pdo;
		$stmt = $pdo -> prepare('SELECT `'.$this->prefix_mysql.role_permission.'`.value, `'.$this->prefix_mysql.permissions.'`.string AS string FROM `'.$this->prefix_mysql.role_permission.'` LEFT JOIN `'.$this->prefix_mysql.permissions.'` ON `'.$this->prefix_mysql.role_permission.'`.id_permission = `'.$this->prefix_mysql.permissions.'`.id WHERE `'.$this->prefix_mysql.role_permission.'`.id_groups = :grupa');
		$stmt -> bindValue(':grupa', $id, PDO::PARAM_INT); 
		$stmt -> execute(); 
		while($row = $stmt -> fetch()){
			$perm[$row['string']] = $row;
		}
		return $perm;
	}
[PHP] pobierz, plaintext 

Ładuje to tak:

[PHP] pobierz, plaintext 
$get_perm = $user_class->get_permission($get_data['group']);
[PHP] pobierz, plaintext 

A wykorzystuje np w templatkach lub php tak:

[PHP] pobierz, plaintext 
if($get_perm['access_edit']['value'] == 1){
//it's works
}
[PHP] pobierz, plaintext 

Czy jest to dobry sposób?

Tak sposób w porządku

Co do kodu to jest wporządku. Problemem jest to co wykorzystałeś do obsługi tego kodu, a mianowicie sesje. W momencie gdy będziesz posiadał hosting współdzielony (a będziesz bo wątpie żebyś kupował dedyka) istnieje możliwość dobrania się do twoich plików przechowujących sesje i ich modyfikacji.

Poczytaj o sesjach i jak je zabezpieczać dopiero wtedy to będzie porządne Ale żebys mnie źle nie zrozumiał od strony kodu można się jedynie o logikę przyczepić ;p

Osobiście polecam ci olać powyższą część mojego posta, a sesje zastąpić jakimś kontenerem w którym narazie pod maską będą sesje np.

[PHP] pobierz, plaintext 
<?php
 
namespace YourFrog;
 
interface SessionInterface
{
	/**
	 *	Pobranie wartości z sesji
	 */
	public function get($key, $defaultValue = null);
 
	/**
	 *	Sprawdzenie czy wartość w sesji istnieje
	 */
	public function hasElement($key);
 
	/**
	 *	Zapisanie wartości w sesji
	 */
	public function set($key, $value);
}
 
/**
 *	Fabryka
 */
class SessionFactory
{
	private static $_instance;
 
	/**
	 *	@return SessionInterface
	 */
	public static function get()
	{
		if( self::$_instance === null )
			self::$_instance = new Session();
 
		return self::$_instance;
	}
}
 
class Session implements SessionInterface
{	
	/**
	 *	Ukryty konsturktor
	 */
	public function __construct()
	{
		$this->sessionStart();
		$this->items = $_SESSION;
	}
 
	private function sessionStart()
	{
		if( session_status() == PHP_SESSION_NONE )
		{
			session_start();
			session_regenerate_id();
		}
	}
 
	public function get($key, $defaultValue = null)
	{
		if( $this->hasElement($key) )
			return $this->items[$key];
 
		return $defaultValue;
	}
 
	public function hasElement($key)
	{
		return isset($this->items[$key]);
	}
 
	public function set($key, $value)
	{
		$this->items[$key] = $value;
	}
}
[PHP] pobierz, plaintext 

Musisz wtedy pamiętać tylko żeby wywoływać fabrykę, a nie bezpośrednio klase session, no ale to też można obejść jakimś wzorcem projektowym jak rejestr.