Security advisory 2 addressed
This commit is contained in:
@@ -108,7 +108,7 @@ Every push to `main` also runs `.gitea/workflows/development.yml` and publishes
|
|||||||
|
|
||||||
Appending a username to the root URL, such as `/alice`, opens that user's public feed and profile information.
|
Appending a username to the root URL, such as `/alice`, opens that user's public feed and profile information.
|
||||||
|
|
||||||
Configuration APIs require a bearer token returned by the login endpoint. 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.
|
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.
|
||||||
|
|
||||||
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 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`.
|
||||||
|
|
||||||
|
|||||||
+7
-6
@@ -54,13 +54,14 @@ These findings are prioritized below. Severity describes the potential security
|
|||||||
|
|
||||||
### SA-002: Bearer tokens accepted in query strings
|
### SA-002: Bearer tokens accepted in query strings
|
||||||
|
|
||||||
**Severity:** High
|
**Severity:** High, remediated in current worktree
|
||||||
**Evidence:** `backend/app/api/auth.py` exposes `GET /api/auth/me?token=...`; the web frontend and Firefox extension use this form.
|
**Evidence before remediation:** `backend/app/api/auth.py` exposed `GET /api/auth/me?token=...`, and web/extension callers used the query form.
|
||||||
**Impact:** Query strings can be recorded by reverse-proxy access logs, browser history, monitoring systems, copied URLs, screenshots, and referrer headers. Anyone obtaining a token can use the bearer session until expiry or revocation. The token also identifies the user session because the token is the credential.
|
**Current state:** `/api/auth/me` now requires the existing bearer-header dependency. The shared web header, admin session check, Firefox settings page, and tests send `Authorization: Bearer <token>`. A query-string token is rejected with `401`.
|
||||||
|
**Residual impact:** Tokens from old URLs may remain in proxy/browser logs and should be treated as exposed until revoked or rotated.
|
||||||
|
|
||||||
**Recommendation:** Make `Authorization: Bearer <token>` the only supported authentication transport for authenticated endpoints. Change the web and extension callers to send the header. If a compatibility transition is required, keep query-token support temporary, disable caching, add a deprecation period, and scrub query parameters from access logs. Consider rotating all existing tokens after migration.
|
**Recommendation:** Rotate existing access tokens after deployment and scrub historical query parameters from logs where possible. Keep the bearer header as the only credential transport.
|
||||||
|
|
||||||
**Priority:** High.
|
**Priority:** Completed in code; token rotation and log hygiene remain.
|
||||||
|
|
||||||
### SA-003: Sensitive secrets stored in plaintext SQLite
|
### SA-003: Sensitive secrets stored in plaintext SQLite
|
||||||
|
|
||||||
@@ -196,7 +197,7 @@ These findings are prioritized below. Severity describes the potential security
|
|||||||
|
|
||||||
- **Authentication transport:** Bearer headers are used by most APIs, but query-string tokens remain a leakage risk. There is no cookie session, which reduces CSRF exposure for current bearer-only API calls. Credentials are now email-based; usernames remain presentation identities.
|
- **Authentication transport:** Bearer headers are used by most APIs, but query-string tokens remain a leakage risk. There is no cookie session, which reduces CSRF exposure for current bearer-only API calls. Credentials are now email-based; usernames remain presentation identities.
|
||||||
- **Email authentication:** Primary and additional addresses are checked independently; additional addresses are unusable for login until their verification token is consumed. The profile exposes status but not verification secrets.
|
- **Email authentication:** Primary and additional addresses are checked independently; additional addresses are unusable for login until their verification token is consumed. The profile exposes status but not verification secrets.
|
||||||
- **Primary email changes:** A replacement remains pending until its verification link is consumed. The existing primary address remains usable during that period, and the same user row retains account permissions and active sessions.
|
- **Primary email selection:** Only an already verified alternative address can be promoted to primary. The same user row retains account permissions and active sessions, and the previous primary is retained as a verified alternative.
|
||||||
- **Password policy:** New and reset passwords require at least eight characters. This is better than no policy but should be replaced with a longer passphrase-oriented policy and breached-password screening after a proper password hash migration.
|
- **Password policy:** New and reset passwords require at least eight characters. This is better than no policy but should be replaced with a longer passphrase-oriented policy and breached-password screening after a proper password hash migration.
|
||||||
- **Email verification:** New administrator-created users cannot log in until verified. The setup-created first administrator is marked verified, which is appropriate for bootstrap but should be protected by the setup controls above.
|
- **Email verification:** New administrator-created users cannot log in until verified. The setup-created first administrator is marked verified, which is appropriate for bootstrap but should be protected by the setup controls above.
|
||||||
- **Password reset:** Tokens are random, hashed, expiring, single-use, and revoke existing access tokens after reset. Reset-email generation errors are intentionally swallowed to preserve generic login behavior, but this should be paired with server-side monitoring.
|
- **Password reset:** Tokens are random, hashed, expiring, single-use, and revoke existing access tokens after reset. Reset-email generation errors are intentionally swallowed to preserve generic login behavior, but this should be paired with server-side monitoring.
|
||||||
|
|||||||
@@ -908,3 +908,9 @@ Remove the entire “New primary email address” block; keep only selecting an
|
|||||||
|
|
||||||
### Assistant outcome
|
### Assistant outcome
|
||||||
Removed the pending primary-email replacement form, client handlers, API routes, service, and obsolete test. The profile now retains only verified alternative promotion, with unverified addresses unable to become primary.
|
Removed the pending primary-email replacement form, client handlers, API routes, service, and obsolete test. The profile now retains only verified alternative promotion, with unverified addresses unable to become primary.
|
||||||
|
|
||||||
|
### User
|
||||||
|
Remove the “New primary email address” functionality and keep only selecting an existing alternative as primary.
|
||||||
|
|
||||||
|
### Assistant outcome
|
||||||
|
Removed the replacement-email workflow entirely. Added cleanup migration 14 for its unused table; verified alternatives remain the only addresses eligible for primary selection.
|
||||||
|
|||||||
@@ -154,6 +154,7 @@
|
|||||||
145. For the new password in the user setting add a validation field to make sure they are the same before submitting
|
145. For the new password in the user setting add a validation field to make sure they are the same before submitting
|
||||||
146. Change so that authentication is based on email address rather than username - maintain the username for presentation purposes
|
146. Change so that authentication is based on email address rather than username - maintain the username for presentation purposes
|
||||||
147. Make sure the web plugin follows same logic
|
147. Make sure the web plugin follows same logic
|
||||||
|
154. Implement the recommendation for ### SA-002: Bearer tokens accepted in query strings
|
||||||
146. Change so that authentication is based on email address rather than username - maintain the username for presentation purposes
|
146. Change so that authentication is based on email address rather than username - maintain the username for presentation purposes
|
||||||
147. Make sure the web plugin follows same logic
|
147. Make sure the web plugin follows same logic
|
||||||
148. In the plugin <span data-i18n="emailLabel">emailLabel</span> should read: <span data-i18n="emailLabel">Email</span>
|
148. In the plugin <span data-i18n="emailLabel">emailLabel</span> should read: <span data-i18n="emailLabel">Email</span>
|
||||||
@@ -162,6 +163,7 @@
|
|||||||
151. Choose primary email from already verified alternative email addresses and increase the number of alternative email addresses allowed to 5.
|
151. Choose primary email from already verified alternative email addresses and increase the number of alternative email addresses allowed to 5.
|
||||||
152. Make sure an email can only be selected when it has been validated.
|
152. Make sure an email can only be selected when it has been validated.
|
||||||
153. Remove the entire "New primary email address" block; keep only selecting an existing alternative as primary.
|
153. Remove the entire "New primary email address" block; keep only selecting an existing alternative as primary.
|
||||||
|
155. Remove the “New primary email address” functionality and keep only selecting an existing alternative as primary.
|
||||||
|
|
||||||
## Future entries
|
## Future entries
|
||||||
|
|
||||||
|
|||||||
@@ -3,9 +3,10 @@
|
|||||||
|
|
||||||
from uuid import uuid4
|
from uuid import uuid4
|
||||||
|
|
||||||
from fastapi import APIRouter, HTTPException
|
from fastapi import APIRouter, Depends, HTTPException
|
||||||
from pydantic import BaseModel
|
from pydantic import BaseModel
|
||||||
|
|
||||||
|
from backend.app.api.dependencies import get_current_user
|
||||||
from backend.app.database import get_connection, init_db
|
from backend.app.database import get_connection, init_db
|
||||||
from backend.app.core.config import settings
|
from backend.app.core.config import settings
|
||||||
from backend.app.services.auth_service import authenticate_user, find_user
|
from backend.app.services.auth_service import authenticate_user, find_user
|
||||||
@@ -96,14 +97,7 @@ def logout(payload: dict):
|
|||||||
|
|
||||||
|
|
||||||
@router.get('/me')
|
@router.get('/me')
|
||||||
def current_user(token: str):
|
def current_user(user: dict = Depends(get_current_user)):
|
||||||
info = validate_token(token)
|
|
||||||
if info is None:
|
|
||||||
raise HTTPException(status_code=401, detail='Token expired or invalid')
|
|
||||||
with get_connection() as conn:
|
|
||||||
user = conn.execute('SELECT * FROM users WHERE id = ?', (info['user_id'],)).fetchone()
|
|
||||||
if user is None:
|
|
||||||
raise HTTPException(status_code=404, detail='User not found')
|
|
||||||
return {
|
return {
|
||||||
'id': user['id'],
|
'id': user['id'],
|
||||||
'username': user['username'],
|
'username': user['username'],
|
||||||
|
|||||||
@@ -213,7 +213,7 @@ CREATE TABLE IF NOT EXISTS email_address_verification_tokens (
|
|||||||
CREATE INDEX IF NOT EXISTS idx_user_email_addresses_user_id ON user_email_addresses(user_id);
|
CREATE INDEX IF NOT EXISTS idx_user_email_addresses_user_id ON user_email_addresses(user_id);
|
||||||
CREATE INDEX IF NOT EXISTS idx_email_address_verification_tokens_address_id ON email_address_verification_tokens(email_address_id);
|
CREATE INDEX IF NOT EXISTS idx_email_address_verification_tokens_address_id ON email_address_verification_tokens(email_address_id);
|
||||||
'''),
|
'''),
|
||||||
(13, '''
|
(13, '''
|
||||||
CREATE TABLE IF NOT EXISTS pending_primary_email_changes (
|
CREATE TABLE IF NOT EXISTS pending_primary_email_changes (
|
||||||
id TEXT PRIMARY KEY,
|
id TEXT PRIMARY KEY,
|
||||||
user_id TEXT NOT NULL UNIQUE,
|
user_id TEXT NOT NULL UNIQUE,
|
||||||
@@ -223,6 +223,9 @@ CREATE TABLE IF NOT EXISTS pending_primary_email_changes (
|
|||||||
created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||||
FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE CASCADE
|
FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE CASCADE
|
||||||
);
|
);
|
||||||
|
'''),
|
||||||
|
(14, '''
|
||||||
|
DROP TABLE IF EXISTS pending_primary_email_changes;
|
||||||
''')
|
''')
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ def test_login_returns_token():
|
|||||||
assert 'access_token' in payload
|
assert 'access_token' in payload
|
||||||
assert payload['token_type'] == 'bearer'
|
assert payload['token_type'] == 'bearer'
|
||||||
|
|
||||||
admin_session = client.get('/api/auth/me', params={'token': payload['access_token']})
|
admin_session = client.get('/api/auth/me', headers={'Authorization': f"Bearer {payload['access_token']}"})
|
||||||
assert admin_session.status_code == 200
|
assert admin_session.status_code == 200
|
||||||
assert admin_session.json()['is_admin'] is True
|
assert admin_session.json()['is_admin'] is True
|
||||||
|
|
||||||
@@ -47,9 +47,10 @@ def test_login_returns_token():
|
|||||||
'email': 'bob@example.com',
|
'email': 'bob@example.com',
|
||||||
'password': 'secret123',
|
'password': 'secret123',
|
||||||
}).json()['access_token']
|
}).json()['access_token']
|
||||||
user_session = client.get('/api/auth/me', params={'token': user_token})
|
user_session = client.get('/api/auth/me', headers={'Authorization': f"Bearer {user_token}"})
|
||||||
assert user_session.status_code == 200
|
assert user_session.status_code == 200
|
||||||
assert user_session.json()['is_admin'] is False
|
assert user_session.json()['is_admin'] is False
|
||||||
|
assert client.get('/api/auth/me', params={'token': payload['access_token']}).status_code == 401
|
||||||
|
|
||||||
|
|
||||||
def test_password_hashes_are_salted_and_legacy_hashes_upgrade_on_login():
|
def test_password_hashes_are_salted_and_legacy_hashes_upgrade_on_login():
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ def test_database_migrations_are_versioned_and_idempotent():
|
|||||||
connection = sqlite3.connect(':memory:')
|
connection = sqlite3.connect(':memory:')
|
||||||
|
|
||||||
apply_migrations(connection)
|
apply_migrations(connection)
|
||||||
assert get_schema_version(connection) == 13
|
assert get_schema_version(connection) == 14
|
||||||
tables = {
|
tables = {
|
||||||
row[0]
|
row[0]
|
||||||
for row in connection.execute(
|
for row in connection.execute(
|
||||||
@@ -27,6 +27,6 @@ def test_database_migrations_are_versioned_and_idempotent():
|
|||||||
assert set(DEFAULT_TAGS) <= seeded_tags
|
assert set(DEFAULT_TAGS) <= seeded_tags
|
||||||
|
|
||||||
apply_migrations(connection)
|
apply_migrations(connection)
|
||||||
assert get_schema_version(connection) == 13
|
assert get_schema_version(connection) == 14
|
||||||
|
|
||||||
connection.close()
|
connection.close()
|
||||||
@@ -190,7 +190,7 @@ async function loadAdminState() {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const sessionResponse = await fetch(`/api/auth/me?token=${encodeURIComponent(accessToken)}`);
|
const sessionResponse = await fetch('/api/auth/me', {headers: authHeaders()});
|
||||||
if (!sessionResponse.ok) {
|
if (!sessionResponse.ok) {
|
||||||
localStorage.removeItem('linklogAccessToken');
|
localStorage.removeItem('linklogAccessToken');
|
||||||
showSignedOutState();
|
showSignedOutState();
|
||||||
|
|||||||
@@ -47,7 +47,7 @@
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
fetch(`/api/auth/me?token=${encodeURIComponent(token)}`)
|
fetch('/api/auth/me', {headers: {Authorization: `Bearer ${token}`}})
|
||||||
.then((response) => {
|
.then((response) => {
|
||||||
if (!response.ok) throw new Error('Session expired');
|
if (!response.ok) throw new Error('Session expired');
|
||||||
return response.json();
|
return response.json();
|
||||||
|
|||||||
@@ -28,7 +28,8 @@ async function loadSettings() {
|
|||||||
if (settings.accessToken && settings.backendUrl) {
|
if (settings.accessToken && settings.backendUrl) {
|
||||||
try {
|
try {
|
||||||
const response = await fetch(
|
const response = await fetch(
|
||||||
`${settings.backendUrl}/api/auth/me?token=${encodeURIComponent(settings.accessToken)}`
|
`${settings.backendUrl}/api/auth/me`,
|
||||||
|
{headers: {Authorization: `Bearer ${settings.accessToken}`}},
|
||||||
);
|
);
|
||||||
if (response.ok) {
|
if (response.ok) {
|
||||||
const user = await response.json();
|
const user = await response.json();
|
||||||
|
|||||||
Reference in New Issue
Block a user