Skip to content

Commit bdd9aa1

Browse files
richard-dennehyrichard-dennehy
committed
execute blocking graph calls on generic thread pool
1 parent 6e2869f commit bdd9aa1

File tree

4 files changed

+68
-52
lines changed

4 files changed

+68
-52
lines changed

‎plugins/microsoft-graph-authz/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,5 +96,5 @@ tasks.named("javadoc").configure { enabled = false }
9696
tasks.named("dependencyLicenses").configure {
9797
mapping from: "microsoft-graph-core", to: "microsoft-graph"
9898
mapping from: /azure-.*/, to: "azure"
99-
mapping from: /jackson/, to: "jackson"
99+
mapping from: /jackson.*/, to: "jackson"
100100
}

‎plugins/microsoft-graph-authz/src/main/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphAuthzPlugin.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@
2020
public class MicrosoftGraphAuthzPlugin extends Plugin implements SecurityExtension {
2121
@Override
2222
public Map<String, Realm.Factory> getRealms(SecurityComponents components) {
23-
return Map.of(MicrosoftGraphAuthzRealmSettings.REALM_TYPE, config -> new MicrosoftGraphAuthzRealm(components.roleMapper(), config));
23+
return Map.of(
24+
MicrosoftGraphAuthzRealmSettings.REALM_TYPE,
25+
config -> new MicrosoftGraphAuthzRealm(components.roleMapper(), config, components.threadPool())
26+
);
2427
}
2528

2629
@Override

‎plugins/microsoft-graph-authz/src/main/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphAuthzRealm.java‎

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.elasticsearch.license.XPackLicenseState;
3131
import org.elasticsearch.logging.LogManager;
3232
import org.elasticsearch.logging.Logger;
33+
import org.elasticsearch.threadpool.ThreadPool;
3334
import org.elasticsearch.xpack.core.XPackPlugin;
3435
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
3536
import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
@@ -42,6 +43,7 @@
4243
import java.util.ArrayList;
4344
import java.util.List;
4445
import java.util.Map;
46+
import java.util.concurrent.ExecutorService;
4547

4648
public class MicrosoftGraphAuthzRealm extends Realm {
4749

@@ -62,8 +64,9 @@ public class MicrosoftGraphAuthzRealm extends Realm {
6264
private final UserRoleMapper roleMapper;
6365
private final GraphServiceClient client;
6466
private final XPackLicenseState licenseState;
67+
private final ExecutorService genericExecutor;
6568

66-
public MicrosoftGraphAuthzRealm(UserRoleMapper roleMapper, RealmConfig config) {
69+
public MicrosoftGraphAuthzRealm(UserRoleMapper roleMapper, RealmConfig config, ThreadPool threadPool) {
6770
super(config);
6871

6972
this.config = config;
@@ -83,15 +86,23 @@ public MicrosoftGraphAuthzRealm(UserRoleMapper roleMapper, RealmConfig config) {
8386

8487
this.client = buildClient(clientSecret);
8588
this.licenseState = XPackPlugin.getSharedLicenseState();
89+
this.genericExecutor = threadPool.generic();
8690
}
8791

8892
// for testing
89-
MicrosoftGraphAuthzRealm(UserRoleMapper roleMapper, RealmConfig config, GraphServiceClient client, XPackLicenseState licenseState) {
93+
MicrosoftGraphAuthzRealm(
94+
UserRoleMapper roleMapper,
95+
RealmConfig config,
96+
GraphServiceClient client,
97+
XPackLicenseState licenseState,
98+
ThreadPool threadPool
99+
) {
90100
super(config);
91101
this.config = config;
92102
this.roleMapper = roleMapper;
93103
this.client = client;
94104
this.licenseState = licenseState;
105+
this.genericExecutor = threadPool.generic();
95106
}
96107

97108
private void require(Setting.AffixSetting<String> setting) {
@@ -123,29 +134,31 @@ public void lookupUser(String principal, ActionListener<User> listener) {
123134
return;
124135
}
125136

126-
try {
127-
final var userProperties = sdkFetchUserProperties(client, principal);
128-
final var groups = sdkFetchGroupMembership(client, principal);
129-
130-
// TODO confirm we don't need any other fields
131-
final var userData = new UserRoleMapper.UserData(principal, null, groups, Map.of(), config);
132-
133-
roleMapper.resolveRoles(userData, listener.delegateFailureAndWrap((l, roles) -> {
134-
final var user = new User(
135-
principal,
136-
roles.toArray(Strings.EMPTY_ARRAY),
137-
userProperties.v1(),
138-
userProperties.v2(),
139-
Map.of(),
140-
true
141-
);
142-
logger.trace("Entra ID user {}", user);
143-
l.onResponse(user);
144-
}));
145-
} catch (Exception e) {
146-
logger.error("failed to authenticate with realm", e);
147-
listener.onFailure(e);
148-
}
137+
genericExecutor.execute(() -> {
138+
try {
139+
final var userProperties = sdkFetchUserProperties(client, principal);
140+
final var groups = sdkFetchGroupMembership(client, principal);
141+
142+
// TODO confirm we don't need any other fields
143+
final var userData = new UserRoleMapper.UserData(principal, null, groups, Map.of(), config);
144+
145+
roleMapper.resolveRoles(userData, listener.delegateFailureAndWrap((l, roles) -> {
146+
final var user = new User(
147+
principal,
148+
roles.toArray(Strings.EMPTY_ARRAY),
149+
userProperties.v1(),
150+
userProperties.v2(),
151+
Map.of(),
152+
true
153+
);
154+
logger.trace("Entra ID user {}", user);
155+
l.onResponse(user);
156+
}));
157+
} catch (Exception e) {
158+
logger.error("failed to authenticate with realm", e);
159+
listener.onFailure(e);
160+
}
161+
});
149162
}
150163

151164
private GraphServiceClient buildClient(SecureString clientSecret) {

‎plugins/microsoft-graph-authz/src/test/java/org/elasticsearch/xpack/security/authz/microsoft/MicrosoftGraphAuthzRealmTests.java‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,13 @@
1111

1212
import com.microsoft.graph.models.DirectoryObjectCollectionResponse;
1313
import com.microsoft.graph.models.Group;
14-
import com.microsoft.graph.models.GroupCollectionResponse;
1514
import com.microsoft.graph.models.odataerrors.MainError;
1615
import com.microsoft.graph.models.odataerrors.ODataError;
1716
import com.microsoft.graph.serviceclient.GraphServiceClient;
1817
import com.microsoft.graph.users.UsersRequestBuilder;
1918
import com.microsoft.graph.users.item.UserItemRequestBuilder;
20-
import com.microsoft.graph.users.item.memberof.MemberOfRequestBuilder;
21-
import com.microsoft.graph.users.item.memberof.graphgroup.GraphGroupRequestBuilder;
2219
import com.microsoft.graph.users.item.transitivememberof.TransitiveMemberOfRequestBuilder;
2320
import com.microsoft.kiota.RequestAdapter;
24-
2521
import org.elasticsearch.ElasticsearchSecurityException;
2622
import org.elasticsearch.action.ActionListener;
2723
import org.elasticsearch.action.support.PlainActionFuture;
@@ -32,10 +28,13 @@
3228
import org.elasticsearch.env.TestEnvironment;
3329
import org.elasticsearch.license.MockLicenseState;
3430
import org.elasticsearch.test.ESTestCase;
31+
import org.elasticsearch.threadpool.TestThreadPool;
32+
import org.elasticsearch.threadpool.ThreadPool;
3533
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
3634
import org.elasticsearch.xpack.core.security.authc.RealmSettings;
3735
import org.elasticsearch.xpack.core.security.authc.support.UserRoleMapper;
3836
import org.elasticsearch.xpack.core.security.user.User;
37+
import org.junit.After;
3938

4039
import java.util.List;
4140
import java.util.Set;
@@ -46,15 +45,14 @@
4645
import static org.hamcrest.Matchers.equalTo;
4746
import static org.mockito.ArgumentMatchers.any;
4847
import static org.mockito.ArgumentMatchers.eq;
49-
import static org.mockito.Mockito.doAnswer;
50-
import static org.mockito.Mockito.mock;
51-
import static org.mockito.Mockito.when;
48+
import static org.mockito.Mockito.*;
5249

5350
public class MicrosoftGraphAuthzRealmTests extends ESTestCase {
5451

5552
private final Settings globalSettings = Settings.builder().put("path.home", createTempDir()).build();
5653
private final Environment env = TestEnvironment.newEnvironment(globalSettings);
5754
private final ThreadContext threadContext = new ThreadContext(globalSettings);
55+
private final ThreadPool threadPool = new TestThreadPool(getClass().getName());
5856

5957
private final String realmName = randomAlphaOfLengthBetween(4, 10);
6058
private final String roleName = randomAlphaOfLengthBetween(4, 10);
@@ -67,6 +65,12 @@ public class MicrosoftGraphAuthzRealmTests extends ESTestCase {
6765
realmName
6866
);
6967

68+
@After
69+
public void tearDown() throws Exception {
70+
super.tearDown();
71+
terminate(threadPool);
72+
}
73+
7074
public void testLookupUser() {
7175
final var roleMapper = mockRoleMapper(Set.of(groupId), Set.of(roleName));
7276

@@ -102,7 +106,7 @@ public void testLookupUser() {
102106
final var licenseState = MockLicenseState.createMock();
103107
when(licenseState.isAllowed(eq(MICROSOFT_GRAPH_FEATURE))).thenReturn(true);
104108

105-
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState);
109+
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState, threadPool);
106110
final var future = new PlainActionFuture<User>();
107111
realm.lookupUser(username, future);
108112
final var user = future.actionGet();
@@ -140,7 +144,7 @@ public void testHandleGetUserPropertiesError() {
140144
final var licenseState = MockLicenseState.createMock();
141145
when(licenseState.isAllowed(eq(MICROSOFT_GRAPH_FEATURE))).thenReturn(true);
142146

143-
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState);
147+
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState, threadPool);
144148
final var future = new PlainActionFuture<User>();
145149
realm.lookupUser(username, future);
146150
final var thrown = assertThrows(ODataError.class, future::actionGet);
@@ -170,22 +174,20 @@ public void testHandleGetGroupMembershipError() {
170174
when(userRequestBuilder.byUserId(eq(username))).thenReturn(userItemRequestBuilder);
171175
when(userItemRequestBuilder.get(any())).thenReturn(msUser);
172176

173-
final var memberOfRequestBuilder = mock(MemberOfRequestBuilder.class);
174-
final var graphGroupRequestBuilder = mock(GraphGroupRequestBuilder.class);
177+
final var memberOfRequestBuilder = mock(TransitiveMemberOfRequestBuilder.class);
175178
final var graphError = new ODataError();
176179
final var error = new MainError();
177180
error.setCode("badRequest");
178181
error.setMessage("bad stuff happened");
179182
graphError.setError(error);
180183

181-
when(userItemRequestBuilder.memberOf()).thenReturn(memberOfRequestBuilder);
182-
when(memberOfRequestBuilder.graphGroup()).thenReturn(graphGroupRequestBuilder);
183-
when(graphGroupRequestBuilder.get(any())).thenThrow(graphError);
184+
when(userItemRequestBuilder.transitiveMemberOf()).thenReturn(memberOfRequestBuilder);
185+
when(memberOfRequestBuilder.get(any())).thenThrow(graphError);
184186

185187
final var licenseState = MockLicenseState.createMock();
186188
when(licenseState.isAllowed(eq(MICROSOFT_GRAPH_FEATURE))).thenReturn(true);
187189

188-
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState);
190+
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState, threadPool);
189191
final var future = new PlainActionFuture<User>();
190192
realm.lookupUser(username, future);
191193
final var thrown = assertThrows(ODataError.class, future::actionGet);
@@ -218,34 +220,32 @@ public void testGroupMembershipPagination() {
218220
when(userRequestBuilder.byUserId(eq(username))).thenReturn(userItemRequestBuilder);
219221
when(userItemRequestBuilder.get(any())).thenReturn(msUser);
220222

221-
final var memberOfRequestBuilder = mock(MemberOfRequestBuilder.class);
222-
final var graphGroupRequestBuilder = mock(GraphGroupRequestBuilder.class);
223+
final var memberOfRequestBuilder = mock(TransitiveMemberOfRequestBuilder.class);
223224
final var group1 = new Group();
224225
group1.setId(groupId);
225-
final var groupMembership1 = new GroupCollectionResponse();
226+
final var groupMembership1 = new DirectoryObjectCollectionResponse();
226227
groupMembership1.setValue(List.of(group1));
227228
groupMembership1.setOdataNextLink("http://localhost:12345/page2");
228229

229230
final var group2 = new Group();
230231
group2.setId(groupId2);
231-
final var groupMembership2 = new GroupCollectionResponse();
232+
final var groupMembership2 = new DirectoryObjectCollectionResponse();
232233
groupMembership2.setValue(List.of(group2));
233234
groupMembership2.setOdataNextLink("http://localhost:12345/page3");
234235

235236
final var group3 = new Group();
236237
group3.setId(groupId3);
237-
final var groupMembership3 = new GroupCollectionResponse();
238+
final var groupMembership3 = new DirectoryObjectCollectionResponse();
238239
groupMembership3.setValue(List.of(group3));
239240

240-
when(userItemRequestBuilder.memberOf()).thenReturn(memberOfRequestBuilder);
241-
when(memberOfRequestBuilder.graphGroup()).thenReturn(graphGroupRequestBuilder);
242-
when(graphGroupRequestBuilder.get(any())).thenReturn(groupMembership1);
241+
when(userItemRequestBuilder.transitiveMemberOf()).thenReturn(memberOfRequestBuilder);
242+
when(memberOfRequestBuilder.get(any())).thenReturn(groupMembership1);
243243
when(requestAdapter.send(any(), any(), any())).thenReturn(groupMembership2, groupMembership3);
244244

245245
final var licenseState = MockLicenseState.createMock();
246246
when(licenseState.isAllowed(eq(MICROSOFT_GRAPH_FEATURE))).thenReturn(true);
247247

248-
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState);
248+
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState, threadPool);
249249
final var future = new PlainActionFuture<User>();
250250
realm.lookupUser(username, future);
251251
final var user = future.actionGet();
@@ -268,7 +268,7 @@ public void testLicenseCheck() {
268268
final var licenseState = MockLicenseState.createMock();
269269
when(licenseState.isAllowed(eq(MICROSOFT_GRAPH_FEATURE))).thenReturn(false);
270270

271-
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState);
271+
final var realm = new MicrosoftGraphAuthzRealm(roleMapper, config, client, licenseState, threadPool);
272272
final var future = new PlainActionFuture<User>();
273273
realm.lookupUser(username, future);
274274
final var thrown = assertThrows(ElasticsearchSecurityException.class, future::actionGet);

0 commit comments

Comments
 (0)