Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose lookup of realm domain config by realm id #106424

Merged

Conversation

albertzaharovits
Copy link
Contributor

@albertzaharovits albertzaharovits commented Mar 18, 2024

The scope here is to expose a method (Realms#getRealmRef) that can be used
to retrieve the realm domain assignments for any realm id.

* for the passed in {@link RealmConfig.RealmIdentifier}.
* If the realm is not currently configured, {@code null} is returned.
*/
public @Nullable Authentication.RealmRef getRealmRef(RealmConfig.RealmIdentifier realmIdentifier) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the goal of this PR.
It exposes the domain configuration (the set of realm ids) for a given realm id.
The plan is to use the realm id of the API Key owner to retrieve the domain using this method. Then retrieve the profiles associated to any of the realms in the domain.

Comment on lines +391 to +394
if (FileRealmSettings.TYPE.equals(realmIdentifier.getType())) {
return getFileRealmRef();
} else if (NativeRealmSettings.TYPE.equals(realmIdentifier.getType())) {
return getNativeRealmRef();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kink here is that the name of the native or file realms is irrelevant, when considering the domain configuration, see also

. Moreover there is always exactly one RealmRef for both file and native realms (which may contain the domain conf). The rationale is that the file and native realms always refer to the same corpus of internal users.

Comment on lines +395 to +396
} else if (ReservedRealm.TYPE.equals(realmIdentifier.getType())) {
return getReservedRealmRef();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reserved realm is less relevant in the domain context, because it cannot be assigned to any.

@albertzaharovits albertzaharovits changed the title Realms with lookup by Expose lookup of realm domain config by realm id Mar 19, 2024
@albertzaharovits albertzaharovits added >refactoring :Security/Security Security issues without another label labels Mar 19, 2024
@albertzaharovits albertzaharovits requested a review from n1v0lg March 19, 2024 13:44
@albertzaharovits albertzaharovits marked this pull request as ready for review March 19, 2024 13:44
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Mar 19, 2024
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean!

builder.put("xpack.security.authc.realms.native." + nativeRealmName + ".order", 4);
String fileRealmName = randomFrom("f" + randomAlphaOfLength(8), FileRealmSettings.DEFAULT_NAME);
builder.put("xpack.security.authc.realms.file." + fileRealmName + ".enabled", randomBoolean());
builder.put("xpack.security.authc.realms.file." + fileRealmName + ".order", 5);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional since that's really not the focus of the PR, but for coverage we might also randomly disable the reserved realm via builder.put("xpack.security.authc.reserved_realm.enabled", randomBoolean());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good tip, thanks!
Pushed 2aa73cd.

@albertzaharovits
Copy link
Contributor Author

Thanks for the speedy review, @n1v0lg!

@albertzaharovits albertzaharovits merged commit dbb7847 into elastic:main Mar 20, 2024
18 checks passed
@albertzaharovits albertzaharovits deleted the realms-with-lookup-by-id branch March 20, 2024 08:05
@lkts lkts mentioned this pull request Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>refactoring :Security/Security Security issues without another label Team:Security Meta label for security team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants