[kohana] Ocena Modelu Autoryzacji Użytkowników Opcje

[k3nsei]

Chciał bym abyście ocenili i skomentowali mój model autoryzacji. Jestem właśnie na etapie jego pisania więc chciał bym się dowiedzieć co zmodyfikować, dodać i usunąć.

[PHP] pobierz, plaintext 
<?php defined('SYSPATH') or die('No direct script access.');
 
class User_Model extends Model
{
	protected $prefix;
	protected $session;
	protected $input;
 
	public function __construct()
	{
		parent::__construct();
		$this->session = new Session;
		$this->input = new Input;
		$this->prefix = config::item('database.default.table_prefix');
	}
 
	public function makeAutoLogIn()
	{
		$cookie_data = cookie::get('Authentication');
		$login = NULL;
		$cookie_key = NULL;
 
		if($cookie_data !== NULL and $this->session->get('isLogin', FALSE) === FALSE)
		{
			$data = explode('-', $cookie_data);
			$cookie_key = $this->session->id();
			$cookie_name = 'Authentication';
			$cookie_value = $data[0].'-'.$cookie_key;
			$cookie_expire = 60*60*24*30;
 
			cookie::delete($cookie_name);
			cookie::set($cookie_name, $cookie_value, $cookie_expire);
 
			$this->db->select('users.user_id, users.user_email, users.user_name, users.user_logins_count, users
.user_last_login, users_roles.role_id');
			$this->db->from('users');
			$this->db->join('users_roles', 'users_roles.user_id = users.user_id');
			$this->db->where(array('users.user_id' => $data[0], 'users.user_cookie_key' => $data[1]));
			$query = $this->db->get()->result_array();
 
			foreach($query as $row)
			{	
				$this->session->set('isLogin', TRUE);
				$this->session->set('id', $row->user_id);
				$this->session->set('login', $row->user_name);
				$this->session->set('email', $row->user_email);
				$this->session->set('lastvisit', $row->user_last_login);
				$this->session->set('role', $row->role_id);
				$this->db->from('users');
				$this->db->set(array('user_logins_count' => $row->user_logins_count+1, 'user_cookie_key' => $cookie_key, 'user_last_ip' => $this->input->ip_address(), 'user_last_login' => mktime()));
				$this->db->where(array('user_id' => $data[0], 'user_cookie_key' => $data[1]));
				$this->db->update();
			}
 
			return (bool) TRUE;
		}
		else
		{
			return (bool) FALSE;
		}
	}
 
	public function makeLogIn($login = NULL, $password = NULL, $auto = FALSE)
	{
		$cookie_key = $this->session->id();
		cookie::delete('Authentication');
		$check = $this->db->select('user_id, user_salt_begin, user_salt_end')->from('users')->where('users.user_name', $login)->get()->current();
 
		if($auto === TRUE)
		{
			$cookie_name = 'Authentication';
			$cookie_value = $check->user_id.'-'.$cookie_key;
			$cookie_expire = 60*60*24*30;
			cookie::set($cookie_name, $cookie_value, $cookie_expire);
		}
 
		if($login !== NULL and $password !== NULL and $this->session->get('isLogin', FALSE) === FALSE)
		{
			$this->db->select('users.user_id, users.user_email, users.user_name, users.user_logins_count, users
.user_last_login, users_roles.role_id');
			$this->db->from('users');
			$this->db->join('users_roles', 'users_roles.user_id = users.user_id');
			$this->db->where(array('users.user_name' => $login, 'users.user_password' => sha1($check->user_salt_begin.md5($password).$check->user_salt_end)));
			$query = $this->db->get()->result_array();
 
			foreach($query as $row)
			{
				$this->session->set('isLogin', TRUE);
				$this->session->set('id', $row->user_id);
				$this->session->set('login', $row->user_name);
				$this->session->set('email', $row->user_email);
				$this->session->set('lastvisit', $row->user_last_login);
				$this->session->set('role', $row->role_id);
				$this->db->from('users');
				$this->db->set(array('user_logins_count' => $row->user_logins_count+1, 'user_cookie_key' => $cookie_key, 'user_last_ip' => $this->input->ip_address(), 'user_last_login' => mktime()));
				$this->db->where(array('users.user_name' => $login, 'users.user_password' => sha1($check->user_salt_begin.md5($password).$check->user_salt_end)));
				$this->db->update();
			}
 
			return (bool) TRUE;
		}
		else
		{
			return (bool) FALSE;
		}
	}
 
	public function makeLogOut()
	{
		$this->session->destroy();
		cookie::delete('Authentication');
	}
 
	public function isAnonymous()
	{
		if($this->session->get('isLogin', FALSE) === FALSE)
		{
			return (bool) TRUE;
		}
		else
		{
			return (bool) FALSE;
		}
	}
 
	public function getData()
	{
		$data = array(
			'isLogin' => (bool) $this->session->get('isLogin', FALSE),
			'id' => (int) $this->session->get('id', 0),
			'login' => (string) $this->session->get('login', NULL),
			'email' => (string) $this->session->get('email', NULL),
			'lastvisit' => (string) date('d.m.Y, H:i:s', $this->session->get('lastvisit', mktime())),
			'role' => (int) $this->session->get('role', 1),
			'IP' => (string) $this->input->ip_address(),
			'Browser' => (string) Kohana::user_agent()
		);
		return (array) $data;
	}
 
	public function getId()
	{
		return (int) $this->session->get('id', 0);
	}
 
	public function getEmail()
	{
		return (string) $this->session->get('email', NULL);
	}
 
	public function getLogin()
	{
		return (string) $this->session->get('login', NULL);
	}
 
	public function getLastvisit()
	{
		return (int) $this->session->get('lastvisit', mktime());
	}
 
	public function getRole()
	{
		return (int) $this->session->get('role', 1);
	}
 
	public function getIp()
	{
		return (string) $this->input->ip_address();
	}
 
	public function getBrowser()
	{
		return (string) Kohana::user_agent();
	}
} // End User Model
?>
[PHP] pobierz, plaintext 

Ten post edytował k3nsei 26.06.2008, 11:53:56

[Cysiaczek]

var $db;

Teraz sobie poczytaj, dlaczego powstał framework Kohana i z jakiego innego projektu się wywodzi. Na stronie głównej projektu zwróć uwagę na słowa

Strict PHP 5 OOP

1. Przenoszę na forum Oceny
2. Oceniam: skończyłem czytać na słówku var

Pozdrawiam.

[.radex]

mógłbyś zamienić [ code ] na [ php ] ? Nieczytelnie jest

[k3nsei]

Teraz sobie poczytaj, dlaczego powstał framework Kohana i z jakiego innego projektu się wywodzi. Na stronie głównej projektu zwróć uwagę na słowa


1. Przenoszę na forum Oceny
2. Oceniam: skończyłem czytać na słówku var

Pozdrawiam.

Dopiero co zaczynam programować w OOP więc nie bądź tak krytyczny, ale i tak już dawno to poprawiłem.

[kwiateusz]

reczne zapytania nie po to wymyslili tam active record?

[k3nsei]

A masz w active record inner join?

Ten post edytował k3nsei 25.06.2008, 15:16:55

[kwiateusz]

ale update jest jak najmniej czystych zapytań powinno sie stosowac jak już framework na to pozwala

co nie zmienia faktu ze za 1 razem nie zauwazylem joina

[Cysiaczek]

@radex_p - nie baw się w moderatora - jest funkcja raportuj

@k3nsei - Oddałeś kod do oceny, więc zakładałemm że wiesz, co robisz
Nie podoba mi się umieszczenie w tej klasie metod makeAutoLogIn(), czy makeLogin() - to powinna robić akcja. Jeśli jednak masz to już tutaj, w modelu, to rozważyłbym przeniesienie metod dostępowych, które raczej są profilem użytkownika do klasy niżej. IMO, najlepiej osobna klasa do logowania. np.

[PHP] pobierz, plaintext 
<?php
class UserControll
{
  function __construct(User_Model $user)
  {
	$this->user=$user;
  }
 
   function makeLogIn(){}
   function makeAutoLogIn(){}
}
?>
[PHP] pobierz, plaintext 

I już mamy większą elastyczność. No, w sumie, jeśli to miałby być komponent wielokrotnego użytku, to właśnie tak bym zrobił

Pozdrawiam.

[k3nsei]

Właściwie to od logowania i wylogowania mam controller.

[bełdzio]

tylko ja w 50 lini widze SQLi ?

A masz w active record inner join?

hmm join( )

[k3nsei]

Wtedy się sypie cały kod. Chyba że sam napiszesz mi active records do makeLogIn i makeAutoLogIn.
Macie jakieś pomysły na dodatkowe funkcję czy zastrzeżenia do obecnych oprócz logowania i autologowania?

Ten post edytował k3nsei 25.06.2008, 16:24:34

[bełdzio]

sypie? ja u siebie korzystam z join( ) i wsio działa jak należy

[k3nsei]

Zresztą quering też jest poprawny i o wiele szybciej mi się pisze i edytuje takie zapytania.

http://k3nsei.cal.pl/news/page/1.html

Login: Demo
Pass: Demo

możesz sobie przetestować.

Dodałem nową poprawioną wersję. Zapraszam do komentowania.

[k3nsei]

Oto mój nowy model autoryzacji użytkowników. Chciał bym abyście go skomentowali, co wam się nie podoba, co byście dodali i co byście zmienili. Jestem otwarty na wasze komentarze, dzięki nim mogę się wiele nauczyć i zrobić lepszą aplikacje.

http://phpfi.com/327959

Ten post edytował k3nsei 29.06.2008, 19:52:01

[Vengeance]

Pierwsze co rzuca mi się w oczy do bezsensowne komentarze w stylu

[PHP] pobierz, plaintext 
<?php
return (bool) TRUE; // Return result
					}
				}
				else
				{
					return (bool) FALSE; // Return result
				}
			}
			else
			{
				return (bool) FALSE; // Return result
			}
		}
		else
		{
			return (bool) FALSE; // Return result
?>
[PHP] pobierz, plaintext 

Poza tym, metoda AddUser czy makeActive bardziej pasuje mi ogólnie do modelu bądź kontrolera użytkownika niż autoryzacji
Kolejna rzecz, czy te metody getEmail, getUserName itd nie łatwiej byłoby zastąpić po prostu np. poprzez pobranie instancji obiektu aktualnie zalogowanego użytkownika, a potem wszystko już z tego obiektu. Czyli zamiast
$auth->getEmail()
to
$auth->getUser()->getEmail()

tu wydaje się bardziej rozbudowane... ale tak nie jest, bo w przypadku dodawania kolejnych rzeczy musisz do klasy Auth dopisywać kolejne metody.

Jak dla mnie Auth powinno odpowiadać za: logowanie, wylogowanie, zalogowanie automatyczne, pobranie obiektu zalogowanego użytkownika, sprawdzanie uprawnień. finito.

Pozdrawiam

[Cysiaczek]

@autor - nie zakładamy nowych topików dla każdej kolejnej wersji. - połączyłem

Pozdrawiam

[k3nsei]

Vengeance przecież jest getData(). Ale nie zawsze trzeba pobierać całość, dlatego są też takie małe funkcje.

[Vengeance]

Chodzi mi oto, że takie rzeczy nie koniecznie muszą być w sesji (przynajmniej ja tak nie robie).

[jarek_bolo]

Witam, odświerzam trochę stary temat, bo chciałem zweryfikować czy dobrze zorumiałem recenzję oraz proponowane poprawki Venganca i Cysiaczka.

Jeśli dobrze was zrozumiałem to klasa Auth powinna być swego rodzaju zewnętrzną biblioteką (ani kontrolerem, ani modelem), którą używało by się w kontrolerach gdzie dostęp musi być autoryzowany.

I teraz ta klasa-zewnętrzna biblioteka Auth powinna mieć dostęp do Kohanowego "input'a" czyli (session, get, post, cookie) oraz modelu Users.
Wykorzystując Model oraz dane z input'a wykonuje odpowiednio, login, autologin, sprawdzenie uprawnień, logout.
Efekty swoich operacji powinna zwrócić do kontrolera aby ten dalej zdecydował co należy zrobić, jaki widok wyświetlić, itd.

I teraz chciałem się upewnić co do użycia takiej biblioteki.
Czy dobrze myślę patrząc w kontekście całej aplikacji, że taki moduł Auth jeśli mowa o ogólno dostępnej stronie powinien być używany w konkretnych, zamkniętych dla ogółu częściach (kontrolerach) aplikacji? Natomiast jeśli mamy na myśli ogólnie panel administratora to taki moduł Auth powinien być wpisany w konstruktor bazowego kontrolera, po którym dziedziczą wszystkie kontrolery panelu administracyjnego.

Czy dobrze sobie to wszystko wyobrażam??