Skip to content

Commit fe01fd3

Browse files
committed
This PR addresses #113535 - a confusing error message when the user attempts to update the password for the elastic superuser in a cloud deployment.
At the heart of the issue is the difference in how the `elastic` superuser is implemented on self-hosted deployments vs. managed cloud deployments. Elasticsearch has two distinct security realms: `file` and `native`. On a self-hosted deployment, the `elastic` superuser is represented as a document in the `.security` index, whereas in a cloud deployment `elastic` is defined in the `ES_PATH_CONF/users` and `ES_PATH_CONF/user_roles` files placed on each node in the cluster. The TransportChangePasswordAction impl is designed to update the password for users in the `native` realm specifically, and a failure on cloud to change the password for `elastic` using the Change Password API fails with the error that the user does not exist. The solution here leverages `fileUserPasswdStore.userExists` to do a low cost check on whether the request username belongs to the `file` realm and will exit early with an informative error message if that is the case.
1 parent 245f5ee commit fe01fd3

File tree

2 files changed

+96
-6
lines changed

2 files changed

+96
-6
lines changed

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordAction.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.action.ActionType;
1212
import org.elasticsearch.action.support.ActionFilters;
1313
import org.elasticsearch.action.support.HandledTransportAction;
14+
import org.elasticsearch.common.ValidationException;
1415
import org.elasticsearch.common.settings.Settings;
1516
import org.elasticsearch.common.util.concurrent.EsExecutors;
1617
import org.elasticsearch.injection.guice.Inject;
@@ -21,25 +22,32 @@
2122
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
2223
import org.elasticsearch.xpack.core.security.user.AnonymousUser;
2324
import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
25+
import org.elasticsearch.xpack.security.authc.file.FileUserPasswdStore;
2426

2527
import java.util.Locale;
2628

29+
import static org.elasticsearch.core.Strings.format;
30+
import static org.elasticsearch.xpack.core.security.user.UsernamesField.ELASTIC_NAME;
31+
2732
public class TransportChangePasswordAction extends HandledTransportAction<ChangePasswordRequest, ActionResponse.Empty> {
2833

2934
public static final ActionType<ActionResponse.Empty> TYPE = new ActionType<>("cluster:admin/xpack/security/user/change_password");
3035
private final Settings settings;
3136
private final NativeUsersStore nativeUsersStore;
37+
private final FileUserPasswdStore fileUserPasswdStore;
3238

3339
@Inject
3440
public TransportChangePasswordAction(
3541
Settings settings,
3642
TransportService transportService,
3743
ActionFilters actionFilters,
38-
NativeUsersStore nativeUsersStore
44+
NativeUsersStore nativeUsersStore,
45+
FileUserPasswdStore fileUserPasswdStore
3946
) {
4047
super(TYPE.name(), transportService, actionFilters, ChangePasswordRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
4148
this.settings = settings;
4249
this.nativeUsersStore = nativeUsersStore;
50+
this.fileUserPasswdStore = fileUserPasswdStore;
4351
}
4452

4553
@Override
@@ -62,6 +70,25 @@ protected void doExecute(Task task, ChangePasswordRequest request, ActionListene
6270
);
6371
return;
6472
}
73+
74+
if (fileUserPasswdStore.userExists(request.username())) {
75+
// File realm users cannot be managed through this API.
76+
// unresolved q: is it possible a username is repeated across file and native realms, such that stopping here is incorrect?
77+
logger.debug(() -> format("failed to change password for user [%s]", request.username()));
78+
ValidationException validationException = new ValidationException();
79+
validationException.addValidationError(
80+
"user ["
81+
+ username
82+
+ "] is file-based and cannot be managed via this API."
83+
+ (ELASTIC_NAME.equalsIgnoreCase(username)
84+
? " To update the '" + ELASTIC_NAME + "' user in a cloud deployment, use the console."
85+
: "")
86+
);
87+
listener.onFailure(validationException);
88+
return;
89+
}
90+
6591
nativeUsersStore.changePassword(request, listener.safeMap(v -> ActionResponse.Empty.INSTANCE));
92+
6693
}
6794
}

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.action.ActionListener;
1111
import org.elasticsearch.action.ActionResponse;
1212
import org.elasticsearch.action.support.ActionFilters;
13+
import org.elasticsearch.common.ValidationException;
1314
import org.elasticsearch.common.settings.Settings;
1415
import org.elasticsearch.tasks.Task;
1516
import org.elasticsearch.test.ESTestCase;
@@ -26,6 +27,8 @@
2627
import org.elasticsearch.xpack.core.security.user.KibanaUser;
2728
import org.elasticsearch.xpack.core.security.user.User;
2829
import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
30+
import org.elasticsearch.xpack.security.authc.file.FileUserPasswdStore;
31+
import org.mockito.Mockito;
2932

3033
import java.util.Collections;
3134
import java.util.Locale;
@@ -56,6 +59,7 @@ public void testAnonymousUser() {
5659
.build();
5760
AnonymousUser anonymousUser = new AnonymousUser(settings);
5861
NativeUsersStore usersStore = mock(NativeUsersStore.class);
62+
FileUserPasswdStore fileUserPasswdStore = mock(FileUserPasswdStore.class);
5963
TransportService transportService = new TransportService(
6064
Settings.EMPTY,
6165
mock(Transport.class),
@@ -69,7 +73,8 @@ public void testAnonymousUser() {
6973
settings,
7074
transportService,
7175
mock(ActionFilters.class),
72-
usersStore
76+
usersStore,
77+
fileUserPasswdStore
7378
);
7479
// Request will fail before the request hashing algorithm is checked, but we use the same algorithm as in settings for consistency
7580
ChangePasswordRequest request = new ChangePasswordRequest();
@@ -96,6 +101,56 @@ public void onFailure(Exception e) {
96101
verifyNoMoreInteractions(usersStore);
97102
}
98103

104+
public void testFileRealmUser() {
105+
final Hasher hasher = getFastStoredHashAlgoForTests();
106+
Settings settings = Settings.builder()
107+
.put(AnonymousUser.ROLES_SETTING.getKey(), "superuser")
108+
.put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), hasher.name())
109+
.build();
110+
ElasticUser elasticUser = new ElasticUser(true);
111+
NativeUsersStore usersStore = mock(NativeUsersStore.class);
112+
FileUserPasswdStore fileUserPasswdStore = mock(FileUserPasswdStore.class);
113+
TransportService transportService = new TransportService(
114+
Settings.EMPTY,
115+
mock(Transport.class),
116+
mock(ThreadPool.class),
117+
TransportService.NOOP_TRANSPORT_INTERCEPTOR,
118+
x -> null,
119+
null,
120+
Collections.emptySet()
121+
);
122+
TransportChangePasswordAction action = new TransportChangePasswordAction(
123+
settings,
124+
transportService,
125+
mock(ActionFilters.class),
126+
usersStore,
127+
fileUserPasswdStore
128+
);
129+
Mockito.when(fileUserPasswdStore.userExists(Mockito.eq(elasticUser.principal()))).thenReturn(true);
130+
ChangePasswordRequest request = new ChangePasswordRequest();
131+
request.username(elasticUser.principal());
132+
request.passwordHash(hasher.hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING));
133+
134+
final AtomicReference<Throwable> throwableRef = new AtomicReference<>();
135+
final AtomicReference<ActionResponse.Empty> responseRef = new AtomicReference<>();
136+
action.doExecute(mock(Task.class), request, new ActionListener<>() {
137+
@Override
138+
public void onResponse(ActionResponse.Empty changePasswordResponse) {
139+
responseRef.set(changePasswordResponse);
140+
}
141+
142+
@Override
143+
public void onFailure(Exception e) {
144+
throwableRef.set(e);
145+
}
146+
});
147+
148+
assertThat(responseRef.get(), is(nullValue()));
149+
assertThat(throwableRef.get(), instanceOf(ValidationException.class));
150+
assertThat(throwableRef.get().getMessage(), containsString("is file-based and cannot be managed via this API"));
151+
verifyNoMoreInteractions(usersStore);
152+
}
153+
99154
public void testValidUser() {
100155
testValidUser(randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe")));
101156
}
@@ -107,6 +162,7 @@ public void testValidUserWithInternalUsername() {
107162
private void testValidUser(User user) {
108163
final Hasher hasher = getFastStoredHashAlgoForTests();
109164
NativeUsersStore usersStore = mock(NativeUsersStore.class);
165+
FileUserPasswdStore fileUserPasswdStore = mock(FileUserPasswdStore.class);
110166
ChangePasswordRequest request = new ChangePasswordRequest();
111167
request.username(user.principal());
112168
request.passwordHash(hasher.hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING));
@@ -132,7 +188,8 @@ private void testValidUser(User user) {
132188
passwordHashingSettings,
133189
transportService,
134190
mock(ActionFilters.class),
135-
usersStore
191+
usersStore,
192+
fileUserPasswdStore
136193
);
137194
final AtomicReference<Throwable> throwableRef = new AtomicReference<>();
138195
final AtomicReference<ActionResponse.Empty> responseRef = new AtomicReference<>();
@@ -157,6 +214,7 @@ public void onFailure(Exception e) {
157214
public void testWithPasswordThatsNotAHash() {
158215
final User user = randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe"));
159216
NativeUsersStore usersStore = mock(NativeUsersStore.class);
217+
FileUserPasswdStore fileUserPasswdStore = mock(FileUserPasswdStore.class);
160218
ChangePasswordRequest request = new ChangePasswordRequest();
161219
request.username(user.principal());
162220
request.passwordHash(randomAlphaOfLengthBetween(14, 20).toCharArray());
@@ -177,7 +235,8 @@ public void testWithPasswordThatsNotAHash() {
177235
passwordHashingSettings,
178236
transportService,
179237
mock(ActionFilters.class),
180-
usersStore
238+
usersStore,
239+
fileUserPasswdStore
181240
);
182241
action.doExecute(mock(Task.class), request, new ActionListener<>() {
183242
@Override
@@ -204,6 +263,7 @@ public void testWithDifferentPasswordHashingAlgorithm() {
204263
final User user = randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe"));
205264
final Hasher hasher = getFastStoredHashAlgoForTests();
206265
NativeUsersStore usersStore = mock(NativeUsersStore.class);
266+
FileUserPasswdStore fileUserPasswdStore = mock(FileUserPasswdStore.class);
207267
ChangePasswordRequest request = new ChangePasswordRequest();
208268
request.username(user.principal());
209269
request.passwordHash(hasher.hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING));
@@ -235,7 +295,8 @@ public void testWithDifferentPasswordHashingAlgorithm() {
235295
passwordHashingSettings,
236296
transportService,
237297
mock(ActionFilters.class),
238-
usersStore
298+
usersStore,
299+
fileUserPasswdStore
239300
);
240301
action.doExecute(mock(Task.class), request, new ActionListener<>() {
241302
@Override
@@ -259,6 +320,7 @@ public void testException() {
259320
final Hasher hasher = getFastStoredHashAlgoForTests();
260321
final User user = randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe"));
261322
NativeUsersStore usersStore = mock(NativeUsersStore.class);
323+
FileUserPasswdStore fileUserPasswdStore = mock(FileUserPasswdStore.class);
262324
ChangePasswordRequest request = new ChangePasswordRequest();
263325
request.username(user.principal());
264326
request.passwordHash(hasher.hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING));
@@ -285,7 +347,8 @@ public void testException() {
285347
passwordHashingSettings,
286348
transportService,
287349
mock(ActionFilters.class),
288-
usersStore
350+
usersStore,
351+
fileUserPasswdStore
289352
);
290353
final AtomicReference<Throwable> throwableRef = new AtomicReference<>();
291354
final AtomicReference<ActionResponse.Empty> responseRef = new AtomicReference<>();

0 commit comments

Comments
 (0)