diff --git a/.env.example b/.env.example index 5a02892..28109ca 100644 --- a/.env.example +++ b/.env.example @@ -20,4 +20,9 @@ SLACK_OAUTH_TOKEN=your-slack-oauth-token JWT_SECRET=your-jwt-secret-key-change-this-in-production # Application URLs -APP_URL=http://localhost:3000 \ No newline at end of file +APP_URL=http://localhost:3000 + +# OIDC Provider (for Outline Wiki SSO) +OIDC_CLIENT_ID=outline-wiki +OIDC_CLIENT_SECRET= +OIDC_COOKIE_SECRET= \ No newline at end of file diff --git a/.gitignore b/.gitignore index 400f35a..a3170a0 100644 --- a/.gitignore +++ b/.gitignore @@ -17,10 +17,10 @@ logs .DS_Store .fleet .idea +docs/* # Local env files .env .env.* !.env.example -/docs/ scripts/*.js diff --git a/app/components/AppNavigation.vue b/app/components/AppNavigation.vue index bdbb47d..b32ed17 100644 --- a/app/components/AppNavigation.vue +++ b/app/components/AppNavigation.vue @@ -211,9 +211,7 @@ const handleLogin = async () => { } catch (err) { console.error("Login error:", err); - if (err.statusCode === 404) { - loginError.value = "No account found"; - } else if (err.statusCode === 500) { + if (err.statusCode === 500) { loginError.value = "Failed to send email"; } else { loginError.value = "Something went wrong"; diff --git a/app/components/LoginModal.vue b/app/components/LoginModal.vue index 543a936..a826505 100644 --- a/app/components/LoginModal.vue +++ b/app/components/LoginModal.vue @@ -167,9 +167,7 @@ const handleLogin = async () => { } catch (err) { console.error('Login error:', err) - if (err.statusCode === 404) { - loginError.value = 'No account found with that email. Please check your email or join Ghost Guild.' - } else if (err.statusCode === 500) { + if (err.statusCode === 500) { loginError.value = 'Failed to send login email. Please try again later.' } else { loginError.value = err.statusMessage || 'Something went wrong. Please try again.' diff --git a/app/composables/useMarkdown.js b/app/composables/useMarkdown.js index 9794192..b66430b 100644 --- a/app/composables/useMarkdown.js +++ b/app/composables/useMarkdown.js @@ -1,12 +1,29 @@ import { marked } from 'marked' +import DOMPurify from 'isomorphic-dompurify' + +const ALLOWED_TAGS = [ + 'a', 'strong', 'em', 'b', 'i', 'u', + 'ul', 'ol', 'li', 'p', 'br', + 'code', 'pre', 'blockquote', + 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', + 'hr', 'table', 'thead', 'tbody', 'tr', 'th', 'td', + 'del', 'sup', 'sub' +] + +const ALLOWED_ATTR = ['href', 'target', 'rel', 'class'] export const useMarkdown = () => { const render = (markdown) => { if (!markdown) return '' - return marked(markdown, { + const raw = marked(markdown, { breaks: true, gfm: true }) + return DOMPurify.sanitize(raw, { + ALLOWED_TAGS, + ALLOWED_ATTR, + ALLOW_DATA_ATTR: false + }) } return { diff --git a/app/middleware/admin.js b/app/middleware/admin.js index 2c643a7..cf2d155 100644 --- a/app/middleware/admin.js +++ b/app/middleware/admin.js @@ -1,18 +1,17 @@ -export default defineNuxtRouteMiddleware((to) => { - // Skip middleware in server-side rendering to avoid errors +export default defineNuxtRouteMiddleware(async (to) => { if (process.server) return - - // TODO: Temporarily disabled for testing - enable when authentication is set up - // Check if user is authenticated (you'll need to implement proper auth state) - // const isAuthenticated = useCookie('auth-token').value - - // if (!isAuthenticated) { - // throw createError({ - // statusCode: 401, - // statusMessage: 'Authentication required' - // }) - // } - - // TODO: Add proper role-based authorization - // For now, we assume anyone with a valid token is an admin -}) \ No newline at end of file + + const { isAuthenticated, memberData, checkMemberStatus } = useAuth() + + if (!isAuthenticated.value) { + await checkMemberStatus() + } + + if (!isAuthenticated.value) { + return navigateTo('/') + } + + if (memberData.value?.role !== 'admin') { + return navigateTo('/members') + } +}) diff --git a/app/pages/oidc/login.vue b/app/pages/oidc/login.vue new file mode 100644 index 0000000..c224c11 --- /dev/null +++ b/app/pages/oidc/login.vue @@ -0,0 +1,96 @@ + + + diff --git a/app/plugins/csrf.client.js b/app/plugins/csrf.client.js new file mode 100644 index 0000000..e5743bc --- /dev/null +++ b/app/plugins/csrf.client.js @@ -0,0 +1,23 @@ +export default defineNuxtPlugin(() => { + const safeMethods = new Set(['GET', 'HEAD', 'OPTIONS']) + + globalThis.$fetch = globalThis.$fetch.create({ + onRequest({ options }) { + const method = (options.method || 'GET').toUpperCase() + if (safeMethods.has(method)) return + + // Read CSRF token from cookie + const csrfToken = useCookie('csrf-token').value + if (csrfToken) { + options.headers = options.headers || {} + if (options.headers instanceof Headers) { + options.headers.set('x-csrf-token', csrfToken) + } else if (Array.isArray(options.headers)) { + options.headers.push(['x-csrf-token', csrfToken]) + } else { + options.headers['x-csrf-token'] = csrfToken + } + } + } + }) +}) diff --git a/docs/SECURITY_EVALUATION.md b/docs/SECURITY_EVALUATION.md new file mode 100644 index 0000000..b337246 --- /dev/null +++ b/docs/SECURITY_EVALUATION.md @@ -0,0 +1,282 @@ +# Ghost Guild Security Evaluation + +**Date:** 2026-02-28 (updated 2026-03-01) +**Framework:** OWASP ASVS v4.0, Level 1 +**Scope:** Full application stack (Nuxt 4 + Nitro server + MongoDB) + +--- + +## Framework Rationale + +Ghost Guild handles payments (Helcim), PII, and user-generated content. We evaluated four frameworks before selecting ASVS: + +| Framework | Why Not | +|---|---| +| PCI-DSS | Too narrow -- only covers payment surface, misses auth/XSS/access control | +| NIST CSF / ISO 27001 | Organizational frameworks, not application-level | +| OWASP Top 10 | Too coarse -- categories without testable requirements | +| **OWASP ASVS L1** | **Selected** -- purpose-built for web apps, testable pass/fail criteria across 14 chapters | + +ASVS Level 1 targets "all software" and is achievable for a small team. + +--- + +## Findings + +### Severity Summary + +| Severity | Count | Key Areas | +|----------|-------|-----------| +| CRITICAL | 5 | Admin auth disabled, payment verification stub, XSS (markdown + emails), cookie flags | +| HIGH | 9 | No rate limiting, no CSRF, JWT secret fallback, unauthenticated endpoints, mass assignment | +| MEDIUM | 5 | Long-lived tokens, Slack secrets unused, devtools, data logging, regex injection | +| LOW | 2 | User enumeration, email format validation | + +--- + +### CRITICAL + +#### C1. Admin endpoints completely unprotected +- **ASVS:** V4.1.1 (Trusted enforcement) +- **Files:** All routes under `server/api/admin/` +- **Evidence:** Auth code is commented out with `// TODO: Temporarily disabled auth for testing`. Anyone can list all members, create/delete events, and view revenue dashboard. The Member model has no `role` or `isAdmin` field. + +#### C2. Payment verification is a stub +- **ASVS:** V10.2.1 (Business logic integrity) +- **File:** `server/api/helcim/verify-payment.post.js` +- **Evidence:** Returns `{ success: true, cardToken: body.cardToken }` without calling the Helcim API. + +#### C3. XSS via unsanitized markdown +- **ASVS:** V5.3.3 (Output encoding for HTML) +- **Files:** `app/composables/useMarkdown.js`, `app/pages/members.vue:247` +- **Evidence:** `marked()` output rendered via `v-html` with no DOMPurify. + +#### C4. XSS in email templates +- **ASVS:** V5.2.6 (Server-side injection) +- **File:** `server/utils/resend.js` (11+ interpolation points) +- **Evidence:** User-supplied values interpolated directly into HTML email bodies. + +#### C5. Session cookie allows JavaScript access +- **ASVS:** V3.2.1 (Cookie attributes) +- **File:** `server/api/auth/verify.get.js:40-45` +- **Evidence:** `httpOnly: false, secure: false`. Session token accessible to any JavaScript. + +--- + +### HIGH + +#### H1. No rate limiting on any endpoint +- **ASVS:** V13.2.5 +- **Evidence:** No rate limiting middleware anywhere. Login, registration, uploads, and payment endpoints are unlimited. + +#### H2. No CSRF protection +- **ASVS:** V3.5.2 +- **Evidence:** No CSRF tokens, double-submit cookies, or CSRF middleware. + +#### H3. Hardcoded JWT fallback secret +- **ASVS:** V6.4.1 (Key management) +- **File:** `nuxt.config.ts:17` +- **Evidence:** `jwtSecret: process.env.JWT_SECRET || 'dev-secret-change-in-production'` + +#### H4. File upload endpoint unauthenticated +- **ASVS:** V4.1.3 +- **File:** `server/api/upload/image.post.js` +- **Evidence:** No auth check. Anyone can upload images. + +#### H5. Helcim payment endpoints mostly unauthenticated +- **ASVS:** V4.1.1 +- **Files:** `verify-payment.post.js`, `initialize-payment.post.js`, `update-billing.post.js`, `subscription.post.js` + +#### H6. Mass assignment: helcimCustomerId in allowedFields +- **ASVS:** V5.1.3 +- **File:** `server/api/members/profile.patch.js:40` +- **Evidence:** A member can change their own Helcim customer ID. + +#### H7. No input validation +- **ASVS:** V5.1.3 +- **Evidence:** Zod is installed but has zero imports. All validation is ad-hoc. + +#### H8. User enumeration on login +- **ASVS:** V2.1.7 +- **File:** `server/api/auth/login.post.js:24-28` +- **Evidence:** Returns 404 "No account found" vs success. + +#### H9. No security headers +- **ASVS:** V9.1.1 +- **Evidence:** No CSP, HSTS, X-Frame-Options, or X-Content-Type-Options. + +--- + +### MEDIUM + +| ID | ASVS | Finding | File | +|----|------|---------|------| +| M1 | V2.5.2 | 30-day static session token, no rotation | `verify.get.js:36` | +| M2 | V10.2.2 | Slack signing secret defined but never used | `nuxt.config.ts:21` | +| M3 | V7.4.1 | DevTools enabled unconditionally | `nuxt.config.ts:4` | +| M4 | V7.1.1 | Console logging of payment data and API token substrings | `helcim/customer.post.js:42` | +| M5 | V5.3.4 | Unescaped regex in directory search | `members/directory.get.js:49-51` | + +### LOW + +| ID | ASVS | Finding | File | +|----|------|---------|------| +| L1 | V2.1.7 | Timing-based enumeration via DB lookup | `auth/login.post.js` | +| L2 | V5.1.3 | No email format validation on login | `auth/login.post.js:15` | + +--- + +## Future Feature Risk Assessment + +| Planned Feature | Risk | Must Address First | +|---|---|---| +| Rich text member updates | Same XSS pattern as C3 | Fix markdown sanitization (C3) | +| Resource library with downloads | Unauthenticated upload (H4), malware distribution | Add upload auth (H4), file validation | +| Etherpad integration | External content rendered unsanitized | Build sanitization utility (C3) | +| Cal.com integration | API credential exposure | Fix secret management (H3) | +| Member-proposed events | No admin role model, no approval workflow | Build RBAC (C1) | +| Advanced search/analytics | Regex injection (M5), privacy leakage | Fix regex escaping (M5) | + +--- + +## Remediation Summary (Phases 0-1 + partial Phase 2) + +All work lives on branch `security/asvs-remediation`. + +### Auth guards (`server/utils/auth.js`) +- `requireAuth(event)` -- Reads JWT from `auth-token` cookie, verifies against `jwtSecret`, loads member from DB, rejects suspended/cancelled accounts (403). Auto-imported by Nitro. +- `requireAdmin(event)` -- Calls `requireAuth`, then checks `member.role === 'admin'` (403 if not). Member model gained `role` field (enum: `member`/`admin`, default `member`). +- Applied to all `server/api/admin/`, `server/api/upload/`, and `server/api/helcim/` endpoints. + +### CSRF (`server/middleware/01.csrf.js` + `app/plugins/csrf.client.js`) +- Double-submit cookie pattern. Middleware generates a random token cookie on first request, then enforces that POST/PATCH/DELETE to `/api/*` include a matching `x-csrf-token` header. +- Client plugin reads the cookie and attaches the header on every `$fetch` request. +- Exempt routes: `/api/helcim/webhook`, `/api/slack/webhook`, `/api/auth/verify`. + +### Security headers (`server/middleware/02.security-headers.js`) +- Always: `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `X-XSS-Protection: 0`, `Referrer-Policy: strict-origin-when-cross-origin`, `Permissions-Policy: camera=(), microphone=(), geolocation=()`. +- Production only: HSTS (1 year, includeSubDomains) and CSP allowing Helcim, Cloudinary, and Plausible sources. + +### Rate limiting (`server/middleware/03.rate-limit.js`) +- `rate-limiter-flexible` with in-memory stores, keyed by client IP. +- Auth endpoints: 5 requests per 5 minutes. Payment endpoints: 10 per minute. Uploads: 10 per minute. General API: 100 per minute. +- Returns 429 with `Retry-After` header on exhaustion. + +### XSS prevention +- **Markdown** (`app/composables/useMarkdown.js`): `marked()` output sanitized through DOMPurify with explicit allowlists for tags and attributes. Strips script/iframe/object/embed/img and all event handler attributes. +- **Email templates** (`server/utils/escapeHtml.js`): Pure function escaping `& < > " '` to HTML entities. Applied to all user-supplied interpolations in `server/utils/resend.js`. + +### Anti-enumeration (`server/api/auth/login.post.js`) +- Login returns identical `{ success: true, message: "If this email is registered, we've sent a login link." }` for both existing and non-existing emails. + +### Mass assignment (`server/api/members/profile.patch.js`) +- Explicit allowlist of profile fields. `helcimCustomerId`, `role`, `status`, `email`, `_id` are excluded from the `$set` update. + +### Session management +- 7-day token expiry with refresh endpoint at `/api/auth/refresh`. + +--- + +## Remediation Roadmap + +### Phase 0: Emergency (before any production traffic) -- COMPLETE + +| # | Finding | Fix | ASVS | Status | +|---|---------|-----|------|--------| +| 1 | C5 | Set `httpOnly: true`, `secure` conditional on NODE_ENV | V3.2.1 | Done | +| 2 | C3 | Install `isomorphic-dompurify`, sanitize `marked()` output | V5.3.3 | Done | +| 3 | C1 | Add `role` field to Member model, re-enable admin auth with role check | V4.1.1 | Done | +| 4 | C2 | Call Helcim API in verify-payment to confirm transactions server-side | V10.2.1 | Done | +| 5 | H3 | Throw on missing JWT_SECRET instead of fallback | V6.4.1 | Done | + +### Phase 1: Pre-launch (before public access) -- COMPLETE + +| # | Finding | Fix | ASVS | Status | +|---|---------|-----|------|--------| +| 6 | C4 | Create `escapeHtml()` utility, apply to all email template interpolations | V5.2.6 | Done | +| 7 | H4 | Add JWT verification to upload endpoint | V4.1.3 | Done | +| 8 | H5 | Add JWT verification to payment endpoints | V4.1.1 | Done | +| 9 | H6 | Remove `helcimCustomerId` from `allowedFields` | V5.1.3 | Done | +| 10 | H2 | Add CSRF double-submit cookie middleware (exempt webhooks) | V3.5.2 | Done | +| 11 | H9 | Add CSP, HSTS, X-Frame-Options, X-Content-Type-Options headers | V9.1.1 | Done | +| 12 | H1 | Add rate limiting to login, registration, upload, payment | V13.2.5 | Done | +| 13 | H8 | Return identical response for existing/non-existing accounts | V2.1.7 | Done | +| 14 | -- | Add `status: 'active'` check to auth endpoints | V4.1.1 | Done | + +### Phase 2: Hardening (within 30 days of launch) -- PARTIAL + +| # | Finding | Fix | ASVS | Status | +|---|---------|-----|------|--------| +| 15 | H7 | Implement Zod validation across all API endpoints | V5.1.3 | Open | +| 16 | M5 | Escape regex in directory search | V5.3.4 | Open | +| 17 | M4 | Remove sensitive console.log statements | V7.1.1 | Open | +| 18 | M3 | Make devtools conditional on NODE_ENV | V7.4.1 | Open | +| 19 | M1 | Shorter session tokens (7d) with refresh endpoint | V2.5.2 | Done | +| 20 | -- | Create shared `requireAuth()`/`requireAdmin()` utilities | V4.1.1 | Done | + +### Phase 3: Before building planned features + +| # | Fix | Status | +|---|-----|--------| +| 21 | Build sanitization utility (DOMPurify wrapper) for all user-generated HTML | Open | +| 22 | Design admin role model with granular permissions | Open | +| 23 | Implement file validation pipeline (type, size, virus scanning) | Open | +| 24 | Design credential management patterns (encrypted at rest) | Open | + +--- + +## Automated Testing + +### Framework + +Vitest with two test projects: + +- **Server tests** (`tests/server/`): Node.js environment, h3 globals stubbed from real h3 functions +- **Client tests** (`tests/client/`): jsdom environment for browser-side composables + +```bash +npm run test # Watch mode +npm run test:run # Single run (CI) +``` + +### Infrastructure + +- `tests/server/setup.js` -- Stubs real h3 functions (`getCookie`, `setCookie`, `getMethod`, `getHeader`, `setHeader`, `getRequestURL`, `createError`, `defineEventHandler`, `readBody`, etc.) as globals to simulate Nitro auto-imports. Also stubs `useRuntimeConfig`. +- `tests/server/helpers/createMockEvent.js` -- Factory that builds real h3 events from Node.js `IncomingMessage`/`ServerResponse` pairs. Accepts `method`, `path`, `headers`, `cookies`, `body`, and `remoteAddress`. Captures response headers via `event._testSetHeaders` for assertions. + +### Test Coverage (79 tests across 8 files) + +| File | Tests | Security Controls Verified | +|------|-------|---------------------------| +| `tests/server/utils/escapeHtml.test.js` | 12 | All 5 HTML entity escapes, null/undefined handling, ` world') + expect(result).not.toContain('') + expect(result).toContain('Hello') + expect(result).toContain('world') + }) + + it('strips onerror attributes', () => { + const result = render('') + expect(result).not.toContain('onerror') + }) + + it('strips onclick attributes', () => { + const result = render('click') + expect(result).not.toContain('onclick') + }) + + it('strips iframe tags', () => { + const result = render('') + expect(result).not.toContain(' { + const result = render('') + expect(result).not.toContain(' { + const result = render('') + expect(result).not.toContain(' { + const result = render('[click me](javascript:alert(1))') + expect(result).not.toContain('javascript:') + }) + + it('strips img tags (not in allowed list)', () => { + const result = render('![alt](https://example.com/img.png)') + expect(result).not.toContain(' { + it('renders bold and italic', () => { + const result = render('**bold** and *italic*') + expect(result).toContain('bold') + expect(result).toContain('italic') + }) + + it('renders links with href', () => { + const result = render('[Ghost Guild](https://ghostguild.org)') + expect(result).toContain(' { + for (let i = 1; i <= 6; i++) { + const hashes = '#'.repeat(i) + const result = render(`${hashes} Heading ${i}`) + expect(result).toContain(``) + } + }) + + it('preserves code blocks', () => { + const result = render('`inline code` and\n\n```\nblock code\n```') + expect(result).toContain('') + expect(result).toContain('
')
+    })
+
+    it('preserves blockquotes', () => {
+      const result = render('> This is a quote')
+      expect(result).toContain('
')
+    })
+
+    it('preserves lists', () => {
+      const result = render('- item 1\n- item 2')
+      expect(result).toContain('