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

[Symfony 6] Enable authenticator manager #14283

Merged
merged 6 commits into from
Sep 15, 2022

Conversation

ernestWarwas
Copy link
Contributor

Q A
Branch? 1.12
Bug fix? yes (For Symfony 6)
New feature? no
BC breaks? no
Deprecations? no
Related tickets relates to #14162
License MIT

@probot-autolabeler probot-autolabeler bot added API APIs related issues and PRs. Shop ShopBundle related issues and PRs. labels Sep 7, 2022
GSadee added a commit that referenced this pull request Sep 14, 2022
…oic425, Zales0123)

This PR was merged into the symfony-6 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | symfony-6                 |
| Bug fix?        | no                                                       |
| New feature?    | no                                                       |
| BC breaks?      | no                                                       |
| Deprecations?   | no |
| Related tickets |  replaces #14183, contributes to #13274 |
| License         | MIT                                                          |

The goal of this PR is to make the whole Sylius installable (not working yet!) with Symfony 6 packages. Packages builds are already working after #14287, #14284 and #14285. 

The next steps would be fixing static checks and PHPSpec, Behat tests are waiting for #14283 🖖 


Commits
-------

7686374 Use UserNotFound exception
140d2ac Checking package versions
f0c6234 Fix packages versions
bbf077c Fix packages versions for doctrine/dbal
a436cbb Upgrade main packages to allow Symfony 6
ef011a5 Do not remove conflict but adjust versions
88215a9 Bump knplabs/gaufrette dependency
a36895f Delete unneeded swiftmailer file
1369d6e Hot-fix with symfony mockery container
dc92fa2 Bump mailer bundle dependencies
ef390c1 Make all dependencies consistent
e5e4e16 Update some Sylius packages dependencies
49aadc8 Remove unneedd conflicts
@ernestWarwas ernestWarwas marked this pull request as ready for review September 15, 2022 06:56
@ernestWarwas ernestWarwas requested a review from a team as a code owner September 15, 2022 06:56
@ernestWarwas ernestWarwas changed the base branch from 1.12 to symfony-6 September 15, 2022 07:04
@ernestWarwas ernestWarwas changed the base branch from symfony-6 to 1.12 September 15, 2022 07:05
Copy link
Member

@GSadee GSadee left a comment

Choose a reason for hiding this comment

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

You need to add changes from security.yaml to the UPGRADE file

@probot-autolabeler probot-autolabeler bot added the Maintenance CI configurations, READMEs, releases, etc. label Sep 15, 2022
loic425 and others added 6 commits September 15, 2022 15:05
Revert lexik guard for api

Commented scenario for test purposes

Test if set token to null fixes the scenario

spec fix
[Events] Fixed cart blamin in API

[Spec] Fix for spec

ShopCartBlamerListener reverted
UPGRADE file updated and review changes

fix services
Comment on lines +43 to +75
```diff
security:
- always_authenticate_before_granting: true
+ enable_authenticator_manager: true
```

and you need to adjust all of your firewalls like that:

```diff
admin:
# ...
form_login:
# ...
- csrf_token_generator: security.csrf.token_manager
+ enable_csrf: true
# ...
new_api_admin_user:
# ...
- anonymous: true
+ entry_point: jwt
# ...
- guard:
# ...
+ jwt: true
```

and also you need to adjust all of your access_control like that:

```diff
- - { path: "%sylius.security.admin_regex%/forgotten-password", role: IS_AUTHENTICATED_ANONYMOUSLY }

+ - { path: "%sylius.security.admin_regex%/forgotten-password", role: PUBLIC_ACCESS }
```
Copy link
Contributor

Choose a reason for hiding this comment

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

In that way, pluses and minuses in diff blocks may not be properly highlighted in the markdown file. It's not a big deal but just wanted to mention it ;)

image

@GSadee GSadee merged commit de79fcd into Sylius:1.12 Sep 15, 2022
@GSadee
Copy link
Member

GSadee commented Sep 15, 2022

Thanks, Ernest! 🎉

Zales0123 referenced this pull request Sep 20, 2022
… in security.yaml (GSadee)

This PR was merged into the 1.12 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.12|
| Bug fix?        | no                                                       |
| New feature?    | no                                                       |
| BC breaks?      | no                                                       |
| Deprecations?   | no|
| Related tickets | fixes https://github.com/Sylius/Sylius/pull/14283|
| License         | MIT                                                          |

As it does not look like a diff now:
<img width="1032" alt="image" src="https://user-images.githubusercontent.com/6140884/190562242-90b5d014-bc11-48b2-a6df-9b76fb500e45.png">

<!--
 - Bug fixes must be submitted against the 1.11 branch
 - Features and deprecations must be submitted against the 1.12 branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

c577356 [Maintenance] Improve note in UPGRADE file about changes in security.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. Maintenance CI configurations, READMEs, releases, etc. Shop ShopBundle related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants