Fix: Hash passwords when 'client_secret' exists in POSTed data

Controller/Component/OAuthComponent.php

@@ -305,7 +305,7 @@ public function user($field = null, $token = null) {
 				));
 			$token = empty($token) ? $this->getBearerToken() : $token;
 			$data = $this->AccessToken->find('first', array(
-				'conditions' => array('oauth_token' => self::hash($token)),
+				'conditions' => array('oauth_token' => $token),
 				'recursive' => 1
 			));
 			if (!$data) {
@@ -400,7 +400,7 @@ public function __call($name, $arguments) {
 	public function checkClientCredentials($client_id, $client_secret = null) {
 		$conditions = array('client_id' => $client_id);
 		if ($client_secret) {
-			$conditions['client_secret'] = self::hash($client_secret);
+			$conditions['client_secret'] = $client_secret;
 		}
 		$client = $this->Client->find('first', array(
 			'conditions' => $conditions,
@@ -442,7 +442,7 @@ public function getClientDetails($client_id) {
  */
 	public function getAccessToken($oauth_token) {
 		$accessToken = $this->AccessToken->find('first', array(
-			'conditions' => array('oauth_token' => self::hash($oauth_token)),
+			'conditions' => array('oauth_token' => $oauth_token),
 			'recursive' => -1,
 		));
 		if ($accessToken) {
@@ -498,7 +498,7 @@ public function checkRestrictedGrantType($client_id, $grant_type) {
  */
 	public function getRefreshToken($refresh_token) {
 		$refreshToken = $this->RefreshToken->find('first', array(
-			'conditions' => array('refresh_token' => self::hash($refresh_token)),
+			'conditions' => array('refresh_token' => $refresh_token),
 			'recursive' => -1
 		));
 		if ($refreshToken) {
@@ -576,7 +576,7 @@ public function checkUserCredentials($client_id, $username, $password) {
  */
 	public function getAuthCode($code) {
 		$authCode = $this->AuthCode->find('first', array(
-			'conditions' => array('code' => self::hash($code)),
+			'conditions' => array('code' => $code),
 			'recursive' => -1
 		));
 		if ($authCode) {

Model/AccessToken.php

@@ -55,6 +55,12 @@ class AccessToken extends OAuthAppModel {
 		),
 	);
 
+	public $actsAs = array(
+		'OAuth.HashedField' => array(
+			'fields' => 'oauth_token',
+		),
+	);
+
 /**
  * belongsTo associations
  *
@@ -70,14 +76,4 @@ class AccessToken extends OAuthAppModel {
 		)
 	);
 
-/**
- * beforeSave method to hash tokens before saving
- *
- * @return boolean
- */
-	public function beforeSave($options = array()) {
-		$this->data['AccessToken']['oauth_token'] = OAuthComponent::hash($this->data['AccessToken']['oauth_token']);
-		return true;
-	}
-
 }

Model/AuthCode.php

@@ -60,6 +60,12 @@ class AuthCode extends OAuthAppModel {
 		),
 	);
 
+	public $actsAs = array(
+		'OAuth.HashedField' => array(
+			'fields' => 'code',
+		),
+	);
+
 /**
  * belongsTo associations
  *
@@ -82,14 +88,4 @@ class AuthCode extends OAuthAppModel {
 		)
 	);
 
-/**
- * beforeSave method to hash codes before saving
- *
- * @return boolean
- */
-	public function beforeSave($options = array()) {
-		$this->data['AuthCode']['code'] = OAuthComponent::hash($this->data['AuthCode']['code']);
-		return true;
-	}
-
 }

Model/Behavior/HashedFieldBehavior.php

@@ -0,0 +1,61 @@
+<?php
+
+App::uses('ModelBehavior', 'Model');
+App::uses('Security', 'Utility');
+
+/**
+ * Enable automatic field hashing when in Model::save() Model::find()
+ */
+class HashedFieldBehavior extends ModelBehavior {
+
+/**
+ * Behavior defaults
+ */
+	protected $_defaults = array(
+		'fields' => array(),
+	);
+
+/**
+ * Setup behavior
+ */
+	public function setup(Model $Model, $config = array()) {
+		parent::setup($Model, $config);
+		$this->settings[$Model->name] = Hash::merge($this->_defaults, $config);
+	}
+
+/**
+ * Hash field when present in model data (POSTed data)
+ */
+	public function beforeSave(Model $Model) {
+		$setting = $this->settings[$Model->name];
+		foreach ((array) $setting['fields'] as $field) {
+			if (array_key_exists($field, $Model->data[$Model->alias])) {
+				$data = $Model->data[$Model->alias][$field];
+				$Model->data[$Model->alias][$field] = Security::hash($data, null, true);
+			}
+		}
+		return true;
+	}
+
+/**
+ * Hash condition when it contains a field specified in setting
+ */
+	public function beforeFind(Model $Model, $queryData) {
+		$setting = $this->settings[$Model->name];
+		$conditions =& $queryData['conditions'];
+		foreach ((array) $setting['fields'] as $field) {
+			$escapeField = $Model->escapeField($field);
+			if (array_key_exists($field, (array)$conditions)) {
+				$queryField = $field;
+			} elseif (array_key_exists($escapeField, (array)$conditions)) {
+				$queryField = $escapeField;
+			}
+			if (isset($queryField)) {
+				$data = $conditions[$queryField];
+				$conditions[$queryField] = Security::hash($data, null, true);
+			}
+		}
+		return $queryData;
+	}
+
+}

Model/Client.php

@@ -1,9 +1,7 @@
 <?php
 
 App::uses('OAuthAppModel', 'OAuth.Model');
-App::uses('OAuthComponent', 'OAuth.Controller/Component');
 App::uses('String', 'Utility');
-App::uses('Security', 'Utility');
 
 /**
  * Client Model
@@ -56,6 +54,14 @@ class Client extends OAuthAppModel {
 		),
 	);
 
+	public $actsAs = array(
+		'OAuth.HashedField' => array(
+			'fields' => array(
+				'client_secret'
+			),
+		),
+	);
+
 /**
  * hasMany associations
  *
@@ -121,7 +127,7 @@ public function add($data = null) {
 		} else {
 			return false;
 		}
-		
+
 		/**
 		 * in case you have additional fields in the clients table such as name, description etc
 		 * and you are using $data['Client']['name'], etc to save
@@ -158,11 +164,6 @@ public function newClientSecret() {
 		return OAuthComponent::hash($str);
 	}
 
-	public function beforeSave($options = array()) {
-		$this->data['Client']['client_secret'] = OAuthComponent::hash($this->data['Client']['client_secret']);
-		return true;
-	}
-
 	public function afterSave($created) {
 		if ($this->addClientSecret) {
 			$this->data['Client']['client_secret'] = $this->addClientSecret;

Model/RefreshToken.php

@@ -55,6 +55,12 @@ class RefreshToken extends OAuthAppModel {
 		),
 	);
 
+	public $actsAs = array(
+		'OAuth.HashedField' => array(
+			'fields' => 'refresh_token',
+		),
+	);
+
 /**
  * belongsTo associations
  *
@@ -70,14 +76,4 @@ class RefreshToken extends OAuthAppModel {
 		)
 	);
 
-/**
- * beforeSave method to hash tokens before saving
- *
- * @return boolean
- */
-	public function beforeSave($options = array()) {
-		$this->data['RefreshToken']['refresh_token'] = OAuthComponent::hash($this->data['RefreshToken']['refresh_token']);
-		return true;
-	}
-
 }

Test/Case/AllOAuthTestsTest.php

@@ -0,0 +1,12 @@
+<?php
+
+class AllOAuthTestsTest extends PHPUnit_Framework_TestSuite {
+
+	public static function suite() {
+		$suite = new CakeTestSuite('OAuth Tests');
+		$path = CakePlugin::path('OAuth') . 'Test' . DS . 'Case' . DS;
+		$suite->addTestDirectory($path . 'Model' . DS . 'Behavior');
+		return $suite;
+	}
+
+}

Test/Case/Model/Behavior/HashedFieldBehaviorTest.php

@@ -0,0 +1,73 @@
+<?php
+
+App::uses('CakeTestCase', 'TestSuite');
+
+class HashedFieldBehaviorTest extends CakeTestCase {
+
+	public $fixtures = array(
+		'plugin.o_auth.access_token',
+	);
+
+	protected $_clientId = '1234567890';
+
+	protected $_token = '0123456789abcdefghijklmnopqrstuvwxyz';
+
+	public function setUp() {
+		$this->AccessToken = ClassRegistry::init('OAuth.AccessToken');
+		$this->AccessToken->recursive = -1;
+	}
+
+	protected function _createToken() {
+		$this->AccessToken->create(array(
+			'client_id' => $this->_clientId,
+			'oauth_token' => $this->_token,
+			'user_id' => 69,
+			'expires' => time() + 86400,
+		));
+		$this->AccessToken->save();
+	}
+
+	public function testBeforeSave() {
+		$this->_createToken();
+
+		$result = $this->AccessToken->findByClientId($this->_clientId);
+		$expected = Security::hash($this->_token, null, true);
+		$this->assertEquals($expected, $result['AccessToken']['oauth_token']);
+	}
+
+	public function testBeforeFind() {
+		$this->_createToken();
+
+		$result = $this->AccessToken->find('first', array(
+			'conditions' => array(
+				'oauth_token' => $this->_token,
+			),
+		));
+		$this->assertEquals($this->_clientId, $result['AccessToken']['client_id']);
+		$this->assertNotEquals($this->_token, $result['AccessToken']['oauth_token']);
+	}
+
+/**
+ * test saving with missing oauth_token in POSTed data does not corrupt value
+ */
+	public function testSavingWithMissingToken() {
+		$this->_createToken();
+
+		$baseline = $this->AccessToken->find('first');
+		$this->assertNull($baseline['AccessToken']['scope']);
+
+		$updated = $baseline;
+		$updated['AccessToken']['scope'] = 'all';
+		unset($updated['AccessToken']['oauth_token']);
+
+		$this->assertFalse(array_key_exists('oauth_token', $updated));
+		$this->AccessToken->save($updated);
+
+		$result = $this->AccessToken->findByClientId($baseline['AccessToken']['client_id']);
+
+		$this->assertEquals('all', $result['AccessToken']['scope']);
+		$expected = Security::hash($this->_token, null, true);
+		$this->assertEquals($expected, $result['AccessToken']['oauth_token']);
+	}
+
+}

Test/Fixture/AccessTokenFixture.php

@@ -0,0 +1,33 @@
+<?php
+/**
+ * AccessTokenFixture
+ *
+ */
+class AccessTokenFixture extends CakeTestFixture {
+
+/**
+ * Fields
+ *
+ * @var array
+ */
+	public $fields = array(
+		'oauth_token' => array('type' => 'string', 'null' => false, 'default' => null, 'length' => 40, 'key' => 'primary', 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
+		'client_id' => array('type' => 'string', 'null' => false, 'default' => null, 'length' => 36, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
+		'user_id' => array('type' => 'integer', 'null' => false, 'default' => null),
+		'expires' => array('type' => 'integer', 'null' => false, 'default' => null),
+		'scope' => array('type' => 'string', 'null' => true, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
+		'indexes' => array(
+			'PRIMARY' => array('column' => 'oauth_token', 'unique' => 1)
+		),
+		'tableParameters' => array('charset' => 'utf8', 'collate' => 'utf8_general_ci', 'engine' => 'MyISAM')
+	);
+
+/**
+ * Records
+ *
+ * @var array
+ */
+	public $records = array(
+	);
+
+}