Passwords stored salt and some change is password logic
This commit is contained in:
@@ -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.
|
||||||
|
- **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.
|
||||||
@@ -831,8 +831,32 @@ Change the title of the field “One-time password” to “One-time password (w
|
|||||||
### 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.
|
||||||
|
|||||||
@@ -148,6 +148,10 @@
|
|||||||
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
|
||||||
|
|
||||||
## Future entries
|
## Future entries
|
||||||
|
|
||||||
|
|||||||
@@ -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
@@ -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')
|
||||||
|
|
||||||
|
|||||||
@@ -2,23 +2,25 @@
|
|||||||
## 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:
|
|
||||||
return sha256(password.encode('utf-8')).hexdigest()
|
|
||||||
|
|
||||||
|
|
||||||
def authenticate_user(username: str, password: str):
|
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 username = ?', (username,)).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(username: str):
|
||||||
|
|||||||
@@ -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={'username': legacy_username, '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
|
||||||
@@ -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
|
||||||
|
|||||||
@@ -44,6 +44,7 @@ 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',
|
'username': 'bob',
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
|
|
||||||
const form = document.querySelector('#login-form');
|
const form = document.querySelector('#login-form');
|
||||||
const status = document.querySelector('#login-status');
|
const status = document.querySelector('#login-status');
|
||||||
|
const otpInput = document.querySelector('#otp');
|
||||||
|
|
||||||
form.addEventListener('submit', async (event) => {
|
form.addEventListener('submit', async (event) => {
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
@@ -12,7 +13,10 @@ 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({
|
||||||
|
...Object.fromEntries(new FormData(form)),
|
||||||
|
otp: otpInput.value.trim() || null,
|
||||||
|
}),
|
||||||
});
|
});
|
||||||
|
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
|
|||||||
@@ -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),
|
||||||
|
|||||||
@@ -45,6 +45,10 @@
|
|||||||
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>
|
||||||
|
|||||||
@@ -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>
|
||||||
|
|||||||
Reference in New Issue
Block a user