Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.9] Add Argon2id Password Support #20855

Merged
merged 4 commits into from Aug 28, 2018
Merged

[3.9] Add Argon2id Password Support #20855

merged 4 commits into from Aug 28, 2018

Conversation

@zero-24
Copy link
Member

@zero-24 zero-24 commented Jun 24, 2018

Pull Request for Issue #20826

Summary of Changes

Per https://wiki.php.net/rfc/argon2_password_hash_enhancements Argon2id password hashes will be supported in PHP 7.3. We should add support for this alongside our existing Argon2i logic.

Testing Instructions

review

Expected result

Argon2id PW supported

Actual result

Argon2id PW NOT supported

Documentation Changes Required

@mbabker can you tell me where you documented the Argon2i support?

zero-24 added 2 commits Jun 24, 2018
@HLeithner
Copy link
Member

@HLeithner HLeithner commented Jun 24, 2018

I think this will not work.
You have to check for "$argon2id" before checking for "$argon2i" in the if Statement because strpos "$argon2i" will already be true for "$argon2id"

@zero-24
Copy link
Member Author

@zero-24 zero-24 commented Jun 24, 2018

fixed by the last commit thanks @HLeithner

@mbabker
Copy link
Contributor

@mbabker mbabker commented Jun 24, 2018

can you tell me where you documented the Argon2i support?

It's not really documented anywhere. We don't have a place covering this part of the API AFAIK.

@zero-24 zero-24 added this to the Joomla 3.9.0 milestone Jun 24, 2018
@HLeithner
Copy link
Member

@HLeithner HLeithner commented Jul 11, 2018

J4i the rfc has been accepted and is already implemented in php 7.3.

@HLeithner
Copy link
Member

@HLeithner HLeithner commented Jul 22, 2018

I have tested this item successfully on c09b09d

Successfully on PHP 7.3.0alpha4


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20855.

@mbabker mbabker merged commit e8652f7 into joomla:3.9-dev Aug 28, 2018
3 of 5 checks passed
3 of 5 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
JTracker/HumanTestResults Human Test Results: 1 Successful 0 Failed.
Details
Hound No violations found. Woof!
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zero-24 zero-24 deleted the zero-24:argon2id branch Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.