Skip to content

Commit c330dbd

Browse files
committed
Fix performance and security issues
- Disable SQLALCHEMY_TRACK_MODIFICATIONS (perf) - Fix N+1 query in activities by using joined relationship - Remove raw exception messages from API responses - Fix Activity.register() to commit properly - Remove unnecessary @DataClass decorators from models
1 parent 470095b commit c330dbd

4 files changed

Lines changed: 21 additions & 30 deletions

File tree

enferno/portal/views.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ def create_workspace():
118118
"workspace": workspace.to_dict(),
119119
}
120120
)
121-
except Exception as e:
122-
return jsonify({"error": str(e)}), 500
121+
except Exception:
122+
return jsonify({"error": "Failed to create workspace"}), 500
123123

124124

125125
@portal.get("/api/workspace/<int:workspace_id>/stats")
@@ -147,9 +147,9 @@ def workspace_update(workspace_id):
147147
try:
148148
db.session.commit()
149149
return jsonify({"message": "Workspace updated successfully"})
150-
except Exception as e:
150+
except Exception:
151151
db.session.rollback()
152-
return jsonify({"message": "Failed to update workspace", "error": str(e)}), 400
152+
return jsonify({"error": "Failed to update workspace"}), 400
153153

154154

155155
@portal.put("/api/profile")
@@ -168,9 +168,9 @@ def profile_update():
168168
try:
169169
db.session.commit()
170170
return jsonify({"message": "Profile updated successfully"})
171-
except Exception as e:
171+
except Exception:
172172
db.session.rollback()
173-
return jsonify({"message": "Failed to update profile", "error": str(e)}), 400
173+
return jsonify({"error": "Failed to update profile"}), 400
174174

175175

176176
@portal.post("/api/workspace/<int:workspace_id>/members")

enferno/settings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class Config:
4242
)
4343
# for postgres
4444
# SQLALCHEMY_DATABASE_URI = os.environ.get('SQLALCHEMY_DATABASE_URI', 'postgresql:///enferno')
45-
SQLALCHEMY_TRACK_MODIFICATIONS = True
45+
SQLALCHEMY_TRACK_MODIFICATIONS = False
4646

4747
# Celery configuration - only set if Redis available and configured
4848
CELERY_BROKER_URL = os.environ.get("CELERY_BROKER_URL") if REDIS_AVAILABLE else None

enferno/user/models.py

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import dataclasses
21
import secrets
32
import string
43
from datetime import datetime
@@ -23,7 +22,6 @@
2322
)
2423

2524

26-
@dataclasses.dataclass
2725
class Role(db.Model, RoleMixin, BaseMixin):
2826
id = db.Column(db.Integer, primary_key=True)
2927
name = db.Column(db.String(80), unique=True, nullable=True)
@@ -38,7 +36,6 @@ def from_dict(self, json_dict):
3836
return self
3937

4038

41-
@dataclasses.dataclass
4239
class User(UserMixin, db.Model, BaseMixin):
4340
id = db.Column(db.Integer, primary_key=True)
4441
username = db.Column(db.String(255), unique=True, nullable=True)
@@ -229,15 +226,12 @@ class Activity(db.Model, BaseMixin):
229226

230227
@classmethod
231228
def register(cls, user_id, action, data=None, workspace_id=None):
232-
"""Register an activity for audit purposes (optionally workspace-scoped).
233-
234-
Note: Does not commit - caller is responsible for transaction management.
235-
This prevents activity logging from rolling back other pending changes.
236-
"""
229+
"""Register an activity for audit purposes (optionally workspace-scoped)."""
237230
activity = cls(
238231
user_id=user_id, action=action, data=data, workspace_id=workspace_id
239232
)
240233
db.session.add(activity)
234+
db.session.commit()
241235
return activity
242236

243237

@@ -249,7 +243,6 @@ class StripeEvent(db.Model, BaseMixin):
249243
event_type = db.Column(db.String(128), nullable=True)
250244

251245

252-
@dataclasses.dataclass
253246
class Workspace(db.Model, BaseMixin):
254247
id = db.Column(db.Integer, primary_key=True)
255248
name = db.Column(db.String(255), nullable=False)
@@ -294,7 +287,6 @@ def generate_slug(name):
294287
return slugify(name)
295288

296289

297-
@dataclasses.dataclass
298290
class Membership(db.Model, BaseMixin):
299291
workspace_id = db.Column(
300292
db.Integer, db.ForeignKey("workspace.id", ondelete="CASCADE"), primary_key=True

enferno/user/views.py

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,9 @@ def api_user_create():
135135
db.session.commit()
136136
Activity.register(current_user.id, "User Create", user.to_dict())
137137
return jsonify({"message": "User successfully created!"})
138-
except Exception as e:
138+
except Exception:
139139
db.session.rollback()
140-
return jsonify({"error": str(e)}), 400
140+
return jsonify({"error": "Failed to create user"}), 400
141141

142142

143143
@bp_user.post("/api/user/<int:id>")
@@ -172,9 +172,9 @@ def api_user_update(id):
172172
{"old": old_user_data, "new": user.to_dict()},
173173
)
174174
return jsonify({"message": "User successfully updated!"})
175-
except Exception as e:
175+
except Exception:
176176
db.session.rollback()
177-
return jsonify({"error": str(e)}), 400
177+
return jsonify({"error": "Failed to update user"}), 400
178178

179179

180180
@bp_user.delete("/api/user/<int:id>")
@@ -204,9 +204,9 @@ def api_user_delete(id):
204204
db.session.commit()
205205
Activity.register(current_user.id, "User Delete", user_data)
206206
return jsonify({"message": "User successfully deleted!"})
207-
except Exception as e:
207+
except Exception:
208208
db.session.rollback()
209-
return jsonify({"error": str(e)}), 400
209+
return jsonify({"error": "Failed to delete user"}), 400
210210

211211

212212
@bp_user.get("/activities/")
@@ -222,18 +222,17 @@ def api_activities():
222222
query = db.select(Activity).order_by(Activity.created_at.desc())
223223
pagination = db.paginate(query, page=page, per_page=per_page)
224224

225-
def activity_to_dict(activity):
226-
"""Convert activity to dict with user info"""
227-
user = db.session.get(User, activity.user_id)
228-
return {
225+
# Activity.user relationship is lazy="joined", so user is already loaded
226+
items = [
227+
{
229228
"id": activity.id,
230-
"user": user.display_name if user else f"User ID: {activity.user_id}",
229+
"user": activity.user.display_name if activity.user else f"User ID: {activity.user_id}",
231230
"action": activity.action,
232231
"data": activity.data,
233232
"created_at": activity.created_at.strftime("%Y-%m-%d %H:%M:%S"),
234233
}
235-
236-
items = [activity_to_dict(activity) for activity in pagination.items]
234+
for activity in pagination.items
235+
]
237236

238237
return Response(
239238
json.dumps(

0 commit comments

Comments
 (0)