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

Fix owner user realm check for API key authentication #84325

Merged
merged 3 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
builder.array(User.Fields.ROLES.getPreferredName(), user.roles());
builder.field(User.Fields.FULL_NAME.getPreferredName(), user.fullName());
builder.field(User.Fields.EMAIL.getPreferredName(), user.email());
if (isAuthenticatedWithServiceAccount()) {
if (isServiceAccount()) {
final String tokenName = (String) getMetadata().get(ServiceAccountSettings.TOKEN_NAME_FIELD);
assert tokenName != null : "token name cannot be null";
final String tokenSource = (String) getMetadata().get(ServiceAccountSettings.TOKEN_SOURCE_FIELD);
Comment on lines -371 to 374
Copy link
Member Author

Choose a reason for hiding this comment

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

Similar to the above: If the service account run-as another user, the service token name and type won't show up in the authentication response. This in theory could be a "breaking change" except we don't have any service account that has "run-as" privilege. So the change here is safe.

Expand Down Expand Up @@ -405,7 +405,7 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
}
builder.endObject();
builder.field(User.Fields.AUTHENTICATION_TYPE.getPreferredName(), getAuthenticationType().name().toLowerCase(Locale.ROOT));
if (isAuthenticatedWithApiKey()) {
if (isApiKey()) {
this.assertApiKeyMetadata();
final String apiKeyId = (String) this.metadata.get(AuthenticationField.API_KEY_ID_KEY);
final String apiKeyName = (String) this.metadata.get(AuthenticationField.API_KEY_NAME_KEY);
Comment on lines -408 to 411
Copy link
Member Author

Choose a reason for hiding this comment

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

This change means we are not going to show API key ID and name in the authentication API response if the request is made by an API key run-as some other user. I think this makes sense. Ideally once we figure out what the effective user is, it should behave mostly the same as the user authentication itself. Also, it is plausible for certain setup, the underlying API key is not mean to reveal to the end-user.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@

import org.elasticsearch.Version;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.VersionUtils;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xpack.core.security.action.service.TokenInfo;
import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType;
import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
Expand All @@ -32,16 +37,21 @@
import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
import org.elasticsearch.xpack.core.security.user.XPackUser;

import java.io.IOException;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;

public class AuthenticationTests extends ESTestCase {
Expand Down Expand Up @@ -313,6 +323,44 @@ public void testDomainSerialize() throws Exception {
}
}

public void testToXContentWithApiKey() throws IOException {
final String apiKeyId = randomAlphaOfLength(20);
final Authentication authentication1 = randomApiKeyAuthentication(randomUser(), apiKeyId);
final String apiKeyName = (String) authentication1.getMetadata().get(AuthenticationField.API_KEY_NAME_KEY);
runWithAuthenticationToXContent(
authentication1,
m -> assertThat(
m,
hasEntry("api_key", apiKeyName != null ? Map.of("id", apiKeyId, "name", apiKeyName) : Map.of("id", apiKeyId))
)
);

final Authentication authentication2 = authentication1.runAs(randomUser(), randomRealmRef(false));
runWithAuthenticationToXContent(authentication2, m -> assertThat(m, not(hasKey("api_key"))));
}

public void testToXContentWithServiceAccount() throws IOException {
final Authentication authentication1 = randomServiceAccountAuthentication();
final String tokenName = (String) authentication1.getMetadata().get(ServiceAccountSettings.TOKEN_NAME_FIELD);
final String tokenType = ServiceAccountSettings.REALM_TYPE
+ "_"
+ authentication1.getMetadata().get(ServiceAccountSettings.TOKEN_SOURCE_FIELD);
runWithAuthenticationToXContent(
authentication1,
m -> assertThat(m, hasEntry("token", Map.of("name", tokenName, "type", tokenType)))
);

final Authentication authentication2 = authentication1.runAs(randomUser(), randomRealmRef(false));
runWithAuthenticationToXContent(authentication2, m -> assertThat(m, not(hasKey("token"))));
}

private void runWithAuthenticationToXContent(Authentication authentication, Consumer<Map<String, Object>> consumer) throws IOException {
try (XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {
authentication.toXContent(builder, ToXContent.EMPTY_PARAMS);
consumer.accept(XContentHelper.convertToMap(BytesReference.bytes(builder), false, XContentType.JSON).v2());
}
}

private void checkCanAccessResources(Authentication authentication0, Authentication authentication1) {
if (authentication0.getAuthenticationType() == authentication1.getAuthenticationType()
|| EnumSet.of(AuthenticationType.REALM, AuthenticationType.TOKEN)
Expand Down Expand Up @@ -415,6 +463,8 @@ public static Authentication randomApiKeyAuthentication(User user, String apiKey
final HashMap<String, Object> metadata = new HashMap<>();
metadata.put(AuthenticationField.API_KEY_ID_KEY, apiKeyId);
metadata.put(AuthenticationField.API_KEY_NAME_KEY, randomBoolean() ? null : randomAlphaOfLengthBetween(1, 16));
metadata.put(AuthenticationField.API_KEY_CREATOR_REALM_NAME, AuthenticationField.API_KEY_CREATOR_REALM_NAME);
metadata.put(AuthenticationField.API_KEY_CREATOR_REALM_TYPE, AuthenticationField.API_KEY_CREATOR_REALM_TYPE);
metadata.put(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, new BytesArray("{}"));
metadata.put(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, new BytesArray("""
{"x":{"cluster":["all"],"indices":[{"names":["index*"],"privileges":["all"]}]}}"""));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1470,13 +1470,13 @@ private void setThreadContextField(ThreadContext threadContext, String threadCon
LogEntryBuilder withAuthentication(Authentication authentication) {
logEntry.with(PRINCIPAL_FIELD_NAME, authentication.getUser().principal());
logEntry.with(AUTHENTICATION_TYPE_FIELD_NAME, authentication.getAuthenticationType().toString());
if (authentication.isAuthenticatedWithApiKey()) {
if (authentication.isApiKey()) {
logEntry.with(API_KEY_ID_FIELD_NAME, (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY));
String apiKeyName = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_NAME_KEY);
if (apiKeyName != null) {
logEntry.with(API_KEY_NAME_FIELD_NAME, apiKeyName);
}
String creatorRealmName = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME);
final String creatorRealmName = ApiKeyService.getCreatorRealmName(authentication);
if (creatorRealmName != null) {
// can be null for API keys created before version 7.7
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, creatorRealmName);
Expand All @@ -1485,11 +1485,15 @@ LogEntryBuilder withAuthentication(Authentication authentication) {
if (authentication.getUser().isRunAs()) {
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, authentication.getLookedUpBy().getName())
.with(PRINCIPAL_RUN_BY_FIELD_NAME, authentication.getUser().authenticatedUser().principal())
// API key can run-as, when that happens, the following field will be _es_api_key,
// not the API key owner user's realm.
.with(PRINCIPAL_RUN_BY_REALM_FIELD_NAME, authentication.getAuthenticatedBy().getName());
// TODO: API key can run-as which means we could use extra fields (#84394)
} else {
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, authentication.getAuthenticatedBy().getName());
}
}
// TODO: service token info is logged in a separate authentication field (#84394)
if (authentication.isAuthenticatedWithServiceAccount()) {
logEntry.with(SERVICE_TOKEN_NAME_FIELD_NAME, (String) authentication.getMetadata().get(TOKEN_NAME_FIELD))
.with(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1314,31 +1314,29 @@ AtomicLong getLastEvictionCheckedAt() {
}

/**
* Returns realm name for the authenticated user.
* If the user is authenticated by realm type {@value AuthenticationField#API_KEY_REALM_TYPE}
* then it will return the realm name of user who created this API key.
* Returns realm name of the owner user of an API key if the effective user is an API Key.
* If the effective user is not an API key, it just returns the source realm name.
*
* @param authentication {@link Authentication}
* @return realm name
*/
public static String getCreatorRealmName(final Authentication authentication) {
if (authentication.isAuthenticatedWithApiKey()) {
if (authentication.isApiKey()) {
return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME);
} else {
return authentication.getSourceRealm().getName();
}
}

/**
* Returns realm type for the authenticated user.
* If the user is authenticated by realm type {@value AuthenticationField#API_KEY_REALM_TYPE}
* then it will return the realm name of user who created this API key.
* Returns realm type of the owner user of an API key if the effective user is an API Key.
* If the effective user is not an API key, it just returns the source realm type.
*
* @param authentication {@link Authentication}
* @return realm type
*/
public static String getCreatorRealmType(final Authentication authentication) {
if (authentication.isAuthenticatedWithApiKey()) {
if (authentication.isApiKey()) {
return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_TYPE);
} else {
return authentication.getSourceRealm().getType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public IngestDocument execute(IngestDocument ingestDocument) throws Exception {
}
break;
case API_KEY:
if (authentication.isAuthenticatedWithApiKey()) {
if (authentication.isApiKey()) {
final String apiKey = "api_key";
final Object existingApiKeyField = userObject.get(apiKey);
@SuppressWarnings("unchecked")
Expand Down
Loading