From 6c898e7c946d696a306f4254e2206f31a514ce61 Mon Sep 17 00:00:00 2001
From: llaffont <laurent.laffont@gmail.com>
Date: Tue, 4 Feb 2014 18:01:11 +0100
Subject: [PATCH] - refacto CAS validation to avoid session collision -
reification of CAS Ticket
---
.../opac/controllers/AuthController.php | 5 +-
.../opac/controllers/CasServerController.php | 33 +++++--------
library/Class/CasTicket.php | 47 +++++++++++++++++++
library/Class/Systeme/ModulesMenu/MusicMe.php | 2 +-
library/ZendAfi/Auth.php | 20 +++++++-
library/ZendAfi/Auth/Storage/Session.php | 14 ++----
.../opac/controllers/AuthControllerTest.php | 24 +++++-----
.../controllers/CasServerControllerTest.php | 16 ++++---
tests/library/ZendAfi/AuthTest.php | 9 ++--
9 files changed, 109 insertions(+), 61 deletions(-)
create mode 100644 library/Class/CasTicket.php
diff --git a/application/modules/opac/controllers/AuthController.php b/application/modules/opac/controllers/AuthController.php
index 33b842bef20..f0553189462 100644
--- a/application/modules/opac/controllers/AuthController.php
+++ b/application/modules/opac/controllers/AuthController.php
@@ -59,7 +59,6 @@ class AuthController extends ZendAfi_Controller_Action {
public function _authenticate() {
- // collect the data from the user
$f = new Zend_Filter_StripTags();
$username = $f->filter($this->_request->getPost('username'));
$password = $f->filter($this->_request->getPost('password'));
@@ -70,13 +69,11 @@ class AuthController extends ZendAfi_Controller_Action {
if (empty($password))
return $this->view->_('Entrez votre mot de passe S.V.P.');
- // do the authentication
if (!ZendAfi_Auth::getInstance()->authenticateLoginPassword($username, $password))
return $this->view->_('Identifiant ou mot de passe incorrect.');
$user = Class_Users::getIdentity();
$this->_helper->trackEvent('authentification', 'connexion', 'utilisateur', $user->getId());
-
}
@@ -347,7 +344,7 @@ class Auth_Strategy_Cas_Abstract extends Auth_Strategy_Abstract{
if ($url_musicme=$this->redirectMusicMe())
return $url_musicme;
- $ticket=md5(Zend_Session::getId());
+ $ticket = (new Class_CasTicket())->getTicketForCurrentUser();
$queries=[];
$url_cas=array_merge(['query'=> '',
'path' => ''],parse_url($this->controller->getCasServerUrl()));
diff --git a/application/modules/opac/controllers/CasServerController.php b/application/modules/opac/controllers/CasServerController.php
index c0f3c432b8a..2fd3938e182 100644
--- a/application/modules/opac/controllers/CasServerController.php
+++ b/application/modules/opac/controllers/CasServerController.php
@@ -32,11 +32,11 @@ class CasServerController extends Zend_Controller_Action {
- public function returnValidTicketResponse($username,$ticket) {
+ public function returnValidTicketResponse($user, $ticket) {
$this->getResponse()->setHeader('Content-Type', 'application/xml;charset=utf-8');
$this->getResponse()->setBody("<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'>
<cas:authenticationSuccess>
- <cas:user>".$username."</cas:user>
+ <cas:user>".$user->getId()."</cas:user>
<cas:proxyGrantingTicket>".$ticket."
</cas:proxyGrantingTicket>
</cas:authenticationSuccess>
@@ -45,13 +45,12 @@ class CasServerController extends Zend_Controller_Action {
- public function returnValidMusicMeResponse($session) {
-
+ public function returnValidMusicMeResponse($user) {
$this->getResponse()->setHeader('Content-Type', 'application/xml;charset=utf-8');
$this->getResponse()->setBody( '<User>
-<ID>'.$session->ID_USER.'</ID>
-<FirstName>'.(isset($session->PRENOM) ? $session->PRENOM:'').'</FirstName>
-<LastName>'.(isset($session->NOM) ? $session->NOM:'').'</LastName>
+<ID>'.$user->getId().'</ID>
+<FirstName>'.$user->getPrenom().'</FirstName>
+<LastName>'.$user->getNom().'</LastName>
<AccountDisabled>false</AccountDisabled>
<AccountExpired>false</AccountExpired>
</User>'
@@ -65,6 +64,8 @@ class CasServerController extends Zend_Controller_Action {
$xml="<User />";
$this->getResponse()->setBody($xml);
}
+
+
public function returnFailureTicketResponse($error,$ticket=null) {
$this->getResponse()->setHeader('Content-Type', 'application/xml;charset=utf-8');
$xml='<cas:serviceResponse xmlns:cas="http://www.yale.edu/tp/cas">';
@@ -93,13 +94,9 @@ class CasServerController extends Zend_Controller_Action {
return $this->returnMusicMeFailureTicketResponse('INVALID_REQUEST');
}
- $serialized_session=Zend_Registry::get('cache')->load($ticket);
- if ($serialized_session) {
- $session = unserialize($serialized_session);
- if ($session)
- return $this->returnValidMusicMeResponse($session);
+ if ($user = (new Class_CasTicket())->userForTicket($ticket))
+ return $this->returnValidMusicMeResponse($user);
- }
return $this->returnMusicMeFailureTicketResponse('INVALID_TICKET',$ticket);
}
@@ -115,16 +112,10 @@ class CasServerController extends Zend_Controller_Action {
return $this->returnFailureTicketResponse('INVALID_REQUEST');
}
- $serialized_session=Zend_Registry::get('cache')->load($ticket);
- if ($serialized_session) {
- $session = unserialize($serialized_session);
- $userid=$session->ID_USER;
- return $this->returnValidTicketResponse($userid,$ticket);
+ if ($user = (new Class_CasTicket())->userForTicket($ticket))
+ return $this->returnValidTicketResponse($user, $ticket);
- }
return $this->returnFailureTicketResponse('INVALID_TICKET',$ticket);
-
-
}
/* Cas server methods not used for now :
diff --git a/library/Class/CasTicket.php b/library/Class/CasTicket.php
new file mode 100644
index 00000000000..47f68c4e696
--- /dev/null
+++ b/library/Class/CasTicket.php
@@ -0,0 +1,47 @@
+<?php
+/**
+ * Copyright (c) 2012, Agence Française Informatique (AFI). All rights reserved.
+ *
+ * AFI-OPAC 2.0 is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE as published by
+ * the Free Software Foundation.
+ *
+ * There are special exceptions to the terms and conditions of the AGPL as it
+ * is applied to this software (see README file).
+ *
+ * AFI-OPAC 2.0 is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+ *
+ * You should have received a copy of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * along with AFI-OPAC 2.0; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+class Class_CasTicket {
+ public function getTicketForCurrentUser() {
+ if ($user = Class_Users::getIdentity())
+ return md5(Zend_Session::getId() . $user->getId());
+ return null;
+ }
+
+
+ public function save() {
+ if ($user = Class_Users::getIdentity())
+ Zend_Registry::get('cache')->save((string)$user->getId(),
+ $this->getTicketForCurrentUser());
+ }
+
+
+ public function clear() {
+ if ($ticket = $this->getTicketForCurrentUser())
+ Zend_Registry::get('cache')->remove($ticket);
+ }
+
+ public function userForTicket($ticket) {
+ if ($id = (int)Zend_Registry::get('cache')->load($ticket))
+ return Class_Users::find($id);
+ return null;
+ }
+}
+?>
\ No newline at end of file
diff --git a/library/Class/Systeme/ModulesMenu/MusicMe.php b/library/Class/Systeme/ModulesMenu/MusicMe.php
index 80ae217e1a8..cdeeb4e7096 100644
--- a/library/Class/Systeme/ModulesMenu/MusicMe.php
+++ b/library/Class/Systeme/ModulesMenu/MusicMe.php
@@ -50,7 +50,7 @@ class Class_Systeme_ModulesMenu_MusicMe extends Class_Systeme_ModulesMenu_Null {
public function getDynamiqueUrl($docid=null) {
$user = Class_Users::getIdentity();
if (!$user) {
- $musicme_link=new Class_MusicMeLink();
+ $musicme_link=new Class_MusicMeLink(null);
return $musicme_link->baseUrl();
}
$url=$this->getMusicMeUrlForUser($user);
diff --git a/library/ZendAfi/Auth.php b/library/ZendAfi/Auth.php
index ff0290b4eac..c17f0258830 100644
--- a/library/ZendAfi/Auth.php
+++ b/library/ZendAfi/Auth.php
@@ -72,13 +72,31 @@ class ZendAfi_Auth extends Zend_Auth {
if (!$this->authenticate($authAdapter)->isValid()) continue;
}
- $this->getStorage()->write($authAdapter->getResultObject());
return true;
}
return false;
}
+
+ /**
+ * Authenticates against the supplied adapter
+ *
+ * @param Zend_Auth_Adapter_Interface $adapter
+ * @return Zend_Auth_Result
+ */
+ public function authenticate(Zend_Auth_Adapter_Interface $adapter) {
+ $result = $adapter->authenticate();
+
+ if ($result->isValid()) {
+ $this->getStorage()->write($adapter->getResultObject());
+ }
+
+ return $result;
+ }
+
+
+
public function md5_base64 ( $data ) {
return preg_replace('/=+$/','',base64_encode(pack('H*',md5($data))));
}
diff --git a/library/ZendAfi/Auth/Storage/Session.php b/library/ZendAfi/Auth/Storage/Session.php
index 796a1e24154..3c6c8b8ae7c 100644
--- a/library/ZendAfi/Auth/Storage/Session.php
+++ b/library/ZendAfi/Auth/Storage/Session.php
@@ -19,23 +19,15 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
class ZendAfi_Auth_Storage_Session extends Zend_Auth_Storage_Session {
-
public function write($contents) {
parent::write($contents);
- Zend_Registry::get('cache')->save(serialize($contents),$this->getCacheKey());
+ (new Class_CasTicket())->save();
}
- public function getCacheKey() {
- return md5(Zend_Session::getId());
- }
-
-
- public function clear()
- {
+ public function clear() {
+ (new Class_CasTicket())->clear();
parent::clear();
- Zend_Registry::get('cache')->remove($this->getCacheKey());
}
-
}
?>
\ No newline at end of file
diff --git a/tests/application/modules/opac/controllers/AuthControllerTest.php b/tests/application/modules/opac/controllers/AuthControllerTest.php
index 9d558a11886..7a7f0591de9 100644
--- a/tests/application/modules/opac/controllers/AuthControllerTest.php
+++ b/tests/application/modules/opac/controllers/AuthControllerTest.php
@@ -195,18 +195,17 @@ class AuthControllerWithProfilPageAbonneSIGBLoggedLogoutTest extends PortailWith
abstract class AuthControllerNobodyLoggedTestCase extends PortailWithOneLoginModuleTestCase {
- protected function _loginHook($account) {
- $account->ROLE = "";
- $account->ROLE_LEVEL = 0;
- $account->ID_USER = "";
- $account->PSEUDO = "";
- }
+ public function setUp() {
+ parent::setUp();
+ ZendAfi_Auth::getInstance()->clearIdentity();
+ }
}
class AuthControllerNobodyLoggedActivateTest extends AuthControllerNobodyLoggedTestCase {
const ERROR_MESSAGE = 'Un probleme est survenu lors de l\'activation';
const OK_MESSAGE = 'Ok, compte cree';
+
public function setUp() {
parent::setUp();
Class_UsersNonValid::beVolatile();
@@ -589,7 +588,6 @@ class AuthControllerPostTest extends AuthControllerNobodyLoggedTestCase {
public function loggingWithCorrectInformation() {
$user = Class_Users::getLoader()->findFirstBy(array());
-
$this->postDispatch('/opac/auth/boite-login?id_module=4',
array('username' => $user->getLogin(),
'password' => $user->getPassword()));
@@ -613,7 +611,7 @@ class AuthControllerPostTest extends AuthControllerNobodyLoggedTestCase {
public function ajaxLoggingWithCorrectInformation() {
$user = Class_Users::getLoader()->findFirstBy(array());
-
+
$this->postDispatch('/opac/auth/ajax-login?id_module=4',
array('username' => $user->getLogin(),
'password' => $user->getPassword()));
@@ -711,9 +709,10 @@ abstract class AuthControllerPostSimpleSuccessfulTestCase extends AuthController
public function setUp() {
parent::setUp();
$this->group_musicme = Class_UserGroup::newInstanceWithId('20', ['libelle' => 'Multimedia',
- 'rights_token' => Class_UserGroup::RIGHT_ACCES_MUSICME]);
+ 'rights_token' => Class_UserGroup::RIGHT_ACCES_MUSICME]);
- $marcel = Class_Users::newInstanceWithId(2, ['nom' => 'Marcel','login' =>'foo'])->beAbonneSIGB()
+ $marcel = Class_Users::newInstanceWithId(2, ['nom' => 'Marcel','login' =>'foo'])
+ ->beAbonneSIGB()
->setUserGroups([$this->group_musicme]);
$this->_auth
@@ -797,7 +796,7 @@ class AuthControllerPostSuccessfulFromCASClientTest extends AuthControllerPostSi
/** @test */
public function responseShouldRedirectToCasClientWithTicket() {
- $this->assertRedirectTo('http://www.numilog.com/view?book=bilbo&ticket='.md5(Zend_Session::getId()));
+ $this->assertRedirectTo('http://www.numilog.com/view?book=bilbo&ticket='.md5(Zend_Session::getId().'2'));
}
/** @test */
@@ -836,14 +835,13 @@ class AuthControllerFromCASClientUserConnectedTest extends AuthControllerNobodyL
public function setUp() {
parent::setUp();
ZendAfi_Auth::getInstance()->logUser(Class_Users::newInstanceWithId('22',['login'=> 'john']));
-
$this->dispatch('/auth/login?service=http://numilog.com/actionredirected');
}
/** @test */
public function responseShouldRedirectToCasClientServiceWithTicket() {
- $this->assertRedirectTo('http://numilog.com/actionredirected?ticket='.md5(Zend_Session::getId()));
+ $this->assertRedirectTo('http://numilog.com/actionredirected?ticket='.md5(Zend_Session::getId().'22'));
}
/** @test */
diff --git a/tests/application/modules/opac/controllers/CasServerControllerTest.php b/tests/application/modules/opac/controllers/CasServerControllerTest.php
index 84a123a38af..a2d141efe3c 100644
--- a/tests/application/modules/opac/controllers/CasServerControllerTest.php
+++ b/tests/application/modules/opac/controllers/CasServerControllerTest.php
@@ -29,7 +29,9 @@ class CasServerControllerValidateActionTest extends AbstractControllerTestCase {
parent::setUp();
$user = new StdClass();
$user->ID_USER=300;
- Zend_Registry::get('cache')->save(serialize($user),md5(Zend_Session::getId()));
+ Class_Users::newInstanceWithId(300);
+ Zend_Registry::get('cache')->save('300',
+ md5(Zend_Session::getId().'300'));
}
@@ -50,7 +52,7 @@ class CasServerControllerValidateActionTest extends AbstractControllerTestCase {
/** @test */
public function requestWithValidTicketShouldRespondValidXML() {
- $this->dispatch('/opac/cas-server/validate?ticket='.md5(Zend_Session::getId()).'&service=http://test.com');
+ $this->dispatch('/opac/cas-server/validate?ticket='.md5(Zend_Session::getId().'300').'&service=http://test.com');
$this->assertContains('<cas:user>300</cas:user>',$this->_response->getBody());
$this->assertContains('<cas:proxyGrantingTicket>',$this->_response->getBody());
}
@@ -68,7 +70,10 @@ class CasServerControllerMusicMeValidateActionTest extends AbstractControllerTes
$user->ID_USER=300;
$user->PRENOM='Tom';
$user->NOM = 'Ate';
- Zend_Registry::get('cache')->save(serialize($user),md5(Zend_Session::getId()));
+ Class_Users::newInstanceWithId(300,
+ ['nom' => 'Ate',
+ 'prenom' => 'Tom']);
+ Zend_Registry::get('cache')->save('300', md5(Zend_Session::getId().'300'));
}
@@ -87,9 +92,8 @@ class CasServerControllerMusicMeValidateActionTest extends AbstractControllerTes
/** @test */
- public function requestMusicMeWithValidTicketShouldRespondValidXML() {
-
- $this->dispatch('/opac/cas-server/validate-musicme?ticket='.md5(Zend_Session::getId()).'&MediaLibraryID=http://test.com');
+ public function requestMusicMeWithValidTicketShouldRespondValidXML() {
+ $this->dispatch('/opac/cas-server/validate-musicme?ticket='.md5(Zend_Session::getId().'300').'&MediaLibraryID=http://test.com');
$this->assertContains('<ID>300</ID>',$this->_response->getBody());
}
diff --git a/tests/library/ZendAfi/AuthTest.php b/tests/library/ZendAfi/AuthTest.php
index 6ea76d24c4b..c9d66468f02 100644
--- a/tests/library/ZendAfi/AuthTest.php
+++ b/tests/library/ZendAfi/AuthTest.php
@@ -60,9 +60,8 @@ class AuthSessionNamespaceTest extends PHPUnit_Framework_TestCase {
/** @test */
- public function validAuthenticationShouldBeStoredInCache() {
- $cached_auth = unserialize($this->cache_mock->getFirstAttributeForLastCallOn('save'));
- $this->assertEquals('sysadm', $cached_auth->LOGIN);
+ public function validAuthenticationUserIdShouldBeStoredInCache() {
+ $this->assertEquals(2, $this->cache_mock->getFirstAttributeForLastCallOn('save'));
}
@@ -95,7 +94,7 @@ class Mock_ZendAfi_Auth_MD5_Adapter implements Zend_Auth_Adapter_Interface {
public function authenticate() {
if ($this->_credential == 'M9h/02RRb2YEEk/Mdv3SeQ')
- return new Zend_Auth_Result(Zend_Auth_Result::SUCCESS, $this->_identity);
+ return new Zend_Auth_Result(Zend_Auth_Result::SUCCESS, $this->getResultObject());
return new Zend_Auth_Result(Zend_Auth_Result::FAILURE_IDENTITY_NOT_FOUND, $this->_identity);
}
@@ -105,7 +104,9 @@ class Mock_ZendAfi_Auth_MD5_Adapter implements Zend_Auth_Adapter_Interface {
public function getResultObject() {
$object = new StdClass;
$object->LOGIN = $this->_identity;
+ $object->ID_USER = 2;
return $object;
}
}
+?>
\ No newline at end of file
--
GitLab