Symfony migration cleanup config #2135
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These changes provide the initial migration of XDMoD's rest framework to Symfony
For whatever reason the `/about/*.php` routes would not match, even though we have other routes w/ `.php` in the url. I've updated the about url's that included `.php` to `.html`. I will revisit these changes in the future.
These changes update so that Symfony code logs to /var/log/xdmod/exceptions.log
These are readability changes meant to make it clear to the reader that the roles are in fact xdmod roles which are different then Symfony Roles.
These changes ensures that we have PHP 8.2 installed w/ all of the dependencies in the XDMoD docker container. Once the XDMoD image is updated to PHP 8.2 by default then these can be removed.
This change ensures that we only copy the playwright related files into the playwright container. While it may only save a second or two, it also makes it clear to anybody who works / reads this later that the full xdmod src is not necessary for the Playwright Tests.
It turns out that we needed a few more directories for playwright tests to be happy, these are them.
These are all changes related to account for PHP 8.2 being more strict about types, as well as moving away from having our own logging levels to just using Monolog Level values. `classes/CCR/CCRDBFormatter`: - Updated the type of `$record` to `LogRecord` from `array` so that it matches `Monolog\Formatter\NormalizerFormatter::format`. `classes/CCR/CCRDBHandler`: - Updated the `$level` default value to be `Monolog\Level:Debug` as opposed to out custom `DEBUG` level. - Removed extraneous code / variables from the `write` function. `classes/CCR/CCRLineFormatter`: - Updated the type of `$record` to `LogRecord` from `array` so that it matches `Monolog\Formatter\LineFormatter::format`. `classes/CCR/Log.php`: - Changed from using custom int log levels (0-7) to the int levels that Monolog provides. - Removed the `convertToMonologLevel` and `convertToCCRLevel` functions and their usage since they are no longer needed ( because we're standardizing on Monolog levels now ). `classes/CCR/Loggable.php`: - Removed usage of the `convertToMonologLevel` function. `tests/component/lib/ETL/IngestorTest.php`: - By default Monolog outputs it's log levels in upper case and since we've chosen to go with Monolog defaults elsewhere we are here as well. `tests/component/lib/LoggerTest.php`: - Updated to use default Monolog log levels ( e.g. Upper Case ). - Updated to use the default integer values for Monolog log levels.
`tests/post/lib/Controllers/MetricExplorerControllerTest.php`: - Just removing the `/` from the begining of the URLs to play nice with Symfony routing. `tests/post/lib/Rest/JobViewerTest.php`: - PHP 8.2 does not like dynamically created properties, this change fixes that.
These changes update `src/Security/Helpers/Tokens.php` with the latest logic from `classes/Models/Services/Tokens.php` with one minor modification. The callers of `src/Security/Helpers/Tokens::authenticate` expect that if there is no token present, null will be returned and they can fall back to standard user / password authentication, but the code in `classes/Models/Services/Tokens.php` only ever threw exceptions. I added a `$strict` arguemnt to the `Helpers/Tokens::authenticate` function that is true by default ( which replicates the original behavior of `Services/Tokens` ) but set it to false for all the new rest endpoints that utilized the old `authenticateToken` function. I also removed the `classes/Models/Services/Tokens.php` class as it's no longer used.
This reverts commit a3c558a.
These changes are the minimum required to get our logging to work w/ PHP 8.2 and the latest version of Monolog. They consist of the following: `classes/CCR/CCRDBFormatter.php`: - Updating the type of the `$record` format to `Monolog\LogRecord`. This is to match the function signature of `Monolog\Formatter\NormalizerFormatter` `classes/CCR/CCRLineFormatter.php`: - Updating the type of the `$record` format to `Monolog\LogRecord`. This is to match the function signature of `Monolog\Formatter\LineFormatter`. `classes/CCR/CCRDBHandler.php`: - This code is no longer needed as we do all the formatting for db log records in `CCRDBFormatter`. `classes/CCR/Log.php`: - Since we handle the conversion of the log level in the constructor of `CCRDBHandler`, this code can be removed. `tests/component/lib/ETL/IngestorTest.php`: - The default format for log level in Monolog messages is to be upper case. We previously expected them to be lower case which is likely a hold over from old PSR logging. We've decided to use standards where ever it makes sense / can, hence this change. `tests/component/lib/LoggerTest.php`: - Same reason / changes as for `IngestorTest`.
The functionality contained in this file has been moved to HomeController / index.html.twig.
So, these changes should have been included when removing the last couple of files.
So this is kind of a half-step commit, ideally there should be one basic way (process) to get / authenticate a user. To get to this point will require some additional work. As a step in the right direction I've pulled out the `detectUser` function ( and the funcitons it depends on ) from `libraries/security.php` and moved them to `src/BaseController.php` so that authentication related code is in one place ( that place being the "new" Symfony code ) and the Symfony controllers that still used `detectUser` have been updated to use new `BaseController::detectUser` function. A future commit ( or maybe PR ) will contain the migration of this code to be more Symfony specific.
These changes should address the following asana tasks: - https://app.asana.com/1/14740318781074/project/1211850781356424/task/1211881442164997?focus=true - https://app.asana.com/1/14740318781074/project/1211850781356424/task/1211881442164990?focus=true The problems that were encountered were that certain css files were not present when viewing the page before logging in. The ultimate cause of the problem was that the logic used to determine whether or not a user was logged in had changed. Previously it was based on the presence of the `xdUser` session variable, in the new code it was based on if an XDUser was returned from the `getXDUser` function ( i.e. was a null returned ), which was always the case. The fix was to add a new template variable `user_logged_in` and set it based on if there was an `xdUser` session variable ( and if the user is not public, since the public user can't be logged in ) and then use this new boolean variable where we had previously tested for `user is not null`. After the above fix went in there was a problem w/ SSO Login. The observed behevaior was that SSO Login would be successful, but the sign in link would not change to the logged in users name. The fix for this was to make sure that the `xdUser` session variable is set in `BaseController.php::getXDUser`.
This is caused by `$request->get('logLevels');` returning an array as opposed
into a string, so when we requested it as a string it freaked out. I've gotten
around this by just using request->get('logLevels'); instead of any of the the
get<type>param functions.
Or at least, they do not cause issues when running our normal suite of automated tests or when logging in via SSO locally.
We no longer need to pre-auth a user before serving them the information, the authentication / authorization is a function of the backend route and if a user isn't authorized to view the information then the route is responsible for handling that.
There are some warnings / deprecations that end up here during the upgrade of PHP and related librarires that was causing the CI build to fail. Since these messages are not actually related to the running of XDMoD I've chosen to truncate the file.
APP_SECRET population will be moved so that it's handled as part of the General Setup ( option 1 in xdmod-setup ).
These changes update the way that the `.env` file is generated. - `.env` is now excluded from being included when building an rpm or source install. - `.env` is now created using a new template `templates/env.template` at the end of the `1. General Setup` process in `xdmod-setup`. - We also use Symfony to pre-compile the `.env` file so that it's not read on every request. The xdmod-setup-start.tcl file has been updated to take this second file generation into account.
This change allows for the `bin/console` file to be used while installed via RPM or Source *and* while doing development work within the XDMoD git repo.
SRI is subresource integrity and informs the browser to check the requested script resource against the provided cryptographic hash. Here is the page used as a reference for changes: https://developer.mozilla.org/en-US/docs/Web/Security/Defenses/Subresource_Integrity and here is the url where the error can be seen: https://sonarcloud.io/project/security_hotspots?id=ubccr_xdmod&pullRequest=2097&sinceLeakPeriod=true
Fixing issue found here: https://app.circleci.com/pipelines/github/ubccr/xdmod/4752/workflows/29b1a380-8ab1-4cb5-872d-943b90d9ca53/jobs/12063 by replacing `strpos` usage ( which does not allow null haystacks ) to `str_contains` which does.
ryanrath
force-pushed
the
symfony-migration_cleanup-config
branch
from
bcfbcea to
916b42b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
Tests performed
Checklist: