Upgrade Stepup-RA to run on Symfony 6.4 and PHP 8.2 #313

parijke · 2024-03-13T14:31:23Z

Upgrade the RA application to support Symfony 6.4 and PHP 8.2

Many structural changes have been made. For details, have a look at the commit history of this PR. Some highlights include:

Introduction of PHPStan
All composer dependencies have been upgraded
Reorganisation of the package configuration
Other QA Tooling was updated

…ker based development environment

This will let the logs go to stdout when running as a container, which is the Docker way to send logs

Bumps [symfony/twig-bridge](https://github.com/symfony/twig-bridge) from 4.4.49 to 4.4.51. - [Release notes](https://github.com/symfony/twig-bridge/releases) - [Changelog](https://github.com/symfony/twig-bridge/blob/6.3/CHANGELOG.md) - [Commits](symfony/twig-bridge@v4.4.49...v4.4.51) --- updated-dependencies: - dependency-name: symfony/twig-bridge dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>

Tests started failing since yesterday: ``` yarn install v1.22.19 info No lockfile found. [1/4] Resolving packages... warning @symfony/webpack-encore > webpack-dev-server > webpack-dev-middleware > [email protected]: this will be v4 [2/4] Fetching packages... error @symfony/[email protected]: The engine "node" is incompatible with this module. Expected version ">=16.0.0". Got "14.21.2" ``` I was able to trace to issue back to a new release of webpack-encore: https://github.com/symfony/webpack-encore/releases/tag/v4.5.0

Feature/docker configs

Bumps [phpseclib/phpseclib](https://github.com/phpseclib/phpseclib) from 3.0.19 to 3.0.34. - [Release notes](https://github.com/phpseclib/phpseclib/releases) - [Changelog](https://github.com/phpseclib/phpseclib/blob/master/CHANGELOG.md) - [Commits](phpseclib/phpseclib@3.0.19...3.0.34) --- updated-dependencies: - dependency-name: phpseclib/phpseclib dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [phpseclib/phpseclib](https://github.com/phpseclib/phpseclib) from 3.0.34 to 3.0.37. - [Release notes](https://github.com/phpseclib/phpseclib/releases) - [Changelog](https://github.com/phpseclib/phpseclib/blob/master/CHANGELOG.md) - [Commits](phpseclib/phpseclib@3.0.34...3.0.37) --- updated-dependencies: - dependency-name: phpseclib/phpseclib dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>

…/phpseclib-3.0.37 Bump phpseclib/phpseclib from 3.0.34 to 3.0.37

MKodde

Hi Paul, I absolutely love the work you put in here. Don't be discouraged by my numerous feedback points, they are mostly on the verge of nitpicking.

Non-code review points
Please also review these non-code review points;

You created this PR from master, Not a bad idea at all. We however have some new code on develop. I'd say, try to rebase on develop to get those changes. Or cherry pick those changes after merging this to master. But I much prefer a rebase onto develop.
After merging, we need a new release/6.0 branch based on the develop branch with your changes
Next, we need to merge the release/6.0 branch to master
Finally develop can be removed. Master can be renamed to main, main can be the default branch from that point on.
A lot of PHPStan warnings ended up in the baseline. I trust you did a best effort attemt to fix the issues when possible.
The config/packages/[dev|prod|smoketest|test] folders should be removed from the project after migrating all env specific config. I see that the web_profiler settings are not yet moved to the framework.yaml
I see quite some unused imports in the project. PHPStorm can super easily remove them for you. In the project pane, right click on the src folder: Click Optimize imports. Bob's your uncle.
Please move the src/**/Resources/* to the /config folder. That way we can finally part with that SF 3 convention.
The app folder can be removed now? If you are not certain please run this past @pmeulen or @phavekes.

Next action
My next action is to use the application in the browser. I'm going to check if I can find some regressions there.

.github/workflows/build-push-docker-image.yml

.github/workflows/test-integration.yml

.scrutinizer.yml

CHANGELOG.md

assets/images/header-logo.png

config/routes/dev/jms_translation.yaml

src/Surfnet/StepupRa/RaBundle/Command/RevokeRecoveryTokenCommand.php

src/Surfnet/StepupRa/RaBundle/Command/SearchRecoveryTokensCommand.php

src/Surfnet/StepupRa/RaBundle/Command/SearchSecondFactorAuditLogCommand.php

src/Surfnet/StepupRa/RaBundle/Controller/ProfileController.php

parijke

Some topics need to be discussed

parijke · 2024-03-22T10:20:49Z

5. A lot of PHPStan warnings ended up in the baseline. I trust you did a best effort attemt to fix the issues when possible.

@MKodde No I did not... I was in the assumption that a baseline is set to avoid overwhelming changes on an existing code base. Maybe we can do a better job, but the question is: how far....?

- rector - sf/flex 2+

LevelSetList::UP_TO_PHP_82, ClassPropertyAssignToConstructorPromotionRector::class, SymfonySetList::SYMFONY_44,

Moved them from the bundle to the globa assets folder Updated the references to these files in the webpack config

This fixed the not present vetting type input fields. Which are added based on the programmed locales.

It can be autoconfigured without any issues

That is more in line with the other QA config

The code style sniffer reported some faulty formatting

Some no longer relevant entries were cleaned up And a couple new ones are added. There is insuficiant time to address them now

Security issues are monitored using dependabot on our VCS. And in addition we scan all projects on a daily scedule with our daily-security-check.yml github action

From `src` to `Surfnet\StepupRa` this sticks to the naming convention we stick to in the other stepup projects

- Remove the repository version of the saml bundle. We can rely on the latest actual release now - Upgrade the monitor bundle - Upgrade any other bundle within the set constraints

Pin them to the 6.4 version we built this app on

phavekes · 2024-04-16T12:49:28Z

config/openconext/parameters.yaml.dist

+    saml_remote_idp_sso_url: https://gateway.dev.openconext.local/authentication/single-sign-on
+    saml_remote_idp_certificate: 'MIIDwTCCAqmgAwIBAgIUYuSUugwc4J4NyW9WGqYJ/liwM4owDQYJKoZIhvcNAQELBQAwcDELMAkGA1UEBhMCTkwxEDAOBgNVBAgMB1V0cmVjaHQxEDAOBgNVBAcMB1V0cmVjaHQxJzAlBgNVBAoMHkRldmVsb3BtZW50IERvY2tlciBlbnZpcm9ubWVudDEUMBIGA1UEAwwLR2F0ZXdheSBJRFAwHhcNMjMwNTE3MTIxNTEyWhcNMzMwNTE0MTIxNTEyWjBwMQswCQYDVQQGEwJOTDEQMA4GA1UECAwHVXRyZWNodDEQMA4GA1UEBwwHVXRyZWNodDEnMCUGA1UECgweRGV2ZWxvcG1lbnQgRG9ja2VyIGVudmlyb25tZW50MRQwEgYDVQQDDAtHYXRld2F5IElEUDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAM2ulQVs5WpbJOAf7Cv/VPDTJqbWHVdUxAmdwZJlcNTRKNFVp4aJzQ3dpiyiGghI5odnzU0/BWBoHZFNYPU/OFr/gzn6iJGxL63L9+mFgE8PR9HpkV5TaRnr21+nZ0EXWjDZk9Px0enERicCItTeQzAUJeA0A9miIcK5IKIz/zSBSR3c802SGD/VelUqY7Z2/UJM97cT92L+4Fz+4zhxxoThbPbrR0CweiROIt82grdwg7zf0+b62MOuVtqFh0yPLRAFfLc4LjHuxFUdUvOHVta7x74dwdmHikqfujM10XN+sNns3LDJde2yPWchU6ktq7cjgbYfIW/vzVzafP1Jk40CAwEAAaNTMFEwHQYDVR0OBBYEFGYn6LWRDZa7+YryUncIlwJB2VorMB8GA1UdIwQYMBaAFGYn6LWRDZa7+YryUncIlwJB2VorMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAJ57lcOF6PWWW56mS2s5gKFImtfRFzlfiyHsF14L7+nQ5NjfOhpU0wRpnTjK91KP0wCwlxzGFXR8yfqfBFJryIV7aDdYPH/RIkwVaNBI0fsD/ozlYb18seieDEGLvQtTlrmc0UNHtWz6FW3L2geM3ENaqpOATl1Ywp4EPML7Dh0CbhhyM8PnPCEsdclouIeP5/B9Swfk3omXehof6bkFbntqA03msFBiW50twkfKeKULcJGXo667hto27KNxZUauqtPbnAGpUQmge8nxSQlN8RPwlvygVM4LVMF9qP9YxloTH0xVNwN4noZUhfMNsKoJ7Hg5Xulaok8oCqmzEiSroEg='
+    loa_required_for_login: 'http://dev.openconext.local/assurance/loa3'
+    authentication_context_class_ref: '%loa_required_for_login%'


authentication_context_class_ref seems to be a new config parameter, and probably does the same as loa_required_for_login. Please use the old parameter and drop this, or explain the purpose of this new parameter

Fair point. Merged them. And kept `authentication_context_class_ref

This reverts commit 965cf09.

The locales and hints were not set correctly set when no hints were found for the chosen institution

The saml bundle now listens for the authentication_context_class_ref param. Our config chekcs if it is set by verifying the required_loa config option. Having parameters for both options makes no sense. So I merged them

Remove deprecated set-output commands Phase out ancient (4 years!) create release action

quartje and others added 18 commits June 19, 2023 15:42

Add docker configs and rebuild the .dist files to reflect the new doc…

2e9bf52

…ker based development environment

Configuring the github actions pipelines for this app

4b46699

Adding the dev image and fixing up some naming conventions

cd646f4

Default docker config: Add mailcatcher host

8c2eeb4

Docker config: Make sure all second factors are the same

9e6ae71

Docker config: Add correct loa1.5

fed2a94

Docker: Add monolog configuration when running as a container

d808143

This will let the logs go to stdout when running as a container, which is the Docker way to send logs

Rename loa's to a more standard name

66298e0

Add repo name to slack notification

8c3958a

Merge pull request #306 from OpenConext/feature/docker_configs

d747775

Feature/docker configs

Moving from CMD to ENTRYPOINT

580f29e

Moving back to CMD

fa6a780

Removing the dev image

c0ddfa0

Merge pull request #312 from OpenConext/dependabot/composer/phpseclib…

93de589

…/phpseclib-3.0.37 Bump phpseclib/phpseclib from 3.0.34 to 3.0.37

parijke requested a review from MKodde March 13, 2024 14:54

MKodde requested changes Mar 19, 2024

View reviewed changes

parijke requested a review from MKodde March 22, 2024 10:16

parijke commented Mar 22, 2024

View reviewed changes

MKodde changed the title ~~Feature/symfony6 upgrade~~ Upgrade Stepup-RA to run on Symfony 6.4 and PHP 8.2 Mar 27, 2024

quartje force-pushed the feature/symfony6-upgrade branch 2 times, most recently from 3c3d076 to ecb5c8d Compare March 28, 2024 14:00

parijke added 4 commits April 1, 2024 16:54

Update and install

157cbbc

- rector - sf/flex 2+

Fix deprecations

5a92ca5

1st Rector run

a8f2193

LevelSetList::UP_TO_PHP_82, ClassPropertyAssignToConstructorPromotionRector::class, SymfonySetList::SYMFONY_44,

Rector remove Action suffix

7dbaa0c

MKodde added 16 commits April 11, 2024 14:13

Reorganize less and js files

a85ed10

Moved them from the bundle to the globa assets folder Updated the references to these files in the webpack config

Inject the locales instead of leaving them empty

b3d9af9

This fixed the not present vetting type input fields. Which are added based on the programmed locales.

Remove unused code and cleanup

19bb4aa

Remove RecTokenController definition

799fd65

It can be autoconfigured without any issues

Remove unused method

d0838b4

Let PHPMD output in text format

850e75d

That is more in line with the other QA config

Clean up getTranslation method

36ffd6f

The code style sniffer reported some faulty formatting

Re generate PHPStan baseline

a46feb3

Some no longer relevant entries were cleaned up And a couple new ones are added. There is insuficiant time to address them now

Remove security checker from the project

89e9ec7

Security issues are monitored using dependabot on our VCS. And in addition we scan all projects on a daily scedule with our daily-security-check.yml github action

Remove app and web folder references

9642b70

Run scrutinizer on default-jammy

1e3e018

Rename Kernel namepace

2d8282c

From `src` to `Surfnet\StepupRa` this sticks to the naming convention we stick to in the other stepup projects

Update Composer dependencies

caa2175

- Remove the repository version of the saml bundle. We can rely on the latest actual release now - Upgrade the monitor bundle - Upgrade any other bundle within the set constraints

Remove unused dotenv component

f3a0b3b

Update Symfony version constraints

8708154

Pin them to the 6.4 version we built this app on

Sync lockfile and set a version number

e17a0e6

MKodde force-pushed the feature/symfony6-upgrade branch from 9de0367 to e17a0e6 Compare April 11, 2024 13:21

phavekes requested changes Apr 16, 2024

View reviewed changes

MKodde added 2 commits April 17, 2024 08:37

Revert "Simplify bundle setup"

4fd7771

This reverts commit 965cf09.

Repair vetting type hints

f48e25b

MKodde force-pushed the feature/symfony6-upgrade branch from 4f7f7f6 to f48e25b Compare April 17, 2024 07:53

MKodde added 3 commits April 17, 2024 09:57

Upgrade Composer dependencies

caf2b4b

Repair vetting type hint controller

02a151d

The locales and hints were not set correctly set when no hints were found for the chosen institution

Remove redundant config option

707ad50

The saml bundle now listens for the authentication_context_class_ref param. Our config chekcs if it is set by verifying the required_loa config option. Having parameters for both options makes no sense. So I merged them

MKodde approved these changes Apr 17, 2024

View reviewed changes

MKodde requested a review from phavekes April 17, 2024 09:59

thijskh and others added 2 commits April 17, 2024 14:44

Also the /internal/{info,health} paths must be publically available

03e3e24

GitHub actions workflows:

57d3e0f

Remove deprecated set-output commands Phase out ancient (4 years!) create release action

MKodde merged commit 32749c6 into master May 15, 2024
5 checks passed

MKodde deleted the feature/symfony6-upgrade branch May 15, 2024 08:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Stepup-RA to run on Symfony 6.4 and PHP 8.2 #313

Upgrade Stepup-RA to run on Symfony 6.4 and PHP 8.2 #313

parijke commented Mar 13, 2024 •

edited by MKodde

Loading

MKodde left a comment •

edited

Loading

parijke left a comment

parijke commented Mar 22, 2024 •

edited

Loading

phavekes Apr 16, 2024 •

edited

Loading

MKodde Apr 17, 2024

Upgrade Stepup-RA to run on Symfony 6.4 and PHP 8.2 #313

Upgrade Stepup-RA to run on Symfony 6.4 and PHP 8.2 #313

Conversation

parijke commented Mar 13, 2024 • edited by MKodde Loading

MKodde left a comment • edited Loading

Choose a reason for hiding this comment

parijke left a comment

Choose a reason for hiding this comment

parijke commented Mar 22, 2024 • edited Loading

phavekes Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

MKodde Apr 17, 2024

Choose a reason for hiding this comment

parijke commented Mar 13, 2024 •

edited by MKodde

Loading

MKodde left a comment •

edited

Loading

parijke commented Mar 22, 2024 •

edited

Loading

phavekes Apr 16, 2024 •

edited

Loading