Tijden veranderen
Vraag ivm authenticatie
classes:
authentication.class.php
current_user.class.php (static) (extends users model)
models:
users.mdl.php
Nu in de database heb ik een 20 tal acties bv (VIEW_NEWS_ITEM, DELETE_NEWS_ITEM, VIEW_ADMIN_PANEL,...)
Het idee is nu om (wanneer de gebruiker inlogd) al deze acties in een session 'rights' te steken dus tijdens het inloggen (stukje uit autentication.class.php:
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
<?php
public function login($p_oUser) {
// Check attempts
if ($this -> checkAttempts()) {
// Only active users can login
$user = $p_oUser;
$user -> password = md5($user -> password);
$user -> active = 1;
// Try to get an user with this parameters
$user -> getData();
if (!empty($user -> id)) {
// An user with this username and password exists
$_SESSION['userID'] = $user -> id;
$_SESSION['ip'] = $this -> getIP();
$_SESSION['attempts'] = 0;
$_SESSION['rights'] = HIER MOET DE SESSIE GEVULD WORDEN
return true;
}
} else {
return false;
}
}
?>
public function login($p_oUser) {
// Check attempts
if ($this -> checkAttempts()) {
// Only active users can login
$user = $p_oUser;
$user -> password = md5($user -> password);
$user -> active = 1;
// Try to get an user with this parameters
$user -> getData();
if (!empty($user -> id)) {
// An user with this username and password exists
$_SESSION['userID'] = $user -> id;
$_SESSION['ip'] = $this -> getIP();
$_SESSION['attempts'] = 0;
$_SESSION['rights'] = HIER MOET DE SESSIE GEVULD WORDEN
return true;
}
} else {
return false;
}
}
?>
Is het nu 'conform' en goed om op de plek van $_SESSION['rights'] = iets te doen als $this->setRights (waarbij ik voor de specifieke user alle rechten/acties uit de db haal en deze return als array), of moet dit toch ergens anders komen?
En moet ik wel met een sessie werken of kan ik beter dit alles in een static array steken?
Excuses voor de mogelijk onduidelijke post, ik heb geprobeerd alles zo goed mogelijk te verwoorden en uit te leggen.
Dan kom je dus op 3 losstaande onderdelen uit:
De Auth-service. Hier zou ik inderdaad een static klasse van maken (of Singleton, heeft persoonlijk mijn voorkeur) en deze, en alleen deze praat met $_SESSION['current_user']. Deze doet ook het inloggen en houdt dingen als ip en attempts bij.
Je User-model waarvan je er ook eentje hebt voor de current user. De Auth-service geeft deze terug als je hem daar opvraagt, zodat je gebruikersnaam en echte naam, emailadres en dat soort info van de huidige gebruiker kan opvragen zoals je dat ook voor alle andere gebruikers doet.
En je User-model heeft ook een 'rights' property. Als je hier een getter van maakt (een functie/method) dan kan je hem zo maken dat hij op dat moment een object aanmaakt waarmee je kan controleren of die gebruiker (waarvan je de rights opvraagt) ook een bepaalde actie mag uitvoeren.
Code (php)
1
2
3
4
5
6
7
8
9
10
11
2
3
4
5
6
7
8
9
10
11
<?php
$auth_service = AuthentificationService::sharedInstance();
$current_user = $auth_service->current_user();
$rights_of_current_user = $current_user->rights();
$rights_of_current_user->grant()
$rights_of_current_user->provoke()
?>
$auth_service = AuthentificationService::sharedInstance();
$current_user = $auth_service->current_user();
$rights_of_current_user = $current_user->rights();
$rights_of_current_user->grant()
$rights_of_current_user->provoke()
?>
Ik zou voor de rechten de database gebruiken (en voor een wat uitgebreidere site is het handig om met rollen te werken, en niet met individuele personen en rechten, misschien is het de moeite waard daar eens naar te kijken) Blijkt achteraf dat je teveel queries draait per request voor de rechten, dan kan je gaan kijken naar een cache of een andere methode. Dat moet niet lastig zijn met deze opbouw. Alleen de klasse waarvan je een instantie bij User::rights() teruggeeft hoef je te veranderen.
Ik zit echter nog met een paar vraagjes:
Jelmer schreef op 23.07.2008 22:46:
Ik zou gewoon de instantie van je User-model die voor de huidige ingelogde user geldt in een sessie stoppen , en via __sleep && __wakeup ervoor zorgen dat deze in de sessie alleen het id opslaat, en de rest uit de database haalt.
Zou je het bovenstaande nog iets verder kunnen verduidelijken ivm een simpel voorbeeldje? Stop je nu enkel de id van de actieve user in een sessie en doe je dan iets als
Code (php)
1
2
3
2
3
$user = new users();
$user -> id = $_SESSION['id'];
$user -> getData();
$user -> id = $_SESSION['id'];
$user -> getData();
Waardoor je object verder gevult wordt?
Jelmer schreef op 23.07.2008 22:46:
Je User-model waarvan je er ook eentje hebt voor de current user. De Auth-service geeft deze terug als je hem daar opvraagt, zodat je gebruikersnaam en echte naam, emailadres en dat soort info van de huidige gebruiker kan opvragen zoals je dat ook voor alle andere gebruikers doet.
Ik zat hier achteraf nog over na te denken, maar waarom een aparte model voor de current user? deze is toch eigenlijk exact hetzelfde als de gewone 'user.mdl.php'? Zeker als hij niets met sessies te maken heeft, sorry als ik het niet helemaal door heb nu ;)[/quote]
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
<?php
class User {
protected $id;
protected $name;
protected $email;
public function __sleep() {
// een lijst met namen van properties
// die opgeslagen moeten worden.
return array('id');
}
public function __wakeup() {
// Alle opgeslagen properties zijn nu al
// weer ingevuld, in ons geval is dat
// alleen User::id.
// zoals jij het doet. $this->id is nu
// dus al ingevuld.
$this->getDate();
}
}
?>
class User {
protected $id;
protected $name;
protected $email;
public function __sleep() {
// een lijst met namen van properties
// die opgeslagen moeten worden.
return array('id');
}
public function __wakeup() {
// Alle opgeslagen properties zijn nu al
// weer ingevuld, in ons geval is dat
// alleen User::id.
// zoals jij het doet. $this->id is nu
// dus al ingevuld.
$this->getDate();
}
}
?>
Je current-user is inderdaad hetzelfde als een normale user, dus daar heb je geen apart model voor nodig volgens mij. Die eerste zin had moeten zijn: "Je User-model waarvan je er ook een instantie hebt voor de current user."
Waarom prefereer je singleton over static voor de authenticatie klasse?
Ik vind static methods een beetje aanvoelen als losse functies. Een instantie voelt meer aan alsof het een bepaalde 'state' heeft, wat het geval is bij de authenticatie klasse.
Zoals het nu is:
users.mdl.php
Code (php)
dan een klasse rights.class.php (moet deze code in auth class?)
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
<?php
class rights{
private $m_aRights;
private $m_oUser;
public function __construct($p_oUser) {
$this->m_oUser = $p_oUser;
}
/**
* isAuthorized, check if the user is authorized or not
* @acces public
* @return true if authorized, false if not
*/
public function isAuthorized($p_sAction) {
$this->fill();
if(!empty($this->m_aRights)){
if (array_key_exists($p_sAction, $this->m_aRights)) {
return "ja";
} else {
return 'nee';
}
}
}
private function fill() {
// Get rights for the user given as parameter
$sSQL = 'SELECT a.title
FROM actionpermissions ap join actions a
ON ap.actionID = a.id
WHERE ap.groupID = "' . $this->m_oUser -> groupID . '"';
$results = array();
$aRows=registry::get('db')->query($sSQL);
// Make an aResult array
foreach ($aRows as $row) {
$results[] = $row;
}
if ($results) {
$numberOfResults = count($results);
for ($i=0;$i<$numberOfResults;$i++) {
$this->m_aRights[$results[$i]['title']] = true;
}
}
return $this->m_aRights;
}
}
?>
class rights{
private $m_aRights;
private $m_oUser;
public function __construct($p_oUser) {
$this->m_oUser = $p_oUser;
}
/**
* isAuthorized, check if the user is authorized or not
* @acces public
* @return true if authorized, false if not
*/
public function isAuthorized($p_sAction) {
$this->fill();
if(!empty($this->m_aRights)){
if (array_key_exists($p_sAction, $this->m_aRights)) {
return "ja";
} else {
return 'nee';
}
}
}
private function fill() {
// Get rights for the user given as parameter
$sSQL = 'SELECT a.title
FROM actionpermissions ap join actions a
ON ap.actionID = a.id
WHERE ap.groupID = "' . $this->m_oUser -> groupID . '"';
$results = array();
$aRows=registry::get('db')->query($sSQL);
// Make an aResult array
foreach ($aRows as $row) {
$results[] = $row;
}
if ($results) {
$numberOfResults = count($results);
for ($i=0;$i<$numberOfResults;$i++) {
$this->m_aRights[$results[$i]['title']] = true;
}
}
return $this->m_aRights;
}
}
?>
De rest (zoals het opvragen van de currentuser aan de auth class) is allemaal goed uitgewerkt en werkt naar mijn zin.
Execuses voor de lap code, het probleem is dus niet dat het niet werkt (want dat doet het wel) maar het probleem is meer of deze opzet goed is... of dat ik beter bv aan de auth class de current user kan opvragen.
Bedankt alvast voor alle moeite om er naar te kijken
in de functie fill maak ik dus een array als array('VIEW_ADMIN_PANEL', 'EDIT_ARTICLE',....);
Zend bijvoorbeeld heeft een soort van 'autoriteit', Zend_Acl, waarvan je 1 instantie hebt die je wss weer via je register vraagt of gebruiker a actie b mag doen. Zij hebben van de rechten dus een op zichzelf staand iets gemaakt. Voor een all-purpuse framework is dat wel een goeie aanpak, maar voor een eigen site, waarvan je zeker weet dat gebruikers rechten nodig hebben zou ik het op deze manier zoals jij het nu hebt gedaan doen.
Overigens zou ik binnen isAuthorized gewoon true of false teruggeven. Er is geen specifieke reden waarom je "ja" of "nee" teruggeeft neem ik aan?
Code (php)
Je code kan nog wat korter. Je loopt nu 3 maal door je query-results array. Een maal is toch genoeg?
Ook zou ik fill() niet een return-value laten geven. Of de state van de instantie aanpassen (properties vullen) of een resultaat teruggeven. Niet beiden tegelijk. Op die manier kan je gemakkelijk onderscheidt maken tussen destructieve en non-destructieve functies. Destructieve functies veranderen het object (en kan je vaak niet terugdraaien) en non-destructieve functies geven alleen een (berekende) waarde maar veranderen niets aan de state van het oorspronkelijke object. Het is ook weer zo'n persoonlijke voorkeur maar door de tijd heeft hij bij mij zich nuttig bevonden :)
Gewijzigd op 01/01/1970 01:00:00 door Jelmer -
Jelmer schreef op 25.07.2008 09:31:
Overigens zou ik binnen isAuthorized gewoon true of false teruggeven. Er is geen specifieke reden waarom je "ja" of "nee" teruggeeft neem ik aan?
Was meer als testje op dat moment, uiteindelijk wordt het natuurlijk geen ja, nee getrek :)
De foreach zal ik even implementeren idd, meestal vind ik tijdens het beta coden een for loop logischer (afwijking van vroeger vermoed ik)
Jelmer schreef op 25.07.2008 09:31:
Het is ook weer zo'n persoonlijke voorkeur maar door de tijd heeft hij bij mij zich nuttig bevonden :)
Dan zal ik dat maar aanhouden, scheelt mij weer een (vervelende) ervaring in de toekomst ;)
edit: bedankt voor alles! Alles werkt nu naar mijn zin!
Gewijzigd op 01/01/1970 01:00:00 door Sjoerd