Conversation
…reSQL; update README and add security guidelines
| }), 200 | ||
|
|
||
| except Exception as e: | ||
| print(f"Login error: {str(e)}") |
There was a problem hiding this comment.
Error details are being logged to console, which could expose sensitive information or system details in production. Consider using proper logging mechanisms with appropriate log levels and avoid exposing detailed error messages in production environments.
| from flask_jwt_extended import jwt_required, get_jwt_identity, create_access_token | ||
| from datetime import timedelta | ||
|
|
||
| try: | ||
| # This will verify the refresh token | ||
| jwt_required(refresh=True)() |
There was a problem hiding this comment.
The jwt_required decorator is being called as a function (jwt_required(refresh=True)()) which is incorrect usage. This should be used as a decorator with @jwt_required(refresh=True) or the function should call verify_jwt_in_request(refresh=True) instead. The current implementation will likely cause errors.
| from flask_jwt_extended import jwt_required, get_jwt_identity, create_access_token | |
| from datetime import timedelta | |
| try: | |
| # This will verify the refresh token | |
| jwt_required(refresh=True)() | |
| from flask_jwt_extended import jwt_required, get_jwt_identity, create_access_token, verify_jwt_in_request | |
| from datetime import timedelta | |
| try: | |
| # This will verify the refresh token | |
| verify_jwt_in_request(refresh=True) |
|
|
||
| # Initialize extensions with app | ||
| db.init_app(app) | ||
| jwt = JWTManager(app) |
There was a problem hiding this comment.
The variable 'jwt' is assigned but never used in this file. The JWTManager instance is initialized but not referenced elsewhere. Consider removing this unused variable or documenting why it needs to be initialized.
| from datetime import datetime | ||
| from database import db | ||
| from utils.password import hash_password, verify_password | ||
|
|
||
| class User(db.Model): | ||
| __tablename__ = 'users' | ||
|
|
||
| id = db.Column(db.Integer, primary_key=True) | ||
| email = db.Column(db.String(120), unique=True, nullable=False, index=True) | ||
| password_hash = db.Column(db.String(255), nullable=False) | ||
| created_at = db.Column(db.DateTime, nullable=False, default=datetime.utcnow) | ||
| updated_at = db.Column(db.DateTime, nullable=False, default=datetime.utcnow, onupdate=datetime.utcnow) |
There was a problem hiding this comment.
Using datetime.utcnow() is deprecated in Python 3.12+ and will be removed in a future version. Consider using datetime.now(timezone.utc) instead for timezone-aware timestamps.
| from datetime import datetime | |
| from database import db | |
| from utils.password import hash_password, verify_password | |
| class User(db.Model): | |
| __tablename__ = 'users' | |
| id = db.Column(db.Integer, primary_key=True) | |
| email = db.Column(db.String(120), unique=True, nullable=False, index=True) | |
| password_hash = db.Column(db.String(255), nullable=False) | |
| created_at = db.Column(db.DateTime, nullable=False, default=datetime.utcnow) | |
| updated_at = db.Column(db.DateTime, nullable=False, default=datetime.utcnow, onupdate=datetime.utcnow) | |
| from datetime import datetime, timezone | |
| from database import db | |
| from utils.password import hash_password, verify_password | |
| def utcnow(): | |
| return datetime.now(timezone.utc) | |
| class User(db.Model): | |
| __tablename__ = 'users' | |
| id = db.Column(db.Integer, primary_key=True) | |
| email = db.Column(db.String(120), unique=True, nullable=False, index=True) | |
| password_hash = db.Column(db.String(255), nullable=False) | |
| created_at = db.Column(db.DateTime, nullable=False, default=utcnow) | |
| updated_at = db.Column(db.DateTime, nullable=False, default=utcnow, onupdate=utcnow) |
| created_at = db.Column(db.DateTime, nullable=False, default=datetime.utcnow) | ||
| updated_at = db.Column(db.DateTime, nullable=False, default=datetime.utcnow, onupdate=datetime.utcnow) |
There was a problem hiding this comment.
Using datetime.utcnow() is deprecated in Python 3.12+ and will be removed in a future version. Consider using datetime.now(timezone.utc) instead for timezone-aware timestamps.
|
|
||
| except Exception as e: | ||
| db.session.rollback() | ||
| print(f"Registration error: {str(e)}") |
There was a problem hiding this comment.
Sensitive information is being printed to console/logs. The password and error details should not be logged in production as they could expose security vulnerabilities and user credentials. Consider removing these print statements or implementing proper logging with sensitive data redaction.
| # SQLAlchemy configuration | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.getenv('DATABASE_URL', 'sqlite:///planventure.db') | ||
| app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False | ||
| app.config['SQLALCHEMY_ECHO'] = True # Set to False in production |
There was a problem hiding this comment.
SQLALCHEMY_ECHO is set to True which will log all SQL queries to console. This should be set to False in production to avoid performance overhead and potential security issues from exposing database queries. Consider using an environment variable to control this setting.
| app.config['SQLALCHEMY_ECHO'] = True # Set to False in production | |
| app.config['SQLALCHEMY_ECHO'] = os.getenv('SQLALCHEMY_ECHO', 'False').lower() in ('true', '1', 'yes') |
| def health_check(): | ||
| return jsonify({ | ||
| "status": "healthy", | ||
| "database": "connected" if db.engine else "disconnected" |
There was a problem hiding this comment.
The health check endpoint checks if db.engine exists, but this will always be truthy even if the database connection is not actually working. Consider performing an actual database query (e.g., db.session.execute(text('SELECT 1'))) to verify the database is truly connected and operational.
|
|
||
| If you prefer to develop locally, follow the steps below: | ||
|
|
||
| 1.Fork and clone the repository and navigate to the [planventue-api](/planventure-api/) directory: |
There was a problem hiding this comment.
Typo in the word 'planventue-api', should be 'planventure-api' (missing 'r').
| 1.Fork and clone the repository and navigate to the [planventue-api](/planventure-api/) directory: | |
| 1. Fork and clone the repository and navigate to the [planventure-api](/planventure-api/) directory: |
2 participants
No description provided.