Support client credentials oauth for inventory

[cconard96]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-

Adds ability to require OAuth Client Credentials authentication between agent and GLPI for inventory submission. I guess there is already basic authentication support, but I can't see how that is possible (where is it handled?).

This was tested only so far as to check if I get an error regarding the missing token or an error regarding the XML.Needs changes on the agent side. The agent will need to request a token through the new API using the client_credentials grant type while requesting the "inventory" scope (/api.php/token), and then send an Authorization Bearer header with the auth token when submitting inventory.

[orthagh]

I guess there is already basic authentication support

Handled by apache module before or a quick patch on cloud instances. There is no code in glpi inventory plugin (nor glpi) to handle that.

We need to develop this option also.

[g-bougard]

Hi @cconard96

I will add support on the glpi-agent side.

Thank you for this adding ;-)

[cconard96]

I guess there is already basic authentication support

Handled by apache module before or a quick patch on cloud instances. There is no code in glpi inventory plugin (nor glpi) to handle that.

We need to develop this option also.

Ah. Ok.

[cconard96]

Ideally, the inventory front file would redirect requests through the new API to de-duplicate authentication handling, but it didn't seem simple with how the inventory feature has its own response object handling. Adding the authentication manually was a workaround. At some point I'd like to revisit that.

[cedric-anne]

Maybe we should, by default, for new GLPI instances, generate a defaut id:password for inventory basic auth and enable by defaut this authentication method. These id:password could be stored encrypted in the DB in a dedicated config field.

[cconard96]

Old patch we use in cloud instances for basic auth

diff --git a/front/inventory.php b/front/inventory.php
index 2220393648..f8d1b8b184 100644
--- a/front/inventory.php
+++ b/front/inventory.php
@@ -42,6 +42,23 @@ if (!defined('GLPI_ROOT')) {
     include(__DIR__ . '/../inc/includes.php');
 }
 
+if (INVENTORY_AUTH_REQUIRED) {
+    $AUTH_USER = INVENTORY_AUTH_USER;
+    $AUTH_PASS = INVENTORY_AUTH_PASSWORD;
+    header('Cache-Control: no-cache, must-revalidate, max-age=0');
+    $has_supplied_credentials = !(empty($_SERVER['PHP_AUTH_USER']) && empty($_SERVER['PHP_AUTH_PW']));
+    $is_not_authenticated = (
+       !$has_supplied_credentials ||
+       $_SERVER['PHP_AUTH_USER'] != $AUTH_USER ||
+       $_SERVER['PHP_AUTH_PW']   != $AUTH_PASS
+    );
+    if ($is_not_authenticated) {
+       header('HTTP/1.1 401 Authorization Required');
+       header('WWW-Authenticate: Basic realm="Access denied"');
+       exit;
+    }
+}
+
 $conf = new Conf();
 if ($conf->enabled_inventory != 1) {
     die("Inventory is disabled");

Can we implement directly ?

Does basic authentication offer any benefit over the OAuth grant types?

[orthagh]

Does basic authentication offer any benefit over the OAuth grant types?

For existing instances, there is no need to deploy again an agent and it's configuration as it's an already supported option. I agree oauth is a better security, and we can add it's recommended with an warning message above the field when no auth or basic auth is selected.

[g-bougard]

Support has been implemented in glpi-agent in 839d48a.

New configuration parameters supported are oauth-client-id & oauth-client-secret. OAUTH_CLIENT_ID & OAUTH_CLIENT_SECRET are the commandline parameters for Windows MSI Installer.

Next glpi-agent nightly build still can be used to test this PR.

[g-bougard]

Also added these options support to glpi-injector script in d4b2cc72.

[cconard96]

Testing the nightly agent (windows) release with OAuth seemed to work OK for me.

[cedric-anne]

Is it possible to add some kind of end-to-end test for this Oauth connection?

[cconard96]

After rebasing, adding the "inventory" scope in the OAuth client (A PR after this one had actually added scope support) and testing again, this no longer seems to be working. The agent is logging: Failed to request oauth access token: 400 Bad Request

[orthagh]

#17218 broke client cred auth type (found with bisect)

[orthagh]

BTW, we may add a dedicated unit test for auth_type=client_credentials authentication to avoid regressions

[cconard96]

@g-bougard The agent will need to send the scope as a string now rather than an array. I guess it changed that it only accepts a space delimited list of scopes rather than an array.

https://github.com/glpi-project/glpi-agent/commit/839d48a24029838d97e4f4b97dc96e5bb643e317#diff-e0f3713032f5dcb1355a170d57906a6d9b8232f9eb32497dc4a2e36f550011d0R413

[g-bougard]

Hi @cconard96

yes, of course, I just pushed the update: glpi-project/glpi-agent@8b70f24c

Next nightly build will include this fix.

[orthagh]

finally found the last blocking issue with the help with @g-bougard , on the same linux where glpi is installed and the agent executed, we found that we triggers the expiration of the jwt. I guess something like 1h shifting is in the story. I added 1 month interval to the token for test, and we passed with the agent.

Not saying we need to set this kind of interval, but we may set something longer than 1hour (6 ?)

[cconard96]

I tested with the latest nightly and confirm it works properly now.

on the same linux where glpi is installed and the agent executed, we found that we triggers the expiration of the jwt. I guess something like 1h shifting is in the story. I added 1 month interval to the token for test, and we passed with the agent.

I'm not able to understand how this is possible unless there is an issue with timezones on the GLPI/DB side. The expiry date should be set to one hour in the future based on the current timezone (new \DateTimeImmutable())->add(new \DateInterval('PT1H')); (in the library code) and the expiry date is checked using the DB NOW() function. The expiry period could be extended to a few hours, but it may just be hiding the real issue.

[g-bougard]

Indeed, on an Alex manual check, I saw he got an answer telling the token was expired. I was also thinking about a problem like you explained, but that's really weird when this happens on the same computer: I mean GLPI, DB & agent on the same computer (if I understood well Alex case).

But anyway, I also think you should report the token expiration case as error. Actually, on OAuthServerException in handleRequest() you always assume the following error Authorization header required to send an inventory. Can you check the exception reports an expired token and provide a better error in that case ? I think you should even check the Authorization header was really missing and then report a more general OAuth server error if the header was indeed set.

[orthagh]

After a quick discussion with Cédric, the patch (check the issued date with php instead MySQL) we had last week could be applied. It doesn't make performance worse, and it strengthens the iat date comparison.

[trasher]

For sur we would not expect the oAuth feature needs the mysql timezone to be properly setup