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

Iepg #1460

Closed
wants to merge 44 commits into from
Closed

Iepg #1460

wants to merge 44 commits into from

Conversation

dsi-iepg
Copy link

Q A
License MIT
Packagist

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) December 13, 2022 10:31
@github-actions
Copy link

github-actions bot commented Dec 13, 2022

Thanks for the PR 😍

How to test these changes in your application

  1. Define the SYMFONY_ENDPOINT environment variable:

    # On Unix-like (BSD, Linux and macOS)
    export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes-contrib/flex/pull-1460/index.json
    # On Windows
    SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes-contrib/flex/pull-1460/index.json
  2. Install the package(s) related to this recipe:

    composer req 'symfony/flex:^1.16'
    composer req 'dsi-iepg/cas-connection:^0.1'
  3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

    # On Unix-like (BSD, Linux and macOS)
    unset SYMFONY_ENDPOINT
    # On Windows
    SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

All the PHP classes should be removed.
Entities won't work if the user is using PHP attributes instead of annotations for instance.
The bundle or the maker bundle should generate such classes. Probably a step to add in a post install text.

auto-merge was automatically disabled January 3, 2023 10:45

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 3, 2023 10:45
"src/": "%SRC_DIR%/"
},
"env": {
"#1": "#### Parameters for CAS connection ###",
Copy link
Member

Choose a reason for hiding this comment

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

This one is not needed as Flex already adds some marker.

"#11": "#Where does the user go according to his role ",
"#12": "#The default value is 'cas_dispatcher'",
"CAS_DISPATCHER_NAME": "",
"#13": "#### end of Cas-connection ####"
Copy link
Member

Choose a reason for hiding this comment

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

Can be removed as well

"#7": "#This value is optional",
"#8": "#if it's 'false' you don't use ceretificat",
"#9": "#THIS SETTING IS NOT RECOMMENDED FOR PRODUCTION!",
"CAS_CA": false,
Copy link
Member

Choose a reason for hiding this comment

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

Should be a string as env vars are always strings.

"env": {
"#1": "#### Parameters for CAS connection ###",
"CAS_HOST": "cas-adresse.com",
"#2": "#This value is optional",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"#2": "#This value is optional",
"#2": "# This value is optional",

Same for all commented lines.

access_control:
- { path: ^/admin, roles: ROLE_ADMIN }
- { path: ^/cas-connection/cas-admin, roles: ROLE_ADMIN }
- { path: ^/cas-connection/cas-user, roles: ROLE_USER }
Copy link
Member

Choose a reason for hiding this comment

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

This file should be removed as it will conflict with the one installed by the security component. Instead, you should add post install file to explain the next step about how to configure the bundle.

auto-merge was automatically disabled January 3, 2023 12:48

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 3, 2023 12:49
firewalls:
main:
provider: app_user_provider
custom_authenticator: Iepg\Bundle\Cas\Controller\CasAuthenticator
Copy link
Member

Choose a reason for hiding this comment

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

The whole file must be removed :)

auto-merge was automatically disabled January 3, 2023 13:00

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 3, 2023 13:01
auto-merge was automatically disabled January 3, 2023 13:12

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 3, 2023 13:12
auto-merge was automatically disabled January 4, 2023 07:17

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 4, 2023 07:17
auto-merge was automatically disabled January 4, 2023 07:26

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 4, 2023 07:26
auto-merge was automatically disabled January 4, 2023 07:28

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 4, 2023 07:28
auto-merge was automatically disabled January 4, 2023 15:53

Head branch was pushed to by a user without write access

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 4, 2023 15:53
@OskarStark
Copy link
Contributor

Any progress here or can we close this PR @dsi-iepg

@fabpot fabpot closed this Jan 1, 2024
auto-merge was automatically disabled January 1, 2024 21:14

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants