Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Klasa Autoryzacji - prośba o opinie
Forum PHP.pl > Forum > PHP
dopelganger
21.08.2013, 09:17:56
Witam,
jestem początkujący w OOP i postanowiłem napisać własną klasę logowania. Poniżej kod klasy i jej użycie. Proszę o opinie, uwagi, będę wdzięczny. Nie wiem czy dobrze rozumię niektóre aspekty oop stąd moje zapytanie.
Dzięki.

[PHP] pobierz, plaintext 
  1. // użycie klasy na stronie poniżej, pomijam formularz logowania HTML, jedynie kod PHP wklejam:
  2.  
  3. <?php
  4.  
  5. 	$user = strip_tags(addslashes(trim($_POST["user"])));
  6. 	$password = strip_tags(addslashes(trim($_POST["password"])));
  7.  
  8. 	$log = new Authorization();
  9. 	$log->setUser($user);
  10. 	$log->setPassword($password);
  11. 	$log->Login();
  12.  
  13. ?>
[PHP] pobierz, plaintext


Kod klasy:

[PHP] pobierz, plaintext 
  1. <?php
  2.  
  3. class Authorization {
  4.  
  5. private $_user;
  6. private $_password;
  7. private $_dbpdo;
  8.  
  9. public function __construct() {
  10. try
  11. {
  12. global $db;
  13. $pdo = new PDO('mysql:host='.$db["Host"].';dbname='.$db["Name"], $db["User"], $db["Password"]);
  14. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  15. $this->_dbpdo = $pdo;
  16. }	
  17. catch(PDOException $error)
  18. {
  19. echo $error->getMessage();
  20. }
  21. }
  22.  
  23. public function setUser($user) {
  24. $this->_user = $user; 
  25. }
  26.  
  27. public function setPassword($password) {
  28. $this->_password = $password; 
  29. }
  30.  
  31. public function getUser() {
  32. return $this->_user;
  33. }
  34.  
  35. public function getPassword() {
  36. return $this->_password;
  37. }
  38.  
  39. public function validateUsr() {
  40. if ( ($this->getUser())=='' || ($this->getPassword())=='' ) {
  41. echo '<p class="errorMsg">Wprowadź login i hasło!</p>';
  42. exit;
  43. }
  44. }
  45.  
  46. public function checkIsUsr() {
  47. $password = @md5($this->getPassword());
  48. $sql = $this->_dbpdo->prepare("SELECT * FROM users WHERE login=:user AND password=:password LIMIT 1");
  49. $sql->bindValue(':user',$this->getUser(),PDO::PARAM_STR);
  50. $sql->bindValue(':password',$password,PDO::PARAM_STR);
  51. $sql->execute();
  52. if ($row = $sql->fetch()) {
  53. return array($row['id'], $row['login']);
  54. } else {
  55. echo '<p class="errorMsg">Nieprawidłowy login i/lub hasło!</p>';
  56. exit;
  57. }
  58. $sql->closeCursor();
  59. }
  60.  
  61. public function Login() {
  62. $this->validateUsr();
  63. $data_user = $this->checkIsUsr();
  64. @session_start();
  65. $_SESSION['id_usr'] = $data_user[0];
  66. $_SESSION['login_usr'] = $data_user[1];
  67. ini_set('session.cookie_httponly', 1);
  68. @session_regenerate_id();
  69. }
  70.  
  71. }
  72.  
  73. ?>
[PHP] pobierz, plaintext
nospor
21.08.2013, 09:26:15
1) addlashes nie sluzy do zabezpieczania danych wkladanych do bazy
2) Skoro uzywasz PDO i bindowania to jestes juz bezpieczny
3) Klasa nie powinna walic na ekran zadnych komunikatow.
4) A juz na pewno klasa nie powinna robic zadnych EXITów
dopelganger
21.08.2013, 09:27:20
Cytat(nospor @ 21.08.2013, 10:26:15 ) *
1) addlashes nie sluzy do zabezpieczania danych wkladanych do bazy
2) Skoro uzywasz PDO i bindowania to jestes juz bezpieczny
3) Klasa nie powinna walic na ekran zadnych komunikatow.
4) A juz na pewno klasa nie powinna robic zadnych EXITów


ok dzięki
nospor
21.08.2013, 09:32:25
1) Trzymanie hasla w czystym md5 jest w dzisiejszych czasach jednoznaczne z trzymaniem hasła w postaci jawnej
2) Wywal te malpy @
3) Klasa nie powinna robic łączenia z bazą. Klasa to połączenie ma otrzymać
dopelganger
21.08.2013, 09:33:07
Cytat(nospor @ 21.08.2013, 10:32:25 ) *
1) Trzymanie hasla w czystym md5 jest w dzisiejszych czasach jednoznaczne z trzymaniem hasła w postaci jawnej
2) Wywal te malpy @


Klasa nie powinna robic łączenia z bazą. Klasa to połączenie ma otrzymać


zamiast md5 stosować inną funkcję ? jeśli tak to jaką, lub w jaki sposób hashować?
nospor
21.08.2013, 09:34:34
http://forum.php.pl/index.php?showtopic=44...t=0&start=0
Przeczytaj caly watek
dopelganger
21.08.2013, 10:02:39
a mam jeszcze jedno pytanie po części powiązane z tym tematem:
czy session_start() i połączenie z bazą wywoływać w momencie kiedy wykonuję jakąś operacje na bazie itp, czy umieścić to tylko raz w pliku np. index.php ?
np:

[PHP] pobierz, plaintext 
  1.  
  2. session_start();
  3. connectDB();
  4.  
  5. include 'strona.php'; // itd, itp..... <body>{$body}</body>
  6.  
[PHP] pobierz, plaintext




Co jest dobrą/ lepszą praktyką?


nospor
21.08.2013, 10:08:57
Z sesjo korzysta sie takze gdy user nie jest zalogowany, wiec sesja powinna sie startowac zawsze na poczatku skryptu glownego. No ale roznie ludzie robią
jackraymund
21.08.2013, 11:13:10
Ja bym nie generował za każdym razem nowej sesji.
session_id zwraca id sesji, więc jeżeli jest pusta można stworzyć nową sesje.(session_start())
Jak chcesz wrócić do starej sesji to ustawiasz session_id($_COOKIE[session_name()]);
nospor
21.08.2013, 11:21:04
Cytat
Ja bym nie generował za każdym razem nowej sesji.
On nie generuje za kazdym razem nowej sesji, tylko podczas logowania zmienia ID sesji - to akurat dobry myk
dopelganger
21.08.2013, 12:36:21
no dobra, troche pozmieniałem, ale też troche sie pogubiłem w tym wszystkim, niby klasa działa, ale ... czuje to "ale" wewnętrzny głos, który mówi mi, że nadal coś może być nie tak. Głównie chodzi mi o usunięcie "echo z klasy. Hmmm wobec tego muszę użyć metod podczas użycia klasy, czy tak? :

[PHP] pobierz, plaintext 
  1. <?php
  2.  
  3. 	$user = strip_tags(trim($_POST["user"]));
  4. 	$password = strip_tags(trim($_POST["password"]));
  5.  
  6. 	$log = new Authorization();
  7. 	$log->setUser($user);
  8.   	$log->setPassword($password);
  9.  
  10. 	if ( ($log->validateUsr()) == false ) {
  11. 	echo '<p class="errorMsg">Wprowadź login i hasło!</p>';
  12. 	} else if ( ($log->Login()) == false ) {
  13. 	echo '<p class="errorMsg">Nieprawidłowy login i/lub hasło!</p>';
  14. 	} else {
  15. 	echo 'Witaj: '.$_SESSION['login_usr'];
  16. 	}
  17. ?>
[PHP] pobierz, plaintext


a dalej kod klasy zmieniony wg. wskazówek (oprócz hashowania, to jeszcze badam teren)

[PHP] pobierz, plaintext 
  1. <?php
  2.  
  3. class Authorization {
  4.  
  5. private $_user;
  6. private $_password;
  7. private $_dbpdo;
  8.  
  9. public function __construct() {
  10. try
  11. {
  12. global $pdo;
  13. $this->_dbpdo = $pdo;
  14. }	
  15. catch(PDOException $error)
  16. {
  17. return $error->getMessage();
  18. }
  19. }
  20.  
  21. public function setUser($user) {
  22. $this->_user = $user; 
  23. }
  24.  
  25. public function setPassword($password) {
  26. $this->_password = $password; 
  27. }
  28.  
  29. public function getUser() {
  30. return $this->_user;
  31. }
  32.  
  33. public function getPassword() {
  34. return $this->_password;
  35. }
  36.  
  37. public function validateUsr() {
  38. if ( ($this->getUser())=='' || ($this->getPassword())=='' ) {
  39. return false;
  40. } else {
  41. return true;
  42. }
  43. }
  44.  
  45. public function Login() {
  46. $password = md5($this->getPassword());
  47. $sql = $this->_dbpdo->prepare("SELECT * FROM users WHERE login=:user AND password=:password LIMIT 1");
  48. $sql->bindValue(':user',$this->getUser(),PDO::PARAM_STR);
  49. $sql->bindValue(':password',$password,PDO::PARAM_STR);
  50. $sql->execute();
  51. if ($row = $sql->fetch()) {
  52. session_start();
  53. $_SESSION['id_usr'] = $row['id'];
  54. $_SESSION['login_usr'] = $row['login'];
  55. ini_set('session.cookie_httponly', 1);
  56. session_regenerate_id();
  57. return true; 
  58. } else {
  59. return false;
  60. }
  61. $sql->closeCursor();
  62. }
  63.  
  64.  
  65. }
  66.  
  67. ?>
[PHP] pobierz, plaintext
jackraymund
21.08.2013, 12:53:02 
[PHP] pobierz, plaintext 
  1. if ( !$log->validateUsr()) {
  2. 	echo '<p class="errorMsg">Wprowadź login i hasło!</p>';
  3. 	} else if ( !$log->Login()) {
  4. 	echo '<p class="errorMsg">Nieprawidłowy login i/lub hasło!</p>';
  5. 	} else {
  6. 	echo 'Witaj: '.$_SESSION['login_usr'];
  7. 	}
[PHP] pobierz, plaintext

tylko to mi się w oczy rzuciło, ! jest zaprzeczeniem np. !true == false, true != false
dopelganger
21.08.2013, 12:59:48
Cytat(nospor @ 21.08.2013, 10:26:15 ) *
3) Klasa nie powinna walic na ekran zadnych komunikatow.


dlaczego? (zrodziło mi się pytanie)


Cytat(jackraymund @ 21.08.2013, 13:53:02 ) *
[PHP] pobierz, plaintext 
  1. if ( !$log->validateUsr()) {
  2. 	echo '<p class="errorMsg">Wprowadź login i hasło!</p>';
  3. 	} else if ( !$log->Login()) {
  4. 	echo '<p class="errorMsg">Nieprawidłowy login i/lub hasło!</p>';
  5. 	} else {
  6. 	echo 'Witaj: '.$_SESSION['login_usr'];
  7. 	}
[PHP] pobierz, plaintext

tylko to mi się w oczy rzuciło, ! jest zaprzeczeniem np. !true == false, true != false


fakt, dzięki smile.gif
nospor
21.08.2013, 13:06:49
Cytat
dlaczego? (zrodziło mi się pytanie)

Bo tak (zrodzial mi sie odpowiedz) tongue.gif

Bo klasa autoryzacja to klasa autoryzacji a nie klasa odpowiedzialna za komunikaty. Za komunikaty odpowiada co innego.
Wezme sobie taka klase od ciebie i nagle jakies komunikaty ni z gruszki ni z pietruszki beda mi sie w systemie pojawiac....
dopelganger
21.08.2013, 13:08:10
Cytat(nospor @ 21.08.2013, 14:06:49 ) *
Bo tak (zrodzial mi sie odpowiedz) tongue.gif

Bo klasa autoryzacja to klasa autoryzacji a nie klasa odpowiedzialna za komunikaty. Za komunikaty odpowiada co innego.
Wezme sobie taka klase od ciebie i nagle jakies komunikaty ni z gruszki ni z pietruszki beda mi sie w systemie pojawiac....


no tak smile.gif
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.