On 2022-05-19 01:44, Michael Jumper wrote: > On Mon, May 16, 2022 at 12:23 PM Dmitry Katsubo <dm...@mail.ru.invalid> wrote: > > Dear Guacamole users, > Dear Nick, > > Sorry I decided to resurrect the 4-years old challenge. I have rebased my > changes on the latest codebase. Not so many changes are required to allow the > user authenticated via auth-header > extension to be provided authentication information / connection settings > from user-mapping.xml. Without the changes the settings are not picked up > from user-mapping.xml. > > > Is there a specific reason that you cannot use the database? It's intended > for what you describe, intended for production use, and will work with header > auth. I think that database is overkill for systems that have a couple of users (e.g. remote admins). Files are easier to maintain and backup, as all Guacamole configuration is basically located in one place. Also imagine the situation when database is down and could be fixed with help of Guacamole unless it is running on the top of that very database. > > Please check my commit b0aa658 > <https://github.com/dmak/guacamole-client/commit/b0aa658043689b8ff37d18db49a75ac443b4cc12>. > If that is OK, then I would provide few unit tests for it. Otherwise > let me know what is missing, preferably in terms so that I can implement > a test. > > > Looking at your commit, I see that one of the primary changes here is > changing the prototype and visibility of the getAuthorizedConfigurations() > function. This will break API and ABI compatibility, > and I do not think we should do this. You mean that there are classes that extend SimpleAuthenticationProvider which are outside Guacamole git? Could be of course, however their adaptation will be trivial. > For the built-in support for user-mapping.xml to be able to accept the > authentication results of other installed extensions, it will need to be > modified to use the less-simple API and implement > AuthenticationProvider and UserContext (rather than use > SimpleAuthenticationProvider). I think that should be possible. AuthenticationProvider is already implemented, probably not the proper way (if so, what is missing?). As for UserContext I am not sure: none of the providers I've checked implement this interface. Maybe you mean that SimpleUserContext should implement that interface in a proper way (again what exactly is missing)? > With user-mapping.xml really being intended for testing only, and with these > changes aimed at allowing user-mapping.xml to be used in a more complex > configuration aimed at production use, I think > these changes really would need to be coupled with a move to a user-mapping > variant that /is/ intended for production (proper salted hashes for passwords > instead of > intentionally-simplified-for-testing hashes, the ability to define a > user/connection association that requires auth from some other extension and > otherwise has no password, etc.). I think there are two things here mixed. The password which is used to authenticate the user against Guacamole is of course salted hashed and stored in guacamole_user SQL table. However in the setup I have the user is already authenticated by the front Web server, hence the password is null. There is nothing to salt or hash. On the other side the password stored in guacamole_connection_attribute table I believe is saved in plaintext, right? In this respect I don't see what else can be improved in user-mapping.xml which is basically another representation of the data in SQL database. >From another side if the changes I suggest break some other flow that you have >in mind, like proper data flow in conjunction with some other extension – >please let me know how can I reproduce the issue, so that I can improve the code changes I suggested.
Many thanks! -- With best regards, Dmitry
