Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

New OpenSSL symmetric adapter + bcrypt refactor for PHP 5.5+ #25

Merged
merged 6 commits into from
May 5, 2016

Conversation

ezimuel
Copy link
Contributor

@ezimuel ezimuel commented May 3, 2016

This PR contains the new OpenSSL symmetric adapter for zend-crypt ver. 3.0. We used this new adapter instead of Mcrypt in FileCipher and we suggested to use it in the documentation for all the other encryption needs. Mcrypt is an outdated extension and it will be deprecated) with PHP 7.1.

Moreover, this PR contains the Zend\Crypt\Password\Bcrypt refactor using the password_hash() and password_verify() functions of PHP 5.5+.

I updated the documentation according to these changes.

Note: All the changes included in this PR are backward compatible. I provided some unit tests as proof.

@ezimuel ezimuel added this to the 3.0.0 milestone May 3, 2016
@ezimuel ezimuel changed the title New OpenSSL symmetric adapter + bcrypt refactor for PHP 5.5+ [WIP] New OpenSSL symmetric adapter + bcrypt refactor for PHP 5.5+ May 3, 2016
@ezimuel ezimuel changed the title [WIP] New OpenSSL symmetric adapter + bcrypt refactor for PHP 5.5+ New OpenSSL symmetric adapter + bcrypt refactor for PHP 5.5+ May 4, 2016
@ezimuel
Copy link
Contributor Author

ezimuel commented May 4, 2016

@weierophinney this PR is ready to be merged in develop, thanks!

* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2016

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samsonasik Thanks, I updated the 2016 in all the code.

'padding' => 'pkcs7'
]);
} catch (Symmetric\Exception\RuntimeException $e) {
$this->markTestSkipped($e->getMessage());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same note as for the mcrypt tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weierophinney even here, we are just testing if OpenSSL extension is enabled.

@weierophinney
Copy link
Member

@ezimuel Looks very good! Just a few general notes:

  • In a number of tests, you mark them skipped if specific extensions are missing and/or functionality is missing. I think for the purposes of testing and for Travis, we should likely mark those as failed, as they're required for the tests.
  • Last item of all arrays should have a trailing comma (reduces the impact of diffs when items are added later).
  • Update the copyrights across the board.
  • Make sure you're using sprintf() when generating exception messages with dynamic elements.

I've added more specific notes elsewhere. Nice work!

@ezimuel
Copy link
Contributor Author

ezimuel commented May 5, 2016

@weierophinney and @samsonasik I updated the PR will all your feedback, thanks!

@weierophinney weierophinney merged commit 1cddcec into zendframework:develop May 5, 2016
weierophinney added a commit that referenced this pull request May 5, 2016
New OpenSSL symmetric adapter + bcrypt refactor for PHP 5.5+
weierophinney added a commit that referenced this pull request May 5, 2016
weierophinney added a commit that referenced this pull request May 5, 2016
@weierophinney weierophinney self-assigned this May 5, 2016
@weierophinney
Copy link
Member

Merged to develop! We can tag 3.0 early next week, @ezimuel !

@ezimuel ezimuel mentioned this pull request May 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants