Skip to content

Commit c688d82

Browse files
Fix NPE on null user in RP code path
Signed-off-by: Darshit Chanpura <[email protected]>
1 parent c243f8a commit c688d82

File tree

2 files changed

+67
-2
lines changed

2 files changed

+67
-2
lines changed

plugin/src/main/java/org/opensearch/ml/helper/ModelAccessControlHelper.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public void validateModelGroupAccess(User user, String modelGroupId, String acti
106106
.onFailure(
107107
new OpenSearchStatusException(
108108
"User "
109-
+ user.getName()
109+
+ (user == null ? null : user.getName())
110110
+ " is not authorized to perform action "
111111
+ action
112112
+ " on ml-model-group id: "
@@ -173,6 +173,7 @@ public void validateModelGroupAccess(
173173
listener.onResponse(true);
174174
return;
175175
}
176+
176177
if (shouldUseResourceAuthz(ML_MODEL_GROUP_RESOURCE_TYPE)) {
177178
var resourceSharingClient = ResourceSharingClientAccessor.getInstance().getResourceSharingClient();
178179
resourceSharingClient.verifyAccess(modelGroupId, ML_MODEL_GROUP_RESOURCE_TYPE, action, ActionListener.wrap(isAuthorized -> {
@@ -181,7 +182,7 @@ public void validateModelGroupAccess(
181182
.onFailure(
182183
new OpenSearchStatusException(
183184
"User "
184-
+ user.getName()
185+
+ (user == null ? null : user.getName())
185186
+ " is not authorized to perform action "
186187
+ action
187188
+ " on ml-model-group id: "

plugin/src/test/java/org/opensearch/ml/helper/ModelAccessControlHelperTests.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,70 @@ public void test_ShouldUseResourceAuthz_FeatureDisabled_And_ClientNull() {
487487
assertFalse(ModelAccessControlHelper.shouldUseResourceAuthz(CommonValue.ML_MODEL_GROUP_RESOURCE_TYPE));
488488
}
489489

490+
public void test_ResourceAuthz_NotAuthorized_UserNull_UsesUnknownName() {
491+
when(resourceSharingClient.isFeatureEnabledForType(CommonValue.ML_MODEL_GROUP_RESOURCE_TYPE)).thenReturn(true);
492+
ResourceSharingClientAccessor.getInstance().setResourceSharingClient(resourceSharingClient);
493+
494+
doAnswer(invocation -> {
495+
ActionListener<Boolean> listener = invocation.getArgument(3);
496+
listener.onResponse(false);
497+
return null;
498+
}).when(resourceSharingClient).verifyAccess(any(), any(), any(), any());
499+
500+
modelAccessControlHelper
501+
.validateModelGroupAccess(
502+
null, // user
503+
mlFeatureEnabledSetting,
504+
"testTenant",
505+
"testGroupID",
506+
"testAction",
507+
client,
508+
sdkClient,
509+
actionListener
510+
);
511+
512+
ArgumentCaptor<Exception> argumentCaptor = ArgumentCaptor.forClass(Exception.class);
513+
verify(actionListener).onFailure(argumentCaptor.capture());
514+
Exception ex = argumentCaptor.getValue();
515+
assertTrue(ex instanceof org.opensearch.OpenSearchStatusException);
516+
assertTrue(ex.getMessage().contains("User null is not authorized"));
517+
518+
ResourceSharingClientAccessor.getInstance().setResourceSharingClient(null);
519+
}
520+
521+
public void test_ResourceAuthz_NotAuthorized_UserPresent_UsesUserName() {
522+
when(resourceSharingClient.isFeatureEnabledForType(CommonValue.ML_MODEL_GROUP_RESOURCE_TYPE)).thenReturn(true);
523+
ResourceSharingClientAccessor.getInstance().setResourceSharingClient(resourceSharingClient);
524+
525+
doAnswer(invocation -> {
526+
ActionListener<Boolean> listener = invocation.getArgument(3);
527+
listener.onResponse(false);
528+
return null;
529+
}).when(resourceSharingClient).verifyAccess(any(), any(), any(), any());
530+
531+
User user = User.parse("owner|IT,HR|myTenant");
532+
533+
modelAccessControlHelper
534+
.validateModelGroupAccess(
535+
user,
536+
mlFeatureEnabledSetting,
537+
"testTenant",
538+
"testGroupID",
539+
"testAction",
540+
client,
541+
sdkClient,
542+
actionListener
543+
);
544+
545+
ArgumentCaptor<Exception> argumentCaptor = ArgumentCaptor.forClass(Exception.class);
546+
verify(actionListener).onFailure(argumentCaptor.capture());
547+
Exception ex = argumentCaptor.getValue();
548+
assertTrue(ex instanceof org.opensearch.OpenSearchStatusException);
549+
assertTrue(ex.getMessage().contains("User owner is not authorized"));
550+
551+
ResourceSharingClientAccessor.getInstance().setResourceSharingClient(null);
552+
}
553+
490554
private GetResponse modelGroupBuilder(List<String> backendRoles, String access, String owner) throws IOException {
491555
MLModelGroup mlModelGroup = MLModelGroup
492556
.builder()

0 commit comments

Comments
 (0)