Correcting behavior of layering allow/deny calls in AuthComponent, fixes #5595, formatting fixes for RequestHandler

git-svn-id: https://svn.cakephp.org/repo/branches/1.2.x.x@7743 3807eeeb-6ff5-0310-8944-8be069107fe0

cake/libs/controller/components/auth.php

@@ -216,6 +216,13 @@ class AuthComponent extends Object {
  * @access public
  */
 	var $params = array();
+/**
+ * Method list for bound controller
+ *
+ * @var array
+ * @access protected
+ */
+	var $_methods = array();
 /**
  * Initializes AuthComponent for use in the controller
  *
@@ -227,6 +234,7 @@ class AuthComponent extends Object {
 		$this->params = $controller->params;
 		$crud = array('create', 'read', 'update', 'delete');
 		$this->actionMap = array_merge($this->actionMap, array_combine($crud, $crud));
+		$this->_methods = get_class_methods($controller);
 
 		$admin = Configure::read('Routing.admin');
 		if (!empty($admin)) {
@@ -256,17 +264,20 @@ class AuthComponent extends Object {
  * @access public
  */
 	function startup(&$controller) {
-		if (strtolower($controller->name) == 'app' || (strtolower($controller->name) == 'tests' && Configure::read() > 0)) {
+		$isBaseOrTests = (
-			return;
+			strtolower($controller->name) == 'app' ||
+			(strtolower($controller->name) == 'tests' && Configure::read() > 0)
+		);
+		if ($isBaseOrTests) {
+			return true;
 		}
-
 		if (!$this->__setDefaults()) {
 			return false;
 		}
 
 		$this->data = $controller->data = $this->hashPasswords($controller->data);
-
 		$url = '';
+
 		if (is_array($this->loginAction)) {
 			$params = $controller->params;
 			$keys = array('pass', 'named', 'controller', 'action', 'plugin');
@@ -294,7 +305,7 @@ class AuthComponent extends Object {
 		);
 
 		if ($loginAction != $url && $isAllowed) {
-			return false;
+			return true;
 		}
 
 		if ($loginAction == $url) {
@@ -354,14 +365,25 @@ class AuthComponent extends Object {
 				if (isset($controller->Acl)) {
 					$this->Acl =& $controller->Acl;
 				} else {
-					trigger_error(__('Could not find AclComponent. Please include Acl in Controller::$components.', true), E_USER_WARNING);
+					$err = 'Could not find AclComponent. Please include Acl in ';
+					$err .= 'Controller::$components.';
+					trigger_error(__($err, true), E_USER_WARNING);
 				}
 			break;
 			case 'model':
 				if (!isset($object)) {
-					if (isset($controller->{$controller->modelClass}) && is_object($controller->{$controller->modelClass})) {
+					$hasModel = (
+						isset($controller->{$controller->modelClass}) &&
+						is_object($controller->{$controller->modelClass})
+					);
+					$isUses = (
+						!empty($controller->uses) && isset($controller->{$controller->uses[0]}) &&
+						is_object($controller->{$controller->uses[0]})
+					);
+
+					if ($hasModel) {
 						$object = $controller->modelClass;
-					} elseif (!empty($controller->uses) && isset($controller->{$controller->uses[0]}) && is_object($controller->{$controller->uses[0]})) {
+					} elseif ($isUses) {
 						$object = $controller->uses[0];
 					}
 				}
@@ -408,15 +430,19 @@ class AuthComponent extends Object {
 		return true;
 	}
 /**
- * Determines whether the given user is authorized to perform an action.  The type of authorization
+ * Determines whether the given user is authorized to perform an action.  The type of
- * used is based on the value of AuthComponent::$authorize or the passed $type param.
+ * authorization used is based on the value of AuthComponent::$authorize or the
+ * passed $type param.
  *
  * Types:
- * 'controller' will validate against Controller::isAuthorized() if controller instance is passed in $object
+ * 'controller' will validate against Controller::isAuthorized() if controller instance is
+ * 				passed in $object
  * 'actions' will validate Controller::action against an AclComponent::check()
  * 'crud' will validate mapActions against an AclComponent::check()
- * array('model'=> 'name'); will validate mapActions against model $name::isAuthorize(user, controller, mapAction)
+ * 		array('model'=> 'name'); will validate mapActions against model
- * 'object' will validate Controller::action against object::isAuthorized(user, controller, action)
+ * 		$name::isAuthorize(user, controller, mapAction)
+ * 'object' will validate Controller::action against
+ * 		object::isAuthorized(user, controller, action)
  *
  * @param string $type Type of authorization
  * @param mixed $object object, model object, or model name
@@ -448,9 +474,18 @@ class AuthComponent extends Object {
 			case 'crud':
 				$this->mapActions();
 				if (!isset($this->actionMap[$this->params['action']])) {
-					trigger_error(sprintf(__('Auth::startup() - Attempted access of un-mapped action "%1$s" in controller "%2$s"', true), $this->params['action'], $this->params['controller']), E_USER_WARNING);
+					$err = 'Auth::startup() - Attempted access of un-mapped action "%1$s" in';
+					$err .= ' controller "%2$s"';
+					trigger_error(
+						sprintf(__($err, true), $this->params['action'], $this->params['controller']),
+						E_USER_WARNING
+					);
 				} else {
-					$valid = $this->Acl->check($user, $this->action(':controller'), $this->actionMap[$this->params['action']]);
+					$valid = $this->Acl->check(
+						$user,
+						$this->action(':controller'),
+						$this->actionMap[$this->params['action']]
+					);
 				}
 			break;
 			case 'model':
@@ -519,8 +554,8 @@ class AuthComponent extends Object {
  */
 	function allow() {
 		$args = func_get_args();
-		if (empty($args)) {
+		if (empty($args) || $args == array('*')) {
-			$this->allowedActions = array('*');
+			$this->allowedActions = $this->_methods;
 		} else {
 			if (isset($args[0]) && is_array($args[0])) {
 				$args = $args[0];

cake/libs/controller/components/request_handler.php

@@ -3,9 +3,9 @@
 /**
  * Request object for handling alternative HTTP requests
  *
- * Alternative HTTP requests can come from wireless units like mobile phones, palmtop computers, and the like.
+ * Alternative HTTP requests can come from wireless units like mobile phones, palmtop computers,
- * These units have no use for Ajax requests, and this Component can tell how Cake should respond to the different
+ * and the like.  These units have no use for Ajax requests, and this Component can tell how Cake
- * needs of a handheld computer and a desktop machine.
+ * should respond to the different needs of a handheld computer and a desktop machine.
  *
  * CakePHP(tm) :  Rapid Development Framework <http://www.cakephp.org/>
  * Copyright 2005-2008, Cake Software Foundation, Inc.
@@ -94,7 +94,11 @@ class RequestHandlerComponent extends Object {
 		'rss'			=> 'application/rss+xml',
 		'atom'			=> 'application/atom+xml',
 		'amf'			=> 'application/x-amf',
-		'wap'			=> array('text/vnd.wap.wml', 'text/vnd.wap.wmlscript', 'image/vnd.wap.wbmp'),
+		'wap'			=> array(
+			'text/vnd.wap.wml',
+			'text/vnd.wap.wmlscript',
+			'image/vnd.wap.wbmp'
+		),
 		'wml'			=> 'text/vnd.wap.wml',
 		'wmlscript'		=> 'text/vnd.wap.wmlscript',
 		'wbmp'			=> 'image/vnd.wap.wbmp',
@@ -188,10 +192,15 @@ class RequestHandlerComponent extends Object {
 		if (!$this->enabled) {
 			return;
 		}
+
 		$this->__initializeTypes();
 		$controller->params['isAjax'] = $this->isAjax();
+		$isRecognized = (
+			!in_array($this->ext, array('html', 'htm')) &&
+			in_array($this->ext, array_keys($this->__requestContent))
+		);
 
-		if (!empty($this->ext) && !in_array($this->ext, array('html', 'htm')) && in_array($this->ext, array_keys($this->__requestContent))) {
+		if (!empty($this->ext) && $isRecognized) {
 			$this->renderAs($controller, $this->ext);
 		} elseif ($this->isAjax()) {
 			$this->renderAs($controller, 'ajax');
@@ -202,6 +211,7 @@ class RequestHandlerComponent extends Object {
 				App::import('Core', 'Xml');
 			}
 			$xml = new Xml(trim(file_get_contents('php://input')));
+
 			if (is_object($xml->child('data')) && count($xml->children) == 1) {
 				$controller->data = $xml->child('data');
 			} else {
@@ -572,7 +582,8 @@ class RequestHandlerComponent extends Object {
 		if (empty($this->__renderType)) {
 			$controller->viewPath .= '/' . $type;
 		} else {
-			$controller->viewPath = preg_replace("/(?:\/{$type})$/", '/' . $type, $controller->viewPath);
+			$remove = preg_replace("/(?:\/{$type})$/", '/' . $type, $controller->viewPath);
+			$controller->viewPath = $remove;
 		}
 		$this->__renderType = $type;
 		$controller->layoutPath = $type;
@@ -582,18 +593,23 @@ class RequestHandlerComponent extends Object {
 		}
 
 		$helper = ucfirst($type);
-		if (!in_array($helper, $controller->helpers) && !array_key_exists($helper, $controller->helpers)) {
+		$isAdded = (
+			in_array($helper, $controller->helpers) ||
+			array_key_exists($helper, $controller->helpers)
+		);
+
+		if (!$isAdded) {
 			if (App::import('Helper', $helper)) {
 				$controller->helpers[] = $helper;
 			}
 		}
 	}
 /**
- * Sets the response header based on type map index name.  If DEBUG is greater
+ * Sets the response header based on type map index name.  If DEBUG is greater than 2, the header
- * than 2, the header is not set.
+ * is not set.
  *
- * @param mixed $type Friendly type name, i.e. 'html' or 'xml', or a full
+ * @param mixed $type Friendly type name, i.e. 'html' or 'xml', or a full content-type,
- *                    content-type, like 'application/x-shockwave'.
+ * 					  like 'application/x-shockwave'.
  * @param array $options If $type is a friendly type name that is associated with
  *                     more than one type of content, $index is used to select
  *                     which content-type to use.
@@ -611,7 +627,8 @@ class RequestHandlerComponent extends Object {
 		if (!array_key_exists($type, $this->__requestContent) && strpos($type, '/') === false) {
 			return false;
 		}
-		$options = array_merge(array('index' => 0, 'charset' => null, 'attachment' => false), $options);
+		$defaults = array('index' => 0, 'charset' => null, 'attachment' => false);
+		$options = array_merge($defaults, $options);
 
 		if (strpos($type, '/') === false && isset($this->__requestContent[$type])) {
 			$cType = null;
@@ -624,6 +641,7 @@ class RequestHandlerComponent extends Object {
 			} else {
 				return false;
 			}
+
 			if (is_array($cType)) {
 				if ($this->prefers($cType)) {
 					$cType = $this->prefers($cType);
@@ -642,7 +660,7 @@ class RequestHandlerComponent extends Object {
 				$header .= '; charset=' . $options['charset'];
 			}
 			if (!empty($options['attachment'])) {
-				header('Content-Disposition: attachment; filename="' . $options['attachment'] . '"');
+				header("Content-Disposition: attachment; filename=\"{$options['attachment']}\"");
 			}
 			if (Configure::read() < 2 && !defined('CAKEPHP_SHELL')) {
 				@header($header);
@@ -650,7 +668,6 @@ class RequestHandlerComponent extends Object {
 			$this->__responseTypeSet = $cType;
 			return true;
 		}
-
 		return false;
 	}
 /**

cake/tests/cases/libs/controller/components/auth.test.php

@@ -294,6 +294,20 @@ class AuthTestController extends Controller {
 		}
 		return true;
 	}
+/**
+ * Mock delete method
+ *
+ * @param mixed $url
+ * @param mixed $status
+ * @param mixed $exit
+ * @access public
+ * @return void
+ */
+	function delete($id = null) {
+		if ($this->TestAuth->testStop !== true && $id !== null) {
+			echo 'Deleted Record: ' . var_export($id, true);
+		}
+	}
 }
 /**
  * AjaxAuthController class
@@ -556,7 +570,6 @@ class AuthTest extends CakeTestCase {
 		$this->assertTrue($this->Controller->Session->check('Message.auth'));
 		$result = $this->Controller->Auth->isAuthorized();
 		$this->assertFalse($result);
-
 	}
 
 /**
@@ -576,19 +589,21 @@ class AuthTest extends CakeTestCase {
 		$this->Controller->Acl->name = 'DbAclTest';
 
 		$this->Controller->Acl->Aro->id = null;
-		$this->Controller->Acl->Aro->create(array('alias'=>'Roles'));
+		$this->Controller->Acl->Aro->create(array('alias' => 'Roles'));
 		$result = $this->Controller->Acl->Aro->save();
 		$this->assertTrue($result);
 
 		$parent = $this->Controller->Acl->Aro->id;
 
-		$this->Controller->Acl->Aro->create(array('parent_id'=> $parent, 'alias'=>'Admin'));
+		$this->Controller->Acl->Aro->create(array('parent_id' => $parent, 'alias' => 'Admin'));
 		$result = $this->Controller->Acl->Aro->save();
 		$this->assertTrue($result);
 
 		$parent = $this->Controller->Acl->Aro->id;
 
-		$this->Controller->Acl->Aro->create(array('model' => 'AuthUser', 'parent_id' => $parent, 'foreign_key' => 1, 'alias'=> 'mariano'));
+		$this->Controller->Acl->Aro->create(array(
+			'model' => 'AuthUser', 'parent_id' => $parent, 'foreign_key' => 1, 'alias'=> 'mariano'
+		));
 		$result = $this->Controller->Acl->Aro->save();
 		$this->assertTrue($result);
 
@@ -612,15 +627,30 @@ class AuthTest extends CakeTestCase {
 		$this->Controller->Auth->actionPath = 'Root/';
 
 		$this->Controller->Auth->startup($this->Controller);
-
-
 		$this->assertTrue($this->Controller->Auth->isAuthorized());
 
 		$this->Controller->Session->del('Auth');
 		$this->Controller->Auth->startup($this->Controller);
 		$this->assertTrue($this->Controller->Session->check('Message.auth'));
 	}
+/**
+ * Tests that deny always takes precedence over allow
+ *
+ * @access public
+ * @return void
+ */
+	function testAllowDenyAll() {
+		$this->Controller->Auth->initialize($this->Controller);
 
+		$this->Controller->Auth->allow('*');
+		$this->Controller->Auth->deny('add');
+
+		$this->Controller->action = 'delete';
+		$this->assertTrue($this->Controller->Auth->startup($this->Controller));
+
+		$this->Controller->action = 'add';
+		$this->assertFalse($this->Controller->Auth->startup($this->Controller));
+	}
 /**
  * testLoginRedirect method
  *
@@ -636,13 +666,17 @@ class AuthTest extends CakeTestCase {
 
 		$_SERVER['HTTP_REFERER'] = false;
 
-		$this->Controller->Session->write('Auth', array('AuthUser' => array('id'=>'1', 'username'=>'nate')));
+		$this->Controller->Session->write('Auth', array(
+			'AuthUser' => array('id' => '1', 'username' => 'nate')
+		));
 
 		$this->Controller->params['url']['url'] = 'users/login';
 		$this->Controller->Auth->initialize($this->Controller);
 
 		$this->Controller->Auth->userModel = 'AuthUser';
-		$this->Controller->Auth->loginRedirect = array('controller' => 'pages', 'action' => 'display', 'welcome');
+		$this->Controller->Auth->loginRedirect = array(
+			'controller' => 'pages', 'action' => 'display', 'welcome'
+		);
 		$this->Controller->Auth->startup($this->Controller);
 		$expected = Router::normalize($this->Controller->Auth->loginRedirect);
 		$this->assertEqual($expected, $this->Controller->Auth->redirect());
@@ -666,14 +700,18 @@ class AuthTest extends CakeTestCase {
 		putenv('HTTP_REFERER=');
 		$url = '/posts/view/1';
 
-		$this->Controller->Session->write('Auth', array('AuthUser' => array('id'=>'1', 'username'=>'nate')));
+		$this->Controller->Session->write('Auth', array(
+			'AuthUser' => array('id' => '1', 'username' => 'nate'))
+		);
 		$this->Controller->testUrl = null;
 		$this->Controller->params = Router::parse($url);
 		$this->Controller->Auth->initialize($this->Controller);
 		$this->Controller->Auth->authorize = 'controller';
 		$this->Controller->params['testControllerAuth'] = true;
 
-		$this->Controller->Auth->loginAction = array('controller' => 'AuthTest', 'action' => 'login');
+		$this->Controller->Auth->loginAction = array(
+			'controller' => 'AuthTest', 'action' => 'login'
+		);
 		$this->Controller->Auth->userModel = 'AuthUser';
 		$this->Controller->Auth->startup($this->Controller);
 		$expected = Router::normalize('/');
@@ -683,8 +721,9 @@ class AuthTest extends CakeTestCase {
 		$this->Controller->Session->del('Auth');
 		$_SERVER['HTTP_REFERER'] = '/admin/';
 
-		$this->Controller->Session->write('Auth', array('AuthUser' => array('id'=>'1', 'username'=>'nate')));
+		$this->Controller->Session->write('Auth', array(
-		
+			'AuthUser' => array('id'=>'1', 'username'=>'nate'))
+		);
 		$this->Controller->params['url']['url'] = 'auth_test/login';
 		$this->Controller->Auth->initialize($this->Controller);
 		$this->Controller->Auth->loginAction = 'auth_test/login';
@@ -834,14 +873,24 @@ class AuthTest extends CakeTestCase {
  */
 	function testCustomRoute() {
 		Router::reload();
-		Router::connect('/:lang/:controller/:action/*', array('lang' => null), array('lang' => '[a-z]{2,3}'));
+		Router::connect(
+			'/:lang/:controller/:action/*',
+			array('lang' => null),
+			array('lang' => '[a-z]{2,3}')
+		);
 
 		$url = '/en/users/login';
 		$this->Controller->params = Router::parse($url);
-		Router::setRequestInfo(array($this->Controller->passedArgs, array('base' => null, 'here' => $url, 'webroot' => '/', 'passedArgs' => array(), 'argSeparator' => ':', 'namedArgs' => array())));
+		Router::setRequestInfo(array($this->Controller->passedArgs, array(
+			'base' => null, 'here' => $url, 'webroot' => '/', 'passedArgs' => array(),
+			'argSeparator' => ':', 'namedArgs' => array()
+		)));
 
 		$this->AuthUser =& new AuthUser();
-		$user = array('id' => 1, 'username' => 'felix', 'password' => Security::hash(Configure::read('Security.salt') . 'cake'));
+		$user = array(
+			'id' => 1, 'username' => 'felix',
+			'password' => Security::hash(Configure::read('Security.salt') . 'cake'
+		));
 		$user = $this->AuthUser->save($user, false);
 
 		$this->Controller->data['AuthUser'] = array('username' => 'felix', 'password' => 'cake');
@@ -871,8 +920,9 @@ class AuthTest extends CakeTestCase {
 		$this->Controller->params['url']['url'] = ltrim($url, '/');
 		Router::setRequestInfo(array(
 			array(
-				'pass' => array(), 'action' => 'index', 'plugin' => null, 'controller' => 'something',
+				'pass' => array(), 'action' => 'index', 'plugin' => null,
-				'admin' => true, 'url' => array('url' => $this->Controller->params['url']['url']),
+				'controller' => 'something', 'admin' => true,
+				'url' => array('url' => $this->Controller->params['url']['url'])
 			),
 			array(
 				'base' => null, 'here' => $url,
@@ -881,7 +931,9 @@ class AuthTest extends CakeTestCase {
 		));
 		$this->Controller->Auth->initialize($this->Controller);
 
-		$this->Controller->Auth->loginAction = array('admin' => true, 'controller' => 'auth_test', 'action' => 'login');
+		$this->Controller->Auth->loginAction = array(
+			'admin' => true, 'controller' => 'auth_test', 'action' => 'login'
+		);
 		$this->Controller->Auth->userModel = 'AuthUser';
 
 		$this->Controller->Auth->startup($this->Controller);