fix: address code review feedback

- Use astimezone() instead of replace() for timezone conversion in is_token_expired
- Log cleanup exceptions with logger.exception() in signup
- Add security warning when STRIPE_WEBHOOK_SECRET is not configured
- Increase Stripe price ID column length from 64 to 128 characters
- Replace alert() with aria-live assertive region in pricing.html
- Convert auth() login tests to use pytest.mark.asyncio and await

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

app/api/billing.py

@@ -238,6 +238,11 @@ async def stripe_webhook(request: Request, db: Session = Depends(get_db)) -> dic
         if settings.stripe_webhook_secret:
             event = stripe.Webhook.construct_event(payload, sig_header, settings.stripe_webhook_secret)
         else:
+            logger.warning(
+                "[SECURITY] STRIPE_WEBHOOK_SECRET is not configured. "
+                "Webhook events are accepted without signature verification. "
+                "Set STRIPE_WEBHOOK_SECRET in production to prevent spoofed events."
+            )
             event = stripe.Event.construct_from(json.loads(payload), stripe.api_key)
     except stripe.SignatureVerificationError:
         logger.warning("[SECURITY] Stripe webhook signature verification failed")

app/api/local_auth.py

@@ -187,6 +187,7 @@ async def signup(request: Request, body: SignupBody, db: DbSession) -> dict[str,
             db.commit()
         except Exception:
             db.rollback()
+            logger.exception("Failed to clean up orphan records for %s after email send failure", body.email)
         logger.warning("Signup email failed for %s: %s", body.email, exc)
         raise HTTPException(
             status_code=status.HTTP_503_SERVICE_UNAVAILABLE,

app/models.py

@@ -288,8 +288,8 @@ class SubscriptionPlan(Base):
     sort_order = Column(Integer, nullable=False, default=0)
     features = Column(Text, nullable=True)  # JSON-encoded list[str]
     api_access = Column(Boolean, nullable=False, default=False)
-    stripe_price_id_monthly = Column(String(64), nullable=True)
+    stripe_price_id_monthly = Column(String(128), nullable=True)
-    stripe_price_id_yearly = Column(String(64), nullable=True)
+    stripe_price_id_yearly = Column(String(128), nullable=True)
 
     created_at = Column(DateTime(timezone=True), server_default=func.now())
     updated_at = Column(DateTime(timezone=True), server_default=func.now(), onupdate=func.now())

app/utils/local_auth.py

@@ -46,7 +46,7 @@ def is_token_expired(sent_at: datetime | None) -> bool:
     """Return True when *sent_at* is None or older than TOKEN_EXPIRY_HOURS."""
     if sent_at is None:
         return True
-    return datetime.now(tz=timezone.utc) > sent_at.replace(tzinfo=timezone.utc) + timedelta(hours=TOKEN_EXPIRY_HOURS)
+    return datetime.now(tz=timezone.utc) > sent_at.astimezone(timezone.utc) + timedelta(hours=TOKEN_EXPIRY_HOURS)
 
 
 def _smtp_send(subject: str, html_body: str, plain_body: str, recipient: str) -> None:

frontend/templates/pricing.html

@@ -397,6 +397,10 @@ async function startCheckout(planId) {
   const cycleEl = document.querySelector('[data-billing-cycle]');
   const billingCycle = (cycleEl && cycleEl.dataset.billingCycle) ? cycleEl.dataset.billingCycle : 'monthly';
 
+  // Clear previous error
+  const errEl = document.getElementById('checkout-error');
+  if (errEl) { errEl.textContent = ''; errEl.hidden = true; }
+
   try {
     const resp = await fetch('/api/billing/create-checkout-session', {
       method: 'POST',
@@ -415,10 +419,17 @@ async function startCheckout(planId) {
       }
     }
     const data = await resp.json().catch(() => ({}));
-    alert(data.detail || 'Unable to start checkout. Please try again.');
+    const msg = data.detail || 'Unable to start checkout. Please try again.';
+    if (errEl) { errEl.textContent = msg; errEl.hidden = false; }
   } catch (e) {
-    alert('Network error. Please try again.');
+    if (errEl) { errEl.textContent = 'Network error. Please try again.'; errEl.hidden = false; }
   }
 }
 </script>
+<div id="checkout-error"
+     role="alert"
+     aria-live="assertive"
+     hidden
+     class="fixed bottom-6 left-1/2 -translate-x-1/2 bg-red-100 border border-red-400 text-red-700 px-6 py-3 rounded-lg shadow-lg text-sm z-50">
+</div>
 {% endblock %}

migrations/versions/018_add_local_users_and_billing.py

@@ -37,8 +37,8 @@ def upgrade() -> None:
         sa.UniqueConstraint("username"),
     )
     op.add_column("user_profiles", sa.Column("stripe_customer_id", sa.String(64), nullable=True))
-    op.add_column("subscription_plans", sa.Column("stripe_price_id_monthly", sa.String(64), nullable=True))
+    op.add_column("subscription_plans", sa.Column("stripe_price_id_monthly", sa.String(128), nullable=True))
-    op.add_column("subscription_plans", sa.Column("stripe_price_id_yearly", sa.String(64), nullable=True))
+    op.add_column("subscription_plans", sa.Column("stripe_price_id_yearly", sa.String(128), nullable=True))
 
 
 def downgrade() -> None:

tests/test_local_auth.py

@@ -550,9 +550,9 @@ def test_reset_password_page(la_client):
 
 
 @pytest.mark.unit
-def test_local_login_success(la_session, active_user):
+@pytest.mark.asyncio
+async def test_local_login_success(la_session, active_user):
     """auth() with valid LocalUser credentials sets session and redirects."""
-    import asyncio
     from unittest.mock import AsyncMock, MagicMock
 
     from fastapi import Request
@@ -563,16 +563,16 @@ def test_local_login_success(la_session, active_user):
     mock_request.form = AsyncMock(return_value={"username": "activeuser", "password": "password123"})
     mock_request.session = {}
 
-    result = asyncio.get_event_loop().run_until_complete(auth(mock_request, db=la_session))
+    result = await auth(mock_request, db=la_session)
     assert result.status_code == 302
     assert "user" in mock_request.session
     assert mock_request.session["user"]["email"] == "active@example.com"
 
 
 @pytest.mark.unit
-def test_local_login_by_email(la_session, active_user):
+@pytest.mark.asyncio
+async def test_local_login_by_email(la_session, active_user):
     """auth() accepts email as username for LocalUser lookup."""
-    import asyncio
     from unittest.mock import AsyncMock, MagicMock
 
     from fastapi import Request
@@ -583,15 +583,15 @@ def test_local_login_by_email(la_session, active_user):
     mock_request.form = AsyncMock(return_value={"username": "active@example.com", "password": "password123"})
     mock_request.session = {}
 
-    result = asyncio.get_event_loop().run_until_complete(auth(mock_request, db=la_session))
+    result = await auth(mock_request, db=la_session)
     assert result.status_code == 302
     assert "user" in mock_request.session
 
 
 @pytest.mark.unit
-def test_local_login_wrong_password(la_session, active_user):
+@pytest.mark.asyncio
+async def test_local_login_wrong_password(la_session, active_user):
     """auth() with wrong password redirects to login with error."""
-    import asyncio
     from unittest.mock import AsyncMock, MagicMock
 
     from fastapi import Request
@@ -602,16 +602,16 @@ def test_local_login_wrong_password(la_session, active_user):
     mock_request.form = AsyncMock(return_value={"username": "activeuser", "password": "wrongpassword"})
     mock_request.session = {}
 
-    result = asyncio.get_event_loop().run_until_complete(auth(mock_request, db=la_session))
+    result = await auth(mock_request, db=la_session)
     assert result.status_code == 302
     assert "/login" in result.headers["location"]
     assert "user" not in mock_request.session
 
 
 @pytest.mark.unit
-def test_local_login_unverified(la_session, pending_user):
+@pytest.mark.asyncio
+async def test_local_login_unverified(la_session, pending_user):
     """auth() for unverified user redirects with verification message."""
-    import asyncio
     from unittest.mock import AsyncMock, MagicMock
 
     from fastapi import Request
@@ -622,6 +622,6 @@ def test_local_login_unverified(la_session, pending_user):
     mock_request.form = AsyncMock(return_value={"username": "pendinguser", "password": "password123"})
     mock_request.session = {}
 
-    result = asyncio.get_event_loop().run_until_complete(auth(mock_request, db=la_session))
+    result = await auth(mock_request, db=la_session)
     assert result.status_code == 302
     assert "verify" in result.headers["location"].lower()