Skip to content

1084 api changes#162

Open
Varaham wants to merge 3 commits intodevfrom
1084-api-changes
Open

1084 api changes#162
Varaham wants to merge 3 commits intodevfrom
1084-api-changes

Conversation

@Varaham
Copy link

@Varaham Varaham commented Aug 7, 2025

Description

The requirement from the customer is as below -
a) To clean up and standardize the outputs for queries to pull members for an app and/or a project with and without admin credentials.

b) Create the below new APIs for Engines / Apps to display the list of users who has access to view (filtering the access permission levels)
/getUsersForEngine
/getUsersForEngineNoCredentials
/getUsersForApp
/getUsersForAppNoCredentials

c) Created the APIs to do the CRUD operations to add / modify / remove users from App (Project) and Engine catalogs.

Changes Made

Changes made in the below classes :
EngineAuthorizationResource
ProjectAuthorizationResource
AdminProjectAuthorizationResource
AdminEngineAuthorizationResource

How to Test

The above APIs can be tested in the POSTMAN directly.

Notes

@github-actions
Copy link

github-actions bot commented Aug 7, 2025

@CodiumAI-Agent /describe

@github-actions
Copy link

github-actions bot commented Aug 7, 2025

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link

Title

1084 api changes


User description

Description

The requirement from the customer is as below -
a) To clean up and standardize the outputs for queries to pull members for an app and/or a project with and without admin credentials.

b) Create the below new APIs for Engines / Apps to display the list of users who has access to view (filtering the access permission levels)
/getUsersForEngine
/getUsersForEngineNoCredentials
/getUsersForApp
/getUsersForAppNoCredentials

c) Created the APIs to do the CRUD operations to add / modify / remove users from App (Project) and Engine catalogs.

Changes Made

Changes made in the below classes :
EngineAuthorizationResource
ProjectAuthorizationResource
AdminProjectAuthorizationResource
AdminEngineAuthorizationResource

How to Test

The above APIs can be tested in the POSTMAN directly.

Notes


PR Type

Enhancement


Description

  • Add admin engine user list/add/remove/update APIs

  • Add admin project user list/add/remove/update APIs

  • Introduce public endpoints for engine and app listing

  • Standardize permission utils calls and input sanitization


Diagram Walkthrough

flowchart LR
  AEAdmin["AdminEngineAuthorizationResource"]
  APAdmin["AdminProjectAuthorizationResource"]
  EAuth["EngineAuthorizationResource"]
  PAuth["ProjectAuthorizationResource"]
  SecAdmin["SecurityAdminUtils"]
  SecEngine["SecurityEngineUtils"]
  SecProject["SecurityProjectUtils"]
  AEAdmin -- "admin user management" --> SecAdmin
  APAdmin -- "admin user management" --> SecAdmin
  EAuth -- "public user listing & add" --> SecEngine
  PAuth -- "public user listing & add" --> SecProject
Loading

File Walkthrough

Relevant files
Enhancement
AdminEngineAuthorizationResource.java
Add admin engine user management endpoints                             

src/prrena/semoss/web/services/local/auth/AdminEngineAuthorizationResource.java

  • Imported SecurityEngineUtils
  • Added admin GET endpoints for engine user listing
  • Added admin POST endpoints for add/update/delete engine permissions
  • Implemented input sanitization and error handling
AdminProjectAuthorizationResource.java
Add admin app user management endpoints                                   

src/prrena/semoss/web/services/local/auth/AdminProjectAuthorizationResource.java

  • Added admin GET endpoints for app user listing
  • Added admin POST endpoints for bulk/single user permissions
  • Integrated graph API support for no-credentials listing
  • Applied input sanitization and error handling
EngineAuthorizationResource.java
Update and add engine public endpoints                                     

src/prrena/semoss/web/services/local/auth/EngineAuthorizationResource.java

  • Updated getEngineUsers calls to getUsersForEngine
  • Added public GET endpoints for engine user listing without credentials
  • Added public POST endpoint for adding engine permissions
  • Enhanced session checks and input sanitization
ProjectAuthorizationResource.java
Update and add app public endpoints                                           

src/prrena/semoss/web/services/local/auth/ProjectAuthorizationResource.java

  • Renamed projectId to appId in methods
  • Updated getProjectUsers to getUsersForApp
  • Added public GET endpoints for app user listing without credentials
  • Added public POST endpoint for adding app permissions

@github-actions
Copy link

github-actions bot commented Aug 7, 2025

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

Possible SQL injection:
several query parameters (e.g., searchTerm, appId, permission) are sanitized with inputSanitizer instead of inputSQLSanitizer. Validate or escape inputs consistently before using them in SQL queries.

⚡ Recommended focus areas for review

Inconsistent Sanitization

Different sanitizers are used for parameters (inputSanitizer vs inputSQLSanitizer), which may lead to SQL injection risk. Ensure all parameters used in database queries are consistently sanitized using inputSQLSanitizer.

	engineId = WebUtility.inputSanitizer(engineId);
    userId = WebUtility.inputSQLSanitizer(userId);
    searchTerm = WebUtility.inputSanitizer(searchTerm);
    permission = WebUtility.inputSanitizer(permission);
NullPointer Risk

Logging in catch blocks calls User.getSingleLogginName(user) but user may be null when ResourceUtility.getUser throws, causing a potential NPE. Add null checks or guard against null user in logs.

try {
	user = ResourceUtility.getUser(request);
} catch (IllegalAccessException e) {
	classLogger.warn(ResourceUtility.getLogMessage(request, request.getSession(false), User.getSingleLogginName(user), "invalid user session trying to access authorization resources"));
	classLogger.error(Constants.STACKTRACE, e);
	Map<String, String> errorMap = new HashMap<String, String>();
Code Duplication

Many API endpoints across AdminEngineAuthorizationResource and AdminProjectAuthorizationResource share nearly identical logic. Consider refactoring common patterns into shared utilities or a base class to reduce maintenance overhead.

@GET
@Produces("application/json")
@Path("getUsersForApp")
public Response getUsersForApp(@Context HttpServletRequest request, 
		@QueryParam("appId") String appId, @QueryParam("userId") String userId, 
		@QueryParam("searchTerm") String searchTerm, @QueryParam("permission") String permission, 
		@QueryParam("limit") long limit, @QueryParam("offset") long offset) {
	appId = WebUtility.inputSQLSanitizer(appId);
    userId = WebUtility.inputSQLSanitizer(userId);
    searchTerm = WebUtility.inputSQLSanitizer(searchTerm);
    permission = WebUtility.inputSQLSanitizer(permission);

	SecurityAdminUtils adminUtils = null;
	User user = null;
	try {
		user = ResourceUtility.getUser(request);
		adminUtils = performAdminCheck(request, user);
	} catch (IllegalAccessException e) {
		classLogger.warn(ResourceUtility.getLogMessage(request, request.getSession(false), User.getSingleLogginName(user), "is trying to pull all the users who use project " + appId + " when not an admin"));
		classLogger.error(Constants.STACKTRACE, e);
		Map<String, String> errorMap = new HashMap<String, String>();
		errorMap.put(Constants.ERROR_MESSAGE, e.getMessage());
		return WebUtility.getResponse(errorMap, 401);
	}
	String searchParam = searchTerm != null ? searchTerm : userId;
	List<Map<String, Object>> members = adminUtils.getProjectUsers(appId, searchParam, permission, limit, offset);
	long totalMembers = SecurityAdminUtils.getProjectUsersCount(appId, searchParam, permission);
	Map<String, Object> ret = new HashMap<String, Object>();
	ret.put("totalMembers", totalMembers);
	ret.put("members", members);
	return WebUtility.getResponse(ret, 200);
}

@QodoAI-Agent
Copy link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Security
Consistent SQL sanitization

All parameters used in DB queries should be SQL-sanitized to prevent injection.
Replace inputSanitizer with inputSQLSanitizer for engineId and searchTerm.

src/prerna/semoss/web/services/local/auth/AdminEngineAuthorizationResource.java [1060-1063]

-engineId = WebUtility.inputSanitizer(engineId);
+engineId = WebUtility.inputSQLSanitizer(engineId);
 userId = WebUtility.inputSQLSanitizer(userId);
-searchTerm = WebUtility.inputSanitizer(searchTerm);
+searchTerm = WebUtility.inputSQLSanitizer(searchTerm);
 permission = WebUtility.inputSanitizer(permission);
Suggestion importance[1-10]: 7

__

Why: Using inputSQLSanitizer for engineId and searchTerm prevents potential SQL injection and improves security.

Medium
Possible issue
Validate pagination bounds

Negative limit or offset values can cause unexpected behavior. Add validation to
reject values below zero with a 400 error.

src/prerna/semoss/web/services/local/auth/AdminEngineAuthorizationResource.java [1055-1059]

 @Path("getUsersForEngine")
 public Response getUsersForEngine(...,
     @QueryParam("limit") long limit, @QueryParam("offset") long offset) {
+  if (limit < 0 || offset < 0) {
+    Map<String,String> err = new HashMap<>();
+    err.put(Constants.ERROR_MESSAGE, "limit and offset must be non-negative");
+    return WebUtility.getResponse(err, 400);
+  }
   ...
Suggestion importance[1-10]: 6

__

Why: Adding checks for negative limit and offset ensures invalid pagination parameters are rejected early, improving robustness.

Low
General
Use generic JSON parsing

Use a TypeToken to parse JSON into the correct generic type and avoid unchecked
conversions.

src/prerna/semoss/web/services/local/auth/AdminEngineAuthorizationResource.java [1231]

 List<Map<String, Object>> permission =
-    new Gson().fromJson(form.getFirst("userpermissions"), List.class);
+    new Gson().fromJson(
+      form.getFirst("userpermissions"),
+      new TypeToken<List<Map<String,Object>>>(){}.getType()
+    );
Suggestion importance[1-10]: 5

__

Why: Parsing JSON with a TypeToken avoids unchecked conversions and makes the code safer and clearer.

Low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants