Open oleks opened 3 years ago
Pinging @elastic/es-security (Team:Security)
Thanks for reporting this issue. I can confirm that it is indeed a bug and we are aware of it. The comma characater is used as a separator to support passing multiple usernames which unfortunately does not work well when the username itself contains a comma. The same problem applies not only to usernames but a few other places as well. Right now the workaround is to filter the full list on the client side.
I tagged this issue with team-discuss
so it is queued for our team discussion and prioritization.
We discussed this. The general feel was that deprecating commas, and marking them as unsupported in the docs is the best option, but we need to have some more conversations internally.
If we do go down that path, we may wish to include other characters in that scope (e.g. /
)
We've reach consensus that we need to go down the path of deprecating and removing some characters.
For compatibility reasons we need to start by deprecation, but it would be helpful if we can allow admins to opt in to it being an error instead.
@ywangd @tvernum there has been a request for clarification about this issue. Does it affect only the native realm, or also those logged in via external authentication providers? In practice, it seems to affect both, but we want to understand if this is really the case or if actually the failure to authenticate external users with commas is being caused by some other problem not explored here. Thanks!
@nerophon This issue affects the GetUsers API. It does not affect authentication. That is, you can still create a user with username ,
and authenticate with it.
By "external authentication providers", do you mean things like SAML idP or custom realm? GetUser API does not apply to either of them. In any case, since you are talking about "failure to authenticate", I suspect the cause is something else. We'll need details of the error to be able to assist further.
@ywangd thanks for the clarification; I've now realised I didn't explain myself properly. The case here is regarding ECE SAML. As per https://github.com/elastic/cloud/issues/85721, that system actually does use the ES Native Realm under the hood, and therefore is limited by this API restriction. So this problem will affect any user of ECE running SAML and having commas in their prinicple
attribute.
Thanks for the additional information @nerophon Indeed, this could cause problem for Cloud SSO if it relies on the GetUser API. Unfortunately, there is no easy workaround for it since filtering on the client side might not be feasible due to the large amount of users. For now I guess documentation for the limitation is probably the way to go.
We have the same problem for the GET roles API as well. I think adopting the same approach for roles as for usernames makes sense.
Docker image:
elasticsearch:7.12.0
Elasticsearch version (
bin/elasticsearch --version
): 7.12.0Plugins installed: []
JVM version (
java -version
): 15.0.1OS version (
uname -a
if on a Unix-like system): Linux 5.4.108 1-NixOS SMP Wed Mar 24 10:26:46 UTC 2021 x86_64 x86_64 x86_64 GNU/LinuxDescription of the problem including expected versus actual behavior:
It is possible to create a user with the username
,
, but trying to retrieve it, you get all the users instead.The create user API states that usernames must be between 1 and 1024 characters long, composed of printable ASCII characters, without leading or trailing spaces. This makes
,
a valid username. However, the behavior of the get users API in response is suspicious.Steps to reproduce:
Start Elasticsearch in a Docker container, with security enabled (I set the password here, as I'm not sure what the default one is):
(Update: I figured out that I could set
xpack.security.enabled
via an environment variable, making the Docker image easier to run.)Then, in another terminal: