Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

GLPIv10 support? #132

Open
DanielWeeber opened this issue Apr 29, 2023 · 18 comments
Open

GLPIv10 support? #132

DanielWeeber opened this issue Apr 29, 2023 · 18 comments

Comments

@DanielWeeber
Copy link

This does not seem to support GLPIv10. Any chance we can get an update?

@DonutsNL
Copy link
Contributor

Im currently developing against the latest glpi version and created a pull request to merge the latest changes in.

@DonutsNL
Copy link
Contributor

If you cant wait: https://github.com/DonutsNL/phpsaml

its not fully tested yet.

@laszlokertesz
Copy link

It is actually working for version 1.20 if you put everything in the database manually.

@DonutsNL
Copy link
Contributor

DonutsNL commented May 3, 2023

No manual insertion should be required, you should be able to install it and configure it using just the plugins config page. In my development environment (running the latest version of GLPI) it is functioning properly against Azure AD - SAML enterprise App.

@AldarisPale
Copy link

AldarisPale commented May 26, 2023

@DonutsNL can confirm that on GLPI 10.0.7 + latest master from https://github.com/DonutsNL/phpsaml results in empty page for /plugins/phpsaml/front/config.php

Login does seem to work though even with modified #120 (comment) applied on top of https://github.com/DonutsNL/phpsaml (the file has changed to plugins/phpsaml/lib/php-saml/src/Saml2/Utils.php)

Edit: https://glpi-project.org/glpi-9-5-x-will-be-discontinued/ is coming sooner than desired

@AldarisPale
Copy link

The error message is:
glpiphplog.CRITICAL: *** Uncaught Exception TypeError: Argument 1 passed to PluginPhpsamlConfig::requested_authn_context() must be of the type string, null given, called in /var/www/plugins/phpsaml/inc/config.class.php on line 162 in /var/www/plugins/phpsaml/inc/config.class.php at line 889 Backtrace : plugins/phpsaml/inc/config.class.php:162 PluginPhpsamlConfig->requested_authn_context() plugins/phpsaml/front/config.php:53 PluginPhpsamlConfig->showForm() public/index.php:73 require()

@DonutsNL
Copy link
Contributor

I added type safety but was not yet able to test all possible conditions. As a work arround remove the type in de method listed. I.e requested_authn_context(string $var) to requested_authn_context($var)

@AldarisPale
Copy link

Thanks, @DonutsNL .
When I change row 889 at https://github.com/DonutsNL/phpsaml/blob/56a15f3aa923588d27f70c4cd6d00729f0a7b533/inc/config.class.php#L817
from
protected function saml_idp_single_logout_service(string $cValue) : void
to
protected function saml_idp_single_logout_service($cValue) : void then /plugins/phpsaml/front/config.php does open, but with errors:

Phpsaml expected 21 configuration items but got 19 items instead
No valid Ipd certificate details provided or available
The optional Service Provider Certificate is not configured, we strongly recommend that you do and enable strict mode
You are sing version 1.2.1 which is also the latest version

If I enable debug mode in GLPI, the additonal notice appears:
PHP Notice (8): Undefined variable: pCert in .../plugins/phpsaml/inc/config.class.php at line 409.

@DonutsNL
Copy link
Contributor

Thats strange. It seems the db schema was not updated or the update was not called. Ill dive into that. See update.php what schema updates are required and run them manually.

@AldarisPale
Copy link

Thanks, @DonutsNL , for looking into it.
Also noticed a typo at https://github.com/DonutsNL/phpsaml/blob/56a15f3aa923588d27f70c4cd6d00729f0a7b533/install/update.class.php#L301
Instead of
Toolbox::logInFile("phpsaml", "INFO -- PHPSAML upgraded to 1.2.1" . "\n", true); it should probably be
Toolbox::logInFile("phpsaml", "INFO -- PHPSAML upgraded to 1.2.2" . "\n", true);

@DonutsNL
Copy link
Contributor

Yea i prob did not update all versions yet and that also causes the update.php to not function. I was going to add a rerun option if less than expected items where found in the database.

@AldarisPale
Copy link

@DonutsNL one more addition regarding glpi 10 - this time about fusioninventory -> glpi inventory / glpi agent migration.

Here's a patch for a couple-days-old phpsaml version, does not directly apply anymore but it should be easy to fix.
It should handle all combinations of fusioninventory agent/ glpi agent and fusioninventory plugin / glpiinventory plugin.

--- a/plugins/phpsaml/inc/phpsaml.class.php
+++ b/plugins/phpsaml/inc/phpsaml.class.php
@@ -182,13 +182,35 @@
                 $cfgObj             = new PluginPhpsamlConfig();
                 $config                 = $cfgObj->getConfig();

-               // Return false for fusioninventory agents
+               // Return false for fusioninventory agent using 
fusioninventory endpoint
                 if ((strpos($_SERVER['HTTP_USER_AGENT'], 
'FusionInventory-Agent_') !== false) &&
                         (strpos($_SERVER['REQUEST_URI'], 
'/plugins/fusioninventory/'))) {

                         return false;
                 }

+               // Return false for fusioninventory agent using 
glpiinventory endpoint
+               if ((strpos($_SERVER['HTTP_USER_AGENT'], 
'FusionInventory-Agent_') !== false) &&
+                       (strpos($_SERVER['REQUEST_URI'], 
'/plugins/glpiinventory/'))) {
+
+                       return false;
+               }
+
+               // Return false for glpi inventory agent using 
fusioninventory endpoint
+               if ((strpos($_SERVER['HTTP_USER_AGENT'], 'GLPI-Agent_') 
!== false) &&
+                       (strpos($_SERVER['REQUEST_URI'], 
'/plugins/fusioninventory/'))) {
+
+                       return false;
+               }
+
+               // Return false for glpi inventory agent using 
glpiinventory endpoint
+               if ((strpos($_SERVER['HTTP_USER_AGENT'], 'GLPI-Agent_') 
!== false) &&
+                       (strpos($_SERVER['REQUEST_URI'], 
'/plugins/glpiinventory/'))) {
+
+                       return false;
+               }
+
+
                 // Return true for files in Excludes
                 foreach (self::EXCLUDED as $value) {
                         if ((PHP_SAPI === 'cli') || 
strpos($_SERVER['REQUEST_URI'], $value) !== false) {

@DonutsNL
Copy link
Contributor

@AldarisPale
Would it be to big a hussle to create a new issue for this problem (if it doesnt allready exist) including a clear description of the issue (being fixed) by the code snippet?

This will help me and @derricksmith out keeping a good overview of issues and things.

Thx

@AldarisPale
Copy link

@DonutsNL separate issue created: #134

@AldarisPale
Copy link

AldarisPale commented May 30, 2023

@DonutsNL about #132 (comment) - when I downloaded bleeding edge from https://github.com/DonutsNL/phpsaml an upgrade was offered and plugin config page does not result in empty page anymore. Thanks!

The current messages are:
No valid Ipd certificate details provided or available
The optional Service Provider Certificate is not configured, we strongly recommend that you do and enable strict mode
A different version of Phpsaml is marked as latest. Version 1.2.1 was found in the repository, you are running 1.2.2

@DonutsNL
Copy link
Contributor

DonutsNL commented May 30, 2023

Thanks for checking and reporting back at me. This helps me greatly 👍

The messages shown are informational mostly and should allow you to check the sanity of the config a little better. To elaborate a bit more:

No valid Ipd certificate details provided or available

  • The new config class will try to decode the provided certificate and report the decoded certificate's CN and Validity date back. If it wasnt able to do so it will report this message. This is either because the idp certificate string is not yet configured, your PHP installation does not have the SSL module enabled or the provided string isnt correct (and thus could not be decoded).

The optional Service Provider Certificate is not configured, we strongly recommend that you do and enable strict mode

  • The config class did not find a SP certificate. Without this certificate the strict config and additional security options will actually break the configuration. The config should also give an warning if you try to enable strict mode without the SP certificate being present. Im not sure I allready implemented it so that it will not allow you to enable strict without this certificate present.

A different version of Phpsaml is marked as latest. Version 1.2.1 was found in the repository, you are running 1.2.2

  • The config class now checks what version is published by @derricksmith and will report any delta's. You are running bleeding-edge and this version is not yet merged and published by @derricksmith.

I hope you like these additions, suggestions for additional validations are welcome ;-)

@AldarisPale
Copy link

AldarisPale commented May 30, 2023

Thanks, @DonutsNL
I'm debugging the Ipd problem (there's a typo in here btw - it should be IdP).
What's strange is that when I copy certificate from "Service Provider Certificate" field and paste it into text file and then run openssl x509 -in certificate.crt -text -noout, everything is valid. Also, it works in the practical world too.

So not clear why it is complaining, cannot see other error messages either. openssl php extension is installed

@DonutsNL
Copy link
Contributor

DonutsNL commented May 30, 2023

you might be hitting a GLPI filtering issue I experienced whenever the first and initial update didnt go right and values are captured by the GLPI _POST handler. GLPI then replaces all line brakes with secure entities (to litteral \r\n) effectivly breaking the certificate. Breaking it because (if i remember correcly) X509 certificates only allows \n for a line breaks in base64 encoded certificates. I tried to correct the filtering issue post filtering with mixed results, i was considering the alternative capturing the $post in the acs.php before glpi would have a chance to filter stuff out. This might cause CSRF compliancy issues (as the CSRF field also being passed by the form). I was also considering an alternative like uploading the certificate file and storing it as CLOB or BLOB in the config field.

DonutsNL added a commit to DonutsNL/phpsaml that referenced this issue May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants