Fixes null Check where a template is created with an admin (null) user #1292

Avinash1423 · 2025-12-23T21:01:56Z

Fix for #1201

Discription:

Added null Check for checkUserPermissions and moved isAdmin() to be executed prior to the checkUserPermissions().

Signed-off-by: Avinash Niyas <[email protected]>

codecov · 2025-12-23T21:08:39Z

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.43%. Comparing base (4ffc945) to head (23934b1).

Files with missing lines	Patch %	Lines
.../org/opensearch/flowframework/util/ParseUtils.java	14.28%	4 Missing and 2 partials ⚠️

❌ Your patch check has failed because the patch coverage (14.28%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1292      +/-   ##
============================================
- Coverage     77.46%   77.43%   -0.03%     
- Complexity     1260     1263       +3     
============================================
  Files           106      106              
  Lines          5901     5904       +3     
  Branches        612      614       +2     
============================================
+ Hits           4571     4572       +1     
  Misses         1034     1034              
- Partials        296      298       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dbwiddis

Thanks for this fix! A suggestion to combine the conditionals into one.

Also need a unit test or integ test validating this new behavior, and a CHANGELOG entry.

dbwiddis · 2025-12-23T23:29:26Z

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

+        if (requestedUser == null || resourceUser == null) {
+            return false;
+        }
        if (resourceUser.getBackendRoles() == null || requestedUser.getBackendRoles() == null) {
            return false;
        }


These lines can probably be combined into one conditional, or maybe we can create a helper method taking varargs User... that we can iterate over and just call if (nullOrNoRoles(requestedUser, resourceUsers)) { return false; }.

@dbwiddis Thanks for the feedback. Sure, will take a look

Hey @Avinash1423 sorry for slowness with the holidays and catching up this week. I'll take a good look at this this weekend.

…ackend roles. Signed-off-by: Avinash Niyas <[email protected]>

dbwiddis

Great progress, see comments below.

dbwiddis · 2026-01-09T19:14:13Z

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

+    private static boolean hasUsersAndRoles(User requestedUser, User resourceUser) {
+
+        return requestedUser != null
+            && resourceUser != null
+            && resourceUser.getBackendRoles() != null
+            && requestedUser.getBackendRoles() != null;
+
+    }


Suggested change

private static boolean hasUsersAndRoles(User requestedUser, User resourceUser) {

return requestedUser != null

&& resourceUser != null

&& resourceUser.getBackendRoles() != null

&& requestedUser.getBackendRoles() != null;

}

private static boolean hasUsersAndRoles(User... users) {

boolean nonNull = true;

for (User user: users) {

if (user == null || user.getBakendRoles() == null) {

nonNull = false;

break;

}

}

return nonNull;

}

Or you can possibly use a stream.

dbwiddis · 2026-01-09T19:15:12Z

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

-            return false;
-        }
+
+        if (!hasUsersAndRoles(requestedUser, resourceUser)) return false;


This works, but given the negation we may want to make the helper method nullUsersOrRoles and reverse the true/false in it.

dbwiddis · 2026-01-09T19:16:48Z

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

        return false;
    }

+    // method to expose checkUserPermissions for testing


It's better to just make the existing method package private with this comment than have a wrapper method to do the same thing.

It actually wouldn't hurt to just make the method public.

dbwiddis · 2026-01-09T19:19:34Z

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

+    public void testCheckUserPermissionsWithNullUsers() throws Exception {
+
+        User mockrequestedUser = null;
+        User mockresourceUser = new User();
+        String mockWorkFlowId = "mockWorkFlowId";
+
+        boolean res = ParseUtils.exposeCheckUserPermissions(mockrequestedUser, mockresourceUser, mockWorkFlowId);
+
+        assertFalse(res);
+
+    }


This is a good test case but you need to test all the conditional cases (null/non-null on both requested and resource user).

dbwiddis · 2026-01-09T19:20:15Z

CHANGELOG.md

 ### Features
 ### Enhancements
 ### Bug Fixes
+- Fixed user permission validation to correctly handle null users and backend roles.


Check the format of our change log entries (see the release notes for where they eventually go) and add a blank line after this so the markdown properly renders.

Fixes null Check where a template is created with an admin (null) user

ecc632b

Signed-off-by: Avinash Niyas <[email protected]>

Avinash1423 requested review from amitgalitz, dbwiddis, jackiehanyang, joshpalis, junweid62, ohltyler and owaiskazi19 as code owners December 23, 2025 21:01

dbwiddis reviewed Dec 23, 2025

View reviewed changes

Fixed user permission validation to correctly handle null users and b…

23934b1

…ackend roles. Signed-off-by: Avinash Niyas <[email protected]>

Avinash1423 force-pushed the nullFix branch from fef720b to 23934b1 Compare December 24, 2025 17:42

Avinash1423 requested a review from dbwiddis December 30, 2025 01:48

dbwiddis reviewed Jan 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixes null Check where a template is created with an admin (null) user #1292

Fixes null Check where a template is created with an admin (null) user #1292

Avinash1423 commented Dec 23, 2025

Uh oh!

codecov bot commented Dec 23, 2025 •

edited

Loading

Uh oh!

dbwiddis left a comment

Uh oh!

dbwiddis Dec 23, 2025 •

edited

Loading

Uh oh!

Avinash1423 Dec 24, 2025

Uh oh!

dbwiddis Jan 9, 2026

Uh oh!

dbwiddis left a comment

Uh oh!

dbwiddis Jan 9, 2026

Uh oh!

dbwiddis Jan 9, 2026

Uh oh!

dbwiddis Jan 9, 2026

Uh oh!

dbwiddis Jan 9, 2026

Uh oh!

dbwiddis Jan 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    private static boolean hasUsersAndRoles(User requestedUser, User resourceUser) {
-        return requestedUser != null
-            && resourceUser != null
-            && resourceUser.getBackendRoles() != null
-            && requestedUser.getBackendRoles() != null;
-    }
+    private static boolean hasUsersAndRoles(User... users) {
+        boolean nonNull = true;
+        for (User user: users) {
+            if (user == null || user.getBakendRoles() == null) {
+                nonNull = false;
+                break;
+            }
+        }
+        return nonNull;
+    }

Fixes null Check where a template is created with an admin (null) user #1292

Are you sure you want to change the base?

Fixes null Check where a template is created with an admin (null) user #1292

Conversation

Avinash1423 commented Dec 23, 2025

Uh oh!

codecov bot commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dbwiddis left a comment

Choose a reason for hiding this comment

Uh oh!

dbwiddis Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Avinash1423 Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

dbwiddis Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

dbwiddis left a comment

Choose a reason for hiding this comment

Uh oh!

dbwiddis Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

dbwiddis Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

dbwiddis Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

dbwiddis Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

dbwiddis Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Dec 23, 2025 •

edited

Loading

dbwiddis Dec 23, 2025 •

edited

Loading