Feature/api secure enpoints#468
Conversation
…dling - Added jakarta.ws.rs-api in pom.xml - Refactored ApiResource to: - Add getPropertyList endpoint with sensitive property redaction - Improve getXml endpoint with proper XML parsing, XXE protection, and sensitive attribute removal - Implement redactSensitiveAttributes method to remove password/pwd fields from XML nodes
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| @GET | ||
| @RolesAllowed({"ADMIN"}) |
Check failure
Code scanning / CodeQL
XPath injection Critical
| <dependency> | ||
| <groupId>jakarta.ws.rs</groupId> | ||
| <artifactId>jakarta.ws.rs-api</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
This is a duplicate - the jakarta.ws.rs-api dependency is already defined on line 281 of this file.
Please remove.
| @Path("/getXml") | ||
| @Produces({MediaType.APPLICATION_XML,MediaType.APPLICATION_JSON}) | ||
| public Response getXml(@Context Request request, | ||
| @QueryParam("filename") String filename, |
There was a problem hiding this comment.
To be clear:
I will not support passing in a filename to be retrieved. It is a security risk since the user could pass a path to any file on the server using '../../etc'
If you are only interested in the partnerships XML file then there is no need to pass in a file name at all.
Additionally, the file names should not need to be known by the user of the UI and the partnerships file could be located anywhere on the server using any user defined name for the file itself so you should be retrieving the file using the call to the partnership factory method.
If you feel there is a need to retrieve more than just the partnerships XML file from the file system then you should pass in an identifier and map the supported set of identifiers to appropriate getters to retrieve the file.
If you want to be able to retrieve the config XML file then please provide a use case as the config.xml is not really intended to be modifed directly but instead controlled by properties set in an external properties file and I do not see what the benefit of retrieving the config XML file would be.
The partnerships file path can be retrieved using this:
String partnershipsFile = getProcessor().getSession(). getPartnershipFactory().getFilename()
There was a problem hiding this comment.
Thanks for the clarification — I agree with your concern around not exposing filenames directly and avoiding path traversal risks.
One point of clarification: there isn’t currently an existing method like
String partnershipsFile = getProcessor().getSession().getPartnershipFactory().getFilename();
in the codebase.
Given that, would it be acceptable if I implement a REST endpoint that returns the list of partnerships using:
getProcessor().getSession().getPartnershipFactory().getPartnerships();
This way, the endpoint exposes the in-memory partnerships directly via the factory without relying on file names, while keeping to the security and design guidelines you’ve outlined.
There was a problem hiding this comment.
Ah yes - the getFilename method in the XMLPartnershipFactory is not public so you would have to change the method to public then do this:
String partnershipsFile = ((XMLPartnershipFactory)getProcessor().getSession().getPartnershipFactory()).getFilename();
However, I will support accessing the list of partnerships directly.
There was a problem hiding this comment.
Thanks , I’d prefer accessing the list of partnerships directly. That approach feels cleaner
uhurusurfa
left a comment
uhurusurfa
left a comment
There was a problem hiding this comment.
Please remove the log file as well
There was a problem hiding this comment.
Please remove this file
3 participants
Add Jakarta WS RS API dependency and enhance ApiResource XML/JSON handling