NEW #22740 Add OpenID Connect impl

[jeritiana]

See context, use case and implementation details in #22740

[hregis]

@jeritiana Thank you for this contribution, but on the other hand when do you check the existence of the user in Dolibarr?

[jeritiana]

@hregis I don't check the existence of the user inside this module (or at least I didn't think I had to) because it is already handled somewhere else downstream. From my understanding of the authentication documentation, the goal of the login modules is to check the validity of a couple user/password. In the case of OpenID, this is delegated to the OpenID Identity Provider.
Which brings us to the following sequence:

1. The user provides his credentials login/password to the OpenID provider
2. This module checks from the OpenID provider if the authentication went well
3. If the authentication fails: the error message is displayed (e.g. Bad login/password)
   If the authentication succeeds on the OpenID provider: this module returns the login value to the calling function.
   Later, if the user (i.e the returned login) does not exist in Dolibarr, the error message "Unable to find user xxx" is displayed.

[hregis]

@jeritiana yes but you don't check the existence of the user in Dolibarr !

[hregis]

@jeritiana you can check the existing openid login method /~https://github.com/Dolibarr/dolibarr/blob/develop/htdocs/core/login/functions_openid.php

[jeritiana]

@hregis Okay I see what you mean. Isn't this already the purpose of main.inc.php#L869, leading to "User not found, connexion refused" and ErrorCantLoadUserFromDolibarrDatabase? i.e. check the existence of the user in Dolibarr?

Or should we also add this check inside an authentication module?

[hregis]

@jeritiana in my opinion it is better to check if the user exists and is "active" before querying OpenID. In addition it will be necessary to add compatibility with Multicompany, this I can take care of.

[hregis]

@jeritiana and why not have improved the OpenID login method already existing in Dolibarr?

[jeritiana]

@jeritiana in my opinion it is better to check if the user exists and is "active" before querying OpenID. In addition it will be necessary to add compatibility with Multicompany, this I can take care of.

Sure enough, let's try that. I will let you know of any progression. Thanks for taking care of the Multicompany compatibility.

@jeritiana and why not have improved the OpenID login method already existing in Dolibarr?

Because the already existing login method is fully featured for OpenID 2.0. Modifying would be a breaking change for Dolibarr and there would be impacts on users, compared to adding a new next gen OpenID Connect support.

[battosai30]

Line 49 there is an error I think :
(GETPOSTISSET['code'])
should be
(GETPOSTISSET('code'))

[FlorentPoinsaut]

Check if the user exists as in the original openid method

[FlorentPoinsaut]

Hi @jeritiana
Do you want to continue to maintain this PR or would you prefer that I create a new one?
Thank you in advance,

[jeritiana]

Hi @FlorentPoinsaut , I will have a look and integrate the suggested changes. Thanks for the ping!

[philippe-opendsi]

Hi,
For the impatient, we have put this PR in a module available here.
https://git.open-dsi.fr/dolibarr-extension/openidconnect
Working with us with a Lemon-LDAP
ping @eldy
Regards

[eldy]

Thanks @jeritiana
Yes, i confirm as you suggest in the issue ticket that parameters may come from the conf.php as alternative instead of database. But this can be done in a next step (for the moment oauth2 ids are into database...
Goal is to have all param related to security in a layer executed before the code (managing the database access) and so safe from any database readability.