Skip to content

[PasswordHasher] Add union types #41640

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

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

ValentineBoineau
Copy link
Contributor

Q A
Branch? 6.0
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

Extracted from #41424

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@ValentineBoineau ValentineBoineau force-pushed the union-types-passwordhasher branch 2 times, most recently from 01da7b0 to fe3fdfd Compare June 9, 2021 13:55
@carsonbot carsonbot changed the title [PasswordHasher] Add union types Add union types Jun 9, 2021
@ValentineBoineau ValentineBoineau marked this pull request as ready for review June 9, 2021 14:04
@ValentineBoineau ValentineBoineau requested a review from chalasr as a code owner June 9, 2021 14:04
@carsonbot carsonbot added this to the 6.0 milestone Jun 9, 2021
@ValentineBoineau ValentineBoineau force-pushed the union-types-passwordhasher branch from fe3fdfd to 5005cdf Compare June 9, 2021 14:28
@carsonbot carsonbot changed the title Add union types [PasswordHasher] Add union types Jun 9, 2021
@ValentineBoineau ValentineBoineau force-pushed the union-types-passwordhasher branch 2 times, most recently from 9df0ac9 to 2af3710 Compare June 10, 2021 17:05
@chalasr chalasr force-pushed the union-types-passwordhasher branch 2 times, most recently from b47497f to e0a3ad3 Compare June 10, 2021 17:42
@ValentineBoineau ValentineBoineau force-pushed the union-types-passwordhasher branch 8 times, most recently from f99c3be to 501b827 Compare June 11, 2021 09:29
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

With some nitpicking.
@chalasr shouldn't we backport some changes to 5.3?

@ValentineBoineau ValentineBoineau force-pushed the union-types-passwordhasher branch from 501b827 to b57bce2 Compare June 11, 2021 10:00
chalasr added a commit that referenced this pull request Jun 11, 2021
…allowed type (chalasr)

This PR was merged into the 5.3 branch.

Discussion
----------

[PasswordHasher] Fix missing PasswordHasherAwareInterface allowed type

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License | MIT
| Doc PR | -

Also backports test changes from #41640

Commits
-------

8d3bea5 [PasswordHasher] Fix missing PasswordHasherAwareInterface allowed type in signatures
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Rebase needed after #41678

chalasr added a commit that referenced this pull request Jun 11, 2021
…e from UserPasswordHasherInterface API (chalasr)

This PR was merged into the 5.3 branch.

Discussion
----------

[PasswordHasher] Remove PasswordHasherAwareInterface type from UserPasswordHasherInterface API

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | no (not yet released)
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

As spotted by `@stof` in #41640 (comment), the methods of this interface should not handle user classes/instances that are not implementing `PasswordAuthenticatedUserInterface`.
This reverts that part from #41678 (not released yet).

Commits
-------

596ba78 [PasswordHasher] Remove PasswordHasherAwareInterface from UserPasswordHasherInterface API
@chalasr chalasr force-pushed the union-types-passwordhasher branch from b57bce2 to 8628479 Compare June 11, 2021 16:50
@chalasr
Copy link
Member

chalasr commented Jun 11, 2021

Thank you @ValentineBoineau.

@chalasr chalasr merged commit e5bbf62 into symfony:6.0 Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants