-
Notifications
You must be signed in to change notification settings - Fork 43
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
HTTP Requests/Responses Logger #5
Comments
Hi guys, Sorry for the delay, couldn't review the change before. Could you please tell me how can I test this? Is it just about pulling the change and every time I run a request I'll get a file in IDEMPIERE_HOME/logs/rest-yyyy-mm-dd-1.log? One more question, why is this a POC solution? what's missing? Thanks a lot! |
For Logging we are using Log4j2. I created initial configuration in com.trekglobal.idempiere.rest.api/log4j2.xml. |
POC is because logger running but Requests body and response not included, coming soon. |
hi, we are almost there. #6
|
Merged the pull request. Please let me know if we can close this pull request or if there's something still pending. |
Hi @norbertbede / @igorpojzl, I just checked again the log files, and I had to revert the commits. There's a big security issue in this implementation. The log file is saving the password plainly in the requestBody of the first POST call (see the attached file for reference). These loggers should not save any sensitive data in the log files, which includes passwords, credit card data, email passwords, etc ... Please go ahead and create the pull request again addressing this issue. Best regards, |
hi. yes, it is not the best, however, the log is totally inside the firewall.
if any else we need specify from the experience the exact list Norbert |
In the response body this sensitive information is hidden, Heng Sin wrote that -> The password that is being exposed is on the request header, so there must be some logic in the RequestLogger that you guys wrote to not write those values on the plain file or at least replace them with *. What needs to be hidden, varies according to countries and industries, you can have:
If you have Nginx, for example, the logs might be store in Nginx. And this is about compliant with the law to avoid legal issues, it doesn't matter in that case if the info is behind a firewall, the laws state specifically that ceratin info should not be logged. You can read more about it here: https://medium.com/ibm-garage/pci-and-pii-compliance-when-logging-data-in-digital-transformation-projects-7739bab159a6 best regards, |
xref -> https://mattermost.idempiere.org/idempiere/pl/jphefce15frb5e6ecbe6gf365a this maybe can be closed? it seems is better to delegate this task to a gateway API |
Motivation
We worked on a project where iDempiere data was transformed by Apache Camel to a REST API developed by an eCommerce software vendor.
The vendor had a lot of troubles and we got an unstable API - non-measurable performance and non-auditable request/responses. We were forced in middleware to route the rest req/responses, times, and endpoints to a logging system for additional processing.
This experience and topic motivated us to implement an out-of-box HTTP request/response logger in REST API.
Solution POC
The functionality logs each HTTP request and response. We mark each request that comes from an API gets a request identifier (UUID). The purpose of the request identifier is to allow referring to any request/response for further analysis (similar like AWS services API's). We decide to use structured JSON (includes: request details, body, HTTP status code (2xx, 4xx, 5xx), response time in milliseconds, and rest endpoint).
Logging Output:
HTTP log is stored to the filesystem at IDEMPIERE_HOME/logs/rest-yyyy-mm-dd-1.log.
Advanced log Processing
Logs can be easily processed/routed by logging software collectors/agents and transfer to a centralized log analyzer like elasti.co (filebeat, journalbeat, metricsbeat)
We assume auth-token can be used in the index for differentiating multiple API's eg. per tenant. (we are using OAuth client id in our private implementation)
The text was updated successfully, but these errors were encountered: