derricksmith / phpsaml

GLPI Plugin - SAML integration using the Onelogin SAML Library
MIT License
32 stars 24 forks source link

Issue Report: phpsaml Plugin Not Functioning After GLPi Update to Version 10.0.11 #153

Open santanaalmeida91 opened 6 months ago

santanaalmeida91 commented 6 months ago

I am writing to bring to your attention an issue I have encountered with the phpsaml plugin in GLPi.

After updating GLPi to version 10.0.11, the phpsaml plugin seems to have stopped functioning correctly. When attempting to access my GLPi the screen becomes unresponsive (/plugins/phpsaml/front/acs.php), and the expected redirection to the GLPi home page does not occur.

I have thoroughly tested this behavior and can confirm that the issue is consistent after the upgrade to GLPi 10.0.11. It appears to be isolated to the phpsaml plugin, as other functionalities within GLPi remain unaffected.

Thank you for your time and assistance in resolving this matter. I look forward to hearing from you soon.

SilvaFernando commented 6 months ago

Hi @DonutsNL and @derricksmith.

Can you help us?

I think this is a bus when includes.php tries to read the POST variable on acs.php, look:

glpiphplog.WARNING:   *** PHP Warning (2): Undefined global variable $_POST in /usr/share/glpi/inc/includes.php at line 102
  Backtrace :
  plugins/phpsaml/front/acs.php:12                   include()
  public/index.php:82                                require()
NateTL commented 6 months ago

I am also experiencing problems with the plugin with 11.0.11 except my issue is with the initial configuration of the plugin within GLPI. After I fill in all of the Identity Provider Configuration settings and click "update" the screen refreshes with all of the settings removed and an error message at the top which says "Error updating settings. Review field values.". This is my first deployment of GLPI and everything else seems to be working fine but I can't get the phpsaml plugin settings to be saved. Thanks.

DonutsNL commented 6 months ago

I will dive into this comming weekend. If you can share logging and the exact version you are using... that would be instrumental. Rgrds

NateTL commented 6 months ago

Not sure if you are asking me or the others. I am running GLPI 11.0.11 on Ubuntu and 1.2.1 of phpsaml. What logging would you like to see? I'm off on holiday until Jan 8th after today so I might not be able to respond before then. Thanks for looking into this.

DonutsNL commented 6 months ago

Please use the latest version from my repository. That version is way ahead of this one and has various bugfixes. Derrick has been away for some time yet and had not yet reviewed the pull request and he still needs to merge the versions.

https://github.com/DonutsNL/phpsaml

NateTL commented 6 months ago

Thanks - my bad. Didn't realize that I didn't pickup the latest version. Will give that a try today.

efriastech commented 6 months ago

Hi DonutsNL, first I want to thank you for keeping this plugin updated on your branch and hope it gets merged. I also have the same issue after updating GLPI and PHP. I am using your latest version.

It was working fine on GLPI 10.0.10 with PHP 8.0.

I have now updated to GLPI 10.0.11 and also to PHP 8.1 and seem to be redirected to: https://glpi.example.com/plugins/phpsaml/front/acs.php with a white page.

(I have also tested with GLPI 10.0.11 but with PHP 8.0 and have the same results. It appears that the only variable here is GLPI 10.0.11.)

My php-error.log also mentions an issue (PHP 8.0 and PHP 8.1), somewhat different to what SilvaFernando showed previously:

[2023-12-20 20:18:12] glpiphplog.CRITICAL:   
*** Uncaught Exception TypeError: count(): Argument #1 ($value) must be of type Countable|array, string given in /var/www/html/10.0.11/glpi/inc/includes.php at line 102
  Backtrace :
  plugins/phpsaml/front/acs.php:6                    include()
---------------------
Operating System: Oracle Linux Server 8.9
Kernel: Linux 4.18.0-513.5.1.el8_9.x86_64
Server version: Apache/2.4.37 (Oracle Linux)
PHP 8.1.27 (cli) (built: Dec 19 2023 20:35:55) (NTS gcc x86_64)
mysql  Ver 15.1 Distrib 10.6.16-MariaDB
---------------------

I leave this here in the hope it sheds more light into the issue. I'm willing to provide more information, if necessary.

NateTL commented 6 months ago

I echo Emanuel's thanks for your work on this very useful plugin. It's pretty much a requirement for us to be able to roll out GLPI at my company. I'm seeing a couple missing field descriptions and missing dropdown list on the second missing field description. Also, have a warning message "Phpsaml expected 21 configuration items but got 19 items instead triggered update" showing. See attached screenshot for more details.
It could be due to these missing config values but when I attempt to logon via PHP SAML it fails with the following message from Microsoft:

Request Id: 2e4cff13-1922-4ada-bd16-cb0b56185300 Correlation Id: 3f9cb68d-333f-4bf1-bdef-77fdad0254fd Timestamp: 2023-12-20T21:01:49Z Message: AADSTS7500522: XML element 'AuthnContextClassRef' in XML namespace 'urn:oasis:names:tc:SAML:2.0:assertion' in the SAML message must be a URI.

Whenever this happens I see the following in the php-errors.log file:

[2023-12-20 21:06:03] glpiphplog.WARNING: *** PHP Warning (2): Undefined array key "proxied" in /var/www/html/glpi/plugins/phpsaml/inc/phpsaml.class.php at line 190 Backtrace : plugins/phpsaml/inc/phpsaml.class.php:424 PluginPhpsamlPhpsaml::auth() plugins/phpsaml/inc/phpsaml.class.php:271 PluginPhpsamlPhpsaml::ssoRequest() plugins/phpsaml/setup.php:203 PluginPhpsamlPhpsaml->processUserLogin() src/Plugin.php:1696 pluginPhpsamlPostInit() src/Plugin.php:276 Plugin::doHook() inc/includes.php:98 Plugin->init() index.php:87 include()

Thanks!

Screenshot 2023-12-20 155844

SilvaFernando commented 6 months ago

Hi @DonutsNL. I've tested using you plugin version and I have same problem :(

SilvaFernando commented 6 months ago

Hi @DonutsNL again rsrs

In my test environment, was copied the GLPI file inc/includes.php from the 10.0.10 version, I think this is not the best way, but maybe this can be a starting point for you.

DonutsNL commented 6 months ago

Yes the column check indicates that for what ever reason an older version of the database schema is present. it is not updated correctly. This is a known bug that i havent addressed yet. Please backup the configuration and fully reinstall the latest version. If there is still an error, please review the /glpi/files/_log/ errorlog.

Ill try to review any errors this weekend and publish a new version if required

SilvaFernando commented 6 months ago

I tried to change the way acs.php calls inc/includes.php, but I'm not a dev and I couldn't find it.

I've already tried reinstalling and the error doesn't change, maybe it's a very annoying bug.

DonutsNL commented 6 months ago

There should be an error in the GLPI/files/_log/phperror log. Could you review it and share any strange errors triggered within the phpsaml directory?

SilvaFernando commented 6 months ago

Unfornately not, just this: *** PHP Warning (2): Undefined global variable $_POST in /usr/share/glpi/inc/includes.php at line 102 Backtrace : plugins/phpsaml/front/acs.php:12 include() public/index.php:82 require()

sblanchouin commented 6 months ago

Hi, same thing here, I just updated the plugin from your repository. Here the log, when the white screen appear.

[2023-12-22 14:19:08] glpiphplog.CRITICAL: *** Uncaught Exception TypeError: count(): Argument #1 ($value) must be of type Countable|array, string given in /var/www/html/glpi/inc/includes.php at line 102 Backtrace : plugins/phpsaml/front/acs.php:6 include() public/index.php:82 require()

sblanchouin commented 6 months ago

And another one

[2023-12-22 14:22:19] glpiphplog.WARNING: *** PHP Warning (2): Cannot modify header information - headers already sent by (output started at /var/www/html/glpi/plugins/phpsaml/hook.php:113) in /var/www/html/glpi/front/inventory.php at line 113 Backtrace : front/inventory.php:113 header() public/index.php:82 require()

DonutsNL commented 6 months ago

I will look into this somewhere this weekend. The type errors are quite clear and surely is something i need to look into. Rgrds,

AldarisPale commented 6 months ago

Looks like getting rid of the specific error message is relatively simple: change https://github.com/DonutsNL/phpsaml/blob/0bb7abba655a79d21ee58cb329a3ff9d0067f78c/front/acs.php#L4 from $_POST = ''; to $_POST = []; but it looks like this is actually the tip of the iceberg, because after that I got error that samlResponse was not valid and browser gave "CORS Missing Allow Origin" errors, so something else must be broken too. It looks like https://github.com/glpi-project/glpi/commit/d264d9074e71daf59af21b25ce4a1adc9389e9b2#diff-8ee42c88039df1e983715c0eeaf0aaf39a5272132a38f5a4cff9553fc00feb08 might be very related to this issue.

gianlucapellegrini commented 6 months ago

Hi, same issue here after updating to 10.0.11. Error: Uncaught Exception TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in /var/www/glpi/inc/includes.php at line 102 Backtrace : plugins/phpsaml/front/acs.php:12 include()

Thanks!

DonutsNL commented 6 months ago

Hi @gianlucapellegrini @AldarisPale @SilvaFernando @NateTL @santanaalmeida91 @efriastech

I corrected an issue where $_POST was not a countable datatype. Please use the latest version in my repository. https://github.com/DonutsNL/phpsaml.

Also addressed other reported issues.

I did not get an CORS error in my dev environment as i dont send security headers from my config. I would love to get more feedback on that issue if possible. I will also check what GLPI documentation sais about CORS, im not sure its supported.

sblanchouin commented 6 months ago

Hi @DonutsNL, I just update from your repository, all is fine now !

Thanks !

DonutsNL commented 6 months ago

Todo: verify if we can send Access-Control-Allow-Origin: https://idp.domain header for CORS enabled environments generated from phpsaml config, from the plugins front/acs.php and front/meta.php. If so figure out a way to test it as well.

AldarisPale commented 6 months ago

Thanks, @DonutsNL. Working now.

I got error that samlResponse was not valid and browser gave "CORS Missing Allow Origin" errors, so something else must be broken too.

Update about this. I had forgotten that in my test environment I had replaced SAML certificate, so when restoring from DB in order to test migration, it restored the old SAML certificate. Meaning, it "samlResponse was not valid" was indeed a completely valid (little pun here) and correct response.

Lets discuss CORS in a separate issue though ? But when I look at web browser logs now, I don't see CORS log entries anymore. Might have been related to the usage of wrong SAML certificate? I am using Google as idP.

DonutsNL commented 6 months ago

Good to hear its working. CORS is an valid issue. The response is from an foreign source and might very well cause CORS issues. Also using CORS to make sure the response is from an expected idp makes total sense. I agree we should create a separate issue for this (i already added it to my phpsaml2 repository) 😉

NateTL commented 5 months ago

Just wanted to confirm that the fixed version of the plug-in is now working for me. Thanks for all your efforts in getting this running again!

DonutsNL commented 5 months ago

Your most Welcome. Rgrds

PediatricsIT commented 5 months ago

Hi @DonutsNL ,

Unfortunately your version of the plugin is not working for me. First I have issues specifying settings. I try to set the "Requested Authn Context" to X509, but it is not getting saved when I hit Update. Also I am getting a warning that the service provider certificate details do not look valid, although this is the same certificate we were using before the upgrade. Finally, we are using Azure AD as our Identity Provider and the error message from them is "AADSTS7500522: XML element 'AuthnContextClassRef' in XML namespace 'urn:oasis:names:tc:SAML:2.0:assertion' in the SAML message must be a URI."

PediatricsIT commented 5 months ago

BTW, I just want to state that this was working before updating to GLPI 10.0.11. The previous version that was working for us was 10.0.10.

PediatricsIT commented 5 months ago

Below is the XML of the SAML authentication request. I notice that although it is not saving the "Requested Authn Context" X509 setting on the configuration page, it looks like there are repeated entries in the request itself.

<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="ONELOGIN_376e6f0a10829a72775d673c253be7ebd9538a9a" Version="2.0" IssueInstant="2024-01-16T22:45:48Z" Destination="https://login.microsoftonline.com/f6b6dd5b-f02f-441a-99a0-162ac5060bd2/saml2" ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" AssertionConsumerServiceURL="https://help.peds.uw.edu/plugins/phpsaml/front/acs.php"

https://help.peds.uw.edu/ urn:oasis:names:tc:SAML:2.0:ac:classes:X509 urn:oasis:names:tc:SAML:2.0:ac:classes:X509 urn:oasis:names:tc:SAML:2.0:ac:classes:X509 urn:oasis:names:tc:SAML:2.0:ac:classes:X509 urn:oasis:names:tc:SAML:2.0:ac:classes:Password urn:oasis:names:tc:SAML:2.0:ac:classes:X509 urn:oasis:names:tc:SAML:2.0:ac:classes:X509 urn:oasis:names:tc:SAML:2.0:ac:classes:Password urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport urn:oasis:names:tc:SAML:2.0:ac:classes:X509
DonutsNL commented 5 months ago

Are there any messages in the glpi php errorlog? GLPI_ROOT/files/_log/?

PediatricsIT commented 5 months ago

Yes, when it installed. To install it I downloaded from your GitHub project, placed the files in the glpi/plugins directory, then installed and enabled it from the glpi web page. The following errors were written to php-errors.log: [2024-01-17 20:34:28] glpiphplog.WARNING: *** PHP User Warning (512): Usage of "utf8_unicode_ci" charset/collation detected, should be "utf8mb4_unicode_ci" in /var/www/glpi/src/DBmysql.php at line 2142 Backtrace : src/DBmysql.php:2142 trigger_error() src/DBmysql.php:394 DBmysql->checkForDeprecatedTableOptions() src/DBmysql.php:352 DBmysql->doQuery() plugins/phpsaml/hook.php:73 DBmysql->query() src/Plugin.php:922 plugin_phpsaml_install() : Plugin->install() src/Marketplace/Controller.php:546 call_user_func() src/Marketplace/Controller.php:436 Glpi\Marketplace\Controller->setPluginState() ajax/marketplace.php:83 Glpi\Marketplace\Controller->installPlugin() public/index.php:82 require()

[2024-01-17 20:34:28] glpiphplog.WARNING: *** PHP User Warning (512): Usage of signed integers in primary or foreign keys is discouraged, please use unsigned integers instead in glpi_plugin_phpsaml_configs.id. in /var/www/glpi/src/DBmysql.php at line 2180 Backtrace : src/DBmysql.php:2180 trigger_error() src/DBmysql.php:394 DBmysql->checkForDeprecatedTableOptions() src/DBmysql.php:352 DBmysql->doQuery() plugins/phpsaml/hook.php:73 DBmysql->query() src/Plugin.php:922 plugin_phpsaml_install() : Plugin->install() src/Marketplace/Controller.php:546 call_user_func() src/Marketplace/Controller.php:436 Glpi\Marketplace\Controller->setPluginState() ajax/marketplace.php:83 Glpi\Marketplace\Controller->installPlugin() public/index.php:82 require()

[2024-01-17 20:35:01] glpiphplog.WARNING: *** PHP Warning (2): Undefined array key "REQUEST_URI" in /var/www/glpi/plugins/phpsaml/setup.php at line 145 Backtrace : src/Plugin.php:320 plugin_init_phpsaml() src/Plugin.php:272 Plugin::load() inc/includes.php:98 Plugin->init() front/cron.php:44 include()

[2024-01-17 20:35:01] glpiphplog.WARNING: *** PHP Warning (2): Undefined array key "REQUEST_URI" in /var/www/glpi/plugins/phpsaml/setup.php at line 145 Backtrace : src/Plugin.php:320 plugin_init_phpsaml() src/Plugin.php:272 Plugin::load() inc/includes.php:98 Plugin->init() front/cron.php:44 include()

[2024-01-17 20:35:01] glpiphplog.WARNING: *** PHP Warning (2): Undefined array key "REQUEST_URI" in /var/www/glpi/plugins/phpsaml/setup.php at line 161 Backtrace : src/Plugin.php:1696 plugin_post_init_phpsaml() src/Plugin.php:276 Plugin::doHook() inc/includes.php:98 Plugin->init() front/cron.php:44 include()

[2024-01-17 20:35:01] glpiphplog.WARNING: *** PHP Warning (2): Undefined array key "REQUEST_URI" in /var/www/glpi/plugins/phpsaml/setup.php at line 161 Backtrace : src/Plugin.php:1696 plugin_post_init_phpsaml() src/Plugin.php:276 Plugin::doHook() inc/includes.php:98 Plugin->init() front/cron.php:44 include()

PediatricsIT commented 5 months ago

After filling in the configuration data and attempting to login, I got more errors that were mostly about REQUEST_URI being an undefined array key. Here are some of the errors with duplicates being omitted: < [2024-01-17 20:46:01] glpiphplog.WARNING: PHP Warning (2): Undefined array key "REQUEST_URI" in /var/www/glpi/plugins/phpsaml/setup.php at line 161 < Backtrace : < src/Plugin.php:1696 plugin_post_init_phpsaml() < src/Plugin.php:276 Plugin::doHook() < inc/includes.php:98 Plugin->init() < front/cron.php:44 include() < < [2024-01-17 20:46:53] glpiphplog.WARNING: PHP Warning (2): Undefined global variable $_POST in /var/www/glpi/inc/includes.php at line 102 < Backtrace : < plugins/phpsaml/front/acs.php:12 include() < public/index.php:82 require() < < [2024-01-17 20:46:53] glpiphplog.CRITICAL: Uncaught Exception TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in /var/www/glpi/inc/includes.php at line 102 < Backtrace : < plugins/phpsaml/front/acs.php:12 include() < public/index.php:82 require() < < [2024-01-17 20:47:01] glpiphplog.WARNING: PHP Warning (2): Undefined array key "REQUEST_URI" in /var/www/glpi/plugins/phpsaml/setup.php at line 145 < Backtrace : < src/Plugin.php:320 plugin_init_phpsaml() < src/Plugin.php:272 Plugin::load() < inc/includes.php:98 Plugin->init() < front/cron.php:44 include()

DonutsNL commented 5 months ago

Hi @PediatricsIT

I am quite sure you are actually not using the latest version of the plugin.

I am sure because of the following reasons:

  1. The indicated array key is not on line 145 (anymore) its actually on line 153 (see screenshot 1).
  2. The latest version checks if the array_key exists before accessing it and therefore prevents the warning from being cast (see screenshot 1).
  3. The "Undefined global variable $_POST" has been addressed some time ago and should not be cast. (see 2nd screenshot)
  4. The "must be of type Countable|array, null" also has to do with $_POST, an issue also adressed somewhere in the 1st week of this year (see 2nd screenshot).

Make sure to use the latest version from this branch: https://github.com/DonutsNL/phpsaml/tree/master.

Dont worry, It is easy to download the wrong version atm. This is because derrick has not yet made them into 'releases' and i am constantly adding fixes to the code. I know this is not ideal and errorprone.

So please try again with the latest 'draft' from my repo linked above and let me know how it goes. Use the screenshots to validate you are using the latest version.

image image

With kind regards,

PediatricsIT commented 5 months ago

My apologies for the mistake. Using the version cloned from your repository it still isn’t working. The errors in php-errors.log that are generated by installing the plugin and trying to authenticate are as follows:

[2024-01-18 18:01:47] glpiphplog.WARNING: *** PHP User Warning (512): Usage of signed integers in primary or foreign keys is discouraged, please use unsigned integers instead in glpi_plugin_phpsaml_configs.id. in /var/www/glpi/src/DBmysql.php at line 2180 Backtrace : src/DBmysql.php:2180 trigger_error() src/DBmysql.php:394 DBmysql->checkForDeprecatedTableOptions() src/DBmysql.php:352 DBmysql->doQuery() plugins/phpsaml/inc/config.class.php:1244 DBmysql->query() plugins/phpsaml/hook.php:58 PluginPhpsamlConfig::install() src/Plugin.php:922 plugin_phpsaml_install() : Plugin->install() src/Marketplace/Controller.php:546 call_user_func() src/Marketplace/Controller.php:436 Glpi\Marketplace\Controller->setPluginState() ajax/marketplace.php:83 Glpi\Marketplace\Controller->installPlugin() public/index.php:82 require()

[2024-01-18 18:04:24] glpiphplog.WARNING: *** PHP Warning (2): Undefined array key "HTTP_X_FORWARDED_PROTO" in /var/www/glpi/plugins/phpsaml/inc/phpsaml.class.php at line 236 Backtrace : plugins/phpsaml/setup.php:203 PluginPhpsamlPhpsaml->processUserLogin() src/Plugin.php:1696 pluginPhpsamlPostInit() src/Plugin.php:276 Plugin::doHook() inc/includes.php:98 Plugin->init() index.php:87 include() public/index.php:82 require()

The result of trying to authenticate to Azure AD is this message: AADSTS7500522: XML element 'AuthnContextClassRef' in XML namespace 'urn:oasis:names:tc:SAML:2.0:assertion' in the SAML message must be a URI.

Pediatrics IT Staff Department of Pediatrics University of Washington 206-543-7337 email: @.**@.>

From: DonutsNL @.> Sent: Thursday, January 18, 2024 2:18 AM To: derricksmith/phpsaml @.> Cc: Peds Computer Support @.>; Mention @.> Subject: Re: [derricksmith/phpsaml] Issue Report: phpsaml Plugin Not Functioning After GLPi Update to Version 10.0.11 (Issue #153)

Hi @PediatricsIThttps://urldefense.com/v3/__https:/github.com/PediatricsIT__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxIRvqRl4$

I am quite sure you are actually not using the latest version of the plugin.

I am sure because of the following reasons:

  1. The indicated array key is not on line 145 (anymore) its actually on line 153 (see screenshot 1).
  2. The latest version checks if the array_key exists before accessing it and therefore prevents the warning from being cast (see screenshot 1).
  3. The "Undefined global variable $_POST" has been addressed some time ago and should not be cast. (see 2nd screenshot)
  4. The "must be of type Countable|array, null" also has to do with $_POST, an issue also adressed somewhere in the 1st week of this year (see 2nd screenshot).

Make sure to use the latest version from this branch: https://github.com/DonutsNL/phpsaml/tree/masterhttps://urldefense.com/v3/__https:/github.com/DonutsNL/phpsaml/tree/master__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxNWtEbPo$.

Dont worry, It is easy to download the wrong version atm. This is because derrick has not yet made them into 'releases' and i am constantly adding fixes to the code. I know this is not ideal and errorprone.

So please try again with the latest 'draft' from my repo linked above and let me know how it goes. Use the screenshots to validate you are using the latest version.

image.png (view on web)https://urldefense.com/v3/__https:/github.com/derricksmith/phpsaml/assets/97617761/3c7ed0e0-5054-4688-b4f2-1261061f1a31__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxQqdZPQA$ image.png (view on web)https://urldefense.com/v3/__https:/github.com/derricksmith/phpsaml/assets/97617761/42736704-b900-4f59-b015-e79c3ff5f0da__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICux8dPUHRw$

With kind regards,

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/derricksmith/phpsaml/issues/153*issuecomment-1898189632__;Iw!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxe4Z8XFI$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ANUKSPDVPFWVWJQSFNTPOGTYPDZELAVCNFSM6AAAAABAVOYUCOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJYGE4DSNRTGI__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxz2P5mKI$. You are receiving this because you were mentioned.Message ID: @.**@.>>

PediatricsIT commented 5 months ago

I found this while Googling for a solution: After further debugging we have found out that Azure AD does NOT support "Compressed SAML Authentication Requests". Ones we have turned this feature OFF on our side, SSO with SAML started to work as expected.

The web page is here: https://learn.microsoft.com/en-us/answers/questions/293577/azure-ad-saml2-request-rejected-aadsts7500525

Does the plugin use compressed SAML authentication requests, and can that feature be disabled? Pediatrics IT Staff Department of Pediatrics University of Washington 206-543-7337 email: @.**@.>

From: DonutsNL @.> Sent: Thursday, January 18, 2024 2:18 AM To: derricksmith/phpsaml @.> Cc: Peds Computer Support @.>; Mention @.> Subject: Re: [derricksmith/phpsaml] Issue Report: phpsaml Plugin Not Functioning After GLPi Update to Version 10.0.11 (Issue #153)

Hi @PediatricsIThttps://urldefense.com/v3/__https:/github.com/PediatricsIT__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxIRvqRl4$

I am quite sure you are actually not using the latest version of the plugin.

I am sure because of the following reasons:

  1. The indicated array key is not on line 145 (anymore) its actually on line 153 (see screenshot 1).
  2. The latest version checks if the array_key exists before accessing it and therefore prevents the warning from being cast (see screenshot 1).
  3. The "Undefined global variable $_POST" has been addressed some time ago and should not be cast. (see 2nd screenshot)
  4. The "must be of type Countable|array, null" also has to do with $_POST, an issue also adressed somewhere in the 1st week of this year (see 2nd screenshot).

Make sure to use the latest version from this branch: https://github.com/DonutsNL/phpsaml/tree/masterhttps://urldefense.com/v3/__https:/github.com/DonutsNL/phpsaml/tree/master__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxNWtEbPo$.

Dont worry, It is easy to download the wrong version atm. This is because derrick has not yet made them into 'releases' and i am constantly adding fixes to the code. I know this is not ideal and errorprone.

So please try again with the latest 'draft' from my repo linked above and let me know how it goes. Use the screenshots to validate you are using the latest version.

image.png (view on web)https://urldefense.com/v3/__https:/github.com/derricksmith/phpsaml/assets/97617761/3c7ed0e0-5054-4688-b4f2-1261061f1a31__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxQqdZPQA$ image.png (view on web)https://urldefense.com/v3/__https:/github.com/derricksmith/phpsaml/assets/97617761/42736704-b900-4f59-b015-e79c3ff5f0da__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICux8dPUHRw$

With kind regards,

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/derricksmith/phpsaml/issues/153*issuecomment-1898189632__;Iw!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxe4Z8XFI$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ANUKSPDVPFWVWJQSFNTPOGTYPDZELAVCNFSM6AAAAABAVOYUCOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJYGE4DSNRTGI__;!!K-Hz7m0Vt54!gfVcXbhavVWzXDFnPqBQBP5PMTRfSW9EPdM-aAegNrlmQ9UHGnsBUA9787p2U_Vt2XI4wBG1aqM7ICuxz2P5mKI$. You are receiving this because you were mentioned.Message ID: @.**@.>>

DonutsNL commented 5 months ago

Hi @PediatricsIT,

Thanks for your research on this. It shouldnt be to hard to add compression as an configuration option.

Ill see if i can address this issues somewhere this weekend.

DonutsNL commented 5 months ago

For completeness:

"usage of signed integers in primary or foreign keys is discouraged" is not a breaking issue but certainly a weird one. The plugin is actually using the GLPI defaul signing. See screenshot. Even when I enforce it by hardcoding 'unsigned', the warning is still being cast by GLPI. I decided not to further analyse this warning as it will not break (and most certainly never, because this would require 2147483647 +1 stored configurations) the function.

The XPROTO warning is cause by a piece of code allready flagged problematic. Currently not breaking the plugin but certainly not implemented optimally. I decided to address this issue in the new version im writing and leave it be in this version. Hopefully someone else is inspired enough to fix this one ;-) Ill add this issue for reference as well.

image image

DonutsNL commented 5 months ago

For other users facing this issue. Compression can be disabled manually be altering the /inc/config.class.php. Locate the lines in the screenshot somewhere on rules : 60-70 and change the value to 'false';

image

Tobiko88 commented 5 months ago

Hi DonutsNL,

after Updating to the latest (2h ago) i cant configure anything.

Bildschirmfoto 2024-01-19 um 13 48 12

in the Database i can see there is a Backuptable created "backup_glpi_plugin_phpsaml_configs" but only a table with excludes...

Bildschirmfoto 2024-01-19 um 13 52 18

is there any issue in the Updateprocess?

Thanks for help :)

Tobias

DonutsNL commented 5 months ago

My version does require a full reinstall of the plugin because its using the migration object of GLPI and no longer the custom installation done previously.

I am not sure what an update from previous version would do. Please try reinstalling. Do save the backup for reference because it will be cleaned by reinstallation. If this still doesnt work, please share the errorlog /files/_log/php_error.log

Tobiko88 commented 5 months ago

Great, thanks for your comment, looks like it should work!

Tobiko88 commented 5 months ago

Just found the issue that the Requested Authn Context will not be safed in the database, after do it manually, it works fine!

Thanks a lot.

Ssshafi commented 1 week ago

Thanks a lot for all your work on this plugin @DonutsNL . After update to glpi 10.0.15, I have the white page problem (acs.php) and your last version is ok after reinstall and reconfigure the plugin. You should take the ownership of this project.

DonutsNL commented 1 week ago

Hi @Ssshafi

Because of a false positive account suspend here at github and support not responding on the reinstatement request for almost two months, I moved my repositories to Codeberg.

I did a full rewrite of the plugin here: codeberg.org/QuinQuies/glpisaml

This version also downloadable using the glpi marketplace. Simply search for glpisaml.

Rgrds,

Ssshafi commented 1 week ago

Thanks @DonutsNL I will look for it !