On 2019-03-22 01:54, Nick Couchman wrote: > On Thu, Mar 21, 2019 at 8:38 PM Dmitry Katsubo <[email protected] > <mailto:[email protected]>> wrote: > > Nick, if you check SimpleAuthenticationProvider.authenticateUser():142 > <https://github.com/apache/guacamole-client/blob/7e7b6fde4cd63ac8ec21e2ee900ae865d15a4c36/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/simple/SimpleAuthenticationProvider.java#L142> > you will see that if there are configurations available, user is created > on-the-fly. > Further look into the source code revealed that things are a bit more > complicated. All modules perform user comparison based on the information > from Credentials instance, see for example > UserService.retrieveAuthenticatedUser():361 > <https://github.com/apache/guacamole-client/blob/658ce7884695cbe0c04b29f0b6fa365312dbe2fd/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java#L361> > and the only place where this object is created at is > TokenRESTService.getCredentials() > <https://github.com/apache/guacamole-client/blob/c890919d5bbb9ccc8243f04caae07c78a032ef07/guacamole/src/main/java/org/apache/guacamole/rest/auth/TokenRESTService.java#L84>. > That in its turn means that Guacamole cannot create Credentials instance > other than from Authorization: Basic HTTP header, which means that front > webserver/proxy authorization (which is not necessarily HTTP basic > authentication) is not possible. > > > I think I understand what you're saying. To be sure, the header module does > work - it will authenticate a user passed through from a Nginx or httpd > header authentication. However, it will not pass through a password to the > File authentication provider (since there is not usually a password present), > so if the File authentication provider module requires that password in order > to retrieve the configuration, it will fail. Maybe this is what you're > saying. > > > > * FileAuthenticationProvider.java.patch – this one overrides > getUserContext() to enable configuration for > authenticatedUser.getIdentifier(). > * AuthenticatedUser_Authorization.patch – this one injects username > from header to Credentials and allows null passwords. > > If you wish to contribute these you'll need to follow the contribution > procedure for the project, which generally means creating a JIRA issue and > then a pull request. I am OK to contribute one of above patches but you / Guacamole team need to decide which way to go:
* Either we go the way that FileAuthenticationProvider "understands" authenticatedUser.getIdentifier() and allows null passwords for that username. * Or we make HTTPHeaderAuthenticationProvider to set username also to Credentials, but then FileAuthenticationProvider still needs to allow null passwords. So what do we choose? > Yes, we removed the NoAuth module without replacing it. The project > determined that it was not worth continuing to keep it in the code, as the > value was limited and the end-goal of the module - transparently > authenticating users into Guacamole - was possible by several other more > secure means (SSO and parameter tokens, in particular). It's also true that > the header module is very simple - it accepts that a user has been > authenticated up-stream and relies on other modules to provide > configurations. This comes with a security caveat of its own - if you use > the header module it *must* be behind a reasonably secure front-end proxy > that won't allow someone to spoof the header that is then accepted by the > authentication module. There are warnings about this in the manual. I agree. On the other hand, even if we make FileAuthenticationProvider work properly, JDBCAuthenticationProviderModule will still not work, as it requires username/password for authentication against the database. So if there is a need to stack JDBC/LDAP on the top of header authentication, one needs to agree how to enable that. -- With best regards, Dmitry
