Skip to content

Commit 72d3d0f

Browse files
ankit--sethiankit--sethi
committed
followups from previous PR -
1. Using IteratingActionListener instead of a for-loop in the main flow of PluggableAuthenticatorChain 2. Adding unit test coverage for PluggableAuthenticatorChain
1 parent 808556a commit 72d3d0f

File tree

4 files changed

+417
-27
lines changed

4 files changed

+417
-27
lines changed

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import static org.elasticsearch.transport.RemoteClusterPortSettings.TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY;
6060
import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
6161
import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
62+
import static org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType.TOKEN;
6263
import static org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef.newAnonymousRealmRef;
6364
import static org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef.newApiKeyRealmRef;
6465
import static org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef.newCloudApiKeyRealmRef;
@@ -83,6 +84,7 @@
8384
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.FALLBACK_REALM_NAME;
8485
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.FALLBACK_REALM_TYPE;
8586
import static org.elasticsearch.xpack.core.security.authc.RealmDomain.REALM_DOMAIN_PARSER;
87+
import static org.elasticsearch.xpack.core.security.authc.Subject.Type.USER;
8688
import static org.elasticsearch.xpack.core.security.authz.RoleDescriptor.Fields.REMOTE_CLUSTER;
8789
import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS;
8890

@@ -424,7 +426,7 @@ public Authentication token() {
424426
assert false == isAuthenticatedInternally();
425427
assert false == isServiceAccount();
426428
assert false == isCrossClusterAccess();
427-
final Authentication newTokenAuthentication = new Authentication(effectiveSubject, authenticatingSubject, AuthenticationType.TOKEN);
429+
final Authentication newTokenAuthentication = new Authentication(effectiveSubject, authenticatingSubject, TOKEN);
428430
return newTokenAuthentication;
429431
}
430432

@@ -603,7 +605,7 @@ public boolean supportsRunAs(@Nullable AnonymousUser anonymousUser) {
603605
// Run-as is supported for authentication with realm, api_key or token.
604606
if (AuthenticationType.REALM == getAuthenticationType()
605607
|| AuthenticationType.API_KEY == getAuthenticationType()
606-
|| AuthenticationType.TOKEN == getAuthenticationType()) {
608+
|| TOKEN == getAuthenticationType()) {
607609
return true;
608610
}
609611

@@ -717,7 +719,7 @@ public boolean canAccessResourcesOf(Authentication resourceCreatorAuthentication
717719
assert EnumSet.of(
718720
Authentication.AuthenticationType.REALM,
719721
Authentication.AuthenticationType.API_KEY,
720-
Authentication.AuthenticationType.TOKEN,
722+
TOKEN,
721723
Authentication.AuthenticationType.ANONYMOUS,
722724
Authentication.AuthenticationType.INTERNAL
723725
).containsAll(EnumSet.of(getAuthenticationType(), resourceCreatorAuthentication.getAuthenticationType()))
@@ -823,6 +825,9 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
823825
apiKeyField.put("managed_by", CredentialManagedBy.CLOUD.getDisplayName());
824826
builder.field("api_key", Collections.unmodifiableMap(apiKeyField));
825827
}
828+
if (metadata.containsKey("managed_by")) {
829+
builder.field("managed_by", metadata.get("managed_by"));
830+
}
826831
}
827832

828833
public static Authentication getAuthenticationFromCrossClusterAccessMetadata(Authentication authentication) {
@@ -982,7 +987,7 @@ private void checkConsistencyForApiKeyAuthenticationType() {
982987
}
983988

984989
private void checkConsistencyForRealmAuthenticationType() {
985-
if (Subject.Type.USER != authenticatingSubject.getType()) {
990+
if (USER != authenticatingSubject.getType()) {
986991
throw new IllegalArgumentException("Realm authentication must have subject type of user");
987992
}
988993
if (isRunAs()) {
@@ -1025,7 +1030,7 @@ private static void checkRunAsConsistency(Subject effectiveSubject, Subject auth
10251030
)
10261031
);
10271032
}
1028-
if (Subject.Type.USER != effectiveSubject.getType()) {
1033+
if (USER != effectiveSubject.getType()) {
10291034
throw new IllegalArgumentException(Strings.format("Run-as subject type cannot be [%s]", effectiveSubject.getType()));
10301035
}
10311036
if (false == effectiveSubject.getMetadata().isEmpty()) {
@@ -1357,7 +1362,7 @@ public static Authentication newServiceAccountAuthentication(User serviceAccount
13571362
final Authentication.RealmRef authenticatedBy = newServiceAccountRealmRef(nodeName);
13581363
Authentication authentication = new Authentication(
13591364
new Subject(serviceAccountUser, authenticatedBy, TransportVersion.current(), metadata),
1360-
AuthenticationType.TOKEN
1365+
TOKEN
13611366
);
13621367
return authentication;
13631368
}
@@ -1384,7 +1389,7 @@ public static Authentication newCloudAccessTokenAuthentication(
13841389
final User user = authResult.getValue();
13851390
return new Authentication(
13861391
new Subject(user, realmRef, TransportVersion.current(), authResult.getMetadata()),
1387-
AuthenticationType.TOKEN
1392+
TOKEN
13881393
);
13891394
}
13901395

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/apikey/CustomAuthenticator.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.xpack.core.security.authc.Authentication;
1414
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
1515
import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
16+
import org.elasticsearch.xpack.core.security.user.User;
1617

1718
/**
1819
* An extension point to provide a custom authenticator implementation. For example, a custom API key or a custom OAuth2
@@ -28,4 +29,6 @@ public interface CustomAuthenticator {
2829

2930
void authenticate(@Nullable AuthenticationToken token, ActionListener<AuthenticationResult<Authentication>> listener);
3031

32+
Authentication getAuthentication(AuthenticationResult<User> result, String nodeName);
33+
3134
}

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

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88
package org.elasticsearch.xpack.security.authc;
99

1010
import org.elasticsearch.action.ActionListener;
11+
import org.elasticsearch.xpack.core.common.IteratingActionListener;
1112
import org.elasticsearch.xpack.core.security.authc.Authentication;
1213
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
1314
import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
1415
import org.elasticsearch.xpack.core.security.authc.apikey.CustomAuthenticator;
1516

1617
import java.util.List;
1718
import java.util.Objects;
19+
import java.util.function.BiConsumer;
1820

1921
public class PluggableAuthenticatorChain implements Authenticator {
2022

@@ -55,28 +57,47 @@ public void authenticate(Context context, ActionListener<AuthenticationResult<Au
5557
}
5658
AuthenticationToken token = context.getMostRecentAuthenticationToken();
5759
if (token != null) {
58-
// TODO switch to IteratingActionListener
59-
for (CustomAuthenticator customAuthenticator : customAuthenticators) {
60-
if (customAuthenticator.supports(token)) {
61-
customAuthenticator.authenticate(token, ActionListener.wrap(response -> {
62-
if (response.isAuthenticated()) {
63-
listener.onResponse(response);
64-
} else if (response.getStatus() == AuthenticationResult.Status.TERMINATE) {
65-
final Exception ex = response.getException();
66-
if (ex == null) {
67-
listener.onFailure(context.getRequest().authenticationFailed(token));
68-
} else {
69-
listener.onFailure(context.getRequest().exceptionProcessingRequest(ex, token));
70-
}
71-
} else if (response.getStatus() == AuthenticationResult.Status.CONTINUE) {
72-
listener.onResponse(AuthenticationResult.notHandled());
73-
}
74-
}, ex -> listener.onFailure(context.getRequest().exceptionProcessingRequest(ex, token))));
75-
return;
76-
}
77-
}
60+
var lis = new IteratingActionListener<>(listener,
61+
getAuthConsumer(context),
62+
customAuthenticators,
63+
context.getThreadContext(),
64+
result -> {
65+
if (result == null) {
66+
// all custom authenticators left the token unhandled
67+
return AuthenticationResult.notHandled();
68+
}
69+
return result;
70+
},
71+
result -> result == null || result.getStatus() == AuthenticationResult.Status.CONTINUE);
72+
lis.run();
73+
return;
7874
}
7975
listener.onResponse(AuthenticationResult.notHandled());
8076
}
8177

78+
private BiConsumer<CustomAuthenticator, ActionListener<AuthenticationResult<Authentication>>> getAuthConsumer(Context context) {
79+
AuthenticationToken token = context.getMostRecentAuthenticationToken();
80+
return (authenticator, iteratingListener) -> {
81+
if (authenticator.supports(token)) {
82+
authenticator.authenticate(token, ActionListener.wrap(response -> {
83+
if (response.isAuthenticated()) {
84+
iteratingListener.onResponse(response);
85+
} else if (response.getStatus() == AuthenticationResult.Status.TERMINATE) {
86+
final Exception ex = response.getException();
87+
if (ex == null) {
88+
iteratingListener.onFailure(context.getRequest().authenticationFailed(token));
89+
} else {
90+
iteratingListener.onFailure(context.getRequest().exceptionProcessingRequest(ex, token));
91+
}
92+
} else if (response.getStatus() == AuthenticationResult.Status.CONTINUE) {
93+
iteratingListener.onResponse(AuthenticationResult.notHandled());
94+
}
95+
}, ex -> iteratingListener.onFailure(context.getRequest().exceptionProcessingRequest(ex, token))));
96+
}
97+
else {
98+
iteratingListener.onResponse(null); // try the next custom authenticator
99+
}
100+
};
101+
}
102+
82103
}

0 commit comments

Comments
 (0)