Skip to content

Commit aab001f

Browse files
ankit--sethiankit--sethi
committed
re-do the ticket in Continuation-passing style. Previous unanswered questions around CI/CD are resolved.
1 parent fe01fd3 commit aab001f

File tree

4 files changed

+167
-47
lines changed

4 files changed

+167
-47
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,13 @@
3333
import org.elasticsearch.action.search.TransportSearchShardsAction;
3434
import org.elasticsearch.action.support.IndexComponentSelector;
3535
import org.elasticsearch.common.Strings;
36+
import org.elasticsearch.common.cache.Cache;
37+
import org.elasticsearch.common.cache.CacheBuilder;
38+
import org.elasticsearch.common.settings.Setting;
39+
import org.elasticsearch.common.settings.Settings;
3640
import org.elasticsearch.common.util.set.Sets;
3741
import org.elasticsearch.core.Nullable;
42+
import org.elasticsearch.core.TimeValue;
3843
import org.elasticsearch.index.seqno.RetentionLeaseActions;
3944
import org.elasticsearch.xpack.core.ccr.action.ForgetFollowerAction;
4045
import org.elasticsearch.xpack.core.ccr.action.PutFollowAction;
@@ -53,7 +58,7 @@
5358
import java.util.Map;
5459
import java.util.Set;
5560
import java.util.SortedMap;
56-
import java.util.concurrent.ConcurrentHashMap;
61+
import java.util.concurrent.ExecutionException;
5762
import java.util.function.Predicate;
5863
import java.util.stream.Collectors;
5964
import java.util.stream.Stream;
@@ -285,7 +290,21 @@ private static Map<String, IndexPrivilege> combineSortedInOrder(
285290
public static final Predicate<String> ACTION_MATCHER = ALL.predicate();
286291
public static final Predicate<String> CREATE_INDEX_MATCHER = CREATE_INDEX.predicate();
287292

288-
private static final ConcurrentHashMap<Set<String>, Set<IndexPrivilege>> CACHE = new ConcurrentHashMap<>();
293+
static final Setting<Integer> CACHE_SIZE = Setting.intSetting(
294+
"xpack.security.privilege.index.cache.size",
295+
10_000,
296+
Setting.Property.NodeScope
297+
);
298+
static final Setting<TimeValue> CACHE_TTL = Setting.timeSetting(
299+
"xpack.security.privilege.index.cache.ttl",
300+
TimeValue.timeValueHours(48),
301+
Setting.Property.NodeScope
302+
);
303+
304+
private static final Cache<Set<String>, Set<IndexPrivilege>> CACHE = CacheBuilder.<Set<String>, Set<IndexPrivilege>>builder()
305+
.setExpireAfterAccess(CACHE_TTL.get(Settings.EMPTY))
306+
.setMaximumWeight(CACHE_SIZE.get(Settings.EMPTY))
307+
.build();
289308

290309
private final IndexComponentSelectorPredicate selectorPredicate;
291310

@@ -340,13 +359,17 @@ public static IndexPrivilege get(String actionOrPrivilege) {
340359
* All raw actions are treated as granting access to the {@link IndexComponentSelector#DATA} selector.
341360
*/
342361
public static Set<IndexPrivilege> resolveBySelectorAccess(Set<String> names) {
343-
return CACHE.computeIfAbsent(names, (theName) -> {
344-
if (theName.isEmpty()) {
345-
return Set.of(NONE);
346-
} else {
347-
return resolve(theName);
348-
}
349-
});
362+
try {
363+
return CACHE.computeIfAbsent(names, (theName) -> {
364+
if (theName.isEmpty()) {
365+
return Set.of(NONE);
366+
} else {
367+
return resolve(theName);
368+
}
369+
});
370+
} catch (ExecutionException e) {
371+
throw new RuntimeException(e);
372+
}
350373
}
351374

352375
@Nullable

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

Lines changed: 60 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,35 +19,40 @@
1919
import org.elasticsearch.transport.TransportService;
2020
import org.elasticsearch.xpack.core.XPackSettings;
2121
import org.elasticsearch.xpack.core.security.action.user.ChangePasswordRequest;
22+
import org.elasticsearch.xpack.core.security.authc.Realm;
23+
import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings;
2224
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
2325
import org.elasticsearch.xpack.core.security.user.AnonymousUser;
26+
import org.elasticsearch.xpack.core.security.user.User;
27+
import org.elasticsearch.xpack.security.authc.Realms;
2428
import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
25-
import org.elasticsearch.xpack.security.authc.file.FileUserPasswdStore;
29+
import org.elasticsearch.xpack.security.authc.file.FileRealm;
2630

2731
import java.util.Locale;
32+
import java.util.Optional;
2833

29-
import static org.elasticsearch.core.Strings.format;
3034
import static org.elasticsearch.xpack.core.security.user.UsernamesField.ELASTIC_NAME;
35+
import static org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore.USER_NOT_FOUND_MESSAGE;
3136

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

3439
public static final ActionType<ActionResponse.Empty> TYPE = new ActionType<>("cluster:admin/xpack/security/user/change_password");
3540
private final Settings settings;
3641
private final NativeUsersStore nativeUsersStore;
37-
private final FileUserPasswdStore fileUserPasswdStore;
42+
private final Realms realms;
3843

3944
@Inject
4045
public TransportChangePasswordAction(
4146
Settings settings,
4247
TransportService transportService,
4348
ActionFilters actionFilters,
4449
NativeUsersStore nativeUsersStore,
45-
FileUserPasswdStore fileUserPasswdStore
50+
Realms realms
4651
) {
4752
super(TYPE.name(), transportService, actionFilters, ChangePasswordRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
4853
this.settings = settings;
4954
this.nativeUsersStore = nativeUsersStore;
50-
this.fileUserPasswdStore = fileUserPasswdStore;
55+
this.realms = realms;
5156
}
5257

5358
@Override
@@ -71,24 +76,57 @@ protected void doExecute(Task task, ChangePasswordRequest request, ActionListene
7176
return;
7277
}
7378

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-
}
79+
// check if user exists in the native realm
80+
nativeUsersStore.getUser(username, new ActionListener<>() {
81+
@Override
82+
public void onResponse(User user) {
83+
Optional<Realm> realm = realms.stream().filter(t -> FileRealmSettings.TYPE.equalsIgnoreCase(t.type())).findAny();
84+
if (user == null && realm.isPresent()) {
85+
// we should check if this request is mistakenly trying to reset a file realm user
86+
FileRealm fileRealm = (FileRealm) realm.get();
87+
fileRealm.lookupUser(username, new ActionListener<>() {
88+
@Override
89+
public void onResponse(User user) {
90+
if (user != null) {
91+
ValidationException validationException = new ValidationException();
92+
validationException.addValidationError(
93+
"user ["
94+
+ username
95+
+ "] is file-based and cannot be managed via this API."
96+
+ (ELASTIC_NAME.equalsIgnoreCase(username)
97+
? " To update the '" + ELASTIC_NAME + "' user in a cloud deployment, use the console."
98+
: "")
99+
);
100+
onFailure(validationException);
101+
} else {
102+
onFailure(createUserNotFoundException());
103+
}
104+
}
90105

91-
nativeUsersStore.changePassword(request, listener.safeMap(v -> ActionResponse.Empty.INSTANCE));
106+
@Override
107+
public void onFailure(Exception e) {
108+
listener.onFailure(e);
109+
}
110+
});
111+
} else if (user == null) {
112+
listener.onFailure(createUserNotFoundException());
113+
} else {
114+
// safe to proceed
115+
nativeUsersStore.changePassword(request, listener.safeMap(v -> ActionResponse.Empty.INSTANCE));
116+
}
117+
}
118+
119+
@Override
120+
public void onFailure(Exception e) {
121+
listener.onFailure(e);
122+
}
123+
});
124+
125+
}
92126

127+
private static ValidationException createUserNotFoundException() {
128+
ValidationException validationException = new ValidationException();
129+
validationException.addValidationError(USER_NOT_FOUND_MESSAGE);
130+
return validationException;
93131
}
94132
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ public class NativeUsersStore {
8282

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

8788
private final Settings settings;
@@ -316,7 +317,7 @@ public void onFailure(Exception e) {
316317
} else {
317318
logger.debug(() -> format("failed to change password for user [%s]", request.username()), e);
318319
ValidationException validationException = new ValidationException();
319-
validationException.addValidationError("user must exist in order to change password");
320+
validationException.addValidationError(USER_NOT_FOUND_MESSAGE);
320321
listener.onFailure(validationException);
321322
}
322323
} else {

0 commit comments

Comments
 (0)