2 Commits
Author SHA1 Message Date
olaf 079eb146f6 Login now based on email
Build LinkLog Development Image / development-image (push) Successful in 10s
2026-08-26 15:03:48 +02:00
olaf dd44b380ce Passwords stored salt and some change is password logic 2026-08-26 14:51:40 +02:00
16 changed files with 432 additions and 52 deletions
+2 -2
View File
@@ -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. User configuration uses the identity in that token. Plugin administration additionally requires an administrator account; the development `alice` account is seeded as an administrator, while `bob` is a standard user. 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.
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`.
@@ -235,7 +235,7 @@ Login:
```sh ```sh
curl -X POST http://localhost:8000/api/auth/login \\ curl -X POST http://localhost:8000/api/auth/login \\
-H 'Content-Type: application/json' \\ -H 'Content-Type: application/json' \\
-d '{"username":"alice","password":"secret123"}' -d '{"email":"alice@example.com","password":"secret123"}'
``` ```
Submit a link using the returned access token: Submit a link using the returned access token:
+256
View File
@@ -0,0 +1,256 @@
# LinkLog Security Audit
**Assessment date:** 2026-08-26
**Scope:** Current LinkLog backend, web frontend, Firefox extension, SQLite persistence, SMTP and Mastodon integrations, Docker/Traefik deployment files, and automated tests.
**Assessment type:** Source-code security review. This is not a penetration test, dependency vulnerability scan, formal threat model sign-off, or production configuration certification.
## Executive Summary
LinkLog has several good security foundations: authenticated API dependencies, administrator authorization checks, owner checks for link operations, token hashing in the database, email verification, password reset token hashing and single-use behavior, TOTP login enforcement, last-administrator protection, parameterized SQLite queries, upload size limits, and non-root application execution in the container.
The current implementation is not ready to expose directly to the public Internet without additional hardening. The most important issues are:
1. Passwords were previously stored as unsalted, fast SHA-256 hashes; this issue has now been addressed in the current worktree with salted scrypt hashes and legacy upgrade support.
2. Access tokens are accepted in query strings by session endpoints, which can leak through logs, browser history, proxies, and referrers.
3. SMTP credentials, Mastodon credentials, OAuth client secrets, and TOTP secrets are stored in plaintext in SQLite.
4. Mastodon instance URLs are user-controlled and the backend makes outbound requests to them, creating an SSRF and egress-control concern.
5. Login has no effective rate limiting or account lockout.
6. The Firefox extension stores bearer tokens in browser local storage and requests broad website access.
7. The Compose setup still exposes the application port directly and relies on deployment-specific Traefik networking and labels.
These findings are prioritized below. Severity describes the potential security impact in a typical Internet-facing deployment, not the likelihood in every environment.
## Positive Controls Already Present
- Bearer authentication is centralized in `backend/app/api/dependencies.py`.
- Administrator routes use `require_admin`; standard users receive `403`.
- Link update, delete, and Mastodon-post operations verify ownership.
- Tokens are generated with UUID material, stored as SHA-256 hashes, expire, and can be revoked.
- Password-reset tokens are random, hashed, expiring, single-use, and revoke existing sessions after reset.
- New administrator-created users require email verification before login.
- OTP uses time-based verification with a one-step clock window and is required before token issuance when enabled.
- The profile API does not return `password_hash` or `otp_secret` after the profile response hardening.
- User privilege changes protect against removing the last administrator and prevent an administrator from changing their own privilege.
- Uploaded avatars have a 2 MB limit, a restricted MIME allow-list, user-scoped filenames, and a persistent data location.
- SQLite foreign keys are enabled and ownership predicates are used for destructive link operations.
- SQL statements use parameters rather than interpolated user values.
- The Docker image runs the application as UID 10001 after startup and defines a health check.
- `.env` and database/runtime files are ignored by Git.
- The XPI build validates archive integrity, required files, and manifest parity.
- Browser rendering generally uses `textContent` for feed data, reducing DOM-based injection risk.
## Findings
### SA-001: Unsalted fast SHA-256 password hashing
**Severity:** Critical, remediated in current worktree
**Evidence before remediation:** `backend/app/database.py` and `backend/app/services/auth_service.py` used unsalted SHA-256 password comparisons.
**Current state:** `backend/app/database.py` now creates salted scrypt hashes in the format `scrypt$N$r$p$salt$digest`. `verify_password()` uses the encoded parameters and constant-time comparison. `authenticate_user()` fetches by username, verifies in Python, and transparently replaces a valid legacy 64-character SHA-256 hash with a new scrypt hash.
**Residual impact:** Existing accounts remain exposed until they successfully authenticate once after deployment. An attacker with a copy of an old database may still attack legacy hashes. Existing credentials should be rotated if the old database may have been exposed.
**Recommendation:** Deploy the current migration, require password rotation for accounts that cannot log in during migration, and monitor for remaining legacy hashes. Review scrypt cost parameters periodically and increase them as hardware changes. Do not revert to a fast general-purpose hash.
**Priority:** Completed in code; operational migration and credential rotation remain.
### SA-002: Bearer tokens accepted in query strings
**Severity:** High
**Evidence:** `backend/app/api/auth.py` exposes `GET /api/auth/me?token=...`; the web frontend and Firefox extension use this 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.
**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.
**Priority:** High.
### SA-003: Sensitive secrets stored in plaintext SQLite
**Severity:** High
**Evidence:** `backend/app/services/email_service.py` stores SMTP settings including `smtp_password` in `app_settings`; `backend/app/services/mastodon_oauth.py` stores Mastodon application secrets and user access tokens in `app_settings` and `user_plugin_config`; `backend/app/api/user_config.py` stores `otp_secret` in the `users` table.
**Impact:** Read access to the database exposes SMTP credentials, Mastodon posting authority, OAuth client secrets, and TOTP seeds. TOTP seeds cannot be changed by a user who loses the database copy. Database backups therefore contain reusable credentials, not just application data.
**Recommendation:** Encrypt secrets at rest using an external secret-management system or an application encryption key held outside the database. At minimum, use a dedicated secret key supplied through a protected environment/secret file, encrypt sensitive values before SQLite storage, restrict file and volume permissions, and document backup key management. Rotate all credentials after a suspected database disclosure. Continue omitting secrets from API responses.
**Priority:** High.
### SA-004: User-controlled Mastodon instance creates SSRF and uncontrolled egress risk
**Severity:** High
**Evidence:** `normalize_instance()` in `backend/app/services/mastodon_oauth.py` and the outbound requests in `backend/app/services/plugin_manager.py` accept an instance supplied by the user and call `urlopen()` against it.
**Impact:** A user can potentially configure an internal hostname, loopback address, cloud metadata endpoint, or other private network destination. The backend may send OAuth registration, token, status, or deletion requests to that destination. In addition to SSRF, this bypasses expected network egress policy and may disclose OAuth-related request data to an unintended host.
**Recommendation:** Validate Mastodon instances as HTTPS public hostnames. Resolve DNS and reject loopback, link-local, private, multicast, unspecified, and reserved IP ranges, including IPv4-mapped IPv6 addresses. Re-check after redirects and disable or strictly limit redirects. Prefer an outbound proxy with an allow-list and network egress policy. Set explicit URL and response-size limits and use a vetted HTTP client with safe redirect handling. Do not accept arbitrary schemes.
**Priority:** High.
### 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.
**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.
**Priority:** High.
### SA-006: Firefox extension has broad host access and stores bearer tokens in local storage
**Severity:** High
**Evidence:** `webextension/manifest.json` declares `host_permissions: ["<all_urls>"]`; `webextension/options.js` and `webextension/popup.js` store and retrieve `accessToken` through `browser.storage.local`.
**Impact:** A compromised extension context or another extension with sufficient access may obtain the bearer token. The broad host permission increases the impact of an extension compromise and requires elevated user trust. The token grants access until expiry or revocation.
**Recommendation:** Minimize permissions to the APIs actually needed. Prefer `activeTab` and explicit user interaction for page capture, and avoid `<all_urls>` unless required by a demonstrated workflow. Store session credentials in the narrowest available extension storage, minimize token lifetime, support refresh-token rotation, and clear all session material on logout or token invalidation. Add a Content Security Policy and review every extension script for dependency and injection risk.
**Priority:** High.
### SA-007: Production Compose configuration exposes the application directly
**Severity:** Medium/High
**Evidence:** `docker-compose.yml` publishes `${APP_PORT:-8000}:8000` while also configuring Traefik labels. The file uses an external `linklog_traefik` network and deployment-specific labels.
**Impact:** The application can bypass the reverse proxy and any TLS, authentication middleware, rate limiting, or security headers configured there. The default `LINKLOG_PUBLIC_URL=localhost` is also unsuitable for a public deployment. A network or label mismatch can silently expose an unprotected direct endpoint or make operators disable controls to restore access.
**Recommendation:** Split local development and production Compose files. In production, do not publish the application port to the host; attach it only to the reverse-proxy network. Ensure the app and Traefik share the same explicitly named network and validate the effective rendered configuration in CI. Require a non-local public hostname, TLS, security headers, request-size limits, and proxy-level rate limiting. Keep the direct port only in a documented local profile.
**Priority:** High for Internet-facing deployments.
### SA-008: Initial setup and SMTP validation are unauthenticated by design
**Severity:** Medium
**Evidence:** `backend/app/api/setup.py` exposes configuration, test-mail, status, and completion routes without a bearer dependency while no administrator exists.
**Impact:** This is necessary for first-run provisioning, but an exposed fresh instance allows anyone who can reach it to attempt setup, modify pending configuration, trigger test mail, and consume the five-send testing quota. The setup pending data includes a password hash and SMTP password in the database.
**Recommendation:** Restrict first-run setup at the network layer until an operator has completed provisioning, or require a one-time bootstrap secret supplied through the environment/console. Bind setup to localhost or a private management interface where possible. Add CSRF protection if setup ever uses cookies, strict request throttling, audit logging, and an explicit setup expiration/cleanup mechanism. Disable setup routes permanently once configuration completes.
**Priority:** Medium, high for exposed fresh deployments.
### SA-009: TOTP enrollment has no recovery codes or reset workflow
**Severity:** Medium
**Evidence:** `POST /api/user/otp/setup` returns the seed/provisioning URI and `POST /api/user/otp` requires a valid current OTP code to disable OTP.
**Impact:** A user who loses the authenticator device or seed can be locked out. Administrators have no documented recovery path that does not weaken authentication. Database readers can also use the plaintext seed as a second factor.
**Recommendation:** Generate one-time recovery codes during enrollment, display them once, hash them at rest, and invalidate each code on use. Require password reauthentication for disabling or replacing OTP. Add a controlled administrative recovery workflow with audit logging and notification. Avoid returning the seed after initial setup and never include it in profile responses.
**Priority:** Medium.
### SA-010: Avatar validation trusts the client MIME type
**Severity:** Medium
**Evidence:** `upload_avatar()` in `backend/app/api/user_config.py` selects the extension from `UploadFile.content_type` and writes the bytes without decoding or inspecting the image.
**Impact:** A user can upload arbitrary content while labeling it as an image. Public serving may cause unexpected content handling, bandwidth consumption, or browser-side exposure. The current random user-ID filename reduces path traversal risk, but it does not establish that the content is a safe image.
**Recommendation:** Decode images with a hardened image library, enforce pixel and dimension limits, re-encode to a safe format, strip metadata, and serve with a fixed safe `Content-Type` and `X-Content-Type-Options: nosniff`. Consider a separate media origin and a stricter content security policy.
**Priority:** Medium.
### SA-011: Error details can disclose infrastructure information
**Severity:** Medium
**Evidence:** Admin SMTP routes return exception text in `503` responses; Mastodon errors include upstream response bodies; the frontend displays these values to the user.
**Impact:** Connection errors can disclose hostnames, ports, TLS details, library messages, upstream response bodies, or internal service information. The behavior is useful for administrators but may expose more detail than intended if an admin session is compromised or error responses are logged.
**Recommendation:** Log full technical details server-side with correlation IDs. Return a stable user-facing message plus a short reference ID. Allow detailed diagnostics only behind an explicit protected troubleshooting mode. Redact credentials, authorization headers, URLs containing secrets, and SMTP/Mastodon response fields before logging or returning them.
**Priority:** Medium.
### SA-012: Token lifecycle has unused refresh-token semantics
**Severity:** Medium
**Evidence:** `issue_token()` returns a `refresh_token` value, but only the access token is inserted into `tokens`; no refresh endpoint or refresh-token hash is implemented.
**Impact:** Clients may assume the refresh token provides renewal or may store a value that cannot be revoked or used. This complicates session reasoning and can lead to unsafe client fallbacks. Access tokens currently live for the configured default of 30 days.
**Recommendation:** Either remove `refresh_token` from the API contract or implement a real refresh-token lifecycle: hash and persist refresh tokens, rotate them on use, detect reuse, bind them to a session/device, expire them separately, and revoke the token family on logout or password change. Reduce access-token lifetime after a real refresh flow is available.
**Priority:** Medium.
### SA-013: No explicit security headers, CORS policy, or request-size policy
**Severity:** Medium
**Evidence:** `backend/app/main.py` does not install security-header or CORS middleware, and the application routes do not define a global request-size limit.
**Impact:** Deployment behavior depends entirely on the reverse proxy. Missing `Content-Security-Policy`, `Strict-Transport-Security`, `X-Content-Type-Options`, `Referrer-Policy`, and frame protections weakens browser-side defenses. An overly permissive future CORS configuration could expose bearer-authenticated APIs. Large request bodies may consume resources even where individual avatar limits exist.
**Recommendation:** Add a documented restrictive security-header policy at the application or guaranteed proxy layer. Use `TrustedHostMiddleware` with an explicit production host list. Do not enable broad CORS; if cross-origin extension access requires it, allow only configured origins. Add global request and upload limits at the proxy and application layers.
**Priority:** Medium.
### SA-014: Development fallback secret is unsafe if the app is run without Compose configuration
**Severity:** Medium
**Evidence:** `Settings.secret_key` in `backend/app/core/config.py` defaults to `dev-secret-key-change-me`.
**Impact:** Local or incorrectly configured deployments can share a known secret. Even if the current token implementation does not use this value for signing, the setting creates a dangerous security assumption and may be used by future features.
**Recommendation:** Fail closed when `APP_ENV=production` and the secret is absent or matches a known development value. Generate secrets during provisioning, validate minimum length and entropy, and never ship a production fallback. Make all cryptographic uses explicit and test them.
**Priority:** Medium.
### SA-015: Some destructive and administrative operations lack audit logging
**Severity:** Low/Medium
**Evidence:** User creation/deletion, privilege changes, SMTP changes, theme changes, OTP enrollment/disablement, link deletion, and Mastodon deletion do not create durable security audit events.
**Impact:** Operators cannot reliably determine who changed privileges, modified delivery credentials, enrolled OTP, or deleted local/remote content. This limits incident response and accountability.
**Recommendation:** Add append-only audit events containing actor ID, action, target type/ID, timestamp, request ID, and outcome. Never store passwords, OTP codes, access tokens, SMTP passwords, or full sensitive request bodies. Export security events to protected logs.
**Priority:** Low/Medium.
## Authentication and Authorization Review
- **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.
- **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.
- **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.
- **OTP:** Login enforcement is present and OTP setup requires confirmation. Recovery codes, secret rotation, reauthentication, and encrypted secret storage are missing.
- **Authorization:** Admin checks and link ownership checks are present. The last-administrator invariant is enforced for privilege changes and deletion. Add authorization tests for every new destructive endpoint as the API grows.
## Data Protection Review
- SQLite is the primary data store and contains profile data, links, password hashes, tokens, SMTP settings, Mastodon credentials, OAuth state, and OTP secrets.
- Database backups must be treated as credential-bearing secrets, encrypted, access-controlled, rotated, and tested for secure deletion.
- Avatar files are persistent and publicly served. Validate and re-encode image content before accepting production uploads.
- Link URLs and comments are intentionally public feed data. Operators should document that users must not submit secrets in URLs or comments.
- SMTP and Mastodon integration errors should be redacted before entering logs or API responses.
## Frontend and Extension Review
- Feed content is generally assigned with `textContent`, which is a good XSS defense.
- User-supplied profile values rendered by Jinja should remain autoescaped; do not mark them safe without a narrowly reviewed reason.
- The web API currently uses bearer headers, so browser CSRF risk is lower than with cookie sessions. Keep it that way unless a CSRF token design is added.
- Browser local storage is exposed to any script running in the same origin. Keep third-party scripts out of authenticated pages and add a restrictive CSP.
- The extension's `<all_urls>` host permission should be reduced if the active-tab workflow is sufficient. Review Mozilla Add-ons policies before publishing signed releases.
- The extension stores access and refresh-token-like values in `browser.storage.local`; implement actual refresh semantics or stop storing/returning unused refresh values.
- Extension error messages should not include tokens or full sensitive URLs.
## Deployment Checklist
Before production exposure:
- [ ] Replace SHA-256 password hashing with Argon2id, scrypt, or bcrypt and migrate existing accounts.
- [ ] Remove query-string token authentication and rotate existing access tokens.
- [ ] Encrypt SMTP, Mastodon, OAuth, and OTP secrets at rest; protect encryption keys separately.
- [ ] Add login, OTP, reset-mail, and setup rate limiting.
- [ ] Restrict Mastodon instance validation and outbound network egress.
- [ ] Disable direct host publication of the application port in production.
- [ ] Configure HTTPS, HSTS, CSP, Referrer-Policy, frame protections, `nosniff`, and trusted hosts.
- [ ] Define a restrictive CORS policy or leave CORS disabled.
- [ ] Add global request-size limits and hardened image decoding/re-encoding.
- [ ] Add OTP recovery codes and a protected recovery workflow.
- [ ] Remove or implement refresh-token behavior.
- [ ] Add security audit events and centralized redacted logging.
- [ ] Rotate all credentials and set a unique high-entropy production secret.
- [ ] Review extension permissions and submit the XPI only after Mozilla policy review.
- [ ] Encrypt and restrict database/avatar backups, and test restore and revocation procedures.
- [ ] Run a dependency vulnerability scan and a dynamic penetration test against a production-like deployment.
## Suggested Priority Order
1. Password hashing migration.
2. Query-token removal and token rotation.
3. Secret-at-rest protection and backup controls.
4. Login/setup/OTP rate limiting.
5. Mastodon SSRF and egress restrictions.
6. Production network and TLS hardening.
7. Extension permission and credential-storage review.
8. Security headers, trusted hosts, request limits, media validation, recovery codes, and audit logging.
## Verification Performed
The repository's automated backend suite was run during this review and passed with 29 tests before the password-storage change; focused password/authentication tests subsequently passed after the remediation. Static Python and JavaScript checks were also used during feature development. These results demonstrate regression coverage, not absence of the findings above. A production release should additionally run a dependency scanner, container scanner, secret scanner, and authenticated dynamic security test.
+42
View File
@@ -825,14 +825,56 @@ Remove `DEFAULT_BACKEND` setting in the plugin.
### Assistant outcome ### Assistant outcome
Removed the hard-coded backend URL fallback from the Firefox extension. The backend URL is now empty until the user enters it, and the README setup instructions reflect that no default server is assumed. Removed the hard-coded backend URL fallback from the Firefox extension. The backend URL is now empty until the user enters it, and the README setup instructions reflect that no default server is assumed.
### User
Change authentication to use email addresses rather than usernames while keeping usernames for presentation.
### Assistant outcome
Switched backend, web login, and Firefox extension login to use trimmed email addresses as credentials. Returned usernames remain available for display, profiles, feed URLs, and authenticated session presentation. Updated login callers, documentation, and tests.
### User
Make sure the web plugin follows the same logic.
### Assistant outcome
Aligned the web and Firefox plugin flows with email-based authentication, including email-form fields, trimmed email/OTP submission, and retention of the returned username for display.
### User
In the plugin, `<span data-i18n="emailLabel">emailLabel</span>` should read `<span data-i18n="emailLabel">Email</span>`.
### Assistant outcome
Changed the Firefox plugin settings email field fallback text to `Email` while retaining the existing `emailLabel` localization key.
### User ### User
Change the title of the field “One-time password” to “One-time password (when configured)”. Change the title of the field “One-time password” to “One-time password (when configured)”.
### Assistant outcome ### Assistant outcome
Updated the OTP field label in the English, Spanish, German, French, and Dutch extension locale catalogs to clarify that the code is needed only when OTP is configured. Updated the OTP field label in the English, Spanish, German, French, and Dutch extension locale catalogs to clarify that the code is needed only when OTP is configured.
### User
For the new password in the user setting add a validation field to make sure they are the same before submitting.
### Assistant outcome
Added a confirm-new-password field to the profile password form. The browser now compares both new-password values and blocks submission with an error message when they differ.
### User ### User
Remove any leading and trailing spaces when entering fields in the plugin settings page. Remove any leading and trailing spaces when entering fields in the plugin settings page.
### Assistant outcome ### Assistant outcome
The extension now trims leading and trailing whitespace from the backend URL, username, and OTP before validation, login, and storage. Password input is preserved exactly so intentional password characters are not changed. The extension now trims leading and trailing whitespace from the backend URL, username, and OTP before validation, login, and storage. Password input is preserved exactly so intentional password characters are not changed.
### User
Do a full security audit document in detail in `Security-audit.md`.
### Assistant outcome
Created [Security-audit.md](../Security-audit.md), documenting the assessed architecture, existing controls, 15 prioritized security findings, authentication and authorization review, data protection, frontend and extension risks, deployment checklist, remediation order, and verification limits.
### User
Address issue 1 and improve password storage.
### Assistant outcome
Replaced unsalted SHA-256 password storage with salted scrypt hashes using standard-library `hashlib.scrypt`. Existing legacy SHA-256 hashes remain verifiable and are transparently upgraded after successful login. Password change and reset flows use the stronger hashing path, regression coverage was added, and `Security-audit.md` was updated to mark SA-001 as remediated in the current worktree.
### User
The login page should ask for OTP password.
### Assistant outcome
Added an optional “One-time password (when configured)” field to the web login page. The form sends a trimmed OTP value to the existing OTP-aware login endpoint and displays the backend’s OTP error when required or invalid.
+9
View File
@@ -148,6 +148,15 @@
139. Remove DEFAULT_BACKEND setting in the plugin. 139. Remove DEFAULT_BACKEND setting in the plugin.
140. Change the title of the field "One-time password" to "One-time password (when configured)". 140. Change the title of the field "One-time password" to "One-time password (when configured)".
141. Remove any leading and trailing spaces when entering fields in the settings page of the plugin. 141. Remove any leading and trailing spaces when entering fields in the settings page of the plugin.
142. Do a full security audit document in what you have done in detail in Security-audit.md
143. Address issue 1. and improve password storage
144. The login page should ask for OTP password
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
147. Make sure the web plugin follows same logic
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
148. In the plugin <span data-i18n="emailLabel">emailLabel</span> should read: <span data-i18n="emailLabel">Email</span>
## Future entries ## Future entries
+3 -3
View File
@@ -21,7 +21,7 @@ init_db()
class LoginRequest(BaseModel): class LoginRequest(BaseModel):
username: str email: str
password: str password: str
otp: str | None = None otp: str | None = None
@@ -33,9 +33,9 @@ class PasswordResetRequest(BaseModel):
@router.post('/login') @router.post('/login')
def login(payload: LoginRequest): def login(payload: LoginRequest):
user = authenticate_user(payload.username, payload.password) user = authenticate_user(payload.email.strip(), payload.password)
if user is None: if user is None:
reset_user = find_user(payload.username) reset_user = find_user(payload.email.strip())
if reset_user and reset_user['email_verified'] and smtp_configured(): if reset_user and reset_user['email_verified'] and smtp_configured():
try: try:
token = create_reset_token(reset_user['id']) token = create_reset_token(reset_user['id'])
+2 -2
View File
@@ -8,7 +8,7 @@ from fastapi import APIRouter, Depends, File, HTTPException, UploadFile
from pydantic import BaseModel from pydantic import BaseModel
from backend.app.api.dependencies import get_current_user from backend.app.api.dependencies import get_current_user
from backend.app.database import AVATARS_DIR, get_connection, hash_password from backend.app.database import AVATARS_DIR, get_connection, hash_password, verify_password
from backend.app.services.link_service import create_label, delete_label, list_user_labels, update_label from backend.app.services.link_service import create_label, delete_label, list_user_labels, update_label
from backend.app.services.otp_service import create_secret, provisioning_uri, verify_code from backend.app.services.otp_service import create_secret, provisioning_uri, verify_code
@@ -86,7 +86,7 @@ def update_current_user_profile(
def update_password(payload: PasswordUpdate, user: dict = Depends(get_current_user)): def update_password(payload: PasswordUpdate, user: dict = Depends(get_current_user)):
if len(payload.new_password) < 8: if len(payload.new_password) < 8:
raise HTTPException(status_code=422, detail='New password must be at least 8 characters') raise HTTPException(status_code=422, detail='New password must be at least 8 characters')
if hash_password(payload.current_password) != user['password_hash']: if not verify_password(payload.current_password, user['password_hash']):
raise HTTPException(status_code=400, detail='Current password is incorrect') raise HTTPException(status_code=400, detail='Current password is incorrect')
with get_connection() as conn: with get_connection() as conn:
+24 -2
View File
@@ -3,7 +3,8 @@
import sqlite3 import sqlite3
import os import os
from hashlib import sha256 from hashlib import scrypt, sha256
import hmac
from pathlib import Path from pathlib import Path
from uuid import uuid4 from uuid import uuid4
@@ -15,7 +16,28 @@ AVATARS_DIR.mkdir(parents=True, exist_ok=True)
def hash_password(password: str) -> str: def hash_password(password: str) -> str:
return sha256(password.encode('utf-8')).hexdigest() salt = os.urandom(16)
digest = scrypt(password.encode('utf-8'), salt=salt, n=16_384, r=8, p=1, dklen=32)
return f'scrypt$16384$8$1${salt.hex()}${digest.hex()}'
def verify_password(password: str, stored_hash: str) -> bool:
if stored_hash.startswith('scrypt$'):
try:
algorithm, cost, block_size, parallelism, salt_hex, digest_hex = stored_hash.split('$')
if algorithm != 'scrypt':
return False
digest = scrypt(
password.encode('utf-8'), salt=bytes.fromhex(salt_hex),
n=int(cost), r=int(block_size), p=int(parallelism), dklen=32,
)
return hmac.compare_digest(digest.hex(), digest_hex)
except (ValueError, TypeError):
return False
if len(stored_hash) == 64:
legacy_digest = sha256(password.encode('utf-8')).hexdigest()
return hmac.compare_digest(legacy_digest, stored_hash)
return False
DEFAULT_TAGS = ('#Internet', '#Cybersecurity', '#Fediverse', '#Food', '#Photography', '#Music', '#AI') DEFAULT_TAGS = ('#Internet', '#Cybersecurity', '#Fediverse', '#Food', '#Photography', '#Music', '#AI')
+18 -16
View File
@@ -2,26 +2,28 @@
## SPDX-License-Identifier: GPL-3.0-or-later ## SPDX-License-Identifier: GPL-3.0-or-later
from datetime import datetime, timezone from datetime import datetime, timezone
from hashlib import sha256 from backend.app.database import get_connection, hash_password, verify_password
from backend.app.database import get_connection
def hash_password(password: str) -> str: def authenticate_user(email: str, password: str):
return sha256(password.encode('utf-8')).hexdigest()
def authenticate_user(username: str, password: str):
password_hash = hash_password(password)
with get_connection() as conn: with get_connection() as conn:
row = conn.execute( row = conn.execute('SELECT * FROM users WHERE email = ?', (email,)).fetchone()
'SELECT * FROM users WHERE username = ? AND password_hash = ?', if row is None or not verify_password(password, row['password_hash']):
(username, password_hash), return None
).fetchone() user = dict(row)
return dict(row) if row else None if not row['password_hash'].startswith('scrypt$'):
conn.execute(
'UPDATE users SET password_hash = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?',
(hash_password(password), row['id']),
)
conn.commit()
user['password_hash'] = conn.execute(
'SELECT password_hash FROM users WHERE id = ?', (row['id'],)
).fetchone()['password_hash']
return user
def find_user(username: str): def find_user(email: str):
with get_connection() as conn: with get_connection() as conn:
row = conn.execute('SELECT * FROM users WHERE username = ?', (username,)).fetchone() row = conn.execute('SELECT * FROM users WHERE email = ?', (email,)).fetchone()
return dict(row) if row else None return dict(row) if row else None
+34 -7
View File
@@ -22,7 +22,7 @@ client = TestClient(app)
def login_headers(username='alice'): def login_headers(username='alice'):
token = client.post('/api/auth/login', json={ token = client.post('/api/auth/login', json={
'username': username, 'email': 'alice@example.com' if username == 'alice' else f'{username}@example.com',
'password': 'secret123', 'password': 'secret123',
}).json()['access_token'] }).json()['access_token']
return {'Authorization': f'Bearer {token}'} return {'Authorization': f'Bearer {token}'}
@@ -31,7 +31,7 @@ def login_headers(username='alice'):
def test_login_returns_token(): def test_login_returns_token():
assert app.version == '0.1.0' assert app.version == '0.1.0'
response = client.post('/api/auth/login', json={ response = client.post('/api/auth/login', json={
'username': 'alice', 'email': 'alice@example.com',
'password': 'secret123', 'password': 'secret123',
}) })
assert response.status_code == 200 assert response.status_code == 200
@@ -44,7 +44,7 @@ def test_login_returns_token():
assert admin_session.json()['is_admin'] is True assert admin_session.json()['is_admin'] is True
user_token = client.post('/api/auth/login', json={ user_token = client.post('/api/auth/login', json={
'username': 'bob', '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', params={'token': user_token})
@@ -52,6 +52,32 @@ def test_login_returns_token():
assert user_session.json()['is_admin'] is False assert user_session.json()['is_admin'] is False
def test_password_hashes_are_salted_and_legacy_hashes_upgrade_on_login():
from hashlib import sha256
from backend.app.database import hash_password
first = hash_password('same-password')
second = hash_password('same-password')
assert first.startswith('scrypt$16384$8$1$')
assert first != second
legacy_username = f'legacy-{uuid4().hex}'
legacy_hash = sha256('legacy-password'.encode('utf-8')).hexdigest()
with get_connection() as conn:
conn.execute(
'''INSERT INTO users
(id, username, email, password_hash, is_admin, email_verified)
VALUES (?, ?, ?, ?, 0, 1)''',
(str(uuid4()), legacy_username, f'{legacy_username}@example.com', legacy_hash),
)
conn.commit()
response = client.post('/api/auth/login', json={'email': legacy_username + '@example.com', 'password': 'legacy-password'})
assert response.status_code == 200
with get_connection() as conn:
upgraded = conn.execute('SELECT password_hash FROM users WHERE username = ?', (legacy_username,)).fetchone()['password_hash']
assert upgraded.startswith('scrypt$16384$8$1$')
def test_configuration_requires_authentication_and_admin_role(): def test_configuration_requires_authentication_and_admin_role():
assert client.get('/api/user/me').status_code == 401 assert client.get('/api/user/me').status_code == 401
assert client.get('/api/admin/plugins').status_code == 401 assert client.get('/api/admin/plugins').status_code == 401
@@ -177,7 +203,7 @@ def test_new_user_must_verify_email_before_login():
assert created.status_code == 201 assert created.status_code == 201
assert created.json()['email_verified'] is False assert created.json()['email_verified'] is False
login = client.post('/api/auth/login', json={'username': username, 'password': 'secret123'}) login = client.post('/api/auth/login', json={'email': f'{username}@example.com', 'password': 'secret123'})
assert login.status_code == 403 assert login.status_code == 403
assert login.json()['detail'] == 'Email address is not verified' assert login.json()['detail'] == 'Email address is not verified'
@@ -197,7 +223,7 @@ def test_email_verification_link_enables_login():
verified = client.get('/api/auth/verify-email', params={'token': verification_token}) verified = client.get('/api/auth/verify-email', params={'token': verification_token})
assert verified.status_code == 200 assert verified.status_code == 200
assert client.post('/api/auth/login', json={'username': username, 'password': 'secret123'}).status_code == 200 assert client.post('/api/auth/login', json={'email': f'{username}@example.com', 'password': 'secret123'}).status_code == 200
assert client.get('/api/auth/verify-email', params={'token': verification_token}).status_code == 400 assert client.get('/api/auth/verify-email', params={'token': verification_token}).status_code == 400
@@ -216,7 +242,7 @@ def test_mistyped_password_sends_reset_link_without_changing_login_error():
with patch('backend.app.api.auth.smtp_configured', return_value=True), \ with patch('backend.app.api.auth.smtp_configured', return_value=True), \
patch('backend.app.api.auth.create_reset_token', return_value='reset-token') as create_token, \ patch('backend.app.api.auth.create_reset_token', return_value='reset-token') as create_token, \
patch('backend.app.api.auth.send_password_reset_email') as send_email: patch('backend.app.api.auth.send_password_reset_email') as send_email:
response = client.post('/api/auth/login', json={'username': username, 'password': 'wrong-password'}) response = client.post('/api/auth/login', json={'email': f'{username}@example.com', 'password': 'wrong-password'})
assert response.status_code == 401 assert response.status_code == 401
assert response.json()['detail'] == 'Invalid username or password' assert response.json()['detail'] == 'Invalid username or password'
@@ -241,7 +267,7 @@ def test_password_reset_is_single_use_and_revokes_sessions():
reset = client.post('/api/auth/reset-password', json={'token': token, 'password': 'new-secret123'}) reset = client.post('/api/auth/reset-password', json={'token': token, 'password': 'new-secret123'})
assert reset.status_code == 200 assert reset.status_code == 200
assert client.post('/api/auth/reset-password', json={'token': token, 'password': 'another-secret'}).status_code == 400 assert client.post('/api/auth/reset-password', json={'token': token, 'password': 'another-secret'}).status_code == 400
assert client.post('/api/auth/login', json={'username': username, 'password': 'new-secret123'}).status_code == 200 assert client.post('/api/auth/login', json={'email': f'{username}@example.com', 'password': 'new-secret123'}).status_code == 200
def test_admin_can_remove_user_with_owned_data(): def test_admin_can_remove_user_with_owned_data():
@@ -515,6 +541,7 @@ def test_public_and_admin_pages_render_html():
assert client.get('/login').status_code == 200 assert client.get('/login').status_code == 200
login_page = client.get('/login').text login_page = client.get('/login').text
assert 'Sign in' in login_page assert 'Sign in' in login_page
assert 'name="otp"' in login_page
assert 'src="/static/logo.svg"' in login_page assert 'src="/static/logo.svg"' in login_page
assert 'id="auth-session" class="auth-session hidden"' in login_page assert 'id="auth-session" class="auth-session hidden"' in login_page
assert 'logout.js?v=3' in login_page assert 'logout.js?v=3' in login_page
+7 -6
View File
@@ -12,7 +12,7 @@ client = TestClient(app)
def test_user_config_api_and_profile_page(): def test_user_config_api_and_profile_page():
login = client.post('/api/auth/login', json={ login = client.post('/api/auth/login', json={
'username': 'alice', 'email': 'alice@example.com',
'password': 'secret123', 'password': 'secret123',
}).json() }).json()
headers = {'Authorization': f"Bearer {login['access_token']}"} headers = {'Authorization': f"Bearer {login['access_token']}"}
@@ -44,9 +44,10 @@ def test_user_config_api_and_profile_page():
assert 'id="auth-profile-link" class="hidden"' in page_response.text assert 'id="auth-profile-link" class="hidden"' in page_response.text
assert '<a id="auth-username" class="user-name" href="/">' in page_response.text assert '<a id="auth-username" class="user-name" href="/">' in page_response.text
assert 'id="auth-avatar"' not in page_response.text assert 'id="auth-avatar"' not in page_response.text
assert 'name="new_password_confirmation"' in page_response.text
bob_login = client.post('/api/auth/login', json={ bob_login = client.post('/api/auth/login', json={
'username': 'bob', 'email': 'bob@example.com',
'password': 'secret123', 'password': 'secret123',
}).json() }).json()
bob_headers = {'Authorization': f"Bearer {bob_login['access_token']}"} bob_headers = {'Authorization': f"Bearer {bob_login['access_token']}"}
@@ -56,7 +57,7 @@ def test_user_config_api_and_profile_page():
}, headers=bob_headers) }, headers=bob_headers)
assert password_response.status_code == 200 assert password_response.status_code == 200
assert client.post('/api/auth/login', json={ assert client.post('/api/auth/login', json={
'username': 'bob', 'email': 'bob@example.com',
'password': 'new-secret-123', 'password': 'new-secret-123',
}).status_code == 200 }).status_code == 200
assert client.put('/api/user/password', json={ assert client.put('/api/user/password', json={
@@ -79,7 +80,7 @@ def test_user_config_api_and_profile_page():
def test_user_can_enable_and_use_otp(): def test_user_can_enable_and_use_otp():
login = client.post('/api/auth/login', json={'username': 'alice', 'password': 'secret123'}).json() login = client.post('/api/auth/login', json={'email': 'alice@example.com', 'password': 'secret123'}).json()
headers = {'Authorization': f"Bearer {login['access_token']}"} headers = {'Authorization': f"Bearer {login['access_token']}"}
setup = client.post('/api/user/otp/setup', headers=headers) setup = client.post('/api/user/otp/setup', headers=headers)
assert setup.status_code == 200 assert setup.status_code == 200
@@ -91,10 +92,10 @@ def test_user_can_enable_and_use_otp():
}) })
assert enabled.status_code == 200 assert enabled.status_code == 200
assert enabled.json()['enabled'] is True assert enabled.json()['enabled'] is True
assert client.post('/api/auth/login', json={'username': 'alice', 'password': 'secret123'}).status_code == 401 assert client.post('/api/auth/login', json={'email': 'alice@example.com', 'password': 'secret123'}).status_code == 401
otp_login = client.post('/api/auth/login', json={ otp_login = client.post('/api/auth/login', json={
'username': 'alice', 'password': 'secret123', 'otp': current_code(secret), 'email': 'alice@example.com', 'password': 'secret123', 'otp': current_code(secret),
}) })
assert otp_login.status_code == 200 assert otp_login.status_code == 200
+5 -1
View File
@@ -12,7 +12,11 @@ form.addEventListener('submit', async (event) => {
const response = await fetch('/api/auth/login', { const response = await fetch('/api/auth/login', {
method: 'POST', method: 'POST',
headers: {'Content-Type': 'application/json'}, headers: {'Content-Type': 'application/json'},
body: JSON.stringify(Object.fromEntries(new FormData(form))), body: JSON.stringify({
email: form.elements.email.value.trim(),
password: form.elements.password.value,
otp: form.elements.otp.value.trim() || null,
}),
}); });
if (!response.ok) { if (!response.ok) {
+8
View File
@@ -192,6 +192,14 @@ if (mastodonParams.get('mastodon_error')) setStatus('#mastodon-status', mastodon
passwordForm.addEventListener('submit', async (event) => { passwordForm.addEventListener('submit', async (event) => {
event.preventDefault(); event.preventDefault();
const password = passwordForm.elements.new_password.value;
const confirmation = passwordForm.elements.new_password_confirmation.value;
if (password !== confirmation) {
const status = document.querySelector('#password-status');
status.textContent = 'New passwords do not match.';
status.style.color = '#f38ba8';
return;
}
const response = await fetch('/api/user/password', { const response = await fetch('/api/user/password', {
method: 'PUT', method: 'PUT',
headers: authHeaders(true), headers: authHeaders(true),
+6 -2
View File
@@ -38,13 +38,17 @@
<section class="link-item settings-panel"> <section class="link-item settings-panel">
<form id="login-form"> <form id="login-form">
<label> <label>
Username Email address
<input id="username" name="username" type="text" autocomplete="username" required /> <input id="email" name="email" type="email" autocomplete="username" required />
</label> </label>
<label> <label>
Password Password
<input id="password" name="password" type="password" autocomplete="current-password" required /> <input id="password" name="password" type="password" autocomplete="current-password" required />
</label> </label>
<label>
One-time password (when configured)
<input id="otp" name="otp" type="text" inputmode="numeric" autocomplete="one-time-code" placeholder="123456" />
</label>
<button type="submit">Sign in</button> <button type="submit">Sign in</button>
<p id="login-status" class="status" role="status"></p> <p id="login-status" class="status" role="status"></p>
<p><small>A mistyped password sends a reset link to your verified email address.</small></p> <p><small>A mistyped password sends a reset link to your verified email address.</small></p>
+4
View File
@@ -72,6 +72,10 @@
New password New password
<input name="new_password" type="password" minlength="8" autocomplete="new-password" required /> <input name="new_password" type="password" minlength="8" autocomplete="new-password" required />
</label> </label>
<label>
Confirm new password
<input name="new_password_confirmation" type="password" minlength="8" autocomplete="new-password" required />
</label>
<button type="submit">Change password</button> <button type="submit">Change password</button>
<p id="password-status" class="status" role="status"></p> <p id="password-status" class="status" role="status"></p>
</form> </form>
+2 -2
View File
@@ -28,8 +28,8 @@
</label> </label>
<label> <label>
<span data-i18n="usernameLabel">Username</span> <span data-i18n="emailLabel">Email</span>
<input id="username" type="text" data-i18n-placeholder="usernamePlaceholder" placeholder="alice" /> <input id="email" type="email" data-i18n-placeholder="emailPlaceholder" placeholder="alice@example.com" />
</label> </label>
<label> <label>
+10 -9
View File
@@ -4,7 +4,7 @@
const statusEl = document.getElementById('status'); const statusEl = document.getElementById('status');
const form = document.getElementById('settings-form'); const form = document.getElementById('settings-form');
const backendUrlInput = document.getElementById('backend-url'); const backendUrlInput = document.getElementById('backend-url');
const usernameInput = document.getElementById('username'); const emailInput = document.getElementById('email');
const passwordInput = document.getElementById('password'); const passwordInput = document.getElementById('password');
const otpInput = document.getElementById('otp'); const otpInput = document.getElementById('otp');
const session = document.getElementById('logged-in'); const session = document.getElementById('logged-in');
@@ -21,9 +21,9 @@ function setStatus(message, isError = false) {
} }
async function loadSettings() { async function loadSettings() {
const settings = await browser.storage.local.get(['backendUrl', 'username', 'accessToken']); const settings = await browser.storage.local.get(['backendUrl', 'email', 'username', 'accessToken']);
backendUrlInput.value = settings.backendUrl || ''; backendUrlInput.value = settings.backendUrl || '';
usernameInput.value = settings.username || ''; emailInput.value = settings.email || '';
if (settings.accessToken && settings.backendUrl) { if (settings.accessToken && settings.backendUrl) {
try { try {
@@ -32,7 +32,7 @@ async function loadSettings() {
); );
if (response.ok) { if (response.ok) {
const user = await response.json(); const user = await response.json();
showLoggedIn(user.username || settings.username, settings.backendUrl); showLoggedIn(user.username || settings.email, settings.backendUrl);
return; return;
} }
} catch (error) { } catch (error) {
@@ -61,11 +61,11 @@ async function clearSession() {
async function saveSettingsAndLogin(event) { async function saveSettingsAndLogin(event) {
event.preventDefault(); event.preventDefault();
const backendUrl = backendUrlInput.value.trim(); const backendUrl = backendUrlInput.value.trim();
const username = usernameInput.value.trim(); const email = emailInput.value.trim();
const password = passwordInput.value; const password = passwordInput.value;
const otp = otpInput.value.trim(); const otp = otpInput.value.trim();
if (!backendUrl || !username || !password) { if (!backendUrl || !email || !password) {
setStatus(t('fillAllFields'), true); setStatus(t('fillAllFields'), true);
return; return;
} }
@@ -74,7 +74,7 @@ async function saveSettingsAndLogin(event) {
const response = await fetch(`${backendUrl}/api/auth/login`, { const response = await fetch(`${backendUrl}/api/auth/login`, {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ username, password, otp: otp || null }) body: JSON.stringify({ email, password, otp: otp || null })
}); });
if (!response.ok) { if (!response.ok) {
@@ -84,14 +84,15 @@ async function saveSettingsAndLogin(event) {
const data = await response.json(); const data = await response.json();
await browser.storage.local.set({ await browser.storage.local.set({
backendUrl, backendUrl,
username, email,
username: data.user?.username || email,
accessToken: data.access_token, accessToken: data.access_token,
tokenType: data.token_type, tokenType: data.token_type,
tokenExpiresAt: data.expires_at, tokenExpiresAt: data.expires_at,
refreshToken: data.refresh_token, refreshToken: data.refresh_token,
}); });
showLoggedIn(data.user?.username || username, backendUrl); showLoggedIn(data.user?.username || email, backendUrl);
passwordInput.value = ''; passwordInput.value = '';
otpInput.value = ''; otpInput.value = '';
setStatus(t('loggedInSuccessfully')); setStatus(t('loggedInSuccessfully'));