Add ECS compliant option for username field

[ywangd]

Add a new option, ecs_compliant, to the SetSecurityUserProcessor configuration. When it is true, it requires the parent field is named as "user" and also save the username as a "name" child field for ECS compliance. The option defaults to true for 8.0 and will be a breaking change. It is possible to opt-out this new behaviour by explicitly set the option to be false so that existing behaviour is retained.

Resolves: #51799

[elasticmachine]

Pinging @elastic/es-security (:Security/Security)

[tvernum]

This is not ideal.

It means if you set
{ "field": "user", "ecs_compliant": true} you get a name field but for { "field": "es_user", "ecs_compliant": true} you get a username field. That's unnecessarily confusing .

My gut-feel preference would be that ecsCompliant mode requires that the field be named "user".
{ "field": "auth", "ecs_compliant": true} should fail as an invalid configuration.

[ywangd]

You are right. It is very confusing. The option is effectively just a "hint". I think tighten the behaviour as you suggested makes sense. Will update. Thanks

[albertzaharovits]

Generally speaking I would abstain from introducing the ecs_compliant configuration option. I believe it's "unatractive" as there's some overhead to learn what it means. I would approach the transition to ECS by simply naming the field differently, eg name instead of username, and having a default value for the field property as user.

In addition, I think we should be mindful that there could be several "users" in an ingest document, so I would follow the recommendation from https://www.elastic.co/guide/en/ecs/current/ecs-user.html#_field_reuse_19 and have a nested user object under a configurable field.

So, to summarize, in 7.x I would simply add a new property name with the same value as username, and in 8 I would create a new user object (containing the name property) under a configurable field, for example ingest.

@ywangd Let me know what you think of this.

[albertzaharovits]

This makes it so that the user can specify the username property when defining the pipeline, but the field is actually named name (in 8 when ecs_compliat is true by default). This is mildly confusing. I would suggest that we add a new property name with the same value as username.

[albertzaharovits]

there are many places with the esc -> ecs inversion (including the PR title).

[ywangd]

Fixed most of them other than the one in the PR branch name. Thanks for spotting this.

[albertzaharovits]

Should the field have a default value of user for 8?

[ywangd]

@albertzaharovits My concern is about backwards compatibility for existing user data. If there is no option to opt-out of this new behaviour, users are forced to reindex their existing data for consistent data structure. This may not be what we want to tell the users?

Therefore I’d like to give users the option to keep existing behaviour even in 8.0 and this is where the ecs_compliant flag comes in. What I’d like to achieve is as the follows:

1. (8.0) The breaking change is turned on by default (ecs_compliant=true) and applies for any newly created processors
2. (8.0) For existing processors, users can opt-out the breaking change (ecs_compliant=false).
3. (7.x) The breaking change is turned off by default (ecs_compliant=false) and users will get deprecating messages if their configurations are not aligned.
4. (7.x) Users can opt-in for the new behaviour (ecs_compliant=true) when creating new processors or if they are willing to re-index their data to prepare for 8.0

Your suggestion of making a nested user object inside the field sounds good to me. It aligns with the ECS recommendation and the internal data structure, i.e. the outer field is authentication not user. The new structure will be something like

{
  "auth": {
    "user": {
      "name": "...",
      "full_name": "...",
      "email": "...",
      "roles": "...",
      "metadata": {}
    },
    "realm": {},
    "api_key": {},
    "authentication_type": "..."
  }
}

In this case, we also need update docs to name the parent field auth instead of user.

[albertzaharovits]

Thank you for describing the ecs_compliant flag ! It makes total sense now.

In this case, we also need update docs to name the parent field auth instead of user.

I believe the parent field is configurable and should stay that way?

This new structure looks more precise, and I prefer it.
But I now wonder, given the extensive changes to the format, whether it would be more appropriate to create a new ingest processor altogether, named, for example "Set Authentication Processor". We would also deprecate the existing processor, and keep it deprecated in 8 and remove it in 9. Ultimately this buys us zero concerns about BWC, in particular no ecs_compliant flag, which I still don't especially like from the admin's perspective.

What are your thoughts about this? I slightly prefer adding a new processor because I believe it's easier on the user to cope with different document structures this way and I hope we can internally reuse the code of the existing processor, but I also think that the approach you've described works well too!
@tvernum might cringe at adding a new processor given #49106 (comment) , although that was a somewhat different discussion where we wanted new user authentication information be included without asking the administrator to configure anything new, but in this case, a change of configuration is required anyway in 8, and I lean for creating a new processor rather than toggling a flag on an existing one.

[ywangd]

I believe the parent field is configurable and should stay that way?

Sorry for the confusion. Yes it is and will stay configurable. What I meant was the example used in the docs page. It configures the field to be named as "user". But we will want to change it to "auth".

BWC indeed needs some more thoughts here. With my proposal, there will still be a problem when user has a mixed cluster of 8.0 and 7.x. Unless users actively do something, an existing processor will be created differently in 8.0 and 7.x since they have different default for ECS compliance. Even though we can put forward upgrade instruction, the chance of user ignoring it and running into this problem is still non-negligible. In fact, even for newly created processors, they will also still be created differently in a mixed cluster ...

I think the overall requirement for this change should satisfy the follows:

• Can opt-in for 7.x
• Can opt-out for 8.0
• Without any user actions, the default behaviour should be consistent across 8.0 and 7.x
• The default behaviour should not break user data structure consistency without user consent

In light of above discussion, I think creating a seperate processor is reasonable. It could potential duplicate quite a bit code and docs but solves the BWC problem easily and better.

[albertzaharovits]

The scenario of creating ingest processors in mixed cluster is a good point!

[ywangd]

@tvernum Does creating a new processor make sense to you? It does feel like a "big" change given the initial perceived scope of this issue. Here is a list of pros and cons for adding a new processor:

Pros

• Clean bwc solution especially for mixed clusters and has clear path for deprecation and removal
• Better consistency and representation of the actual information (it's auth not just user) and hence future proof

Cons

• A rather big change
• A new processor implies many overhead and duplication including code, test, docs

The alternative is to add a new ecs_compliant flag and let it control the ingestor's behaviour. In addition, version 8.0 will mandate the flag to be set explicity by users while 7.x has a default value of false (for mixed cluster scenario discussed above). If not explicitly set for 8.0, we should at least refuse to create the pipeline to avoid inconsistent data. We could also fail to start the node entirely, but this feels too harsh for users.

Pros

• Smaller change

Cons

• Deprecation and removal path way is less clear. Ideally, the ecs_compliant flag itself should be removed ultimately. So we are adding temporary thing and its removal requires at least another user side cooperation.
• Neither "refuse to create the pipeline" nor "refuse to start the node" is a great solution. Both of them give users a non-completely-functional cluster.

[tvernum]

To be honest, my feeling is that this probably isn't an issue we need to solve at all right now.

It would be nice to have this processor create ECS compliant data, but unless someone is specifically asking for it (thinking mostly of SIEM and/or Obersability-Ingest teams), then maybe it would be better to spend our time elsewhere.

For a lot of use cases, the user that ingests the document is not the most interesting user from an ECS point of view. If I ingest Elasticsearch audit logs using beats, the user in the log messages is the interesting part, not the user that runs beats. I think that's going to be true for almost all SIEM related ingestion and a lot of observability as well.

That said:

• I don't think the mixed-cluster issues are too bad. The upgrade assistant in 7.x can tell people that their processor is in a mode that won't be supported in 8.0, if they run a mixed cluster while ignoring that advice, then they will get bad (inconsistent) output, but it won't break their cluster.
• I think a new processor with the addition of a new root object is a reasonable approach.

[ywangd]

Thanks Tim. I agree it's not a priority. By creating a new processor, the plan is to keep the old one available but deprecated in 8.0 and full removal only for 9.0. It's still quite a while to come. I'll deprioritize it for now. Having a consensus on the approach also allows us to come back to it quickly when needed.

[ywangd]

I am closing this as I don't see it getting picked up again anytime soon. The issue will stay open so we can always come back to it when necessary.