-
Notifications
You must be signed in to change notification settings - Fork 51
fix: ensure get login response passes right response #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -352,32 +352,35 @@ private void createUserMapping(User mUser, JSONObject resMap, JSONObject service | |||||||||||||||||||||||||||||||||||||||
| if (mUser.getDesignation() != null) { | ||||||||||||||||||||||||||||||||||||||||
| resMap.put("designation", new JSONObject(mUser.getDesignation().toString())); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| for (UserServiceRoleMapping m_UserServiceRoleMapping : mUser.getM_UserServiceRoleMapping()) { | ||||||||||||||||||||||||||||||||||||||||
| serviceRoleMultiMap.put( | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getM_ServiceMaster().getServiceName(), | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_Role().getRoleName()); | ||||||||||||||||||||||||||||||||||||||||
| String serv = m_UserServiceRoleMapping.getM_ProviderServiceMapping().getM_ServiceMaster().getServiceName(); | ||||||||||||||||||||||||||||||||||||||||
| if (!previlegeObj.has(serv)) { | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.put(serv, new JSONObject( | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getM_ServiceMaster().toString())); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("serviceName", serv); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("serviceID", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getProviderServiceMapID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("providerServiceMapID", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getProviderServiceMapID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("apimanClientKey", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getAPIMANClientKey()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("roles", new JSONArray()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("stateID", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getStateID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("agentID", m_UserServiceRoleMapping.getAgentID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("agentPassword", m_UserServiceRoleMapping.getAgentPassword()); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| JSONArray roles = previlegeObj.getJSONObject(serv).getJSONArray("roles"); | ||||||||||||||||||||||||||||||||||||||||
| if (null != mUser.getM_UserServiceRoleMapping()) { | ||||||||||||||||||||||||||||||||||||||||
| for (UserServiceRoleMapping m_UserServiceRoleMapping : mUser.getM_UserServiceRoleMapping()) { | ||||||||||||||||||||||||||||||||||||||||
| serviceRoleMultiMap.put( | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getM_ServiceMaster().getServiceName(), | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_Role().getRoleName()); | ||||||||||||||||||||||||||||||||||||||||
| String serv = m_UserServiceRoleMapping.getM_ProviderServiceMapping().getM_ServiceMaster() | ||||||||||||||||||||||||||||||||||||||||
| .getServiceName(); | ||||||||||||||||||||||||||||||||||||||||
| if (!previlegeObj.has(serv)) { | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.put(serv, new JSONObject( | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getM_ServiceMaster().toString())); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("serviceName", serv); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("serviceID", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getProviderServiceMapID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("providerServiceMapID", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getProviderServiceMapID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("apimanClientKey", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getAPIMANClientKey()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("roles", new JSONArray()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("stateID", | ||||||||||||||||||||||||||||||||||||||||
| m_UserServiceRoleMapping.getM_ProviderServiceMapping().getStateID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("agentID", m_UserServiceRoleMapping.getAgentID()); | ||||||||||||||||||||||||||||||||||||||||
| previlegeObj.getJSONObject(serv).put("agentPassword", m_UserServiceRoleMapping.getAgentPassword()); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| JSONArray roles = previlegeObj.getJSONObject(serv).getJSONArray("roles"); | ||||||||||||||||||||||||||||||||||||||||
| // roles.put(new JSONObject(m_UserServiceRoleMapping.getM_Role().toString())); | ||||||||||||||||||||||||||||||||||||||||
| JSONObject roleObject = new JSONObject(m_UserServiceRoleMapping.getM_Role().toString()); | ||||||||||||||||||||||||||||||||||||||||
| roleObject.put("isSanjeevani", m_UserServiceRoleMapping.getIsSanjeevani()); | ||||||||||||||||||||||||||||||||||||||||
| roles.put(roleObject); | ||||||||||||||||||||||||||||||||||||||||
| JSONObject roleObject = new JSONObject(m_UserServiceRoleMapping.getM_Role().toString()); | ||||||||||||||||||||||||||||||||||||||||
| roleObject.put("isSanjeevani", m_UserServiceRoleMapping.getIsSanjeevani()); | ||||||||||||||||||||||||||||||||||||||||
| roles.put(roleObject); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| Iterator<String> keySet = serviceRoleMultiMap.keys(); | ||||||||||||||||||||||||||||||||||||||||
| while (keySet.hasNext()) { | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -483,8 +486,50 @@ public String superUserAuthenticate( | |||||||||||||||||||||||||||||||||||||||
| public String getLoginResponse(HttpServletRequest request) { | ||||||||||||||||||||||||||||||||||||||||
| OutputResponse response = new OutputResponse(); | ||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||
| response.setResponse(sessionObject.getSessionObject(request.getHeader("Authorization"))); | ||||||||||||||||||||||||||||||||||||||||
| } catch (RedisSessionException e) { | ||||||||||||||||||||||||||||||||||||||||
| String authHeader = request.getHeader("Authorization"); | ||||||||||||||||||||||||||||||||||||||||
| if (authHeader.isEmpty()) { | ||||||||||||||||||||||||||||||||||||||||
| // Try JWT token from header first | ||||||||||||||||||||||||||||||||||||||||
| String jwtToken = request.getHeader("Jwttoken"); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // If not in header, try cookie | ||||||||||||||||||||||||||||||||||||||||
| if (jwtToken == null) { | ||||||||||||||||||||||||||||||||||||||||
| Cookie[] cookies = request.getCookies(); | ||||||||||||||||||||||||||||||||||||||||
| if (cookies != null) { | ||||||||||||||||||||||||||||||||||||||||
| for (Cookie cookie : cookies) { | ||||||||||||||||||||||||||||||||||||||||
| if ("jwtToken".equals(cookie.getName())) { | ||||||||||||||||||||||||||||||||||||||||
| jwtToken = cookie.getValue(); | ||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (jwtToken == null) { | ||||||||||||||||||||||||||||||||||||||||
| throw new IEMRException("No authentication token found in header or cookie"); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Extract user ID from the JWT token | ||||||||||||||||||||||||||||||||||||||||
| String userId = jwtUtil.getUserIdFromToken(jwtToken); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+507
to
+513
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate JWT token before extracting userId. The code extracts userId from the JWT token without validating it first. This could lead to security issues if an invalid or expired token is provided. Add JWT validation before extracting the userId: if (jwtToken == null) {
throw new IEMRException("No authentication token found in header or cookie");
}
+// Validate the JWT token first
+Claims claims = jwtUtil.validateToken(jwtToken);
+if (claims == null) {
+ throw new IEMRException("Invalid or expired JWT token");
+}
+
// Extract user ID from the JWT token
String userId = jwtUtil.getUserIdFromToken(jwtToken);π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
| // Get user details and prepare response | ||||||||||||||||||||||||||||||||||||||||
| User user = iemrAdminUserServiceImpl.getUserById(Long.parseLong(userId)); | ||||||||||||||||||||||||||||||||||||||||
| if (user == null) { | ||||||||||||||||||||||||||||||||||||||||
| throw new IEMRException("User not found"); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| String remoteAddress = request.getHeader("X-FORWARDED-FOR"); | ||||||||||||||||||||||||||||||||||||||||
| if (remoteAddress == null || remoteAddress.trim().length() == 0) { | ||||||||||||||||||||||||||||||||||||||||
| remoteAddress = request.getRemoteAddr(); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Use the helper method to prepare response | ||||||||||||||||||||||||||||||||||||||||
| JSONObject responseObj = prepareAuthenticationResponse(user, remoteAddress, request.getRemoteHost()); | ||||||||||||||||||||||||||||||||||||||||
| response.setResponse(responseObj.toString()); | ||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||
| response.setResponse(sessionObject.getSessionObject(authHeader)); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||||||||||||||||||||||
| logger.error("getLoginResponse failed with error " + e.getMessage(), e); | ||||||||||||||||||||||||||||||||||||||||
| response.setError(e); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| return response.toString(); | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1057,4 +1102,30 @@ public ResponseEntity<String> getJwtTokenFromCookie(HttpServletRequest httpReque | |||||||||||||||||||||||||||||||||||||||
| return ResponseEntity.status(HttpStatus.NOT_FOUND).body("JWT token not found"); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| private JSONObject prepareAuthenticationResponse(User mUser, String remoteAddress, String remoteHost) throws Exception { | ||||||||||||||||||||||||||||||||||||||||
| JSONObject resMap = new JSONObject(); | ||||||||||||||||||||||||||||||||||||||||
| JSONObject serviceRoleMultiMap = new JSONObject(); | ||||||||||||||||||||||||||||||||||||||||
| JSONObject serviceRoleMap = new JSONObject(); | ||||||||||||||||||||||||||||||||||||||||
| JSONArray serviceRoleList = new JSONArray(); | ||||||||||||||||||||||||||||||||||||||||
| JSONObject previlegeObj = new JSONObject(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (mUser != null) { | ||||||||||||||||||||||||||||||||||||||||
| mUser.setM_UserServiceRoleMapping(iemrAdminUserServiceImpl.getUserServiceRoleMapping(mUser.getUserID())); | ||||||||||||||||||||||||||||||||||||||||
| createUserMapping(mUser, resMap, serviceRoleMultiMap, serviceRoleMap, serviceRoleList, previlegeObj); | ||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||
| resMap.put("isAuthenticated", false); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| JSONObject responseObj = new JSONObject(resMap.toString()); | ||||||||||||||||||||||||||||||||||||||||
| JSONArray previlageObjs = new JSONArray(); | ||||||||||||||||||||||||||||||||||||||||
| Iterator<?> services = previlegeObj.keys(); | ||||||||||||||||||||||||||||||||||||||||
| while (services.hasNext()) { | ||||||||||||||||||||||||||||||||||||||||
| String service = (String) services.next(); | ||||||||||||||||||||||||||||||||||||||||
| previlageObjs.put(previlegeObj.getJSONObject(service)); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| responseObj.put("previlegeObj", previlageObjs); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return iemrAdminUserServiceImpl.generateKeyAndValidateIP(responseObj, remoteAddress, remoteHost); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -90,6 +90,10 @@ public long getRefreshTokenExpiration() { | |||||||||||||||||||||||||||||||
| return REFRESH_EXPIRATION_TIME; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| public String getUserIdFromToken(String token) { | ||||||||||||||||||||||||||||||||
| return getAllClaimsFromToken(token).get("userId", String.class); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
Comment on lines
+93
to
+96
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for token validation and missing claims. The method should handle potential exceptions and validate that the userId claim exists before returning it. Consider wrapping in try-catch or documenting the expected exceptions. Apply this diff to add error handling: public String getUserIdFromToken(String token) {
- return getAllClaimsFromToken(token).get("userId", String.class);
+ try {
+ Claims claims = getAllClaimsFromToken(token);
+ String userId = claims.get("userId", String.class);
+ if (userId == null || userId.isEmpty()) {
+ throw new IllegalArgumentException("JWT token does not contain userId claim");
+ }
+ return userId;
+ } catch (Exception e) {
+ throw new IllegalArgumentException("Failed to extract userId from token", e);
+ }
}π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| // Additional helper methods | ||||||||||||||||||||||||||||||||
| public String getJtiFromToken(String token) { | ||||||||||||||||||||||||||||||||
| return getAllClaimsFromToken(token).getId(); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π οΈ Refactor suggestion
Use consistent header naming convention.
The header name "Jwttoken" should follow a consistent naming convention. Consider using "JWT-Token", "JwtToken", or "Authorization" with "Bearer" prefix as per standard practices.
π€ Prompt for AI Agents