#560 SeedDMS_Core_DMS->addUser() / $role argument confusion

[Nicolas DUFAILLY]

Hi,

Related to investigations achieved by a sucessful SSO setup with SeedDMS 6.0.25 and Keycloak (hints added to this thread : https://sourceforge.net/p/seeddms/discussion/general/thread/9ee1b010/) ;

One of the amendments being made in OIDC-Extension's "ext/oidc/class.oidc.php" is related to a call to SeedDMS_Core_DMS->addUser(file location pear/vendor/seeddms/core/Core/inc.ClassDMS.php) that is confusing :

Indeed, comment attached to this method give following hint for $role parameter : "(can be 0=normal, 1=admin, 2=guest)"

However, this don't match then while calling SeedDMS_Core_Role::getInstance (file location pear/vendor/seeddms/core/Core/inc.ClassUser.php). Indeed, 0/1/2 values matches the tblRoles.role column, but not the tblRoles.id which is queried by this method.

I think the 0/1/2 was coming from previous versions of SeedDMS when roles didn't exists (dont know for 5.x, direclty jumping from 4.x to 6.x).

So the comment should probably changed accordingly by something to "(can be 3=normal, 1=admin, 2=guest)", to be aligned with the default provided data after a fresh setup.

In addition, to avoid being limited by the 3 default roles, I would suggest the following change as well :
Considering SeedDMS_Core_DMS->addUser() is already flexible, accepting both string and int ;
Considering as well SeedDMS_Core_Role::getInstance() has an optional "$by" argument to query about role by its name ;
Would it be possible that, if parameter sent to Considering SeedDMS_Core_DMS->addUser() is of string type, the subsequent call to SeedDMS_Core_Role::getInstance() would be made with '$by = "name"' ; whereas called with '$by = "id"/null' when called with an int ?
Thus, features that would require using addUser (as OIDC-Extension) could be able to select an appropriate role whatever default provided or not (and even for default provided, using more meaningful naming).
Don't know however if this can break currenlty existing implementations.

[Uwe Steinmann]

It looks like the extension your are using it just outdated and not official. Where did you get it from?

[Nicolas DUFAILLY]

I found it using one of the external resources' links on bottom of https://www.seeddms.org/download/ page.

[Nicolas DUFAILLY]

Hi,
After looking more in details at SeedDMS's internal while pushing further the OIDC-Extension, it appears the proposed change at the end is no longer necessary. Indeed, SeedDMS_Core_DMS->addUser() can already accept a role object, and also has useful methods to find role by name upstream in client code.
For this ticket, would so only remains to solve the small discrepancy between SeedDMS_Core_DMS->addUser's method comment and it's implementation. Clearly not a priority (bet as well as simple to implement as changing a comment). PLease so close the ticket at your convenience.