Authentication under domains

[albertzaharovits]

This makes the ubiquitous Authentication object contain the domain information that will later be used for access control decisions related to ownership.
The domain information is a Set of RealmIdentifiers of the authentication realms configured under the same domain name.

[ywangd]

I scanned through the code and didn't read too closely. I have an overall "design" question:

What do you think if we attach all domain information to RealmRef instead of split them between RealmRef and Authentication? I am also thinking promoting Domain into it's own class. So it's something like:

public record Domain(String name, Set<RealmRef> realmRefs) {}

public class RealmRef {
  String name;
  String type;
  String nodeName;
  @Nullable Domain domain;
}

One advantage of this approach is that we can keep all domain information for both authenticatedBy and lookupBy realms. It is also more expandable if we decide to keep delegate realm info in the future as well. Semantically, domain is also more akin to realm. Another advantage is that it requires no cascading changes to AuthenticationContext.

[elasticsearchmachine]

Hi @albertzaharovits, I've created a changelog YAML for you.

[albertzaharovits]

What do you think if we attach all domain information to RealmRef instead of split them between RealmRef and Authentication? I am also thinking promoting Domain into it's own class.

We discussed it on another channel, and I agreed that it is a good suggestion that I will follow.

[elasticsearchmachine]

Hi @albertzaharovits, I've created a changelog YAML for you.

[ywangd]

This is looking great! Thanks for the iterations! I had quite a few comments. But none of them is critical. Also:

• I suggest we update the PR description to replace realm ref with realm id as well.
• Nothing really to do for this, but I'd like to double click that the equals and/or hashCode method of RealmRef (and in turn Authentication) cannot be relied upon for ownership checking because domain configuration can change independantly.

[ywangd]

Nit: Are these assertions necessary given the Authentication object is directly instantiated here?

[albertzaharovits]

I would like to keep the isAssignedToDomain asserts. I like that they document which authentications are outside domains.

[albertzaharovits]

Test failure https://gradle-enterprise.elastic.co/s/sqmaob7d4novq is already tracked (and muted) #83516 .

[elasticsearchmachine]

Hi @albertzaharovits, I've updated the changelog YAML for you.

[elasticsearchmachine]

Hi @albertzaharovits, I've created a changelog YAML for you.

[albertzaharovits]

🎉

[albertzaharovits]

Like we discussed I'm planing to open follow-up PRs for some refactorings that were reverted by 0ebd996 .