Skip to content
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.

Sign up for GitHub

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

feat: Add middleware to log all post requests. #651

Merged
merged 3 commits into from
Oct 11, 2024
Merged

feat: Add middleware to log all post requests. #651

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6336c74
feat: Add middleware to log all post requests.
Ashutosh619-sudo Oct 8, 2024
bfee7cf
comment resolved
Ashutosh619-sudo Oct 9, 2024
5f7ac41
Merge branch 'master' into log-post-api
Ashutosh619-sudo Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions fyle_netsuite_api/logging_middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import json
import traceback
import os
import random
Expand Down Expand Up @@ -56,3 +57,21 @@ def filter(self, record):
worker_id = getattr(record, 'worker_id', '')
record.worker_id = worker_id
return True

class LogPostRequestMiddleware:
def __init__(self, get_response):
self.get_response = get_response

def __call__(self, request):
if request.method in ['POST', 'PUT'] :
try:
body_unicode = request.body.decode('utf-8')
request_body = json.loads(body_unicode)
logger.info("POST request to %s: %s", request.path, request_body)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using the custom logger for consistency

The new middleware is using the root logger, while there's a custom get_logger() function available in this file. For consistency and to benefit from the worker ID logging, consider using the custom logger.

Replace the use of logger with the custom logger:

-logger.info("POST request to %s: %s", request.path, request_body)
+get_logger().info("POST request to %s: %s", request.path, request_body)

-logger.warning("Failed to decode POST request body for %s", request.path)
+get_logger().warning("Failed to decode POST request body for %s", request.path)

This change will ensure that the worker ID is included in the log messages from the new middleware, maintaining consistency with other parts of the application.

Also applies to: 72-72

except (json.JSONDecodeError, UnicodeDecodeError):
logger.warning("Failed to decode POST request body for %s", request.path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also do 1 general exception to be safe, just do logger.info there

except Exception as e:
logger.info('Something went wrong when logging post call - %s', e)

response = self.get_response(request)
return response
Comment on lines +61 to +77
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

🛠️ Refactor suggestion

Implement Data Masking or Filtering for Sensitive Information in Logs

The current implementation logs all POST and PUT request bodies without masking or filtering sensitive information. This can lead to exposure of sensitive data such as PII.

Recommended Actions:

  1. Implement Data Masking:
    • Use the custom get_logger() function to ensure consistency.
    • Introduce a mechanism to identify and mask sensitive fields before logging. For example: 
      class LogPostRequestMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response
        self.logger = get_logger()

    def __call__(self, request):
        if request.method in ['POST', 'PUT']:
            try:
                body_unicode = request.body.decode('utf-8')
                request_body = json.loads(body_unicode)
                masked_body = mask_sensitive_data(request_body)
                self.logger.info("POST request to %s: %s", request.path, masked_body)
            except (json.JSONDecodeError, UnicodeDecodeError):
                self.logger.warning("Failed to decode POST request body for %s", request.path)
            except Exception as e:
                self.logger.error('Unexpected error when logging post call - %s', str(e), exc_info=True)

        response = self.get_response(request)
        return response

def mask_sensitive_data(data):
    sensitive_fields = ['password', 'ssn', 'credit_card']
    for field in sensitive_fields:
        if field in data:
            data[field] = '****'
    return data
  2. Review Log Levels:
    • Change logger.info to logger.error for unexpected exceptions to improve issue visibility.
  3. Audit Logging Practices:
    • Regularly review logs to ensure no sensitive information is inadvertently captured.
🔗 Analysis chain

Implement custom logger and review exception handling

  1. As suggested in the previous review, please use the custom get_logger() function instead of the root logger for consistency and to benefit from worker ID logging.

  2. The general exception handler has been added as requested, but consider using logger.error instead of logger.info for unexpected exceptions. This will make it easier to identify and troubleshoot issues.

  3. Be cautious about logging all POST and PUT request bodies, as they may contain sensitive information. Consider implementing a mechanism to filter out or mask sensitive data before logging.

Here's a suggested implementation addressing points 1 and 2:

class LogPostRequestMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response
        self.logger = get_logger()

    def __call__(self, request):
        if request.method in ['POST', 'PUT']:
            try:
                body_unicode = request.body.decode('utf-8')
                request_body = json.loads(body_unicode)
                self.logger.info("POST request to %s: %s", request.path, request_body)
            except (json.JSONDecodeError, UnicodeDecodeError):
                self.logger.warning("Failed to decode POST request body for %s", request.path)
            except Exception as e:
                self.logger.error('Unexpected error when logging post call - %s', str(e), exc_info=True)

        response = self.get_response(request)
        return response

To address point 3, let's check if there are any existing mechanisms for handling sensitive data in the project:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing data masking or filtering mechanisms
rg -i "mask|filter|sensitive|pii" --type python

Length of output: 152

Script:

#!/bin/bash
# Search for existing data masking or filtering mechanisms
rg -i "mask|filter|sensitive|pii" --type py

Length of output: 101832

1 change: 1 addition & 0 deletions fyle_netsuite_api/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@

MIDDLEWARE = [
'request_logging.middleware.LoggingMiddleware',
'fyle_netsuite_api.logging_middleware.LogPostRequestMiddleware',
'fyle_netsuite_api.logging_middleware.ErrorHandlerMiddleware',
'corsheaders.middleware.CorsMiddleware',
'django.middleware.common.CommonMiddleware',
Expand Down
Loading