Skip to content

Add bcrypt support to Security::hash() #717

Closed
wants to merge 1 commit into from

3 participants

@sitedyno

Fixed to work as described here #697 (comment)

Let me know what else I need to do :)

@markstory markstory commented on an outdated diff Jul 17, 2012
lib/Cake/Utility/Security.php
* @return string Hash
*/
public static function hash($string, $type = null, $salt = false) {
+ if (($type === null && self::$hashType === 'blowfish') || $type === 'blowfish') {
+ if ($salt === true || ($salt !== false && !is_string($salt))) {
@markstory
CakePHP member
markstory added a note Jul 17, 2012

Couldn't this be expressed as:

if ($type === null) {
   $type = self::$hashType;
}

Instead of the nested ifs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory
CakePHP member

It might be useful to have a test case for comparing hashes as well, just so its easily added to the docs/api as well.

@sitedyno

You mean this? sitedyno/cakephp@dcb91e4#L0R142

$blowfishHash = '$2a$10$l4ztO6LP8/ib/CcpqQtev.rw3RXT9yw1TItDCSPq.J6gCqKe4Tfby'; // a hash of 'someKey'
$result = Security::hash($key, null, $blowfishHash);
$this->assertSame($result, $blowfishHash);
@shama shama commented on an outdated diff Jul 17, 2012
lib/Cake/Utility/Security.php
@@ -189,4 +217,71 @@ public static function rijndael($text, $key, $operation) {
return $out;
}
+/**
+ * Generates a psuedo random salt suitable for use with php's crypt() function.
@shama
shama added a note Jul 17, 2012

s/psuedo/pseudo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sitedyno

Will squash once I get the ok. Anything else? :)

@markstory
CakePHP member

@sitedyno I was more thinking of a test that would be useful as an example of doing a password check. But perhaps that kind of test belongs somewhere else.

@sitedyno

@markstory better?

@markstory
CakePHP member

@sitedyno Perfect :D You could use assertNotSame to make the test a little less wordy but otherwise that's exactly what I was thinking.

@markstory
CakePHP member

Sweet, I'll get this merged in :D

@markstory markstory was assigned Jul 21, 2012
@sitedyno

OK squashed it up.

@markstory
CakePHP member

Merged in [434d3a7]

@markstory markstory closed this Jul 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.