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

Fix validation of REST array parameters. #1768

Merged

Conversation

aaronweeden
Copy link
Contributor

@aaronweeden aaronweeden commented Aug 31, 2023

Description

This PR fixes a bug in the validation of REST parameters in the getParam() method of BaseControllerProvider in which array parameters were not properly caught as being invalid. filter_var() was returning false but only being checked for null.

This PR also fixes a bug in XdmodApplicationFactory with processing array POST data that was exposed when testing the bug above; if non-string data was sent in a POST, it would still attempt to json_decode it. Now, instead, it decodes that data as null.

This PR also adds the ability to make PUT requests with XdmodTestHelper, which is needed by ubccr/xdmod-appkernels#95.

Tests performed

This PR adds integration tests of array parameters for endpoints that call getParam().

Additional integration tests are added in ubccr/xdmod-appkernels#95 and ubccr/xdmod-supremm#350.

I developed the tests incrementally while running in the tools-ext-01.ccr.xdmod.org/xdmod10.0:rockylinux8.5-0.5 Docker container upgraded to the latest XDMoD.

Also, in a Docker container running tools-ext-01.ccr.xdmod.org/xdmod-10.5.0-x86_64:rockylinux8.5-0.3:

  1. Run the following commands:
    export COMPOSER_ALLOW_SUPERUSER=1
    export XDMOD_REALMS='jobs,storage,cloud'
    export XDMOD_IS_CORE=yes
    export XDMOD_INSTALL_DIR=/xdmod
    export XDMOD_TEST_MODE=upgrade
    git clone https://github.com/ubccr/xdmod /xdmod
    cd /xdmod
    openssl genrsa -rand /proc/cpuinfo:/proc/dma:/proc/filesystems:/proc/interrupts:/proc/ioports:/proc/uptime 2048 > /etc/pki/tls/private/localhost.key
    /usr/bin/openssl req -new -key /etc/pki/tls/private/localhost.key -x509 -sha256 -days 365 -set_serial $RANDOM -extensions v3_req -out /etc/pki/tls/certs/localhost.crt -subj "/C=XX/L=Default City/O=Default Company Ltd"
    composer install
    mkdir ~/phpunit
    mkdir /tmp/screenshots
    ~/bin/buildrpm xdmod
    ./tests/ci/bootstrap.sh
    ./tests/ci/validate.sh
    composer install --no-progress
    mv ./configuration/organization.json ./configuration/organization.json.old
    mv ./configuration/portal_settings.ini ./configuration/portal_settings.ini.old
    cp /etc/xdmod/portal_settings.ini ./configuration/portal_settings.ini
    cp /etc/xdmod/organization.json ./configuration/organization.json
    ./tests/ci/scripts/qa-test-setup.sh
    ./tests/regression/runtests.sh
    cd tests/integration
    ../../vendor/bin/phpunit --testsuite default --group UserAdminTest.createUsers --debug > /before.txt
    ../../vendor/bin/phpunit --testsuite default --exclude-group UserAdminTest.createUsers --debug >> /before.txt
    git clone https://github.com/aaronweeden/xdmod -b fix-rest-array-param-validation /xdmod-new
    unalias rm
    rm -r lib
    cp -r /xdmod-new/tests/integration/lib .
    rm -r /usr/share/xdmod/classes/Rest
    cp -r {/xdmod-new,/usr/share/xdmod}/classes/Rest
    ../../vendor/bin/phpunit --testsuite default --group UserAdminTest.createUsers --debug > /after.txt
    ../../vendor/bin/phpunit --testsuite default --exclude-group UserAdminTest.createUsers --debug >> /after.txt
    
  2. Compare /before.txt and /after.txt and make sure the differences are correct:
    1. Different random values for ControllerTest::testEnumTargetAddresses.
    2. New tests for:
      • MetricExplorerTest::testCreateQueryParamValidation.
      • MetricExplorerTest::testUpdateQueryByIdParamValidation.
      • UserControllerProviderTest::testUpdateCurrentUser.
      • AuthenticationControllerProviderTest::testGetIdpRedirect.
      • DashboardControllerProviderTest::testSetLayout.
      • DashboardControllerProviderTest::testGetStatisticsParamValidation.
      • WarehouseControllerProviderTest::testCreateSearchHistory.
      • WarehouseControllerProviderTest::testGetHistoryById.
      • WarehouseControllerProviderTest::testUpdateHistory.
      • WarehouseControllerProviderTest::testDeleteHistory.
      • WarehouseControllerProviderTest::testDeleteAllHistory.
      • WarehouseControllerProviderTest::testGetDimensions.
      • WarehouseExportControllerProviderTest::testCreateRequestParamValidation.
    3. New test arguments for:
      • AdminControllerProviderTest::testResetUserTourViewed.
      • DashboardControllerProviderTest::testSetViewedUserTour.
      • WarehouseControllerProviderTest::testGetSearchHistory.
      • WarehouseControllerProviderTest::testSearchJobs.
      • WarehouseControllerProviderTest::testSearchJobsByPeers.
      • WarehouseControllerProviderTest::testSearchJobsByTimeseries.
      • WarehouseControllerProviderTest::testGetAggregateDataMalformedRequests.
      • WarehouseControllerProviderTest::testGetDimensionValues.
      • WarehouseControllerProviderTest::testGetRawData.
    4. Last group of tests take about 0.25 min more and go from 784 tests -> 877 tests, 7141 assertions -> 7440 assertions.

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

@aaronweeden aaronweeden added the bug Bugfixes label Aug 31, 2023
@aaronweeden aaronweeden added this to the 11.0.0 milestone Aug 31, 2023
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch 2 times, most recently from 7bfda44 to afbc7e5 Compare October 6, 2023 00:30
@aaronweeden aaronweeden changed the base branch from xdmod10.5 to xdmod11.0 October 6, 2023 00:50
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch 4 times, most recently from 49064e0 to f4d5652 Compare October 6, 2023 16:26
@aaronweeden aaronweeden marked this pull request as ready for review October 6, 2023 16:47
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch 2 times, most recently from 4ddeb41 to cd86575 Compare October 13, 2023 21:20
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch from cd86575 to 3b2f8d0 Compare October 19, 2023 13:40
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch from 3b2f8d0 to 17b944f Compare November 7, 2023 18:43
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch from b4f7f2b to ada284d Compare November 20, 2023 21:02
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch 4 times, most recently from b673a8a to ce0f619 Compare December 7, 2023 17:48
@aaronweeden aaronweeden force-pushed the fix-rest-array-param-validation branch from ce0f619 to d4dedd6 Compare December 7, 2023 19:18
@aaronweeden aaronweeden merged commit a7ac07c into ubccr:xdmod11.0 Dec 11, 2023
4 checks passed
@aaronweeden aaronweeden deleted the fix-rest-array-param-validation branch December 11, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants