GUACAMOLE-1239: Configurable username letter case #956
Conversation
|
Thanks for jumping in on this. Please note:
|
StephanRichter
force-pushed
the
feature/configurable-case
branch
from
dfb95fb to
fdad9c7
Compare
StephanRichter
changed the title
|
Thanks for your suggestions. I re-created the commit basing it on the current main branch. I am well aware, that these changes would need an update to the documentation, too. If someone could point me towards the place to do this… |
|
@StephanRichter Thanks. One other thing - there's already a PR out there that deals with this - #902. While that is my PR, I'm not opposed to someone else stepping in and working on it; however, there's already been a fair amount of discussion about how to handle it, and the other PR deals more broadly with how to handle username comparison across multiple authentication modules, versus the approach of just forcing the username to a given case and assuming everything will handle it correctly from there. The approach you've taken, here, is much simpler, but I'm not sure that it covers all of the possible scenarios of dealing with multiple modules (JDBC + SSO or JDBC + LDAP, for example). I'm happy to have further discussion on the merits of one approach versus another, and I know that the previously mentioned PR has languished a bit, so, again, I'm not opposed to others stepping in to help with this. |
|
@necouchman Hi! I've seen the other PR. Basically, I don't prefer my "solution" above yours. I just wanted to suggest an easier solution, which was developed with JDBC+LDAP in mind. Also this solution does not alter any behaviour, if left unconfigured. To be honest, I did not study the other solution in detail, as I only spotted it when preparing my PR ;) |
| @@ -55,13 +57,22 @@ public class TokenRESTService { | |||
| * Logger for this class. | |||
| */ | |||
| private static final Logger logger = LoggerFactory.getLogger(TokenRESTService.class); | |||
| private static final String DEFAULT_LETTER_CASE = "mixed-case"; | |||
There was a problem hiding this comment.
All constants should be documented.
| Optional.ofNullable(credentials.getUsername()).map(String::toLowerCase).ifPresent(credentials::setUsername); | ||
| break; | ||
| case "upper-case": | ||
| Optional.ofNullable(credentials.getUsername()).filter(un -> !"guacadmin".equals(un)).map(String::toUpperCase).ifPresent(credentials::setUsername); |
There was a problem hiding this comment.
We shouldn't expect any particular username - what is this check intending to accomplish?
|
Closing in favor of #902, which has now been merged. |
|
Damn. Right when I managed to assign some time to work on this. The other approach feels a bit complicated. But nevermind, as long as the Issue is resolved. I'm looking forward to the next release ;) |
This is another option to make guacamole case-insensitive.
More specific, it can be configured, whether usernames shall be treated as uppercase, lowercase or mixed-case.
If that seems reasonable, but needs more work, don't hesitate to ask.