Skip to content

Commit 9f842ae

Browse files
manavgupclaude
andcommitted
fix(pr-555): Add authorization checks and improve security
Addresses critical security vulnerabilities and code quality issues identified in PR #555 review: **1. Authorization Security** (CRITICAL FIX): - Add verify_user_authorization() helper function to validate user access - Implement authorization checks in ALL endpoints (create, get, update, delete, toggle, list) - Only admins can create GLOBAL configs - Users can only access their own configs (or admin can access any) - Proper logging for authorization violations **2. Exception Handling** (HIGH PRIORITY): - Replace generic Exception catching with specific exception types: - ValidationError for Pydantic validation failures (422 status) - IntegrityError for unique constraint violations (409 status) - ValueError for not found errors (404 status) - Use logger.exception() for unexpected errors (better stack traces) - Return generic "Internal server error" message for security **3. Performance Optimization** (HIGH PRIORITY): - Add composite database indexes for common query patterns: - idx_runtime_config_user_lookup (scope, category, user_id, is_active) - idx_runtime_config_collection_lookup (scope, category, collection_id, is_active) - idx_runtime_config_global_lookup (scope, category, is_active) - Optimizes get_effective_config() hierarchical queries **4. Code Quality Improvements**: - Import ConfigScope for scope validation - Import IntegrityError and ValidationError for proper exception handling - Improve docstrings with authorization documentation - Better error messages and logging - Ruff formatting applied **Testing**: Local validation pending **Related**: Addresses review feedback from PR #555 (Dynamic Configuration System) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent c183637 commit 9f842ae

File tree

2 files changed

+145
-70
lines changed

2 files changed

+145
-70
lines changed

backend/rag_solution/models/runtime_config.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
from datetime import datetime
2727
from typing import TYPE_CHECKING, Any
2828

29-
from sqlalchemy import Boolean, DateTime, Enum, String, Text, UniqueConstraint
29+
from sqlalchemy import Boolean, DateTime, Enum, Index, String, Text, UniqueConstraint
3030
from sqlalchemy.dialects.postgresql import JSONB, UUID
3131
from sqlalchemy.orm import Mapped, mapped_column
3232
from sqlalchemy.sql import func
@@ -84,9 +84,7 @@ class RuntimeConfig(Base): # pylint: disable=too-few-public-methods
8484
__tablename__ = "runtime_configs"
8585

8686
# 🆔 Identification
87-
id: Mapped[uuid.UUID] = mapped_column(
88-
UUID(as_uuid=True), primary_key=True, default=IdentityService.generate_id
89-
)
87+
id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), primary_key=True, default=IdentityService.generate_id)
9088

9189
# ⚙️ Configuration Attributes
9290
scope: Mapped[ConfigScope] = mapped_column(
@@ -123,8 +121,9 @@ class RuntimeConfig(Base): # pylint: disable=too-few-public-methods
123121
DateTime(timezone=True), server_default=func.now(), onupdate=func.now()
124122
)
125123

126-
# 🔐 Unique Constraint
124+
# 🔐 Unique Constraint & Performance Indexes
127125
# Ensures only one config per scope/category/key/user/collection combination
126+
# Composite indexes optimize common query patterns in get_effective_config()
128127
__table_args__ = (
129128
UniqueConstraint(
130129
"scope",
@@ -134,6 +133,15 @@ class RuntimeConfig(Base): # pylint: disable=too-few-public-methods
134133
"collection_id",
135134
name="uq_runtime_config_scope_category_key_user_collection",
136135
),
136+
# Composite index for USER-scoped config lookups (get_effective_config)
137+
# Query pattern: scope='USER' AND category=? AND user_id=? AND is_active=true
138+
Index("idx_runtime_config_user_lookup", "scope", "category", "user_id", "is_active"),
139+
# Composite index for COLLECTION-scoped config lookups
140+
# Query pattern: scope='COLLECTION' AND category=? AND collection_id=? AND is_active=true
141+
Index("idx_runtime_config_collection_lookup", "scope", "category", "collection_id", "is_active"),
142+
# Composite index for GLOBAL-scoped config lookups
143+
# Query pattern: scope='GLOBAL' AND category=? AND is_active=true
144+
Index("idx_runtime_config_global_lookup", "scope", "category", "is_active"),
137145
{"extend_existing": True},
138146
)
139147

0 commit comments

Comments
 (0)