Skip to content

Commit ba958e2

Browse files
authored
Fix owner user realm check for API key authentication (#84325)
API Key can run-as since #79809. There are places in the code where we assume API key cannot run-as. Most of them are corrected in #81564. But there are still a few things got missed. This PR fixes the methods for checking owner user realm for API key. This means, when API Keys "running-as" (impersonating other users), we do not expose the authenticating key ID and name to the end-user such as the Authenticate API and the SetSecurityUseringest processor. Only the effective user is revealed, just like in the regular case of a realm user run as. For audit logging, the key's ID and name are not exposed either. But this is mainly because there are no existing fields suitable for these information. We do intend to add them later (#84394) because auditing logging is to consumed by system admin instead of end-users. Note the resource sharing check (canAccessResourcesOf) also needs to be fixed, this will be handled by #84277
1 parent c0fb9c9 commit ba958e2

File tree

7 files changed

+117
-39
lines changed

7 files changed

+117
-39
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
369369
builder.array(User.Fields.ROLES.getPreferredName(), user.roles());
370370
builder.field(User.Fields.FULL_NAME.getPreferredName(), user.fullName());
371371
builder.field(User.Fields.EMAIL.getPreferredName(), user.email());
372-
if (isAuthenticatedWithServiceAccount()) {
372+
if (isServiceAccount()) {
373373
final String tokenName = (String) getMetadata().get(ServiceAccountSettings.TOKEN_NAME_FIELD);
374374
assert tokenName != null : "token name cannot be null";
375375
final String tokenSource = (String) getMetadata().get(ServiceAccountSettings.TOKEN_SOURCE_FIELD);
@@ -406,7 +406,7 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
406406
}
407407
builder.endObject();
408408
builder.field(User.Fields.AUTHENTICATION_TYPE.getPreferredName(), getAuthenticationType().name().toLowerCase(Locale.ROOT));
409-
if (isAuthenticatedWithApiKey()) {
409+
if (isApiKey()) {
410410
this.assertApiKeyMetadata();
411411
final String apiKeyId = (String) this.metadata.get(AuthenticationField.API_KEY_ID_KEY);
412412
final String apiKeyName = (String) this.metadata.get(AuthenticationField.API_KEY_NAME_KEY);

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@
99

1010
import org.elasticsearch.Version;
1111
import org.elasticsearch.common.bytes.BytesArray;
12+
import org.elasticsearch.common.bytes.BytesReference;
1213
import org.elasticsearch.common.io.stream.BytesStreamOutput;
1314
import org.elasticsearch.common.io.stream.StreamInput;
1415
import org.elasticsearch.common.settings.Settings;
16+
import org.elasticsearch.common.xcontent.XContentHelper;
1517
import org.elasticsearch.test.ESTestCase;
1618
import org.elasticsearch.test.VersionUtils;
19+
import org.elasticsearch.xcontent.ToXContent;
20+
import org.elasticsearch.xcontent.XContentBuilder;
21+
import org.elasticsearch.xcontent.XContentType;
1722
import org.elasticsearch.xpack.core.security.action.service.TokenInfo;
1823
import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType;
1924
import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
@@ -32,16 +37,21 @@
3237
import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
3338
import org.elasticsearch.xpack.core.security.user.XPackUser;
3439

40+
import java.io.IOException;
3541
import java.util.Arrays;
3642
import java.util.EnumSet;
3743
import java.util.HashMap;
3844
import java.util.Locale;
3945
import java.util.Map;
4046
import java.util.Set;
47+
import java.util.function.Consumer;
4148
import java.util.function.Supplier;
4249
import java.util.stream.Collectors;
4350

51+
import static org.hamcrest.Matchers.hasEntry;
52+
import static org.hamcrest.Matchers.hasKey;
4453
import static org.hamcrest.Matchers.is;
54+
import static org.hamcrest.Matchers.not;
4555
import static org.hamcrest.Matchers.nullValue;
4656

4757
public class AuthenticationTests extends ESTestCase {
@@ -283,6 +293,44 @@ public void testDomainSerialize() throws Exception {
283293
}
284294
}
285295

296+
public void testToXContentWithApiKey() throws IOException {
297+
final String apiKeyId = randomAlphaOfLength(20);
298+
final Authentication authentication1 = randomApiKeyAuthentication(randomUser(), apiKeyId);
299+
final String apiKeyName = (String) authentication1.getMetadata().get(AuthenticationField.API_KEY_NAME_KEY);
300+
runWithAuthenticationToXContent(
301+
authentication1,
302+
m -> assertThat(
303+
m,
304+
hasEntry("api_key", apiKeyName != null ? Map.of("id", apiKeyId, "name", apiKeyName) : Map.of("id", apiKeyId))
305+
)
306+
);
307+
308+
final Authentication authentication2 = authentication1.runAs(randomUser(), randomRealmRef(false));
309+
runWithAuthenticationToXContent(authentication2, m -> assertThat(m, not(hasKey("api_key"))));
310+
}
311+
312+
public void testToXContentWithServiceAccount() throws IOException {
313+
final Authentication authentication1 = randomServiceAccountAuthentication();
314+
final String tokenName = (String) authentication1.getMetadata().get(ServiceAccountSettings.TOKEN_NAME_FIELD);
315+
final String tokenType = ServiceAccountSettings.REALM_TYPE
316+
+ "_"
317+
+ authentication1.getMetadata().get(ServiceAccountSettings.TOKEN_SOURCE_FIELD);
318+
runWithAuthenticationToXContent(
319+
authentication1,
320+
m -> assertThat(m, hasEntry("token", Map.of("name", tokenName, "type", tokenType)))
321+
);
322+
323+
final Authentication authentication2 = authentication1.runAs(randomUser(), randomRealmRef(false));
324+
runWithAuthenticationToXContent(authentication2, m -> assertThat(m, not(hasKey("token"))));
325+
}
326+
327+
private void runWithAuthenticationToXContent(Authentication authentication, Consumer<Map<String, Object>> consumer) throws IOException {
328+
try (XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {
329+
authentication.toXContent(builder, ToXContent.EMPTY_PARAMS);
330+
consumer.accept(XContentHelper.convertToMap(BytesReference.bytes(builder), false, XContentType.JSON).v2());
331+
}
332+
}
333+
286334
private void checkCanAccessResources(Authentication authentication0, Authentication authentication1) {
287335
if (authentication0.getAuthenticationType() == authentication1.getAuthenticationType()
288336
|| EnumSet.of(AuthenticationType.REALM, AuthenticationType.TOKEN)
@@ -385,6 +433,8 @@ public static Authentication randomApiKeyAuthentication(User user, String apiKey
385433
final HashMap<String, Object> metadata = new HashMap<>();
386434
metadata.put(AuthenticationField.API_KEY_ID_KEY, apiKeyId);
387435
metadata.put(AuthenticationField.API_KEY_NAME_KEY, randomBoolean() ? null : randomAlphaOfLengthBetween(1, 16));
436+
metadata.put(AuthenticationField.API_KEY_CREATOR_REALM_NAME, AuthenticationField.API_KEY_CREATOR_REALM_NAME);
437+
metadata.put(AuthenticationField.API_KEY_CREATOR_REALM_TYPE, AuthenticationField.API_KEY_CREATOR_REALM_TYPE);
388438
metadata.put(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, new BytesArray("{}"));
389439
metadata.put(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, new BytesArray("""
390440
{"x":{"cluster":["all"],"indices":[{"names":["index*"],"privileges":["all"]}]}}"""));

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,13 +1470,13 @@ private void setThreadContextField(ThreadContext threadContext, String threadCon
14701470
LogEntryBuilder withAuthentication(Authentication authentication) {
14711471
logEntry.with(PRINCIPAL_FIELD_NAME, authentication.getUser().principal());
14721472
logEntry.with(AUTHENTICATION_TYPE_FIELD_NAME, authentication.getAuthenticationType().toString());
1473-
if (authentication.isAuthenticatedWithApiKey()) {
1473+
if (authentication.isApiKey()) {
14741474
logEntry.with(API_KEY_ID_FIELD_NAME, (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY));
14751475
String apiKeyName = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_NAME_KEY);
14761476
if (apiKeyName != null) {
14771477
logEntry.with(API_KEY_NAME_FIELD_NAME, apiKeyName);
14781478
}
1479-
String creatorRealmName = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME);
1479+
final String creatorRealmName = ApiKeyService.getCreatorRealmName(authentication);
14801480
if (creatorRealmName != null) {
14811481
// can be null for API keys created before version 7.7
14821482
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, creatorRealmName);
@@ -1485,11 +1485,15 @@ LogEntryBuilder withAuthentication(Authentication authentication) {
14851485
if (authentication.getUser().isRunAs()) {
14861486
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, authentication.getLookedUpBy().getName())
14871487
.with(PRINCIPAL_RUN_BY_FIELD_NAME, authentication.getUser().authenticatedUser().principal())
1488+
// API key can run-as, when that happens, the following field will be _es_api_key,
1489+
// not the API key owner user's realm.
14881490
.with(PRINCIPAL_RUN_BY_REALM_FIELD_NAME, authentication.getAuthenticatedBy().getName());
1491+
// TODO: API key can run-as which means we could use extra fields (#84394)
14891492
} else {
14901493
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, authentication.getAuthenticatedBy().getName());
14911494
}
14921495
}
1496+
// TODO: service token info is logged in a separate authentication field (#84394)
14931497
if (authentication.isAuthenticatedWithServiceAccount()) {
14941498
logEntry.with(SERVICE_TOKEN_NAME_FIELD_NAME, (String) authentication.getMetadata().get(TOKEN_NAME_FIELD))
14951499
.with(

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,31 +1314,29 @@ AtomicLong getLastEvictionCheckedAt() {
13141314
}
13151315

13161316
/**
1317-
* Returns realm name for the authenticated user.
1318-
* If the user is authenticated by realm type {@value AuthenticationField#API_KEY_REALM_TYPE}
1319-
* then it will return the realm name of user who created this API key.
1317+
* Returns realm name of the owner user of an API key if the effective user is an API Key.
1318+
* If the effective user is not an API key, it just returns the source realm name.
13201319
*
13211320
* @param authentication {@link Authentication}
13221321
* @return realm name
13231322
*/
13241323
public static String getCreatorRealmName(final Authentication authentication) {
1325-
if (authentication.isAuthenticatedWithApiKey()) {
1324+
if (authentication.isApiKey()) {
13261325
return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME);
13271326
} else {
13281327
return authentication.getSourceRealm().getName();
13291328
}
13301329
}
13311330

13321331
/**
1333-
* Returns realm type for the authenticated user.
1334-
* If the user is authenticated by realm type {@value AuthenticationField#API_KEY_REALM_TYPE}
1335-
* then it will return the realm name of user who created this API key.
1332+
* Returns realm type of the owner user of an API key if the effective user is an API Key.
1333+
* If the effective user is not an API key, it just returns the source realm type.
13361334
*
13371335
* @param authentication {@link Authentication}
13381336
* @return realm type
13391337
*/
13401338
public static String getCreatorRealmType(final Authentication authentication) {
1341-
if (authentication.isAuthenticatedWithApiKey()) {
1339+
if (authentication.isApiKey()) {
13421340
return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_TYPE);
13431341
} else {
13441342
return authentication.getSourceRealm().getType();

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public IngestDocument execute(IngestDocument ingestDocument) throws Exception {
140140
}
141141
break;
142142
case API_KEY:
143-
if (authentication.isAuthenticatedWithApiKey()) {
143+
if (authentication.isApiKey()) {
144144
final String apiKey = "api_key";
145145
final Object existingApiKeyField = userObject.get(apiKey);
146146
@SuppressWarnings("unchecked")

0 commit comments

Comments
 (0)