[2.1] Added the support of Apache's passwords #3243

Merged
merged 2 commits into from Jan 4, 2013

Projects

None yet

2 participants

@ezimuel
Zend Framework member

Added the support of Apache's passwords in Zend\Crypt\Password\Apache.
I implemented the different password formats supported by htpasswd, reported here:
http://httpd.apache.org/docs/2.2/misc/password_encryptions.html

@weierophinney weierophinney and 1 other commented on an outdated diff Jan 3, 2013
library/Zend/Crypt/Password/Apache.php
+ );
+ }
+ foreach ($options as $key => $value) {
+ switch (strtolower($key)) {
+ case 'format':
+ $this->setFormat($value);
+ break;
+ case 'authname':
+ $this->setAuthName($value);
+ break;
+ case 'username':
+ $this->setUserName($value);
+ break;
+ }
+ }
+ }
@weierophinney
weierophinney Jan 3, 2013

I think this can be cleaned up a bit. I'd do the following:

if (empty($options)) {
    return;
}
if (!is_array($options) && !$options instanceof Traversable) {
    throw new Exception\InvalidArgumentException(/* must be array or traversable */);
}
foreach ($options as $key => $value) {
    // set options as above
}

This way, the bulk of the work is not in a conditional, and you're not needing to cast Traversables to an array (as you're simply iterating over them).

@weierophinney weierophinney and 1 other commented on an outdated diff Jan 3, 2013
library/Zend/Crypt/Password/Apache.php
+ }
+ }
+ }
+ }
+
+ /**
+ * Generate the hash of a password
+ *
+ * @param string $password
+ * @throws Exception\RuntimeException
+ * @return string
+ */
+ public function create($password)
+ {
+ if (empty($this->format)) {
+ throw new Exception\InvalidArgumentException(
@weierophinney
weierophinney Jan 3, 2013

I'd use RuntimeException or DomainException here -- $this->format is not an argument to the method, which makes InvalidArgumentException confusing.

@ezimuel
ezimuel Jan 4, 2013

OK, I will use RuntimeException.

@weierophinney weierophinney commented on an outdated diff Jan 3, 2013
library/Zend/Crypt/Password/Apache.php
+ }
+
+ /**
+ * Verify if a password is correct against an hash value
+ *
+ * @param string $password
+ * @param string $hash
+ * @return boolean
+ */
+ public function verify($password, $hash)
+ {
+ if (substr($hash, 0, 5) === '{SHA}') {
+ $hash2 = '{SHA}' . base64_encode(sha1($password, true));
+
+ return ($hash === $hash2);
+ } elseif (substr($hash, 0, 6) === '$apr1$') {
@weierophinney
weierophinney Jan 3, 2013

Since the previous block returns, do this as a new if statement. Same goes for the next elseif.

@weierophinney weierophinney and 1 other commented on an outdated diff Jan 3, 2013
library/Zend/Crypt/Password/Apache.php
+ 'The APR1 password format is not valid'
+ );
+ }
+ $hash2 = $this->apr1Md5($password, $token[2]);
+
+ return ($hash === $hash2);
+ } elseif (strlen($hash) > 13) { // digest
+ if (empty($this->userName) || empty($this->authName)) {
+ throw new Exception\RuntimeException(
+ 'You must specify UserName and AuthName (realm) to verify the digest'
+ );
+ }
+ $hash2 = md5($this->userName . ':' . $this->authName . ':' .$password);
+
+ return ($hash === $hash2);
+ } else { // crypt(3)
@weierophinney
weierophinney Jan 3, 2013

Per the above, you can remove the else block -- all previous blocks return, so this block can be promoted to the method body.

@ezimuel
ezimuel Jan 4, 2013

Yes, make sense.

@weierophinney weierophinney was assigned Jan 3, 2013
@weierophinney
Zend Framework member

Looks good, @ezimuel -- make the changes as suggested, and I can merge.

@weierophinney weierophinney added a commit that referenced this pull request Jan 4, 2013
@weierophinney weierophinney [#3243] CS fixes
- trailing whitespace
- add comma after last array element
66ffb9c
@weierophinney weierophinney merged commit 77a77b1 into zendframework:develop Jan 4, 2013

1 check failed

Details default The Travis build failed
@katalonec katalonec pushed a commit to katalonec/zf2 that referenced this pull request Jan 4, 2013
@weierophinney weierophinney Merge branch 'feature/auth-apache-password' into develop
Close #3243
4d2d94c
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney [#3243] CS fixes
- trailing whitespace
- add comma after last array element
79383a9
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'feature/auth-apache-password' into develop
Close #3243
992854c
@weierophinney weierophinney added a commit to zendframework/zend-crypt that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#3243] CS fixes
- trailing whitespace
- add comma after last array element
499f91f
@weierophinney weierophinney added a commit to zendframework/zend-crypt that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'feature/auth-apache-password' into develop ac9318c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment