-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add text summarization endpoint - PR-9 #157
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
8e7140e
d6e125f
e6b6c3d
ca8589b
fa2988c
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 | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,11 @@ | ||||||||||||||||||||||||||||||||
| from functools import wraps | ||||||||||||||||||||||||||||||||
| from flask import request, jsonify | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| def require_api_key(f): | ||||||||||||||||||||||||||||||||
| @wraps(f) | ||||||||||||||||||||||||||||||||
| def decorated_function(*args, **kwargs): | ||||||||||||||||||||||||||||||||
| api_key = request.headers.get('X-API-Key') | ||||||||||||||||||||||||||||||||
| if api_key != 'your-secret-key': # Replace with actual key or env var | ||||||||||||||||||||||||||||||||
|
Comment on lines
+3
to
+8
|
||||||||||||||||||||||||||||||||
| def require_api_key(f): | |
| @wraps(f) | |
| def decorated_function(*args, **kwargs): | |
| api_key = request.headers.get('X-API-Key') | |
| if api_key != 'your-secret-key': # Replace with actual key or env var | |
| import os | |
| def require_api_key(f): | |
| @wraps(f) | |
| def decorated_function(*args, **kwargs): | |
| api_key = request.headers.get('X-API-Key') | |
| expected_api_key = os.environ.get('API_KEY') | |
| if expected_api_key is None: | |
| return jsonify({'error': 'Server misconfiguration: API key not set'}), 500 | |
| if api_key != expected_api_key: |
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.
Hardcoding secrets like API keys is a major security risk. This key should be loaded from an environment variable or a secure configuration service to prevent accidental exposure. Please also remember to add import os at the top of the file.
| if api_key != 'your-secret-key': # Replace with actual key or env var | |
| return jsonify({'error': 'API key required'}), 401 | |
| expected_key = os.environ.get('API_KEY') | |
| if not expected_key or api_key != expected_key: | |
| return jsonify({'error': 'API key required or invalid'}), 401 |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,128 @@ | ||||||||||||||||
| from flask import Blueprint, request, jsonify | ||||||||||||||||
| from flask_restx import Api, Resource, fields | ||||||||||||||||
| import logging | ||||||||||||||||
| from typing import Dict, Any, Optional | ||||||||||||||||
| import time | ||||||||||||||||
|
|
||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||
|
|
||||||||||||||||
| # Create emotion endpoint blueprint | ||||||||||||||||
| emotion_bp = Blueprint('emotion', __name__, url_prefix='/api/analyze') | ||||||||||||||||
|
|
||||||||||||||||
| # Create API namespace | ||||||||||||||||
| api = Api(emotion_bp, doc=False, title='Emotion Analysis API', version='1.0') | ||||||||||||||||
|
|
||||||||||||||||
| # Define request/response models | ||||||||||||||||
| emotion_request = api.model('EmotionRequest', { | ||||||||||||||||
| 'text': fields.String(required=True, description='Text to analyze for emotions'), | ||||||||||||||||
| 'generate_summary': fields.Boolean(required=False, default=False, description='Generate text summary') | ||||||||||||||||
| }) | ||||||||||||||||
|
|
||||||||||||||||
| emotion_response = api.model('EmotionResponse', { | ||||||||||||||||
| 'emotions': fields.List(fields.String, description='Detected emotions'), | ||||||||||||||||
| 'confidence_scores': fields.List(fields.Float, description='Confidence scores for each emotion'), | ||||||||||||||||
| 'summary': fields.String(description='Text summary (if requested)'), | ||||||||||||||||
| 'processing_time': fields.Float(description='Processing time in seconds'), | ||||||||||||||||
| 'text_length': fields.Integer(description='Length of input text'), | ||||||||||||||||
| 'timestamp': fields.String(description='Analysis timestamp') | ||||||||||||||||
| }) | ||||||||||||||||
|
|
||||||||||||||||
| @api.route('/journal') | ||||||||||||||||
| class EmotionAnalysis(Resource): | ||||||||||||||||
| """Emotion analysis endpoint for journal entries.""" | ||||||||||||||||
|
|
||||||||||||||||
| @api.expect(emotion_request) | ||||||||||||||||
| @api.marshal_with(emotion_response) | ||||||||||||||||
| def post(self): | ||||||||||||||||
| """Analyze emotions in journal text.""" | ||||||||||||||||
| try: | ||||||||||||||||
| start_time = time.time() | ||||||||||||||||
|
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. issue (code-quality): We've found these issues:
|
||||||||||||||||
|
|
||||||||||||||||
| # Get request data | ||||||||||||||||
| data = request.get_json() | ||||||||||||||||
| if not data or 'text' not in data: | ||||||||||||||||
| return {'error': 'Text is required'}, 400 | ||||||||||||||||
|
Comment on lines
+43
to
+44
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. suggestion: Error responses do not use Flask's jsonify, which may lead to inconsistent response formatting. Use 'jsonify' for error responses to maintain consistent response formatting and content-type. |
||||||||||||||||
|
|
||||||||||||||||
| text = data['text'] | ||||||||||||||||
| generate_summary = data.get('generate_summary', False) | ||||||||||||||||
|
|
||||||||||||||||
| # Validate input | ||||||||||||||||
| if not isinstance(text, str) or len(text.strip()) == 0: | ||||||||||||||||
| return {'error': 'Text must be a non-empty string'}, 400 | ||||||||||||||||
|
|
||||||||||||||||
| if len(text) > 10000: # 10k character limit | ||||||||||||||||
| return {'error': 'Text too long (max 10,000 characters)'}, 400 | ||||||||||||||||
|
|
||||||||||||||||
| # Mock emotion analysis (replace with actual model integration) | ||||||||||||||||
| emotions, confidence_scores = self._analyze_emotions(text) | ||||||||||||||||
|
|
||||||||||||||||
| # Generate summary if requested | ||||||||||||||||
| summary = None | ||||||||||||||||
| if generate_summary: | ||||||||||||||||
| summary = self._generate_summary(text) | ||||||||||||||||
|
|
||||||||||||||||
| processing_time = time.time() - start_time | ||||||||||||||||
|
|
||||||||||||||||
| # Prepare response | ||||||||||||||||
| response = { | ||||||||||||||||
| 'emotions': emotions, | ||||||||||||||||
| 'confidence_scores': confidence_scores, | ||||||||||||||||
| 'summary': summary, | ||||||||||||||||
| 'processing_time': round(processing_time, 3), | ||||||||||||||||
| 'text_length': len(text), | ||||||||||||||||
| 'timestamp': time.strftime('%Y-%m-%d %H:%M:%S UTC', time.gmtime()) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| logger.info(f"Emotion analysis completed: {len(emotions)} emotions detected in {processing_time:.3f}s") | ||||||||||||||||
| return response, 200 | ||||||||||||||||
|
|
||||||||||||||||
| except Exception as e: | ||||||||||||||||
| logger.error(f"Emotion analysis failed: {e}") | ||||||||||||||||
| return {'error': 'Emotion analysis failed'}, 500 | ||||||||||||||||
|
Comment on lines
+79
to
+81
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. Catching a generic |
||||||||||||||||
|
|
||||||||||||||||
| def _analyze_emotions(self, text: str) -> tuple[list[str], list[float]]: | ||||||||||||||||
| """Analyze emotions in text (mock implementation).""" | ||||||||||||||||
| # Mock emotion detection - replace with actual SAMO BERT model | ||||||||||||||||
| emotions = [] | ||||||||||||||||
| confidence_scores = [] | ||||||||||||||||
|
|
||||||||||||||||
| # Simple keyword-based emotion detection for demo | ||||||||||||||||
| text_lower = text.lower() | ||||||||||||||||
|
|
||||||||||||||||
| emotion_keywords = { | ||||||||||||||||
| 'joy': ['happy', 'excited', 'joyful', 'cheerful', 'delighted'], | ||||||||||||||||
| 'sadness': ['sad', 'depressed', 'melancholy', 'gloomy', 'sorrowful'], | ||||||||||||||||
| 'anger': ['angry', 'mad', 'furious', 'irritated', 'annoyed'], | ||||||||||||||||
| 'fear': ['afraid', 'scared', 'terrified', 'anxious', 'worried'], | ||||||||||||||||
| 'surprise': ['surprised', 'shocked', 'amazed', 'astonished'], | ||||||||||||||||
| 'disgust': ['disgusted', 'revolted', 'repulsed', 'sickened'] | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| for emotion, keywords in emotion_keywords.items(): | ||||||||||||||||
| confidence = sum(1 for keyword in keywords if keyword in text_lower) / len(keywords) | ||||||||||||||||
|
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. suggestion (code-quality): Simplify constant sum() call (
Suggested change
ExplanationAssum add the values it treats True as 1, and False as 0. We make useof this fact to simplify the generator expression inside the sum call.
|
||||||||||||||||
| if confidence > 0.1: # Threshold for detection | ||||||||||||||||
| emotions.append(emotion) | ||||||||||||||||
| confidence_scores.append(min(confidence * 2, 1.0)) # Scale to 0-1 | ||||||||||||||||
|
|
||||||||||||||||
| # If no emotions detected, add neutral | ||||||||||||||||
| if not emotions: | ||||||||||||||||
| emotions = ['neutral'] | ||||||||||||||||
| confidence_scores = [0.5] | ||||||||||||||||
|
|
||||||||||||||||
| return emotions, confidence_scores | ||||||||||||||||
|
|
||||||||||||||||
| def _generate_summary(self, text: str) -> str: | ||||||||||||||||
| """Generate text summary (mock implementation).""" | ||||||||||||||||
| # Mock summarization - replace with actual T5 model | ||||||||||||||||
| words = text.split() | ||||||||||||||||
| if len(words) <= 20: | ||||||||||||||||
| return text | ||||||||||||||||
|
|
||||||||||||||||
| # Simple extractive summary (first 20 words) | ||||||||||||||||
| summary_words = words[:20] | ||||||||||||||||
| return ' '.join(summary_words) + '...' | ||||||||||||||||
|
Comment on lines
+118
to
+123
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. suggestion (code-quality): We've found these issues:
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| def register_emotion_endpoints(app): | ||||||||||||||||
| """Register emotion endpoints with the Flask app.""" | ||||||||||||||||
| app.register_blueprint(emotion_bp) | ||||||||||||||||
| logger.info("Emotion endpoints registered: /api/analyze/journal") | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| from flask import Blueprint, jsonify, request | ||
| from health_monitor import health_monitor | ||
| import logging | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Create health endpoints blueprint | ||
| health_bp = Blueprint('health', __name__, url_prefix='/api/health') | ||
|
|
||
| @health_bp.route('/', methods=['GET']) | ||
| def health_check(): | ||
| """Basic health check endpoint.""" | ||
| try: | ||
| summary = health_monitor.get_health_summary() | ||
| status_code = 200 if summary["status"] in ["healthy", "warning"] else 503 | ||
| return jsonify(summary), status_code | ||
| except Exception as e: | ||
| logger.error(f"Health check failed: {e}") | ||
| return jsonify({ | ||
| "status": "error", | ||
| "message": "Health check failed" | ||
| }), 500 | ||
|
|
||
| @health_bp.route('/detailed', methods=['GET']) | ||
| def detailed_health(): | ||
| """Detailed health check with system metrics.""" | ||
| try: | ||
| health_data = health_monitor.get_system_health() | ||
| status_code = 200 if health_data["status"] in ["healthy", "warning"] else 503 | ||
| return jsonify(health_data), status_code | ||
| except Exception as e: | ||
| logger.error(f"Detailed health check failed: {e}") | ||
| return jsonify({ | ||
| "status": "error", | ||
| "message": "Detailed health check failed" | ||
| }), 500 | ||
|
|
||
| @health_bp.route('/ready', methods=['GET']) | ||
| def readiness_check(): | ||
| """Kubernetes readiness probe endpoint.""" | ||
| try: | ||
| health_data = health_monitor.get_system_health() | ||
| if health_data["status"] in ["healthy", "warning"]: | ||
| return jsonify({"ready": True}), 200 | ||
| else: | ||
| return jsonify({"ready": False, "reason": health_data["status"]}), 503 | ||
| except Exception as e: | ||
| logger.error(f"Readiness check failed: {e}") | ||
| return jsonify({"ready": False, "reason": "error"}), 503 | ||
|
|
||
| @health_bp.route('/live', methods=['GET']) | ||
| def liveness_check(): | ||
| """Kubernetes liveness probe endpoint.""" | ||
| try: | ||
| # Simple liveness check - just verify the service is responding | ||
| return jsonify({"alive": True}), 200 | ||
| except Exception as e: | ||
| logger.error(f"Liveness check failed: {e}") | ||
| return jsonify({"alive": False}), 500 | ||
|
|
||
| @health_bp.route('/metrics', methods=['GET']) | ||
| def health_metrics(): | ||
| """Health metrics endpoint for monitoring systems.""" | ||
| try: | ||
| health_data = health_monitor.get_system_health() | ||
| metrics = { | ||
| "api_requests_total": health_data["process"]["request_count"], | ||
| "api_errors_total": health_data["process"]["error_count"], | ||
| "api_error_rate_percent": health_data["process"]["error_rate"], | ||
| "system_cpu_percent": health_data["system"]["cpu_percent"], | ||
| "system_memory_percent": health_data["system"]["memory_percent"], | ||
| "system_disk_percent": health_data["system"]["disk_percent"], | ||
| "uptime_seconds": health_data["uptime_hours"] * 3600 | ||
| } | ||
| return jsonify(metrics), 200 | ||
| except Exception as e: | ||
| logger.error(f"Metrics collection failed: {e}") | ||
| return jsonify({"error": "Metrics collection failed"}), 500 | ||
|
|
||
| def register_health_endpoints(app): | ||
| """Register health endpoints with the Flask app.""" | ||
| app.register_blueprint(health_bp) | ||
| logger.info("Health endpoints registered: /api/health/*") |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,90 @@ | ||||||||||||||||||||||||
| import time | ||||||||||||||||||||||||
| import psutil | ||||||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||||||
| from typing import Dict, Any, Optional | ||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| class HealthMonitor: | ||||||||||||||||||||||||
| """Health monitoring system for API endpoints and system resources.""" | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def __init__(self): | ||||||||||||||||||||||||
| self.start_time = time.time() | ||||||||||||||||||||||||
| self.request_count = 0 | ||||||||||||||||||||||||
| self.error_count = 0 | ||||||||||||||||||||||||
| self.last_health_check = None | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def get_system_health(self) -> Dict[str, Any]: | ||||||||||||||||||||||||
| """Get comprehensive system health metrics.""" | ||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||
| # System resource usage | ||||||||||||||||||||||||
| cpu_percent = psutil.cpu_percent(interval=1) | ||||||||||||||||||||||||
|
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.
Suggested change
|
||||||||||||||||||||||||
| memory = psutil.virtual_memory() | ||||||||||||||||||||||||
| disk = psutil.disk_usage('/') | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Process information | ||||||||||||||||||||||||
| process = psutil.Process() | ||||||||||||||||||||||||
| process_memory = process.memory_info().rss / 1024 / 1024 # MB | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Uptime calculation | ||||||||||||||||||||||||
| uptime_seconds = time.time() - self.start_time | ||||||||||||||||||||||||
| uptime_hours = uptime_seconds / 3600 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| health_data = { | ||||||||||||||||||||||||
| "status": "healthy", | ||||||||||||||||||||||||
| "timestamp": datetime.utcnow().isoformat(), | ||||||||||||||||||||||||
| "uptime_hours": round(uptime_hours, 2), | ||||||||||||||||||||||||
| "system": { | ||||||||||||||||||||||||
| "cpu_percent": cpu_percent, | ||||||||||||||||||||||||
| "memory_percent": memory.percent, | ||||||||||||||||||||||||
| "memory_available_gb": round(memory.available / 1024**3, 2), | ||||||||||||||||||||||||
| "disk_percent": disk.percent, | ||||||||||||||||||||||||
| "disk_free_gb": round(disk.free / 1024**3, 2) | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| "process": { | ||||||||||||||||||||||||
| "memory_mb": round(process_memory, 2), | ||||||||||||||||||||||||
| "request_count": self.request_count, | ||||||||||||||||||||||||
| "error_count": self.error_count, | ||||||||||||||||||||||||
| "error_rate": round(self.error_count / max(self.request_count, 1) * 100, 2) | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| "last_health_check": self.last_health_check | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Determine overall health status | ||||||||||||||||||||||||
| if cpu_percent > 90 or memory.percent > 90 or disk.percent > 90: | ||||||||||||||||||||||||
| health_data["status"] = "warning" | ||||||||||||||||||||||||
| if cpu_percent > 95 or memory.percent > 95 or disk.percent > 95: | ||||||||||||||||||||||||
| health_data["status"] = "critical" | ||||||||||||||||||||||||
| if self.error_count > 0 and self.error_count / max(self.request_count, 1) > 0.1: | ||||||||||||||||||||||||
| health_data["status"] = "degraded" | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| self.last_health_check = health_data["timestamp"] | ||||||||||||||||||||||||
| return health_data | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||
| logger.error(f"Health check failed: {e}") | ||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||
| "status": "error", | ||||||||||||||||||||||||
| "timestamp": datetime.utcnow().isoformat(), | ||||||||||||||||||||||||
| "error": str(e) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def record_request(self, success: bool = True): | ||||||||||||||||||||||||
| """Record a request for health monitoring.""" | ||||||||||||||||||||||||
| self.request_count += 1 | ||||||||||||||||||||||||
| if not success: | ||||||||||||||||||||||||
| self.error_count += 1 | ||||||||||||||||||||||||
|
Comment on lines
+73
to
+77
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. The
Suggested change
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def get_health_summary(self) -> Dict[str, Any]: | ||||||||||||||||||||||||
| """Get a simplified health summary for quick checks.""" | ||||||||||||||||||||||||
| health = self.get_system_health() | ||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||
| "status": health["status"], | ||||||||||||||||||||||||
| "uptime_hours": health["uptime_hours"], | ||||||||||||||||||||||||
| "request_count": health["process"]["request_count"], | ||||||||||||||||||||||||
| "error_rate": health["process"]["error_rate"] | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Global health monitor instance | ||||||||||||||||||||||||
| health_monitor = HealthMonitor() | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from collections import defaultdict | ||
| from datetime import datetime, timedelta | ||
| from flask import abort, current_app | ||
|
|
||
| # Simple rate limiter using memory (use Redis for production) | ||
| rate_limit = defaultdict(list) | ||
|
|
||
| def rate_limit(max_requests=100, window_minutes=1): | ||
| def decorator(f): | ||
| @wraps(f) | ||
|
||
| def decorated_function(*args, **kwargs): | ||
| client_ip = request.remote_addr | ||
|
||
| now = datetime.utcnow() | ||
| window_start = now - timedelta(minutes=window_minutes) | ||
| rate_limit[client_ip] = [req_time for req_time in rate_limit[client_ip] if req_time > window_start] | ||
| if len(rate_limit[client_ip]) >= max_requests: | ||
| abort(429, description="Rate limit exceeded") | ||
| rate_limit[client_ip].append(now) | ||
| return f(*args, **kwargs) | ||
| return decorated_function | ||
| return decorator | ||
|
Comment on lines
+4
to
+21
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. This rate limiter has several critical issues that will cause it to fail or work incorrectly in a production environment:
This implementation is also not suitable for multi-process workers (like Gunicorn) as state is not shared. The comment correctly suggests Redis for production. from functools import wraps
from threading import Lock
from flask import request, abort
# Simple rate limiter using memory (use Redis for production). This version is thread-safe.
_rate_limit_requests = defaultdict(list)
_lock = Lock()
def rate_limit(max_requests=100, window_minutes=1):
def decorator(f):
@wraps(f)
def decorated_function(*args, **kwargs):
client_ip = request.remote_addr
now = datetime.utcnow()
window_start = now - timedelta(minutes=window_minutes)
with _lock:
# Filter out old requests
user_requests = [req_time for req_time in _rate_limit_requests[client_ip] if req_time > window_start]
_rate_limit_requests[client_ip] = user_requests
if len(_rate_limit_requests[client_ip]) >= max_requests:
abort(429, description="Rate limit exceeded")
_rate_limit_requests[client_ip].append(now)
return f(*args, **kwargs)
return decorated_function
return decorator |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,45 @@ | ||||||||||||||||||||||||
| from fastapi import Depends, HTTPException, status | ||||||||||||||||||||||||
| from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials | ||||||||||||||||||||||||
| from jose import JWTError, jwt | ||||||||||||||||||||||||
| from passlib.context import CryptContext | ||||||||||||||||||||||||
| from datetime import datetime, timedelta | ||||||||||||||||||||||||
| from typing import Optional | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Security settings | ||||||||||||||||||||||||
| SECRET_KEY = "your-secret-key" # Should be loaded from config | ||||||||||||||||||||||||
|
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. π¨ issue (security): Hardcoded secret key in security/auth.py is a security risk. Replace the hardcoded secret key with one loaded from environment variables or a secure configuration source.
Comment on lines
+7
to
+9
|
||||||||||||||||||||||||
| # Security settings | |
| SECRET_KEY = "your-secret-key" # Should be loaded from config | |
| import os | |
| # Security settings | |
| SECRET_KEY = os.environ.get("SECRET_KEY") | |
| if not SECRET_KEY: | |
| raise RuntimeError("SECRET_KEY environment variable not set") |
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.
Hardcoding secrets like this SECRET_KEY is a critical security vulnerability. It should be loaded from an environment variable or a secrets management system to prevent it from being checked into version control. Remember to import os.
| SECRET_KEY = "your-secret-key" # Should be loaded from config | |
| SECRET_KEY = os.environ.get("SECRET_KEY") # Should be loaded from config |
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.
suggestion (code-quality): We've found these issues:
- Add single value to dictionary directly rather than using update() (
simplify-dictionary-update) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| to_encode.update({"exp": expire}) | |
| encoded_jwt = jwt.encode(to_encode, SECRET_KEY, algorithm=ALGORITHM) | |
| return encoded_jwt | |
| to_encode["exp"] = expire | |
| return jwt.encode(to_encode, SECRET_KEY, algorithm=ALGORITHM) |
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.
suggestion (code-quality): Explicitly raise from a previous error (raise-from-previous-error)
| except JWTError: | |
| raise credentials_exception | |
| except JWTError as e: | |
| raise credentials_exception from e |
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.
π¨ issue (security): Hardcoded API key in auth.py should be replaced with a secure config value.
Consider loading the API key from an environment variable or config file to avoid exposing sensitive information in the codebase.