Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/174#discussion_r126514786
--- Diff:
extensions/guacamole-auth-header/src/main/java/org/apache/guacamole/auth/header/AuthenticationProviderService.java
---
@@ -73,6 +73,8 @@ public AuthenticatedUser authenticateUser(Credentials
credentials)
String username =
request.getHeader(confService.getHttpAuthHeader());
if (username != null) {
+ // Write username to the credentials object to make
tokenfilter work
--- End diff --
"to make tokenfilter work" sounds like this is a stopgap measure / hack
just to get things working for a very specific case, which isn't really the
kind of change that should ever be made to any part of the Guacamole codebase.
If `TokenFilter` is not working as expected because this value isn't
populated, and the fact that it isn't populated is a bug, then that's a
legitimate reason to make this change, and there's no need to call out the
specific things that break because it's absent. If it's necessary, it's
necessary.
If this truly is a hack, however, it might be worth instead looking into
ways that `TokenFilter` could be modified to not depend in the username in the
`Credentials` object. Grabbing the identifier of the `AuthenticatedUser` might
be an alternative, though I haven't thought through whether that would go
against the defined semantics of the `GUAC_USERNAME` token.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
