Minervis-GmbH / BigBlueButton-Ilias-Plugin

BigBlueButton Plugin for ILIAS LMS
https://docu.ilias.de/goto_docu_dcl_3342_1_73.html
GNU General Public License v3.0
8 stars 18 forks source link

ilInitialisationGuest::initIlias should be compatible with ilInitialisation::initILIAS() #36

Closed thwien closed 1 year ago

thwien commented 1 year ago

Due to multiple CSS vulnerabilities in Ilias < 7.16 we decided to upgrade to Ilias 7.16 immediately. Unfortunately we got an error message while upgrading related to this BBB plugin:

_PHP Warning: Declaration of ilInitialisationGuest::initIlias($client_id, $clienttoken = '') should be compatible with ilInitialisation::initILIAS() in [...]/ilias/Customizing/global/plugins/Services/Repository/RepositoryObject/BigBlueButton/guest.php on line 40

It seems we have already the lasted version of your BBB plugin, checked with "git pull". We decided to return to previous Ilias version. But due to the urgency of fixing Ilias vulnerabilities and due to necessity of this BBB plugin, we would appreciate if you could have a look to this error message soon and have maybe a fix. Thanks a lot in advance.

Information about the vulnerabilities in Ilias < 7.16:

https://wid.cert-bund.de/portal/wid/securityadvisory?name=WID-SEC-2022-2172 https://docu.ilias.de/goto_docu_pg_140768_35.html

thwien commented 1 year ago

@xJuvi

Thanks a lot for reviewing these code lines. We retried updating Ilias with your patch today but run still in errors unfortunately. Therefore we returned to the previous Ilias version again.

One error is related to the line "use Exception;".

PHP Warning: The use statement with non-compound name 'Exception' has no effect in [...]/ilias/Customizing/global/plugins/Services/Repository/RepositoryObject/BigBlueButton/guest.php on line 5

It seems that this line is not necessary. Therefore I remove it.

I guess you wanted to use \Exception and therefore I added a backslash to the line you throw the exception:

throw new \Exception("There has been an error. Try it again.");

But still this BBB plugin cause problems while composer installation:

# rm -rf libs/vendor/composer

# composer install

Script @php setup/cli.php build-artifacts --yes handling the post-autoload-dump event returned with error code 255 

If I remove [...]/ilias/Customizing/global/plugins/Services/Repository/RepositoryObject/BigBlueButton/ the composer installation finishes completely.

If you need any further information, do not hesitate to ask me.

xJuvi commented 1 year ago

Hey @thwien i removed the "use" statement. That's my fault. Also i correct the exception. Now there shouldn't throw composer errors. For production use, the display error should not be enabled, to is commented this out.

Can you post here more error from composer? The lines before your line, composer should tell you in which file the error exists.

Maybe you can try also ``composer diagnose```to check composer errors. On my testcase, the composer doesn't throw any errors.

Kind regards

thwien commented 1 year ago

@xJuvi Thanks a lot for your reply. I replayed all update steps and applied your current patch. Unfortunately I get still the same composer error.

I am not used to work with composer, so thanks for your advice running "composer diagnose". Output:

# composer diagnose
Do not run Composer as root/super user! See https://getcomposer.org/root for details
Continue as root/super user [yes]? yes
Checking composer.json: WARNING
License "GPL v3" is not a valid SPDX license identifier, see https://spdx.org/licenses/ if you use an open license.
If the software is closed-source, you may use "proprietary" as license.
require.ezyang/htmlpurifier : exact version constraints (4.13.0) should be avoided if the package follows semantic versioning
require.guzzlehttp/psr7 : exact version constraints (1.8.5) should be avoided if the package follows semantic versioning
require.league/flysystem : exact version constraints (1.1.4) should be avoided if the package follows semantic versioning
Checking platform settings: OK
Checking git settings: OK
Checking http connectivity to packagist: OK
Checking https connectivity to packagist: OK
Checking github.com rate limit: OK
Checking disk free space: OK
Composer version: 2.0.9
PHP version: 7.4.33
PHP binary path: /usr/bin/php7.4
OpenSSL version: OpenSSL 1.1.1n  15 Mar 2022
cURL version: 7.74.0 libz 1.2.11 ssl OpenSSL/1.1.1n
zip: extension present, unzip present

Are these warnings relevant? If not, how to get debugging log information about causing this error? I tried also "composer clear-cache" and "composer -vvv install" without any further error hint, just abandoned warnings:

Package imsglobal/lti is abandoned, you should avoid using it. No replacement was suggested.
Package simplesamlphp/simplesamlphp-module-authfacebook is abandoned, you should avoid using it. No replacement was suggested.
Package simplesamlphp/simplesamlphp-module-authwindowslive is abandoned, you should avoid using it. No replacement was suggested.
Package simplesamlphp/simplesamlphp-module-oauth is abandoned, you should avoid using it. No replacement was suggested.
Package simplesamlphp/simplesamlphp-module-riak is abandoned, you should avoid using it. No replacement was suggested.
Package symfony/debug is abandoned, you should avoid using it. Use symfony/error-handler instead.
Package technosophos/libris is abandoned, you should avoid using it. No replacement was suggested.
Package twig/extensions is abandoned, you should avoid using it. No replacement was suggested.
Package php-cs-fixer/diff is abandoned, you should avoid using it. No replacement was suggested.
Package phpunit/php-token-stream is abandoned, you should avoid using it. No replacement was suggested.

Do you have more ideas to suggest? Do you see differences between your and my system?

Here some more information about our environment:

Debian 11 PHP-FPM 7.4.33 (deb.sury.org) MariaDB 10.5.15 Ilias 7.16

xJuvi commented 1 year ago

Hey @thwien i'll build a new test enviroment next week to debug the source code and find the error. Today i added a second patch to PT #38

Composer erro code 255 says that there was a logical error on source code. With your tests that everything is fine when you remove this plugin we can be sure that this plugin has an logical error. But we doesn't use automated tests like PHPUnit, so we must debug all the code manualy to find the error. Maybe it's enought when you remove the guest.php file for debug reasons to check, if you can install your composer deps.

Let me hear from you. This results are interesting for me.

Kind regards

thwien commented 1 year ago

@xJuvi Thank you. I tested removing just guest.php from the plugin directory and composer building runs completely. Than I removed guest.php back into place and patched classes/class.ilBigBlueButtonProtocol.php as you mentioned. I checked classes/class.ilBigBlueButtonProtocol.php related to syntax and everything is fine. But composer building still output an error code 255. How I can debug or log what's wrong with loading this plugin? I hope you can find the error. Thanks a lot in advance.

xJuvi commented 1 year ago

@thwien So i did understand correct, that composer run successful if you delete the 'guest.php' file? Then there was an logical error inside. I'll check the code on the next days. But the whole file is dirty i think. So we need to rebuild some parts. Maybe i find a quick solution to fix this bug.

Thanks for your hints and debug outputs.

thwien commented 1 year ago

@xJuvi Yes, you did understand correctly.

xJuvi commented 1 year ago

@thwien I found the bug this afternoon and will fix it tomorrow. It's an easy logical mistake.

xJuvi commented 1 year ago

@thwien I added two more commits to the PR. Now it works completely. I checked this also on my dev system, there are no more errors on composer and i can join the meeting as a guest.

thwien commented 1 year ago

After adding your code changes I can confirm that now the composer runs successfully on my test machine. I will try to update the production environment very soon. I will let you know about the result. Thanks a lot for your time reviewing the plugin code.

thwien commented 1 year ago

@xJuvi Upgrade is working. Thanks a lot. Great job. Keep up your good work.

Best regards.