Skip to content

Commit feab123

Browse files
ywangdywangd
authored
Fix API key role descriptors rewrite bug for upgraded clusters (#62917)
This PR ensures that API key role descriptors are always rewritten to a target node compatible format before a request is sent.
1 parent be33573 commit feab123

File tree

4 files changed

+95
-15
lines changed

4 files changed

+95
-15
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java

+30-12
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.common.settings.Settings;
1515
import org.elasticsearch.common.util.concurrent.ThreadContext;
1616
import org.elasticsearch.common.util.concurrent.ThreadContext.StoredContext;
17+
import org.elasticsearch.common.xcontent.XContentBuilder;
1718
import org.elasticsearch.common.xcontent.XContentHelper;
1819
import org.elasticsearch.common.xcontent.XContentType;
1920
import org.elasticsearch.node.Node;
@@ -167,19 +168,36 @@ public void executeAfterRewritingAuthentication(Consumer<StoredContext> consumer
167168

168169
private Map<String, Object> rewriteMetadataForApiKeyRoleDescriptors(Version streamVersion, Authentication authentication) {
169170
Map<String, Object> metadata = authentication.getMetadata();
170-
if (authentication.getAuthenticationType() == AuthenticationType.API_KEY
171-
&& authentication.getVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES)
172-
&& streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) {
173-
metadata = new HashMap<>(metadata);
174-
metadata.put(
175-
API_KEY_ROLE_DESCRIPTORS_KEY,
176-
XContentHelper.convertToMap(
177-
(BytesReference) metadata.get(API_KEY_ROLE_DESCRIPTORS_KEY), false, XContentType.JSON).v2());
178-
metadata.put(
179-
API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY,
180-
XContentHelper.convertToMap(
181-
(BytesReference) metadata.get(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), false, XContentType.JSON).v2());
171+
if (authentication.getAuthenticationType() == AuthenticationType.API_KEY) {
172+
if (authentication.getVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES)
173+
&& streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) {
174+
metadata = new HashMap<>(metadata);
175+
metadata.put(API_KEY_ROLE_DESCRIPTORS_KEY,
176+
convertRoleDescriptorsBytesToMap((BytesReference) metadata.get(API_KEY_ROLE_DESCRIPTORS_KEY)));
177+
metadata.put(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY,
178+
convertRoleDescriptorsBytesToMap((BytesReference) metadata.get(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY)));
179+
} else if (authentication.getVersion().before(VERSION_API_KEY_ROLES_AS_BYTES)
180+
&& streamVersion.onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES)) {
181+
metadata = new HashMap<>(metadata);
182+
metadata.put(API_KEY_ROLE_DESCRIPTORS_KEY,
183+
convertRoleDescriptorsMapToBytes((Map<String, Object>)metadata.get(API_KEY_ROLE_DESCRIPTORS_KEY)));
184+
metadata.put(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY,
185+
convertRoleDescriptorsMapToBytes((Map<String, Object>) metadata.get(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY)));
186+
}
182187
}
183188
return metadata;
184189
}
190+
191+
private Map<String, Object> convertRoleDescriptorsBytesToMap(BytesReference roleDescriptorsBytes) {
192+
return XContentHelper.convertToMap(roleDescriptorsBytes, false, XContentType.JSON).v2();
193+
}
194+
195+
private BytesReference convertRoleDescriptorsMapToBytes(Map<String, Object> roleDescriptorsMap) {
196+
try(XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {
197+
builder.map(roleDescriptorsMap);
198+
return BytesReference.bytes(builder);
199+
} catch (IOException e) {
200+
throw new UncheckedIOException(e);
201+
}
202+
}
185203
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java

+22-3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import org.elasticsearch.Version;
99
import org.elasticsearch.common.bytes.BytesArray;
10+
import org.elasticsearch.common.bytes.BytesReference;
1011
import org.elasticsearch.common.settings.Settings;
1112
import org.elasticsearch.common.util.concurrent.ThreadContext;
1213
import org.elasticsearch.common.util.concurrent.ThreadContext.StoredContext;
@@ -28,6 +29,7 @@
2829
import java.util.Map;
2930
import java.util.concurrent.atomic.AtomicReference;
3031

32+
import static org.elasticsearch.xpack.core.security.authc.Authentication.VERSION_API_KEY_ROLES_AS_BYTES;
3133
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY;
3234
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY;
3335
import static org.hamcrest.Matchers.instanceOf;
@@ -136,7 +138,7 @@ public void testExecuteAfterRewritingAuthentication() throws IOException {
136138
assertEquals(original, securityContext.getAuthentication());
137139
}
138140

139-
public void testExecuteAfterRewritingAuthenticationShouldRewriteApiKeyMetadataForBwc() throws IOException {
141+
public void testExecuteAfterRewritingAuthenticationWillConditionallyRewriteNewApiKeyMetadata() throws IOException {
140142
User user = new User("test", null, new User("authUser"));
141143
RealmRef authBy = new RealmRef("_es_api_key", "_es_api_key", "node1");
142144
final Map<String, Object> metadata = Map.of(
@@ -147,16 +149,23 @@ API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, new BytesArray("{\"limitedBy role\": {\"cl
147149
AuthenticationType.API_KEY, metadata);
148150
original.writeToContext(threadContext);
149151

152+
// If target is old node, rewrite new style API key metadata to old format
150153
securityContext.executeAfterRewritingAuthentication(originalCtx -> {
151154
Authentication authentication = securityContext.getAuthentication();
152155
assertEquals(Map.of("a role", Map.of("cluster", List.of("all"))),
153156
authentication.getMetadata().get(API_KEY_ROLE_DESCRIPTORS_KEY));
154157
assertEquals(Map.of("limitedBy role", Map.of("cluster", List.of("all"))),
155158
authentication.getMetadata().get(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY));
156159
}, Version.V_7_8_0);
160+
161+
// If target is new node, no need to rewrite the new style API key metadata
162+
securityContext.executeAfterRewritingAuthentication(originalCtx -> {
163+
Authentication authentication = securityContext.getAuthentication();
164+
assertSame(metadata, authentication.getMetadata());
165+
}, VersionUtils.randomVersionBetween(random(), VERSION_API_KEY_ROLES_AS_BYTES, Version.CURRENT));
157166
}
158167

159-
public void testExecuteAfterRewritingAuthenticationShouldNotRewriteApiKeyMetadataForOldAuthenticationObject() throws IOException {
168+
public void testExecuteAfterRewritingAuthenticationWillConditionallyRewriteOldApiKeyMetadata() throws IOException {
160169
User user = new User("test", null, new User("authUser"));
161170
RealmRef authBy = new RealmRef("_es_api_key", "_es_api_key", "node1");
162171
final Map<String, Object> metadata = Map.of(
@@ -166,9 +175,19 @@ public void testExecuteAfterRewritingAuthenticationShouldNotRewriteApiKeyMetadat
166175
final Authentication original = new Authentication(user, authBy, authBy, Version.V_7_8_0, AuthenticationType.API_KEY, metadata);
167176
original.writeToContext(threadContext);
168177

178+
// If target is old node, no need to rewrite old style API key metadata
169179
securityContext.executeAfterRewritingAuthentication(originalCtx -> {
170180
Authentication authentication = securityContext.getAuthentication();
171181
assertSame(metadata, authentication.getMetadata());
172-
}, randomFrom(Version.V_8_0_0, Version.V_7_8_0));
182+
}, Version.V_7_8_0);
183+
184+
// If target is new old, ensure old map style API key metadata is rewritten to bytesreference
185+
securityContext.executeAfterRewritingAuthentication(originalCtx -> {
186+
Authentication authentication = securityContext.getAuthentication();
187+
assertEquals("{\"a role\":{\"cluster\":[\"all\"]}}",
188+
((BytesReference)authentication.getMetadata().get(API_KEY_ROLE_DESCRIPTORS_KEY)).utf8ToString());
189+
assertEquals("{\"limitedBy role\":{\"cluster\":[\"all\"]}}",
190+
((BytesReference)authentication.getMetadata().get(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY)).utf8ToString());
191+
}, VersionUtils.randomVersionBetween(random(), VERSION_API_KEY_ROLES_AS_BYTES, Version.CURRENT));
173192
}
174193
}

‎x-pack/qa/full-cluster-restart/build.gradle

+2
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ for (Version bwcVersion : BuildParams.bwcVersions.indexCompatible) {
6363
setting 'xpack.security.transport.ssl.certificate', 'testnode.crt'
6464
keystore 'xpack.security.transport.ssl.secure_key_passphrase', 'testnode'
6565
setting 'logger.org.elasticsearch.xpack.watcher', 'DEBUG'
66+
67+
setting 'xpack.security.authc.api_key.enabled', 'true'
6668
}
6769
}
6870

‎x-pack/qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java

+41
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.apache.http.util.EntityUtils;
1111
import org.elasticsearch.Version;
1212
import org.elasticsearch.client.Request;
13+
import org.elasticsearch.client.RequestOptions;
1314
import org.elasticsearch.client.Response;
1415
import org.elasticsearch.client.ResponseException;
1516
import org.elasticsearch.client.RestClient;
@@ -52,6 +53,7 @@
5253
import static org.hamcrest.Matchers.containsString;
5354
import static org.hamcrest.Matchers.equalTo;
5455
import static org.hamcrest.Matchers.everyItem;
56+
import static org.hamcrest.Matchers.greaterThan;
5557
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
5658
import static org.hamcrest.Matchers.hasEntry;
5759
import static org.hamcrest.Matchers.hasItems;
@@ -222,6 +224,45 @@ public void testWatcher() throws Exception {
222224
}
223225
}
224226

227+
@SuppressWarnings("unchecked")
228+
public void testWatcherWithApiKey() throws Exception {
229+
final Request getWatchStatusRequest = new Request("GET", "/_watcher/watch/watch_with_api_key");
230+
getWatchStatusRequest.addParameter("filter_path", "status");
231+
232+
if (isRunningAgainstOldCluster()) {
233+
final Request createApiKeyRequest = new Request("PUT", "/_security/api_key");
234+
createApiKeyRequest.setJsonEntity("{\"name\":\"key-1\"}");
235+
final Response response = client().performRequest(createApiKeyRequest);
236+
final Map<String, Object> createApiKeyResponse = entityAsMap(response);
237+
238+
Request createWatchWithApiKeyRequest = new Request("PUT", "/_watcher/watch/watch_with_api_key");
239+
createWatchWithApiKeyRequest.setJsonEntity(loadWatch("simple-watch.json"));
240+
final byte[] keyBytes =
241+
(createApiKeyResponse.get("id") + ":" + createApiKeyResponse.get("api_key")).getBytes(StandardCharsets.UTF_8);
242+
final String authHeader = "ApiKey " + Base64.getEncoder().encodeToString(keyBytes);
243+
createWatchWithApiKeyRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", authHeader));
244+
client().performRequest(createWatchWithApiKeyRequest);
245+
246+
assertBusy(() -> {
247+
final Map<String, Object> getWatchStatusResponse = entityAsMap(client().performRequest(getWatchStatusRequest));
248+
final Map<String, Object> status = (Map<String, Object>) getWatchStatusResponse.get("status");
249+
assertEquals("executed", status.get("execution_state"));
250+
});
251+
252+
} else {
253+
final Map<String, Object> getWatchStatusResponse = entityAsMap(client().performRequest(getWatchStatusRequest));
254+
final Map<String, Object> status = (Map<String, Object>) getWatchStatusResponse.get("status");
255+
final int version = (int) status.get("version");
256+
257+
assertBusy(() -> {
258+
final Map<String, Object> newGetWatchStatusResponse = entityAsMap(client().performRequest(getWatchStatusRequest));
259+
final Map<String, Object> newStatus = (Map<String, Object>) newGetWatchStatusResponse.get("status");
260+
assertThat((int) newStatus.get("version"), greaterThan(version + 2));
261+
assertEquals("executed", newStatus.get("execution_state"));
262+
});
263+
}
264+
}
265+
225266
/**
226267
* Tests that a RollUp job created on a old cluster is correctly restarted after the upgrade.
227268
*/

0 commit comments

Comments
 (0)