flipboxfactory / saml-sp

SAML Service Provider (SP) Plugin for Craft CMS
https://saml-sp.flipboxfactory.com/
Other
19 stars 5 forks source link

Craftcms update 4.5.11.1 Psr logging error #197

Closed SETU-WEB closed 11 months ago

SETU-WEB commented 11 months ago

Hi Since updating to craft 4.5.11.1 (which fixed a PHP error that occurred due to a conflict with psr/log v3.) I am getting this error which looks related to update

Declaration of flipbox\saml\core\containers\Logger::emergency($message, array $context = []) must be compatible with Psr\Log\LoggerInterface::emergency(Stringable|string $message, array $context = []): void

Heres what in my error log

[24-Nov-2023 15:55:41 Europe/Dublin] PHP Fatal error: Declaration of flipbox\saml\core\containers\Logger::emergency($message, array $context = []) must be compatible with Psr\Log\LoggerInterface::emergency(Stringable|string $message, array $context = []): void in /var/www/vendor/flipboxfactory/saml-core/src/containers/Logger.php on line 91 [24-Nov-2023 15:56:10 Europe/Dublin] PHP Fatal error: Declaration of flipbox\saml\core\containers\Logger::emergency($message, array $context = []) must be compatible with Psr\Log\LoggerInterface::emergency(Stringable|string $message, array $context = []): void in /var/www/vendor/flipboxfactory/saml-core/src/containers/Logger.php on line 91 [24-Nov-2023 15:58:16 Europe/Dublin] PHP Fatal error: Declaration of flipbox\saml\core\containers\Logger::emergency($message, array $context = []) must be compatible with Psr\Log\LoggerInterface::emergency(Stringable|string $message, array $context = []): void in /var/www/vendor/flipboxfactory/saml-core/src/containers/Logger.php on line 91

Craft Cms Pro 4.5.11.1 SAML Service Provider 4.0.5

Thank you for your help.

dsmrt commented 11 months ago

I merged your PR but it looks like it actually broke things. I see craft v4.5.11.1 using psr\log 1.1.4

Are you using a different version than this?

dsmrt commented 11 months ago

I'm following the psr\log 1.1.4 as seen here: https://github.com/php-fig/log/blob/1.1.4/Psr/Log/LoggerInterface.php

superextinct commented 11 months ago

Hi @dsmrt, thanks for having a look at this. A couple of insights to share: Craft requires samdark/yii2-psr-log-target:^1.1.3 as of 4.5.11. Nothing breaks when performing the update with composer itself. However, when updating Craft through its cli command ./craft update craft a couple of things happen:

Perhaps @WITwebmaster can chime in, if this is also his method of updating craft.

I hadn't noticed this before, so I understand my pull request might not be suitable for all environments. Would you consider adding psr/log:^1.0.0 as a root dependency in saml-core?

dsmrt commented 11 months ago

The psr/log constraint is probably a good idea. I'll put that in place. Hopefully that will prevent the unwanted upgrade.

SETU-WEB commented 11 months ago

Hi all I just ran the update via command line, and I am now on version 4.0.6.1 but I'm still getting the same error. I updated to Craft 4 4.5.11.1 a few days ago via command line

dsmrt commented 11 months ago

OK ... I'll do some more testing

dsmrt commented 11 months ago

I bumped the versions in saml-core and saml-sp. You can update saml-sp to 4.0.7. @superextinct I did what you recommended and pinned psr/log in core at 1.1.4. Can y'all give the update a try and let me know how that goes for you?

Thanks for the help!! @WITwebmaster @superextinct

SETU-WEB commented 11 months ago

Ok that seems to have fixed the Psr issue, but I am now getting an error I haven't had before Exception Missing SAMLRequest or SAMLResponse parameter.

Nothing has changed in my setup which was working before

dsmrt commented 11 months ago

That happens when the login process isn't kicked off properly. For example, you maybe refreshing an errored page from before without sending the POST data and the message is missing. Can you try initiating the sso process from the beginning? Initiating sso can be from the Craft Admin/CP using the via button or sometimes your IdP has a button to login from there.

superextinct commented 11 months ago

Looking good on my end, @dsmrt. Thanks for the quick fix!

SETU-WEB commented 11 months ago

Yea its all working now, thanks for your help

dsmrt commented 11 months ago

Thank you both for the help here!

dsmrt commented 6 months ago

Just a heads up. I've added an update for Craft 4 to handle both psr/log v1 and v3.

https://github.com/flipboxfactory/saml-sp/pull/217/files

https://github.com/flipboxfactory/saml-core/tree/4.2.0