Początki w PHP OOP - logowanie.

0

Witam,

Potrzebuje pomocy. Staram się obiektowo napisać skrypt logowania. Nie wiem dlaczego zaraz po załadowaniu index.php zmienna $_SESSION['logged'] równa jest true oraz widzę napis "Zalogowano". Przecież nie wprowadzono jeszcze nic do formularza. Przez to nie może wykonać się funkcja login. Dopiero zacząłem łapać o co chodzi z tym OOP. Proszę pomóżcie...

index.php:

<?php
 
require_once 'core/init.php';
 
$logowanie = DB::getInstance();
 
$login = trim($_POST['login']);
$password = trim($_POST['password']);
 
$logowanie->login($login, $password);
print_r($_SESSION['logged']);
if($_SESSION['logged'])
{
	echo "Zalogowano";
}
else
{
	echo "Nie zalogowano";
}
?>
 

db.php:

<?php
 
class DB
{
	private static $_instance = null;
	private $_pdo, 
	//konstruktor klasy BD, 
	private function __construct()
	{
		try
		{
			$this->_pdo = new PDO('mysql:host=localhost;dbname=informator', 'root', '');
			echo "Connected";
		}
		catch(PDOException $e)
		{
			die($e->getMessage());
		}
	}
 
	//sprawdߠczy klasa DB juߠistnieje,
	public static function getInstance()
	{
		if(!isset(self::$_instance))
		{
			self::$_instance = new DB();
		}
		return self::$_instance;
	}
 
	public function login($login, $password)
	{
		if(!isset($_SESSION['logged']))
		{
			if(isset($_POST['submit']))
			{
				//$login = trim($_POST['login']);
				//$password = trim($_POST['password']);
 
				try
				{					
					$pdo = self::getInstance();
					$stmt = $pdo->prepare('SELECT * FROM users WHERE login = :login');
					$stmt->bindValue(':login', $login, PDO::PARAM_STR);
					$stmt->execute();
					$row = $stmt->fetch(PDO::FETCH_OBJ);
					$pass = $row->password;
					$usr = $row->login;
				if(password_verify($password, $pass) && $login ===$usr)
				{
					session_start();
					echo "Witaj ".$login;
					$_SESSION['logged'] = true;
					$_SESSION['login'] = $login;
				}
				else
				{
					echo "Podano nieprawidłowe dane";
				}
				}
				catch(PDOException $e)
				{
					echo "Połączenie nie mogło zostac utworzone" .$e->getMessage();
				}
			}
		}
 
	}
}
 
?>
 
0

Ponieważ używasz isset.

$login = trim($_POST['login']); // $login = "" 
$password = trim($_POST['password']); // $password = "" 

isset sprawdza, czy wartość jest ustawiona. W twoim przypadku jest ustawiona, na pusty string, ale jest. :) Sprawdzaj !empty()

0

Nie rozumiem... Przecież w index.php nie ma żadnego isset. O którym miejscu mowa?

0

A gdzie sprawdzasz, czy jest zalogowany? W index, czy w db.php?

0

@Panicz74 sprawdź czy on wchodzi do tego warunku za każdym razem:

if(isset($_POST['submit']))

I wyczyść sesję.

1

@Panicz74: nie rozumiesz podstawowej idei OOP. Nie chodzi o to, aby na pałę wciskać wszystko do jednej klasy - co ma wspólnego logowanie z bazą danych, co?

Nie mówiąc już o tym, że:

  1. Zależność metody login od $_SESSION['logged'] wcale nie jest oczywista - trzeba dopiero poddać kod dokładnej analizie, aby zrozumieć, co się tam dzieje - czyli dochodzi niepotrzebne marnowanie czasu.
  2. Proszę Cię, mamy XXI wiek, terabajtowe dyski... a Ty skracasz nazwę klasy do DB. Kod przede wszystkim powinien być czytelny.
  3. W jakim celu wykorzystujesz podkreślnik (underscore: _) na początku nazw zmiennych? "bo tak kod wygląda fajniej"? Underscore'y to przeszłość i nie powinieneś od nich zaczynać nazw (tyczy się to również nazewnictwa metod).
  4. Zupełnie nie rozumiem po co Ci metoda getInstance - możesz wytłumaczyć? Przecież i tak masz czysty singleton, więc po prostu zrób wszystko statyczne, tak jak bozia przykazała.
  5. Nie musisz nullować ręcznie pól - domyślnie mają one właśnie wartość null.
  6. Konstruktor nie powinien rzucać wyjątków, ani nie powinien robić niczego, co mogłoby rzucić wyjątek.
  7. W jakim celu wykonujesz porównanie $login ===$usr? Przecież już i tak pobrałeś z bazy danych użytkownika, który ma ten login, więc siłą rzeczy nie ma takiej możliwości, aby kiedykolwiek to porównanie się nie spełniło.
  8. Po co Ci $_SESSION['logged']? Wystarczy przecież sprawdzić, czy istnieje $_SESSION['login'].
  9. W złym miejscu rozpoczynasz sesję (patrz niżej).
  10. Komentuj! (patrz: PHPDoc).

Zatem poprawiając:

<?php

/**
 * Klasa odpowiedialna za zarządzanie połączeniem z bazą danych.
 */
class Database {

	/**
	 * @var PDO
	 */
    private static $pdo;
	
	/**
	 * Nawiązuje połączenie z bazą danych.
	 * @return void
	 * @throws Exception
	 */
	public static function connect() {
		self::$pdo = new PDO('mysql:host=localhost;dbname=informator', 'root', '');
	}
	
	/**
	 * Zwraca instancję połączenia.
	 * @return PDO|null
	 */
	public static function getPdo() {
		return self::$pdo;
	}
	
	/**
	 * Przygotowuje i zwraca zapytanie PDO.
	 * @var string $query
	 * @return PDOStatement
	 */
	public static function prepareQuery($query) {
		return self::$pdo->prepare($query);
	}
}

/**
 * Klasa odpowiedzialna za logowanie użytkownika do serwisu.
 */
class UserAuthorizer {
 
	/**
	 * Dokonuje logowania użytkownika.
	 * W razie niepowodzenia rzuca wyjątek.
	 * @return void
	 * @throws Exception
	 */
	public static function authorize($login, $password) {
		$stmt = Database::prepareQuery('SELECT * FROM `users` WHERE `login` = :login');
		$stmt->bindValue(':login', $login);
		$stmt->execute();
		
		$row = $stmt->fetch();
		
		if (empty($row)) {
			throw new Exception('Użytkownik o podanym loginie nie istnieje.');
		}
		
		if (!password_verify($password, $row->password)) {
			throw new Exception('Błędne hasło.');
		}
		
		$_SESSION['login'] = $login;
	}
	
	/**
	 * Sprawdza czy użytkownik jest już zalogowany - zwraca prawdę, jeśli tak.
	 * @return bool
	 */
	public static function isAuthorized() {
		return isset($_SESSION['login']);
	}
	
	/**
	 * Wylogowywuje aktualnie zalogowanego użytkownika.
	 * @return void
	 */
	public static function logout() {
		if (self::isAuthorized()) {
			unset($_SESSION['login']);
		}
		
		session_destroy();
	}
}

Prawda, że czytelniej?

Potem w głównym skrypcie:

<?php

session_start(); // rozpoczynanie sesji przychodzi tutaj! session_start() ma być wywoływane *zawsze na początku* skryptu, a nie w środku
Database::connect();

if (isset($_POST['login']) && isset($_POST['password'])) {
	if (UserAuthorizer::isAuthorized()) {
		die('już jesteś zalogowany');
	}
	
	try {
		UserAuthorizer::authorize($_POST['login'], $_POST['password']);
	} catch (Exception $ex) {
		die('logowanie się nie powiodło: ' . $ex->getMessage());
	}
	
	die('yay!');
}

To jest w dalszym ciągu kijowe rozwiązanie, choć musisz przyznać, że czytelniejsze od Twojego ;)
Polecam w najbliższym czasie zapoznać się z architekturą MVC oraz przyczepić jakiegoś frameworka (np. Symfony, Zend Framework, CakePHP, Yii (...)).

0

Nawet nie wiesz ile taka lekcja dla mnie znaczy. Dziękuję, że chciało Ci się to dla mnie pisać, na pewno mi się przyda :)

1 użytkowników online, w tym zalogowanych: 0, gości: 1