Skip to content

Commit fef720b

Browse files
committed
Fixed user permission validation to correctly handle null users and backend roles.
Signed-off-by: Avinash Niyas <avinashNiyaz1423@gmail.com>
1 parent ecc632b commit fef720b

File tree

3 files changed

+39
-7
lines changed

3 files changed

+39
-7
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
77
### Features
88
### Enhancements
99
### Bug Fixes
10+
- Fixed user permission validation to correctly handle null users and backend roles.
1011
### Infrastructure
1112
### Documentation
1213
### Maintenance

src/main/java/org/opensearch/flowframework/util/ParseUtils.java

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,14 @@ public static void resolveUserAndExecute(
366366
}
367367
}
368368

369+
// helper method to check if varargs User... is null
370+
private static boolean hasUsersAndRoles(User requestedUser, User resourceUser){
371+
372+
return requestedUser != null && resourceUser != null && resourceUser.getBackendRoles() != null && requestedUser.getBackendRoles() != null;
373+
374+
375+
}
376+
369377
/**
370378
* Check if requested user has backend role required to access the resource
371379
* @param requestedUser the user to execute the request
@@ -374,13 +382,12 @@ public static void resolveUserAndExecute(
374382
* @return boolean if the requested user has backend role required to access the resource
375383
* @throws Exception exception
376384
*/
385+
386+
377387
private static boolean checkUserPermissions(User requestedUser, User resourceUser, String workflowId) throws Exception {
378-
if (requestedUser == null || resourceUser == null) {
379-
return false;
380-
}
381-
if (resourceUser.getBackendRoles() == null || requestedUser.getBackendRoles() == null) {
382-
return false;
383-
}
388+
389+
if (!hasUsersAndRoles(requestedUser, resourceUser)) return false;
390+
384391
// Check if requested user has backend role required to access the resource
385392
for (String backendRole : requestedUser.getBackendRoles()) {
386393
if (resourceUser.getBackendRoles().contains(backendRole)) {
@@ -398,6 +405,13 @@ private static boolean checkUserPermissions(User requestedUser, User resourceUse
398405
return false;
399406
}
400407

408+
// method to expose checkUserPermissions for testing
409+
static boolean exposeCheckUserPermissions(User requestedUser, User resourceUser, String workflowId) throws Exception{
410+
411+
return checkUserPermissions(requestedUser, resourceUser, workflowId);
412+
413+
}
414+
401415
/**
402416
* Check if filter by backend roles is enabled and validate the requested user
403417
* @param requestedUser the user to execute the request

src/test/java/org/opensearch/flowframework/util/ParseUtilsTests.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99
package org.opensearch.flowframework.util;
1010

11+
import org.mockito.Mockito;
1112
import org.opensearch.common.xcontent.XContentFactory;
1213
import org.opensearch.commons.authuser.User;
1314
import org.opensearch.core.rest.RestStatus;
@@ -29,7 +30,6 @@
2930
import java.util.List;
3031
import java.util.Map;
3132
import java.util.Set;
32-
3333
import static org.opensearch.flowframework.util.ParseUtils.isAdmin;
3434

3535
public class ParseUtilsTests extends OpenSearchTestCase {
@@ -444,4 +444,21 @@ public void testIsAdminBackendRoleIsAllAccess() {
444444
public void testIsAdminNull() {
445445
assertFalse(isAdmin(null));
446446
}
447+
448+
449+
public void testCheckUserPermissionsWithNullUsers() throws Exception {
450+
451+
User mockrequestedUser=null;
452+
User mockresourceUser=new User();
453+
String mockWorkFlowId="mockWorkFlowId";
454+
455+
boolean res= ParseUtils.exposeCheckUserPermissions(mockrequestedUser,mockresourceUser,mockWorkFlowId);
456+
457+
assertFalse(res);
458+
459+
}
460+
461+
462+
447463
}
464+

0 commit comments

Comments
 (0)