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

[Security] Make PersistentToken immutable and tell TokenProviderInterface::updateToken() implementations should accept DateTimeInterface #50290

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 10, 2023

Q A
Branch? 6.4
Bug fix? no
New feature? no
Deprecations? no
Tickets Part of #47580
License MIT
Doc PR -

@nicolas-grekas
Copy link
Member Author

PR rebased and ready for merge in 6.3 if desired.

@nicolas-grekas
Copy link
Member Author

PR ready for 6.3.

@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
fabpot added a commit that referenced this pull request Jun 3, 2023
This PR was merged into the 6.4 branch.

Discussion
----------

Remove unnecessary usages of DateTime

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #47580
| License       | MIT
| Doc PR        | -

Together with #50290, this PR removes `DateTime` everywhere possible.

What remains is:
- test cases that test both DateTime and DateTimeImmutable - legit to keep
- date/time form-types and transformers in the Form component - we'd need a separate effort for them, related to #50295
- `PersistentTokenInterface::getLastUsed(): \DateTime` - separate effort also

Commits
-------

8b08294 Remove unnecessary usages of DateTime
@fabpot
Copy link
Member

fabpot commented Jun 3, 2023

@nicolas-grekas This PR should be adapted for 6.4 instead of 6.3.

@nicolas-grekas
Copy link
Member Author

PR updated, ready for 6.4

@nicolas-grekas nicolas-grekas force-pushed the sec-immut branch 2 times, most recently from 8091528 to 7e7f384 Compare June 5, 2023 20:04
@xabbuh
Copy link
Member

xabbuh commented Jun 6, 2023

The low deps failures are related, aren't they?

@nicolas-grekas
Copy link
Member Author

The low deps failures are related, aren't they?

Right, fixed!

…terface::updateToken()` implementations should accept `DateTimeInterface`
@fabpot
Copy link
Member

fabpot commented Jul 13, 2023

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 31bad80 into symfony:6.4 Jul 13, 2023
4 of 9 checks passed
@nicolas-grekas nicolas-grekas deleted the sec-immut branch July 25, 2023 15:45
This was referenced Oct 21, 2023
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.

None yet

7 participants