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

EZP-32250: Exposed PasswordHashService APIs #150

Merged
merged 9 commits into from
Dec 23, 2020
Merged

EZP-32250: Exposed PasswordHashService APIs #150

merged 9 commits into from
Dec 23, 2020

Conversation

adamwojs
Copy link
Member

@adamwojs adamwojs commented Dec 22, 2020

Question Answer
JIRA issue EZP-32250
Type feature
Target eZ Platform version v3.3
BC breaks no
Doc needed yes

Added methods

  • getSupportedHashTypes(): array
  • isHashTypeSupported(int $hashType): bool

to \eZ\Publish\Core\Repository\User\PasswordHashServiceInterface and refactor existing \eZ\Publish\API\Repository\Values\User\User::SUPPORTED_PASSWORD_HASHES usages

This allows to decorate PasswordHashService in 3rd-party bundles and add support for custom password hash types.

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (master for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ezsystems/php-dev-team).

@adamwojs adamwojs added Improvement Changes not fixing or changing behavior DX labels Dec 22, 2020
@adamwojs adamwojs self-assigned this Dec 22, 2020
@ezsystems ezsystems deleted a comment from sonarcloud bot Dec 22, 2020
@adamwojs adamwojs requested review from glye and a team December 22, 2020 22:36
interface PasswordHashServiceInterface
{
public function getDefaultHashType(): int;

public function getSupportedHashTypes(): array;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since other methods specifically ask for int.

Suggested change
public function getSupportedHashTypes(): array;
/**
* @return int[]
*/
public function getSupportedHashTypes(): array;

@@ -24,6 +24,9 @@
*/
abstract class User extends Content implements UserReference
{
/**
* @var int[] List of supported (by default) hash types.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Side note: to speed up checks if hash is supported this can be turned into array<int, int>. Then to check support for hash you'd just see if:

isset(User::SUPPORTED_PASSWORD_HASHES[$hashType]);

Copy link
Member Author

Choose a reason for hiding this comment

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

Premature optimization is the root of all evil

D. Knuth

@ezsystems ezsystems deleted a comment from sonarcloud bot Dec 23, 2020
@alongosz alongosz changed the title EZP-32250: As a Developer, I want add support for custom password hash type EZP-32250: Exposed PasswordHashService APIs Dec 23, 2020
@ezsystems ezsystems deleted a comment from sonarcloud bot Dec 23, 2020
@lserwatka lserwatka merged commit 6834f1d into master Dec 23, 2020
@lserwatka lserwatka deleted the ezp_32250 branch December 23, 2020 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Improvement Changes not fixing or changing behavior Ready for QA
4 participants