SA-005 addressed by updateing ratelimiting
Build LinkLog Development Image / development-image (push) Successful in 8s
Build LinkLog Development Image / development-image (push) Successful in 8s
This commit is contained in:
@@ -110,6 +110,8 @@ Appending a username to the root URL, such as `/alice`, opens that user's public
|
||||
|
||||
Configuration APIs require a bearer token returned by the login endpoint. Send it in the `Authorization: Bearer ...` header; query-string tokens are not accepted. Users authenticate with their email address; the username remains the public presentation identity used in profiles and feed URLs. User configuration uses the identity in that token. Plugin administration additionally requires an administrator account.
|
||||
|
||||
Login failures are throttled per client IP and email. Five failures within 15 minutes trigger a two-minute lockout, including invalid OTP attempts; successful authentication clears the failure counter.
|
||||
|
||||
Users can change their password from the profile page. The current password is required, new passwords must contain at least 8 characters, and the endpoint is `PUT /api/user/password`.
|
||||
|
||||
Users can configure a time-based one-time password from the profile page using an authenticator app. The profile displays a provisioning secret and authenticator URI during setup, then requires a current six-digit code to enable or disable OTP. When OTP is enabled, both the web login and Firefox extension settings login require the code. The TOTP secret is never returned by the profile API after setup.
|
||||
|
||||
+5
-5
@@ -89,13 +89,13 @@ These findings are prioritized below. Severity describes the potential security
|
||||
|
||||
### SA-005: Login endpoint lacks rate limiting and lockout
|
||||
|
||||
**Severity:** High
|
||||
**Evidence:** `POST /api/auth/login` in `backend/app/api/auth.py` has no IP, username, or account rate limit. The OTP verification path is also not rate-limited separately.
|
||||
**Impact:** Attackers can perform password guessing and OTP guessing at high speed. Sending a password-reset email after failed authentication can also be abused to generate mail volume and user harassment, even though the response remains generic.
|
||||
**Severity:** High, remediated in current worktree
|
||||
**Evidence before remediation:** `POST /api/auth/login` had no IP, email, or account rate limit, and OTP failures were not throttled separately.
|
||||
**Current state:** Login failures are tracked in SQLite by a SHA-256 key derived from client IP and normalized email. Five failures within 15 minutes cause a two-minute lockout; the endpoint returns `429` with `Retry-After`, and successful password plus OTP authentication clears the counter. Password-reset mail remains generic and should still be rate-limited operationally.
|
||||
|
||||
**Recommendation:** Add a distributed rate limiter keyed by IP and normalized username, with conservative burst limits, exponential backoff, and monitoring. Rate-limit password-reset issuance independently and avoid sending reset mail for every failed attempt. Consider temporary account protection after repeated failures without creating a user-enumeration oracle. Return `Retry-After` where appropriate.
|
||||
**Recommendation:** Use a distributed limiter for multi-instance deployments, add monitoring, and rate-limit password-reset issuance independently. Keep responses generic to avoid account enumeration.
|
||||
|
||||
**Priority:** High.
|
||||
**Priority:** Completed for the single-instance SQLite deployment; distributed limiting and reset-mail controls remain.
|
||||
|
||||
### SA-006: Firefox extension has broad host access and stores bearer tokens in local storage
|
||||
|
||||
|
||||
@@ -957,6 +957,12 @@ Address SA-004 by validating Mastodon instances as HTTPS public hostnames, rejec
|
||||
### Assistant outcome
|
||||
Implemented and validated SA-004 with centralized Mastodon URL checks, DNS resolution and blocked-range detection, IPv4-mapped IPv6 handling, and no-redirect outbound requests for OAuth, posting, and deletion. The rebuilt container suite passed with 42 tests.
|
||||
|
||||
### User
|
||||
Implement SA-005: login endpoint lacks rate limiting and lockout.
|
||||
|
||||
### Assistant outcome
|
||||
Added persistent IP/email login throttling with a five-failure limit in a 15-minute window, two-minute lockout, `Retry-After` responses, OTP failure coverage, and counter reset after successful authentication. Updated documentation and audit status.
|
||||
|
||||
### User
|
||||
Address SA-004 by validating Mastodon instances as HTTPS public hostnames, rejecting unsafe DNS/IP ranges including mapped IPv6, and blocking redirects.
|
||||
|
||||
@@ -1002,5 +1008,11 @@ Made popup authentication states mutually exclusive using the DOM `hidden` prope
|
||||
### User
|
||||
The authenticated session text and sign-in block are still shown together.
|
||||
|
||||
### User
|
||||
Use the VIBE directory to log interactions
|
||||
|
||||
### Assistant outcome
|
||||
Appended this visible interaction to the project VIBE chat and prompt logs in the repo’s append-only format.
|
||||
|
||||
### Assistant outcome
|
||||
Made popup state transitions defensive by synchronizing both `hidden` attributes and CSS classes. Non-authentication tag-loading errors no longer switch the popup to signed-out state; only a rejected session does. Added `display: none !important` guards for both authentication blocks.
|
||||
|
||||
@@ -171,6 +171,7 @@
|
||||
155. Remove the “New primary email address” functionality and keep only selecting an existing alternative as primary.
|
||||
166. Address SA-003 by using a dedicated secret key supplied through a protected environment/secret file, encrypt sensitive values before SQLite storage
|
||||
167. Address SA-004 by validating Mastodon instances as HTTPS public hostnames, rejecting unsafe DNS/IP ranges including mapped IPv6, and blocking redirects
|
||||
168. Implement SA-005: login endpoint lacks rate limiting and lockout
|
||||
167. Address SA-004 by validating Mastodon instances as HTTPS public hostnames, rejecting unsafe DNS/IP ranges including mapped IPv6, and blocking redirects.
|
||||
158. When the user is logged in the webplugin should not display "Please sign in to use LinkLog."
|
||||
156. When the user is signed in the plugin should not display "Please sign in to use LinkLog." and the link to the settings
|
||||
@@ -179,6 +180,7 @@
|
||||
166. Address SA-003 by using a dedicated secret key supplied through a protected environment/secret file, encrypt sensitive values before SQLite storage
|
||||
161. The popup still shows the sign-in block even though the authenticated session text is displayed; show the block only when signed out.
|
||||
162. The authenticated session text and sign-in block are still shown together.
|
||||
163. Use the VIBE directory to log interactions.
|
||||
|
||||
## Future entries
|
||||
|
||||
|
||||
+14
-4
@@ -3,7 +3,7 @@
|
||||
|
||||
from uuid import uuid4
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||
from pydantic import BaseModel
|
||||
|
||||
from backend.app.api.dependencies import get_current_user
|
||||
@@ -17,6 +17,7 @@ from backend.app.services.token_service import issue_token, revoke_token, valida
|
||||
from backend.app.services.otp_service import verify_code
|
||||
from backend.app.services.secret_store import decrypt_secret
|
||||
from backend.app.services.email_addresses import verify_user_email_address
|
||||
from backend.app.services.login_throttle import check_login_allowed, clear_login_failures, record_login_failure
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
@@ -35,10 +36,17 @@ class PasswordResetRequest(BaseModel):
|
||||
|
||||
|
||||
@router.post('/login')
|
||||
def login(payload: LoginRequest):
|
||||
user = authenticate_user(payload.email.strip(), payload.password)
|
||||
def login(payload: LoginRequest, request: Request):
|
||||
email = payload.email.strip()
|
||||
ip_address = request.client.host if request.client else 'unknown'
|
||||
retry_after = check_login_allowed(ip_address, email)
|
||||
if retry_after is not None:
|
||||
raise HTTPException(status_code=429, detail='Too many failed login attempts. Try again later.', headers={'Retry-After': str(retry_after)})
|
||||
|
||||
user = authenticate_user(email, payload.password)
|
||||
if user is None:
|
||||
reset_user = find_user(payload.email.strip())
|
||||
record_login_failure(ip_address, email)
|
||||
reset_user = find_user(email)
|
||||
if reset_user and reset_user['email_verified'] and smtp_configured():
|
||||
try:
|
||||
token = create_reset_token(reset_user['id'])
|
||||
@@ -50,8 +58,10 @@ def login(payload: LoginRequest):
|
||||
if not user['email_verified']:
|
||||
raise HTTPException(status_code=403, detail='Email address is not verified')
|
||||
if user['otp_enabled'] and not verify_code(decrypt_secret(user['otp_secret']), payload.otp):
|
||||
record_login_failure(ip_address, email)
|
||||
raise HTTPException(status_code=401, detail='One-time password required or invalid')
|
||||
|
||||
clear_login_failures(ip_address, email)
|
||||
token_data = issue_token(user['id'], user['username'])
|
||||
return {
|
||||
'access_token': token_data['access_token'],
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
## Copyright © 2026 Olaf Kolkman
|
||||
## SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from hashlib import sha256
|
||||
import json
|
||||
|
||||
from backend.app.database import get_connection
|
||||
|
||||
MAX_FAILURES = 5
|
||||
FAILURE_WINDOW = timedelta(minutes=15)
|
||||
LOCKOUT_DURATION = timedelta(minutes=2)
|
||||
|
||||
|
||||
def _setting_name(ip_address: str, email: str) -> str:
|
||||
key = sha256(f'{ip_address}\0{email.casefold()}'.encode('utf-8')).hexdigest()
|
||||
return f'login_rate:{key}'
|
||||
|
||||
|
||||
def _read_rate(setting_name: str) -> dict:
|
||||
with get_connection() as conn:
|
||||
row = conn.execute('SELECT value FROM app_settings WHERE name = ?', (setting_name,)).fetchone()
|
||||
if not row:
|
||||
return {}
|
||||
try:
|
||||
return json.loads(row['value'])
|
||||
except (TypeError, json.JSONDecodeError):
|
||||
return {}
|
||||
|
||||
|
||||
def check_login_allowed(ip_address: str, email: str) -> int | None:
|
||||
rate = _read_rate(_setting_name(ip_address, email))
|
||||
now = datetime.now(timezone.utc)
|
||||
locked_until = datetime.fromisoformat(rate['locked_until']) if rate.get('locked_until') else None
|
||||
if locked_until and locked_until > now:
|
||||
return int((locked_until - now).total_seconds()) + 1
|
||||
return None
|
||||
|
||||
|
||||
def record_login_failure(ip_address: str, email: str) -> None:
|
||||
setting_name = _setting_name(ip_address, email)
|
||||
now = datetime.now(timezone.utc)
|
||||
rate = _read_rate(setting_name)
|
||||
first_failure = datetime.fromisoformat(rate['first_failure']) if rate.get('first_failure') else now
|
||||
if now - first_failure >= FAILURE_WINDOW:
|
||||
rate = {}
|
||||
first_failure = now
|
||||
failures = int(rate.get('failures', 0)) + 1
|
||||
updated = {'failures': failures, 'first_failure': first_failure.isoformat()}
|
||||
if failures >= MAX_FAILURES:
|
||||
updated['locked_until'] = (now + LOCKOUT_DURATION).isoformat()
|
||||
with get_connection() as conn:
|
||||
conn.execute(
|
||||
'''INSERT INTO app_settings (name, value, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP)
|
||||
ON CONFLICT(name) DO UPDATE SET value = excluded.value, updated_at = CURRENT_TIMESTAMP''',
|
||||
(setting_name, json.dumps(updated)),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
|
||||
def clear_login_failures(ip_address: str, email: str) -> None:
|
||||
with get_connection() as conn:
|
||||
conn.execute('DELETE FROM app_settings WHERE name = ?', (_setting_name(ip_address, email),))
|
||||
conn.commit()
|
||||
@@ -13,6 +13,7 @@ from fastapi.testclient import TestClient
|
||||
from backend.app.main import app
|
||||
from backend.app.database import get_connection
|
||||
from backend.app.services.email_service import get_smtp_settings
|
||||
from backend.app.services.login_throttle import clear_login_failures
|
||||
from backend.app.services.password_reset import create_reset_token
|
||||
from backend.app.services.token_service import issue_token
|
||||
|
||||
@@ -53,6 +54,20 @@ def test_login_returns_token():
|
||||
assert client.get('/api/auth/me', params={'token': payload['access_token']}).status_code == 401
|
||||
|
||||
|
||||
def test_login_rate_limit_locks_out_after_five_failures_and_resets_on_success():
|
||||
email = f'unknown-{uuid4().hex}@example.com'
|
||||
for attempt in range(5):
|
||||
response = client.post('/api/auth/login', json={'email': email, 'password': 'wrong-password'})
|
||||
assert response.status_code == 401, attempt
|
||||
locked = client.post('/api/auth/login', json={'email': email, 'password': 'wrong-password'})
|
||||
assert locked.status_code == 429
|
||||
assert int(locked.headers['Retry-After']) > 0
|
||||
|
||||
clear_login_failures('testclient', email)
|
||||
valid = client.post('/api/auth/login', json={'email': 'alice@example.com', 'password': 'secret123'})
|
||||
assert valid.status_code == 200
|
||||
|
||||
|
||||
def test_password_hashes_are_salted_and_legacy_hashes_upgrade_on_login():
|
||||
from hashlib import sha256
|
||||
from backend.app.database import hash_password
|
||||
|
||||
Reference in New Issue
Block a user