Skip to content

[SecurityBundle] avoid collision between ldap and normal authenticators #57215

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

Closed
wants to merge 8,715 commits into from

Conversation

MindfulPol
Copy link

@MindfulPol MindfulPol commented May 28, 2024

Q A
Branch? 7.1
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #41892
License MIT

fabpot and others added 30 commits April 8, 2024 13:30
… version (MatTheCat)

This PR was merged into the 7.1 branch.

Discussion
----------

[WebProfilerBundle] Inline flowchart-only Mermaid version

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | Fix symfony#54416
| License       | MIT

From mermaid-js/mermaid#2920 (comment) there is no official way to generate a diagram-specific script, but it is possible by patching the `packages/mermaid/src/diagram-api/diagram-orchestration.ts` file before building.

This PR comes with a Makefile whose default recipe does so, and the `mermaid-flowchart-v2.min.js` file it generated from v10.9.0.

Bumping the script’s version will require to update the Makefile’s `tag` variable to the corresponding `mermaid-js/mermaid`’s (from https://github.com/mermaid-js/mermaid/tags) and running `make`. The recipe depends on cURL, GNU tar, and pnpm.

Commits
-------

32612e7 [WebProfilerBundle] Inline flowchart-only Mermaid version
…reateFromTimestamp() (derrabus)

This PR was merged into the 7.1 branch.

Discussion
----------

[Clock] Add a polyfill for DateTimeImmutable::createFromTimestamp()

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | N/A
| License       | MIT

This PR adds the new `createFromTimestamp()` method introduced in php/php-src#12413 to our `DatePoint` class. It comes with a polyfill implementation for PHP 8.2/8.3.

Commits
-------

9e48e0d [Clock] Polyfill DateTimeImmutable::createFromTimestamp()
…ssing `intl` extension (alexandre-daubois)

This PR was merged into the 7.1 branch.

Discussion
----------

[TwigBundle] Don't register emoji extension on missing `intl` extension

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Appveyor is failing a log because Twig bundle tries to load Emoji transliterator even if ext-intl is not present. Snippet of the error:

```
8) Symfony\Bundle\SecurityBundle\Tests\Functional\CsrfFormLoginTest::testFormLoginRedirectsToProtectedResourceAfterLogin with data set #0 (array('CsrfFormLogin', 'config.yml'))
LogicException: You cannot use the "Symfony\Component\Emoji\EmojiTransliterator" class as the "intl" extension is not installed. See https://php.net/intl.

C:\projects\symfony\src\Symfony\Component\Emoji\EmojiTransliterator.php:17
C:\projects\symfony\src\Symfony\Component\ErrorHandler\DebugClassLoader.php:304
C:\projects\symfony\src\Symfony\Bundle\TwigBundle\DependencyInjection\Compiler\ExtensionPass.php:35
C:\projects\symfony\src\Symfony\Component\DependencyInjection\Compiler\Compiler.php:73
C:\projects\symfony\src\Symfony\Component\DependencyInjection\ContainerBuilder.php:750
C:\projects\symfony\src\Symfony\Component\HttpKernel\Kernel.php:495
C:\projects\symfony\src\Symfony\Component\HttpKernel\Kernel.php:732
C:\projects\symfony\src\Symfony\Component\HttpKernel\Kernel.php:120
C:\projects\symfony\src\Symfony\Bundle\FrameworkBundle\Test\KernelTestCase.php:67
C:\projects\symfony\src\Symfony\Bundle\FrameworkBundle\Test\WebTestCase.php:44
C:\projects\symfony\src\Symfony\Bundle\SecurityBundle\Tests\Functional\CsrfFormLoginTest.php:111
```

Commits
-------

1a90ba8 [TwigBundle] Don't register emoji extension on missing `intl` extension
…en if `recipients` is defined in `EnvelopeListener` (lyrixx)

This PR was merged into the 7.1 branch.

Discussion
----------

[Mailer] Add support for allowing some users even if `recipients` is defined in `EnvelopeListener`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | -
| License       | MIT

I'm migrate an application from SwiftMailer to symfony/mailer, and this options was used.

---

depends on symfony#54292

Commits
-------

6228896 [Mailer] Add support for allowing some users even if `recipients` is defined in `EnvelopeListener`
…ent to 8.0 (xabbuh)

This PR was merged into the 7.1 branch.

Discussion
----------

[HttpFoundation] defer addition of new method argument to 8.0

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix symfony#51502 (comment)
| License       | MIT

Commits
-------

8fadaca defer addition of new method argument to 8.0
…RROR` (MatTheCat)

This PR was merged into the 7.1 branch.

Discussion
----------

[Validator] Deprecate `Bic::INVALID_BANK_CODE_ERROR`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Issues        | N/A
| License       | MIT

Follow-up of symfony#54219: now that `Bic::INVALID_BANK_CODE_ERROR` is no longer referenced, it should be deprecated then removed.

Commits
-------

1f73fbd [Validator] Deprecate `Bic::INVALID_BANK_CODE_ERROR`
…bbuh)

This PR was merged into the 7.1 branch.

Discussion
----------

[TwigBundle] re-allow Twig bridge 6.4 and 7.0

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

The change was not necessary in symfony#54432.

Commits
-------

40d2155 re-allow Twig bridge 6.4 and 7.0
…them using `#[Autowire(env: '...')]` depending on the signature of the corresponding parameter
This PR was merged into the 6.4 branch.

Discussion
----------

[Messenger] fix tests

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

Commits
-------

452fac4 fix tests
…efore modifying the date (xabbuh)

This PR was merged into the 6.4 branch.

Discussion
----------

[Clock] initialize the current time with midnight before modifying the date

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix symfony#54497
| License       | MIT

Commits
-------

260b834 initialize the current time with midnight before modifying the date
* 6.4:
  initialize the current time with midnight before modifying the date
  fix tests
  [HtmlSanitizer] Ignore Processing Instructions
…e-daubois)

This PR was merged into the 6.4 branch.

Discussion
----------

[Serializer] Use explicit nullable type

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

A few more for PHP 8.4.

Commits
-------

dfac61f [Serializer] Use explicit nullable type
…izer with MaxDepth enabled (jaydiablo)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Serializer] Fixing PHP warning in the ObjectNormalizer with MaxDepth enabled

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix symfony#54378
| License       | MIT

This fixes a specific issue I ran into when testing an application in Symfony 6 that uses the rompetompe/inertia-bundle to serialize PHP data to JSON for use by Inertia.js.

Added a test that is as minimal as necessary for the Warning to be thrown. I tested against Symfony 5.4 as well, but the issue doesn't seem to be present there (could have something to do with 5.4 not having the AttributeLoader).

Commits
-------

31e3bde [Serializer] Fixing PHP warning in the ObjectNormalizer with MaxDepth enabled
* 5.4:
  explicitly mark nullable parameters as nullable
  fix low deps tests
  [HttpKernel] Fix datacollector caster for reference object property
  bug symfony#51578 [Cache] always select database for persistent redis connections
  [Security] Validate that CSRF token in form login is string similar to username/password
  [validator] validated Dutch translation
  Improve dutch translations
  [Translation] Skip state=needs-translation entries only when source == target
  [HttpKernel] Ensure controllers are not lazy
  [Validator] Fill in trans-unit id 113: This URL does not contain a TLD.
  [Validator] added missing Polish translation for unit 113
  [Validator] add missing lv translation
  [HttpClient] Let curl handle transfer encoding
  [Messenger] Make Doctrine connection ignore unrelated tables on setup
  [HttpFoundation] Set content-type header in RedirectResponse
  add translations for the requireTld constraint option message
  [Serializer] Fix unexpected allowed attributes
  [FrameworkBundle] Fix registration of the bundle path to translation
fabpot and others added 9 commits May 7, 2024 08:17
…on fails (valtzu)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Messenger] Don't drop stamps when message validation fails

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| License       | MIT

`ValidationMiddleware` has the same issue as `DispatchAfterCurrentBusMiddleware` did before the [fix](symfony#51468): if message validation fails, stamps added by previous middlewares in the stack are dropped. What this means in practice is:

1. `bin/console messenger:consume --bus=external` receives a message and `AddBusNameStampMiddleware` adds the `BusNameStamp` so that in case of failure it would be routed to a correct bus
2. The message fails validation – the original envelope without the added `BusNameStamp` is sent to failure queue
3. When running `bin/console messenger:failed:retry`, the message is dispatched on wrong bus (the default one)

This has really bad implications if you handle the message differently depending on which bus it is received.

Some refactoring was done to reduce duplication, similar to `WrappedExceptionsTrait`/`WrappedExceptionsInterface`.

Commits
-------

fed32dc [Messenger] Don't drop stamps when message validation fails
* 6.4:
  [Messenger] Don't drop stamps when message validation fails
  [WebProfilerBundle] Fix assignment to constant variable
  [Mailer] [Sendgrid] Use DataPart::getContentId() when DataPart::setContentId() is used.
* 7.0:
  [Messenger] Don't drop stamps when message validation fails
  [WebProfilerBundle] Fix assignment to constant variable
  [Mailer] [Sendgrid] Use DataPart::getContentId() when DataPart::setContentId() is used.
This PR was merged into the 7.1 branch.

Discussion
----------

[Messenger] Fix missing import

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

[Fixes CI](https://github.com/symfony/symfony/actions/runs/8980645540/job/24664656941#step:8:1304)

Commits
-------

36a1636 [Messenger] Remove ContainerInterface mock
…nterface` internal (valtzu)

This PR was merged into the 7.1 branch.

Discussion
----------

[Messenger] Don't mark `EnvelopeAwareExceptionInterface` internal

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| License       | MIT

As discussed [here](symfony#54842 (comment)), let's expose `EnvelopeAwareExceptionInterface` to allow custom Messenger middlewares throw a custom exception without causing stamps from previous middlewares being dropped.

Commits
-------

7713fd1 Don't mark EnvelopeAwareExceptionInterface internal
>
>
Co-authored-by: xopino <madrona.91196@gmail.com>
>
>
Co-authored-by: xopino <madrona.91196@gmail.com>
@carsonbot carsonbot added this to the 7.0 milestone May 28, 2024
@MindfulPol MindfulPol changed the base branch from 7.0 to 7.1 May 28, 2024 22:06
@carsonbot carsonbot changed the title fix / avoid collision between ldap and normal authenticators [SecurityBundle] fix / avoid collision between ldap and normal authenticators May 29, 2024
@OskarStark OskarStark changed the title [SecurityBundle] fix / avoid collision between ldap and normal authenticators fix / avoid collision between ldap and normal authenticators May 29, 2024
@OskarStark OskarStark changed the title fix / avoid collision between ldap and normal authenticators [SecurityBundke] avoid collision between ldap and normal authenticators May 29, 2024
@xabbuh xabbuh changed the title [SecurityBundke] avoid collision between ldap and normal authenticators [SecurityBundle] avoid collision between ldap and normal authenticators May 29, 2024
@MindfulPol MindfulPol changed the base branch from 7.1 to 5.4 June 1, 2024 18:23
@MindfulPol
Copy link
Author

The bug should be resolved on the branch it was found, I'm sorry but going to close this one and open the one with the proper version.

@MindfulPol MindfulPol closed this Jun 1, 2024
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.