Skip to content
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
fe01fd3
This PR addresses #113535 - a confusing error message when the user a…
ankit--sethi May 1, 2025
e4681be
Update docs/changelog/127621.yaml
ankit--sethi May 1, 2025
7b5d0d5
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 1, 2025
9efe82e
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 2, 2025
d3f5c35
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 5, 2025
aab001f
re-do the ticket in Continuation-passing style. Previous unanswered q…
ankit--sethi May 7, 2025
885cfc7
Merge remote-tracking branch 'origin/feature/misleading-error-message…
ankit--sethi May 7, 2025
96158ff
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 7, 2025
8d39db9
link issue
ankit--sethi May 7, 2025
bf32356
accidental commit, reverting
ankit--sethi May 7, 2025
d0faee2
Update docs/changelog/127621.yaml
ankit--sethi May 7, 2025
54889da
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 8, 2025
3fc8aa9
try to check for membership in all non-native realms
ankit--sethi May 8, 2025
76c1afc
Merge remote-tracking branch 'origin/feature/misleading-error-message…
ankit--sethi May 8, 2025
448081d
[CI] Auto commit changes from spotless
May 8, 2025
028b2b8
improve checks to fix failing tests
ankit--sethi May 20, 2025
e40fd67
improve
ankit--sethi May 20, 2025
d0f7971
Merge remote-tracking branch 'origin/feature/misleading-error-message…
ankit--sethi May 20, 2025
173032a
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 20, 2025
099707e
improve
ankit--sethi May 21, 2025
9f2d92c
Merge remote-tracking branch 'origin/main' into feature/misleading-er…
ankit--sethi May 21, 2025
4c19a6f
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 21, 2025
e3eacf3
improve logic with GroupedActionListener
ankit--sethi May 23, 2025
6436e4d
Merge remote-tracking branch 'upstream/main' into feature/misleading-…
ankit--sethi May 23, 2025
7dcd5bd
return early
ankit--sethi May 23, 2025
615146b
extra spaces
ankit--sethi May 23, 2025
163d667
revert
ankit--sethi May 23, 2025
2ab8733
Merge branch 'main' into feature/misleading-error-message
ankit--sethi May 28, 2025
d0608f2
Merge branch 'main' into feature/misleading-error-message
ankit--sethi Jun 2, 2025
ceca41c
Merge remote-tracking branch 'upstream/main' into feature/misleading-…
ankit--sethi Jun 3, 2025
8484bbc
Merge remote-tracking branch 'origin/feature/misleading-error-message…
ankit--sethi Jun 3, 2025
04b17d3
Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…
ankit--sethi Jun 12, 2025
d831eaf
Update x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/…
ankit--sethi Jun 12, 2025
732d64c
PR feedback
ankit--sethi Jun 12, 2025
3640f46
Merge remote-tracking branch 'origin/feature/misleading-error-message…
ankit--sethi Jun 12, 2025
3deef98
fix imports
ankit--sethi Jun 12, 2025
8b6acf4
fix test
ankit--sethi Jun 12, 2025
be80d78
add test
ankit--sethi Jun 12, 2025
e16640e
Merge branch 'main' into feature/misleading-error-message
ankit--sethi Jun 12, 2025
e5fa208
Merge branch 'main' into feature/misleading-error-message
ankit--sethi Jun 13, 2025
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
5 changes: 5 additions & 0 deletions docs/changelog/127621.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 127621
summary: Fix error message when changing the password for a user in the file realm
area: Security
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,57 @@
import org.elasticsearch.action.ActionType;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.HandledTransportAction;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.injection.guice.Inject;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.action.user.ChangePasswordRequest;
import org.elasticsearch.xpack.core.security.authc.Realm;
import org.elasticsearch.xpack.core.security.authc.esnative.ClientReservedRealm;
import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.user.AnonymousUser;
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.security.authc.Realms;
import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.xpack.core.security.user.UsernamesField.ELASTIC_NAME;
import static org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore.USER_NOT_FOUND_MESSAGE;

public class TransportChangePasswordAction extends HandledTransportAction<ChangePasswordRequest, ActionResponse.Empty> {

public static final ActionType<ActionResponse.Empty> TYPE = new ActionType<>("cluster:admin/xpack/security/user/change_password");
private final Settings settings;
private final NativeUsersStore nativeUsersStore;
private final Realms realms;

@Inject
public TransportChangePasswordAction(
Settings settings,
TransportService transportService,
ActionFilters actionFilters,
NativeUsersStore nativeUsersStore
NativeUsersStore nativeUsersStore,
Realms realms
) {
super(TYPE.name(), transportService, actionFilters, ChangePasswordRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
this.settings = settings;
this.nativeUsersStore = nativeUsersStore;
this.realms = realms;
}

@Override
Expand All @@ -62,6 +84,86 @@ protected void doExecute(Task task, ChangePasswordRequest request, ActionListene
);
return;
}
nativeUsersStore.changePassword(request, listener.safeMap(v -> ActionResponse.Empty.INSTANCE));

// check if user exists in the native realm
nativeUsersStore.getUser(username, new ActionListener<>() {
@Override
public void onResponse(User user) {
if (ClientReservedRealm.isReserved(username, settings) == false && user == null) {
List<Realm> nonNativeRealms = realms.getActiveRealms()
.stream()
.filter(t -> Set.of(NativeRealmSettings.TYPE, ReservedRealm.TYPE).contains(t.type()) == false) // Reserved realm is
// implemented in the
// native store
.toList();
if (nonNativeRealms.isEmpty()) {
listener.onFailure(createUserNotFoundException());
}
CountDownLatch latch = new CountDownLatch(nonNativeRealms.size());
List<Exception> realmErrors = Collections.synchronizedList(new ArrayList<>());
nonNativeRealms.forEach(realm -> {
// we should check if this request is mistakenly trying to reset a user in some other realm
realm.lookupUser(username, new ActionListener<>() {
@Override
public void onResponse(User user) {
if (user != null) {
ValidationException validationException = new ValidationException();
validationException.addValidationError(
"user ["
+ username
+ "] is a(n)"
+ realm
+ " user and cannot be managed via this API."
+ (ELASTIC_NAME.equalsIgnoreCase(username)
? " To update the '" + ELASTIC_NAME + "' user in a cloud deployment, use the console."
: "")
);
onFailure(validationException);
} else {
onFailure(null);
}
}

@Override
public void onFailure(Exception e) {
if (e != null) {
realmErrors.add(e);
}
latch.countDown();
}
});
});
try {
latch.await();
// combine errors across realms
ValidationException validationException = new ValidationException();
List<String> errors = realmErrors.stream()
.map(t -> ((ValidationException) t).validationErrors())
.flatMap((Function<List<String>, Stream<String>>) Collection::stream)
.collect(Collectors.toList());
validationException.addValidationErrors(errors);
listener.onFailure(validationException);
} catch (InterruptedException e) {
logger.info("Interrupted while waiting for user lookups across realms", e);
listener.onFailure(e);
}
} else {
// safe to proceed
nativeUsersStore.changePassword(request, listener.safeMap(v -> ActionResponse.Empty.INSTANCE));
}
}

@Override
public void onFailure(Exception e) {
listener.onFailure(e);
}
});

}

private static ValidationException createUserNotFoundException() {
ValidationException validationException = new ValidationException();
validationException.addValidationError(USER_NOT_FOUND_MESSAGE);
return validationException;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public class NativeUsersStore {

public static final String USER_DOC_TYPE = "user";
public static final String RESERVED_USER_TYPE = "reserved-user";
public static final String USER_NOT_FOUND_MESSAGE = "user must exist in order to change password";
private static final Logger logger = LogManager.getLogger(NativeUsersStore.class);

private final Settings settings;
Expand Down Expand Up @@ -240,7 +241,7 @@ private void getUserAndPassword(final String user, final ActionListener<UserAndP
() -> executeAsyncWithOrigin(
client.threadPool().getThreadContext(),
SECURITY_ORIGIN,
client.prepareGet(SECURITY_MAIN_ALIAS, getIdForUser(USER_DOC_TYPE, user)).request(),
client.prepareGet(SECURITY_MAIN_ALIAS, getIdForUser(getDocType(user), user)).request(),
new ActionListener<GetResponse>() {
@Override
public void onResponse(GetResponse response) {
Expand Down Expand Up @@ -279,12 +280,7 @@ public void onFailure(Exception t) {
*/
public void changePassword(final ChangePasswordRequest request, final ActionListener<Void> listener) {
final String username = request.username();
final String docType;
if (ClientReservedRealm.isReserved(username, settings)) {
docType = RESERVED_USER_TYPE;
} else {
docType = USER_DOC_TYPE;
}
final String docType = getDocType(username);

securityIndex.forCurrentProject().prepareIndexIfNeededThenExecute(listener::onFailure, () -> {
executeAsyncWithOrigin(
Expand Down Expand Up @@ -316,7 +312,7 @@ public void onFailure(Exception e) {
} else {
logger.debug(() -> format("failed to change password for user [%s]", request.username()), e);
ValidationException validationException = new ValidationException();
validationException.addValidationError("user must exist in order to change password");
validationException.addValidationError(USER_NOT_FOUND_MESSAGE);
listener.onFailure(validationException);
}
} else {
Expand All @@ -329,6 +325,16 @@ public void onFailure(Exception e) {
});
}

private String getDocType(String username) {
final String docType;
if (ClientReservedRealm.isReserved(username, settings)) {
docType = RESERVED_USER_TYPE;
} else {
docType = USER_DOC_TYPE;
}
return docType;
}

/**
* Asynchronous method to create the elastic superuser with the given password hash. The cache for the user will be
* cleared after the document has been indexed.
Expand Down
Loading
Loading