diff --git a/.github/workflows/backend-academy.yml b/.github/workflows/backend-academy.yml index 5ec16e524..3ab502f7b 100644 --- a/.github/workflows/backend-academy.yml +++ b/.github/workflows/backend-academy.yml @@ -1,5 +1,29 @@ name: BackendAcademy CI +# Issue #692 — BA-124: Add comprehensive lint, typecheck, and CI gates +# +# This workflow runs on every push / PR touching BackendAcademy sources. +# All jobs must pass before a PR can be merged. +# +# Job overview +# ───────────────────────────────────────────────── +# typecheck TypeScript compiler check (no emit) +# lint ESLint on all src/** TypeScript files +# unit-tests Jest unit tests (*.spec.ts) in-process +# integration-tests Learner-journey and AI integration specs +# build Production nest build (must succeed after lint+types pass) +# +# Failure guidance +# ───────────────────────────────────────────────── +# typecheck: Fix TypeScript errors reported by `npm run typecheck`. +# Common causes: missing types, wrong generics, strict-null mismatches. +# lint: Run `npm run lint` locally. Auto-fixable issues: `npm run lint -- --fix`. +# unit-tests: Run `npm run test:unit` locally with NODE_ENV=test. +# Check the failing spec file reported in the output. +# integration-tests: Run `npx jest --config jest.config.ts --runInBand src/integration/learner-journey.spec.ts src/ai` +# Requires no external dependencies — uses in-memory fixtures. +# build: Run `npm run build` locally. Usually fails when typecheck fails first. + on: push: branches: [ main ] @@ -12,13 +36,103 @@ on: - 'BackendAcademy/**' - '.github/workflows/backend-academy.yml' +defaults: + run: + working-directory: BackendAcademy + jobs: + # ───────────────────────────────────────────── + # 1. TypeScript type checking + # ───────────────────────────────────────────── + typecheck: + name: TypeScript typecheck + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: BackendAcademy/package-lock.json + + - name: Install dependencies + # --legacy-peer-deps: joi-to-typescript (devDependency) pins joi@17 + # while the app targets joi@18. + run: npm ci --no-audit --no-fund --legacy-peer-deps + + - name: Run TypeScript compiler check + run: npm run typecheck + + # ───────────────────────────────────────────── + # 2. Lint + # ───────────────────────────────────────────── + lint: + name: ESLint + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: BackendAcademy/package-lock.json + + - name: Install dependencies + run: npm ci --no-audit --no-fund --legacy-peer-deps + + - name: Run ESLint + run: npm run lint + + # ───────────────────────────────────────────── + # 3. Unit tests + # ───────────────────────────────────────────── + unit-tests: + name: Unit tests + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: BackendAcademy/package-lock.json + + - name: Install dependencies + run: npm ci --no-audit --no-fund --legacy-peer-deps + + # BA-124: Run all unit specs (*.spec.ts) isolated from integration tests. + # Tests run in-band to avoid shared-state flakiness from parallel workers. + - name: Run unit tests + env: + NODE_ENV: test + run: npm run test:unit + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: BackendAcademy/coverage/ + retention-days: 7 + + # ───────────────────────────────────────────── + # 4. Integration & AI tests + # ───────────────────────────────────────────── integration-and-ai-tests: name: Learner journey & AI tests runs-on: ubuntu-latest - defaults: - run: - working-directory: BackendAcademy steps: - name: Checkout code @@ -28,6 +142,8 @@ jobs: uses: actions/setup-node@v4 with: node-version: '20' + cache: 'npm' + cache-dependency-path: BackendAcademy/package-lock.json - name: Install dependencies # --legacy-peer-deps: joi-to-typescript (devDependency) pins joi@17 @@ -41,3 +157,35 @@ jobs: env: NODE_ENV: test run: npx jest --config jest.config.ts --runInBand src/integration/learner-journey.spec.ts src/ai + + # ───────────────────────────────────────────── + # 5. Production build + # ───────────────────────────────────────────── + build: + name: Production build + runs-on: ubuntu-latest + needs: [typecheck, lint] + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: BackendAcademy/package-lock.json + + - name: Install dependencies + run: npm ci --no-audit --no-fund --legacy-peer-deps + + - name: Build + run: npm run build + + - name: Upload build artifact + uses: actions/upload-artifact@v4 + with: + name: dist + path: BackendAcademy/dist/ + retention-days: 3 diff --git a/BackendAcademy/docs/OPERATIONS.md b/BackendAcademy/docs/OPERATIONS.md new file mode 100644 index 000000000..9ce8e8460 --- /dev/null +++ b/BackendAcademy/docs/OPERATIONS.md @@ -0,0 +1,614 @@ +# BackendAcademy — Operational Documentation & Runbooks + +> **BA-125** — This document fulfils the operational documentation requirement. +> It covers setup, module wiring, required services, environment variables, +> deployment, rollback, data recovery, and incident-response procedures. + +--- + +## Table of Contents + +1. [Service Overview](#1-service-overview) +2. [Required Infrastructure](#2-required-infrastructure) +3. [Environment Variables](#3-environment-variables) +4. [Module Wiring](#4-module-wiring) +5. [Local Development Setup](#5-local-development-setup) +6. [Deployment Guide](#6-deployment-guide) +7. [Running Database Migrations](#7-running-database-migrations) +8. [CI/CD Gates](#8-cicd-gates) +9. [Rollback Runbook](#9-rollback-runbook) +10. [Data Recovery Runbook](#10-data-recovery-runbook) +11. [Incident Response Runbook](#11-incident-response-runbook) +12. [Troubleshooting Guide](#12-troubleshooting-guide) +13. [Health Checks & Monitoring](#13-health-checks--monitoring) +14. [Secrets Management](#14-secrets-management) + +--- + +## 1. Service Overview + +**BackendAcademy** is the NestJS API server powering RustAcademy. +It provides: + +- REST API for courses, lessons, tasks, submissions, users, and rewards +- AI Mentor integration (Claude / OpenAI / mock) +- Notification delivery (email, push, in-app) with circuit-breaker resilience +- Social feed with ownership and moderation enforcement +- Background job queue for grading and certificate minting +- Stellar / Soroban smart-contract integration for on-chain rewards + +**Default port:** `3000` (configurable via `PORT`). +**Health endpoint:** `GET /health` + +--- + +## 2. Required Infrastructure + +| Service | Purpose | Required in prod? | +|---------------|--------------------------------------------|:-----------------:| +| PostgreSQL 15+ | Primary datastore for all application data | **Yes** | +| Redis 7+ | Pub/sub, job queue, rate-limiting cache | **Yes** | +| Anthropic Claude API (optional) | AI Mentor chat and grading | No (mock fallback) | +| OpenAI API (optional) | AI Mentor alternative | No (mock fallback) | +| Stellar Horizon | Blockchain account/payment lookups | No (stubbed) | +| SMTP / SendGrid / SES | Email delivery | No (logged only in dev) | +| FCM / APNs | Push notifications | No (logged only in dev) | + +--- + +## 3. Environment Variables + +Copy `.env.example` as a starting point: + +```bash +cp .env.example .env +``` + +### 3.1 Required in Production + +| Variable | Description | Constraints | +|-----------------------|---------------------------------------------------|----------------------------------------------| +| `DATABASE_URL` | PostgreSQL connection string | Full URL including user/password/db name | +| `REDIS_HOST` | Redis hostname or IP | Must be reachable from the app container | +| `JWT_SECRET` | JWT signing secret | ≥ 32 chars; must not be a placeholder value | +| `ASSET_SIGNING_SECRET`| HMAC secret for signed asset download URLs | Must not be empty; empty = forgeable URLs | + +### 3.2 Optional / Provider-specific + +| Variable | Default | Description | +|------------------------|--------------|---------------------------------------------------| +| `PORT` | `3000` | HTTP port the server listens on | +| `NODE_ENV` | `development`| Runtime mode: `development`, `production`, `test` | +| `CORS_ORIGIN` | `*` | Allowed CORS origins (comma-separated or `*`) | +| `REDIS_PORT` | `6379` | Redis port | +| `REDIS_PASSWORD` | — | Redis auth password (if required) | +| `JWT_CLOCK_SKEW_SECONDS` | `30` | Allowed JWT clock drift in seconds (0–120) | +| `API_KEY_SECRET` | — | Shared secret for API-key authentication | +| `AI_PROVIDER` | `mock` | `claude` | `openai` | `mock` | +| `ANTHROPIC_API_KEY` | — | Required when `AI_PROVIDER=claude` | +| `OPENAI_API_KEY` | — | Required when `AI_PROVIDER=openai` | +| `AI_MODEL` | — | Override the default AI model | +| `AI_MAX_TOKENS` | `4096` | Token budget per AI response | +| `AI_TEMPERATURE` | `0.7` | Sampling temperature for AI responses | +| `AI_RETRY_MAX_ATTEMPTS`| `3` | AI provider retry attempts (429/5xx) | +| `AI_RETRY_BASE_DELAY_MS`| `250` | Base back-off delay in milliseconds | +| `AI_RETRY_MAX_DELAY_MS`| `5000` | Maximum back-off delay in milliseconds | +| `ASSETS_UPLOAD_DIR` | `./data/uploads` | Where uploaded assets are stored on disk | +| `ASSETS_STATIC_DIR` | `./public` | Read-only static asset directory | +| `ASSETS_BASE_URL` | `/api/v1/assets` | Base URL in asset metadata responses | +| `ASSETS_MAX_SIZE_MB` | `10` | Per-file upload size limit (MB) | +| `ASSETS_MAX_TOTAL_MB` | `1024` | Aggregate asset storage quota (MB) | +| `ASSETS_MAX_COUNT` | `10000` | Maximum number of stored assets | +| `LOCALE` | `en` | Localization locale | +| `CRON_CLEANUP_SCHEDULE`| `0 0 * * *` | Daily cleanup cron (midnight UTC) | +| `CRON_ANALYTICS_SCHEDULE`| `0 */6 * * *` | Analytics aggregation every 6 hours | +| `CRON_NOTIFICATIONS_SCHEDULE`| `*/30 * * * *` | Notification batch flush every 30 min | + +> **Production startup guardrails:** When `NODE_ENV=production`, the app +> refuses to boot if `DATABASE_URL`, `REDIS_HOST`, `JWT_SECRET`, or +> `ASSET_SIGNING_SECRET` are missing, empty, or set to placeholder values +> from `.env.example`. This prevents silent misconfiguration in production. + +--- + +## 4. Module Wiring + +The application entry-point is `src/main.ts`. All NestJS modules are registered in `src/app.module.ts`. + +| Module | Key providers | Notes | +|-----------------------|----------------------------------------------------|------------------------------------------| +| `ConfigModule` | `ConfigService`, env validation via `Joi` | Global; validates all env vars at startup | +| `DatabaseModule` | `DatabaseService`, `MigrationService` | Wraps `@supabase/supabase-js` client | +| `AuthModule` | `AuthSessionService`, JWT guards, decorators | JWT + API-key auth | +| `UsersModule` | `UsersService`, `UserProfileService` | | +| `CoursesModule` | `CourseService`, `CertificateService`, `CourseRatingService` | Progress tracking | +| `LessonsModule` | `LessonService` | | +| `TasksModule` | `TaskService`, `TaskOrchestratorService` | | +| `SubmissionsModule` | `SubmissionService`, `TutorReviewService` | Grading pipeline | +| `JobsModule` | `JobsService`, `GradingJobService` | In-memory job queue | +| `DeadLetterQueueModule` | `DlqService`, `GradingRetryPolicy` | | +| `AiModule` | `AiService`, `PromptTemplateService`, `ClaudeProvider` | Pluggable AI backend | +| `NotificationsModule` | `NotificationsService`, `EmailNotificationProvider`, `PushNotificationProvider`, `InAppNotificationProvider` | Resilient delivery | +| `SocialModule` | `SocialService` | Post ownership + moderation enforcement | +| `ChatModule` | `ChatService`, rate limiting | | +| `RewardsModule` | `RewardsService`, `StreakService`, `ReferralService` | XLM reward logic | +| `WalletModule` | `WalletService` | Stellar wallet ops | +| `ContractsModule` | `ContractsService`, `ContractRegistryService` | Soroban contract calls | +| `BadgesModule` | `BadgesService` | NFT badge minting | +| `SecurityModule` | `SecurityService`, `AntiCheatService` | | +| `AdminModule` | `AdminService` | Admin-only operations | +| `MonitoringModule` | `MetricsService`, `MonitoringService` | Prometheus metrics | +| `RedisModule` | `RedisService` | Redis client wrapper | +| `HealthModule` | `HealthController` | `GET /health` | +| `LoggingModule` | `CorrelationLoggerService`, `ErrorTrackingService` | | + +--- + +## 5. Local Development Setup + +### Prerequisites + +- Node.js 20+ +- npm 9+ (or pnpm 9+) +- Docker + Docker Compose (for PostgreSQL and Redis) + +### Steps + +```bash +# 1. Install dependencies +cd BackendAcademy +npm install + +# 2. Copy and fill in environment variables +cp .env.example .env +# Edit .env — set DATABASE_URL, REDIS_HOST, JWT_SECRET, etc. + +# 3. Start PostgreSQL and Redis via Docker +docker-compose up -d # from the repo root + +# 4. Run database migrations +npx ts-node src/database/migration-cli.ts migrate + +# 5. Start the development server (hot reload) +npm run start:dev +# → http://localhost:3000 + +# 6. Verify the server is healthy +curl http://localhost:3000/health +``` + +### Running tests locally + +```bash +# Unit tests only (fast, no external dependencies) +npm run test:unit + +# All tests +npm test + +# TypeScript check +npm run typecheck + +# Lint +npm run lint +``` + +--- + +## 6. Deployment Guide + +### 6.1 Docker (recommended) + +```bash +# Build the image +docker build -t rustacademy-backend:latest ./BackendAcademy + +# Run in production +docker run -d \ + --name rustacademy-backend \ + -p 3000:3000 \ + --env-file /path/to/prod.env \ + rustacademy-backend:latest +``` + +> The Dockerfile uses a multi-stage build with a non-root `appuser` for least-privilege execution. + +### 6.2 Railway / Render + +1. Point the service root to `BackendAcademy/`. +2. Set `BUILD_COMMAND` to `npm run build` and `START_COMMAND` to `node dist/main.js`. +3. Add all required environment variables from [Section 3](#3-environment-variables) in the platform dashboard. +4. Enable the health check endpoint at `/health`. + +### 6.3 Pre-deployment checklist + +- [ ] All required env vars are set (see [Section 3.1](#31-required-in-production)) +- [ ] `JWT_SECRET` and `ASSET_SIGNING_SECRET` are ≥ 32 chars and not placeholder values +- [ ] Database migrations have been reviewed and staged (`migration-cli.ts dry-run`) +- [ ] Feature flags for risky changes are set to `false` initially +- [ ] Rollback plan confirmed (see [Section 9](#9-rollback-runbook)) +- [ ] Team notified of deployment window +- [ ] Monitoring dashboards open and baselining + +--- + +## 7. Running Database Migrations + +BackendAcademy uses a custom migration service (`src/database/migration.service.ts`). + +```bash +# Inside BackendAcademy directory (or Docker exec into the container) + +# Run all pending migrations (auto-detects by file timestamp) +npx ts-node src/database/migration-cli.ts migrate + +# List applied migrations +npx ts-node src/database/migration-cli.ts status + +# Rollback the last batch of migrations +npx ts-node src/database/migration-cli.ts rollback + +# Rollback a specific number of steps +npx ts-node src/database/migration-cli.ts rollback --steps 2 +``` + +> **Always take a database snapshot before running migrations in production.** +> See [Section 10](#10-data-recovery-runbook) for recovery procedures. + +--- + +## 8. CI/CD Gates + +All CI jobs are defined in `.github/workflows/backend-academy.yml`. +The following gates **must all pass** before a PR can be merged: + +| Job | Command | What it checks | +|------------------------|----------------------------|------------------------------------------| +| `typecheck` | `npm run typecheck` | TypeScript compiler errors (no emit) | +| `lint` | `npm run lint` | ESLint rules on `src/**/*.ts` | +| `unit-tests` | `npm run test:unit` | All `*.spec.ts` unit tests | +| `integration-tests` | Jest on learner-journey | Integration + AI provider suites | +| `build` | `npm run build` | NestJS production build (requires typecheck+lint) | + +See the workflow file for actionable failure guidance per job. + +--- + +## 9. Rollback Runbook + +### Trigger + +Deploy produced errors (5xx spike, health check failures, data corruption). + +### Procedure + +**Step 1 — Assess the blast radius** + +```bash +# Check recent error rate from monitoring +curl http://:3000/metrics | grep http_requests_total +# Or check health +curl http://:3000/health +``` + +**Step 2 — Roll back the application** + +*Docker:* +```bash +# Pull and run the previous image tag +docker stop rustacademy-backend +docker run -d \ + --name rustacademy-backend \ + -p 3000:3000 \ + --env-file /path/to/prod.env \ + rustacademy-backend: +``` + +*Railway/Render:* Use the platform's **Rollback** or **Redeploy Previous** button in the dashboard. + +*Git-based:* +```bash +# In CI: re-trigger the pipeline on the previous good commit +git revert HEAD --no-edit +git push origin main +``` + +**Step 3 — Roll back database migrations (if needed)** + +Only run if the new migration caused the issue: + +```bash +# Inside the old container or via db migration CLI +npx ts-node src/database/migration-cli.ts rollback +``` + +> ⚠️ Only rollback if the migration is reversible. Destructive migrations +> (column drops, data transformations) require manual data recovery. +> See [Section 10](#10-data-recovery-runbook). + +**Step 4 — Verify recovery** + +```bash +curl http://:3000/health +# Expect: { "status": "ok", ... } +``` + +**Step 5 — Post-mortem** + +After recovery, document in a GitHub issue: +- What failed and when +- Root cause +- Fix and prevention steps + +--- + +## 10. Data Recovery Runbook + +### 10.1 Before any migration in production + +```bash +# Supabase — create a manual backup via the dashboard +# Or via psql: +pg_dump $DATABASE_URL --format=custom --file=rustacademy_$(date +%Y%m%d_%H%M%S).dump +``` + +### 10.2 Restore from backup + +```bash +# Stop the application to prevent writes during restore +docker stop rustacademy-backend + +# Restore +pg_restore --clean --dbname $DATABASE_URL rustacademy_.dump + +# Restart +docker start rustacademy-backend +``` + +### 10.3 Recovering deleted records + +The application does not currently implement soft-deletes globally. +If records are accidentally deleted: + +1. Stop the application immediately to prevent further writes. +2. Restore from the most recent pre-deletion backup. +3. Use a binary diff (WAL-based) recovery if the PostgreSQL instance has + point-in-time recovery (PITR) enabled via Supabase. + +### 10.4 Redis data loss + +Redis is used for ephemeral state (sessions, rate-limit counters, job queues). +Its data is not durably persisted by default. + +- **Sessions:** Users will be logged out and need to re-authenticate. +- **Rate-limit counters:** Will reset; clients may temporarily exceed limits. +- **Job queue:** Any in-flight jobs will be lost. Trigger a manual requeue if needed. + +**Prevention:** Enable Redis AOF persistence or use a managed Redis with +automatic replication (e.g., Upstash, Redis Cloud). + +--- + +## 11. Incident Response Runbook + +### 11.1 Severity levels + +| Level | Description | Response SLA | +|-------|------------------------------------------|-------------| +| P1 | Full service outage / payment failures | < 15 min | +| P2 | Partial outage / degraded AI/rewards | < 1 hour | +| P3 | Single feature broken, workaround exists | < 4 hours | +| P4 | Minor issue / cosmetic bug | Next sprint | + +### 11.2 Common incidents + +#### Service fails to start + +**Symptoms:** Container exits immediately; health check never responds. + +**Diagnosis:** +```bash +docker logs rustacademy-backend 2>&1 | head -50 +``` + +**Common causes:** +- Missing required env var — look for `ConfigValidationError` in logs. +- Database unreachable — look for `ECONNREFUSED` or `ETIMEDOUT` on `DATABASE_URL`. +- Port already in use — `EADDRINUSE :3000`. + +**Fix:** +- Add the missing env var to the deployment config. +- Ensure the database and Redis containers are running and reachable. +- Free the port or change `PORT`. + +--- + +#### High error rate (5xx spike) + +**Symptoms:** Monitoring shows elevated `http_requests_total{status="5xx"}`. + +**Diagnosis:** +```bash +# Application logs +docker logs rustacademy-backend 2>&1 | grep ERROR | tail -30 + +# Health breakdown +curl http://:3000/health | jq +``` + +**Common causes:** +- Database connection pool exhausted → check `DATABASE_URL` connectivity. +- Redis connection lost → check `REDIS_HOST` / `REDIS_PORT`. +- Uncaught exception in a NestJS handler. + +**Fix:** +- Restart the service to reset connection pool. +- Check infrastructure for outages. +- If the 5xx is from a specific endpoint, use feature flags or deploy a patch. + +--- + +#### Notification provider circuit open + +**Symptoms:** Notifications are silently failing; circuit metrics show `OPEN`. + +**Diagnosis:** +```bash +curl http://:3000/health | jq '.providers' +# Look for: { "providerId": "email", "healthy": false } +``` + +**Fix:** +- Check the external provider (SendGrid, FCM, etc.) status page. +- The circuit will self-recover after 30 seconds of recovery timeout. +- To force reset without a redeploy, restart the container (circuit resets on startup). + +--- + +#### AI provider errors (429 / 5xx) + +**Symptoms:** AI Mentor responses fail; learners see error messages. + +**Diagnosis:** +```bash +docker logs rustacademy-backend 2>&1 | grep "AI" | tail -20 +``` + +**Fix:** +- Check `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` are valid. +- Check the AI provider's status page for outages. +- Set `AI_PROVIDER=mock` temporarily to fall back to offline responses. +- Adjust `AI_RETRY_MAX_ATTEMPTS` and `AI_RETRY_BASE_DELAY_MS` if rate-limited. + +--- + +#### Database migration failure + +**Symptoms:** Migration CLI exits with an error; service may fail to start +if a required table is missing. + +**Diagnosis:** +```bash +npx ts-node src/database/migration-cli.ts status +``` + +**Fix:** +1. Revert the failing migration script. +2. Run `rollback` to undo any partial changes. +3. Fix the migration and redeploy. +4. If data was mutated, see [Section 10.3](#103-recovering-deleted-records). + +--- + +## 12. Troubleshooting Guide + +### JWT errors / users getting logged out unexpectedly + +- Check `JWT_SECRET` matches between app instances (in multi-instance setups). +- Adjust `JWT_CLOCK_SKEW_SECONDS` if distributed clocks are drifting (`> 30 s`). +- Run `GET /health` to confirm the auth configuration is valid. + +### CORS errors in the browser + +- Check `CORS_ORIGIN` is set to the frontend origin (e.g., `https://app.rustacademy.xyz`). +- Do not use `*` in production if the frontend sends credentials. + +### Signed asset URL 403 errors + +- Verify `ASSET_SIGNING_SECRET` matches the value used when the URL was created. +- Check that the URL has not expired (`expiresAt` claim in the query string). + +### Jobs not processing + +- Check Redis is reachable: `redis-cli -h $REDIS_HOST ping` should return `PONG`. +- Check the dead-letter queue via `GET /api/v1/admin/dlq` (admin auth required). +- Inspect `JobsService` logs for error patterns. + +### Metrics not appearing + +- Prometheus metrics are exposed at `GET /metrics`. +- The `@willsoto/nestjs-prometheus` library is registered in `MonitoringModule`. +- If metrics return 404, confirm `MonitoringModule` is imported in `AppModule`. + +--- + +## 13. Health Checks & Monitoring + +### Health endpoint + +``` +GET /health +``` + +Response shape: + +```json +{ + "status": "ok", + "info": { + "database": { "status": "up" }, + "redis": { "status": "up" } + }, + "details": { ... } +} +``` + +Returns `503 Service Unavailable` if any critical dependency is down. + +### Prometheus metrics + +``` +GET /metrics +``` + +Key metrics: + +| Metric | Description | +|---------------------------------|----------------------------------------| +| `http_requests_total` | Request count by method / route / status | +| `http_request_duration_seconds` | Request latency histogram | +| `notification_delivery_total` | Notification deliveries by provider | +| `circuit_breaker_state` | Circuit state per provider (0=CLOSED, 1=OPEN) | + +### Recommended alerts + +| Condition | Severity | +|----------------------------------------------|----------| +| `http_requests_total{status="5xx"}` rate > 1% | P2 | +| Health endpoint returns non-200 | P1 | +| Circuit breaker OPEN for > 5 min | P2 | +| Database connection failures | P1 | + +--- + +## 14. Secrets Management + +> ⚠️ **Never commit secrets to the repository.** +> All `.env` files are listed in `.gitignore`. + +### Rotation procedure + +**JWT_SECRET rotation:** + +1. Generate a new secret: `openssl rand -hex 32` +2. Deploy the new secret alongside the old one (dual-secret verification period). +3. After all existing tokens expire (default: 24 h), remove the old secret. + +**ASSET_SIGNING_SECRET rotation:** + +1. Generate a new secret: `openssl rand -hex 32` +2. Deploy with the new secret. Old signed URLs will immediately become invalid. +3. Notify users with active signed URLs to regenerate them. + +**AI provider key rotation:** + +1. Generate a new key in the Anthropic/OpenAI dashboard. +2. Update `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` in the deployment config. +3. Redeploy. Zero-downtime rolling restart is safe. + +### Storage recommendations + +- Store production secrets in a dedicated secrets manager + (AWS Secrets Manager, HashiCorp Vault, Doppler, Railway Env). +- Do **not** store secrets in Git, CI logs, or Slack. +- Rotate secrets if they are accidentally exposed. diff --git a/BackendAcademy/package-lock.json b/BackendAcademy/package-lock.json index faeb5cd02..f8d9a6341 100644 --- a/BackendAcademy/package-lock.json +++ b/BackendAcademy/package-lock.json @@ -18,7 +18,6 @@ "@nestjs/typeorm": "^10.0.2", "@willsoto/nestjs-prometheus": "^6.0.0", "axios": "^1.7.9", - "@willsoto/nestjs-prometheus": "^6.1.0", "class-transformer": "^0.5.1", "class-validator": "^0.14.4", "file-type": "^19.6.0", diff --git a/BackendAcademy/package.json b/BackendAcademy/package.json index 0476cf472..256cbb43c 100644 --- a/BackendAcademy/package.json +++ b/BackendAcademy/package.json @@ -8,7 +8,9 @@ "start": "node dist/main", "start:dev": "nest start --watch", "lint": "eslint \"{src,test}/**/*.ts\"", + "typecheck": "tsc --noEmit", "test": "jest --config jest.config.ts", + "test:unit": "NODE_ENV=test jest --config jest.config.ts --runInBand --testPathPattern='\\.spec\\.ts$'", "test:ci": "NODE_ENV=test jest --config jest.config.ts --runInBand" }, "dependencies": { diff --git a/BackendAcademy/src/notifications/providers/email.provider.ts b/BackendAcademy/src/notifications/providers/email.provider.ts index 9aab61c5e..c911388ff 100644 --- a/BackendAcademy/src/notifications/providers/email.provider.ts +++ b/BackendAcademy/src/notifications/providers/email.provider.ts @@ -6,6 +6,15 @@ import { } from '../interfaces/notification-provider.interface'; import { Notification } from '../interfaces/notifications.interface'; import { sanitiseTemplateValue } from '../email.service'; +import { + CircuitBreaker, + CircuitBreakerMetrics, + resilientCall, + RetryOptions, + TimeoutError, + CircuitOpenError, + HttpStatusError, +} from './provider-resilience'; /** * Default fallback values for missing personalization fields. @@ -21,6 +30,17 @@ const FALLBACKS: Record = { rewardAmount: 'a reward', }; +/** Per-attempt timeout for email delivery calls (ms). */ +const EMAIL_TIMEOUT_MS = 5_000; + +/** Retry configuration for email delivery. */ +const EMAIL_RETRY: RetryOptions = { + maxAttempts: 3, + initialDelayMs: 300, + backoffFactor: 2, + maxDelayMs: 5_000, +}; + /** * Email delivery adapter implementing the INotificationProvider interface. * @@ -31,6 +51,11 @@ const FALLBACKS: Record = { * All user-supplied values are HTML-escaped and dangerous constructs * (script, iframe, etc.) are stripped before interpolation to prevent * XSS attacks in email content. + * + * Resilience (Issue #674): + * - Per-attempt timeout of 5 s. + * - Exponential-backoff retry (up to 3 attempts) for transient failures. + * - Circuit breaker: opens after 5 consecutive failures; probes after 30 s. */ @Injectable() export class EmailNotificationProvider implements INotificationProvider { @@ -38,6 +63,17 @@ export class EmailNotificationProvider implements INotificationProvider { readonly providerName = 'Email Notification Provider'; private readonly logger = new Logger(EmailNotificationProvider.name); + private readonly circuitBreaker = new CircuitBreaker('email', { + failureThreshold: 5, + recoveryTimeoutMs: 30_000, + halfOpenProbes: 1, + }); + + /** Expose circuit metrics for the health endpoint. */ + get circuitMetrics(): CircuitBreakerMetrics { + return this.circuitBreaker.metrics; + } + async send( notification: Notification, context: DeliveryContext, @@ -54,36 +90,17 @@ export class EmailNotificationProvider implements INotificationProvider { }; } - const subject = this.renderSubject(notification, context); - const body = this.renderBody(notification, context); - try { - // In production this would integrate with SendGrid, SES, etc. - this.logger.log( - `[EMAIL] To: ${recipientEmail} | Subject: "${subject}"`, + const result = await resilientCall( + () => this.doSend(notification, context, recipientEmail), + this.circuitBreaker, + EMAIL_TIMEOUT_MS, + EMAIL_RETRY, + `email:${context.userId}`, ); - - // Simulate email sending delay - await new Promise((resolve) => setTimeout(resolve, 200)); - - this.logger.log( - `[EMAIL] Successfully delivered to ${recipientEmail}`, - ); - - return { - success: true, - message: `Email delivered to ${recipientEmail}`, - deliveredAt: new Date(), - }; - } catch (error) { - this.logger.error( - `[EMAIL] Failed to deliver to ${recipientEmail}: ${(error as Error).message}`, - ); - return { - success: false, - message: `Delivery failed: ${(error as Error).message}`, - deliveredAt: new Date(), - }; + return result; + } catch (err) { + return this.buildErrorResult(err, context.userId, 'EMAIL'); } } @@ -103,27 +120,72 @@ export class EmailNotificationProvider implements INotificationProvider { } async healthCheck(): Promise { - // In production this would verify SMTP/API connectivity - this.logger.log('[EMAIL] Health check OK'); - return true; + const { state } = this.circuitBreaker.metrics; + const circuitOk = state !== 'OPEN'; + this.logger.log( + `[EMAIL] Health check: circuit=${state} healthy=${circuitOk}`, + ); + return circuitOk; + } + + // ── Internal delivery ───────────────────────────────────────── + + private async doSend( + notification: Notification, + context: DeliveryContext, + recipientEmail: string, + ): Promise { + const subject = this.renderSubject(notification, context); + this.logger.log( + `[EMAIL] To: ${recipientEmail} | Subject: "${subject}"`, + ); + + // In production this would integrate with SendGrid, SES, etc. + await new Promise((resolve) => setTimeout(resolve, 200)); + + this.logger.log( + `[EMAIL] Successfully delivered to ${recipientEmail}`, + ); + + return { + success: true, + message: `Email delivered to ${recipientEmail}`, + deliveredAt: new Date(), + }; + } + + // ── Error handling ──────────────────────────────────────────── + + private buildErrorResult( + err: unknown, + userId: string, + tag: string, + ): DeliveryResult { + let message: string; + if (err instanceof CircuitOpenError) { + this.logger.warn(`[${tag}] Circuit open — skipping user ${userId}: ${err.message}`); + message = `Provider unavailable (circuit open): ${err.message}`; + } else if (err instanceof TimeoutError) { + this.logger.error(`[${tag}] Timeout for user ${userId}: ${err.message}`); + message = `Delivery timed out: ${err.message}`; + } else if (err instanceof HttpStatusError) { + this.logger.error(`[${tag}] HTTP ${err.status} for user ${userId}`); + message = `HTTP error ${err.status}`; + } else { + const msg = err instanceof Error ? err.message : String(err); + this.logger.error(`[${tag}] Failed for user ${userId}: ${msg}`); + message = `Delivery failed: ${msg}`; + } + return { success: false, message, deliveredAt: new Date() }; } // ── Template rendering with fallback support (#387, Task 2) ──────── - /** - * Renders the email subject, applying personalization fields - * with fallback values for any that are missing. - */ private renderSubject( notification: Notification, context: DeliveryContext, ): string { - let subject = notification.title; - - // Replace template placeholders like {{name}}, {{courseName}}, etc. - subject = this.applyPersonalization(subject, context); - - return subject; + return this.applyPersonalization(notification.title, context); } /** @@ -179,7 +241,6 @@ export class EmailNotificationProvider implements INotificationProvider { if (value !== undefined && value !== null && value !== '') { return sanitiseTemplateValue(value); } - // Use fallback or a safe placeholder return FALLBACKS[key] || `[${key}]`; }); } diff --git a/BackendAcademy/src/notifications/providers/in-app.provider.ts b/BackendAcademy/src/notifications/providers/in-app.provider.ts index e5b2f9e4b..1b381f4d5 100644 --- a/BackendAcademy/src/notifications/providers/in-app.provider.ts +++ b/BackendAcademy/src/notifications/providers/in-app.provider.ts @@ -5,12 +5,37 @@ import { DeliveryContext, } from '../interfaces/notification-provider.interface'; import { Notification } from '../interfaces/notifications.interface'; +import { + CircuitBreaker, + CircuitBreakerMetrics, + resilientCall, + RetryOptions, + TimeoutError, + CircuitOpenError, + HttpStatusError, +} from './provider-resilience'; + +/** Per-attempt timeout for in-app delivery calls (ms). */ +const IN_APP_TIMEOUT_MS = 2_000; + +/** Retry configuration for in-app delivery. */ +const IN_APP_RETRY: RetryOptions = { + maxAttempts: 2, + initialDelayMs: 100, + backoffFactor: 2, + maxDelayMs: 1_000, +}; /** * In-app notification delivery adapter. * * Handles storing and displaying notifications inside the application. * These notifications are persisted and displayed in the user's inbox. + * + * Resilience (Issue #674): + * - Per-attempt timeout of 2 s. + * - Exponential-backoff retry (up to 2 attempts) for transient failures. + * - Circuit breaker: opens after 5 consecutive failures; probes after 30 s. */ @Injectable() export class InAppNotificationProvider implements INotificationProvider { @@ -18,31 +43,31 @@ export class InAppNotificationProvider implements INotificationProvider { readonly providerName = 'In-App Notification Provider'; private readonly logger = new Logger(InAppNotificationProvider.name); + private readonly circuitBreaker = new CircuitBreaker('in-app', { + failureThreshold: 5, + recoveryTimeoutMs: 30_000, + halfOpenProbes: 1, + }); + + /** Expose circuit metrics for the health endpoint. */ + get circuitMetrics(): CircuitBreakerMetrics { + return this.circuitBreaker.metrics; + } + async send( notification: Notification, context: DeliveryContext, ): Promise { try { - // In-app notifications are stored in the notification store - // and rendered in the user's notification feed. - this.logger.log( - `[IN-APP] Stored notification for user: ${context.userId}`, - ); - - return { - success: true, - message: `In-app notification stored for user ${context.userId}`, - deliveredAt: new Date(), - }; - } catch (error) { - this.logger.error( - `[IN-APP] Failed for user ${context.userId}: ${(error as Error).message}`, + return await resilientCall( + () => this.doSend(notification, context), + this.circuitBreaker, + IN_APP_TIMEOUT_MS, + IN_APP_RETRY, + `in-app:${context.userId}`, ); - return { - success: false, - message: `In-app delivery failed: ${(error as Error).message}`, - deliveredAt: new Date(), - }; + } catch (err) { + return this.buildErrorResult(err, context.userId); } } @@ -62,7 +87,54 @@ export class InAppNotificationProvider implements INotificationProvider { } async healthCheck(): Promise { - this.logger.log('[IN-APP] Health check OK'); - return true; + const { state } = this.circuitBreaker.metrics; + const circuitOk = state !== 'OPEN'; + this.logger.log( + `[IN-APP] Health check: circuit=${state} healthy=${circuitOk}`, + ); + return circuitOk; + } + + // ── Internal delivery ───────────────────────────────────────── + + private async doSend( + _notification: Notification, + context: DeliveryContext, + ): Promise { + // In-app notifications are stored in the notification store + // and rendered in the user's notification feed. + this.logger.log( + `[IN-APP] Stored notification for user: ${context.userId}`, + ); + + return { + success: true, + message: `In-app notification stored for user ${context.userId}`, + deliveredAt: new Date(), + }; + } + + // ── Error handling ──────────────────────────────────────────── + + private buildErrorResult( + err: unknown, + userId: string, + ): DeliveryResult { + let message: string; + if (err instanceof CircuitOpenError) { + this.logger.warn(`[IN-APP] Circuit open — skipping user ${userId}: ${err.message}`); + message = `Provider unavailable (circuit open): ${err.message}`; + } else if (err instanceof TimeoutError) { + this.logger.error(`[IN-APP] Timeout for user ${userId}: ${err.message}`); + message = `Delivery timed out: ${err.message}`; + } else if (err instanceof HttpStatusError) { + this.logger.error(`[IN-APP] HTTP ${err.status} for user ${userId}`); + message = `HTTP error ${err.status}`; + } else { + const msg = err instanceof Error ? err.message : String(err); + this.logger.error(`[IN-APP] Failed for user ${userId}: ${msg}`); + message = `In-app delivery failed: ${msg}`; + } + return { success: false, message, deliveredAt: new Date() }; } } diff --git a/BackendAcademy/src/notifications/providers/provider-resilience.spec.ts b/BackendAcademy/src/notifications/providers/provider-resilience.spec.ts new file mode 100644 index 000000000..753d9d18e --- /dev/null +++ b/BackendAcademy/src/notifications/providers/provider-resilience.spec.ts @@ -0,0 +1,286 @@ +/** + * Tests for provider resilience utilities — Issue #674 + * + * Covers timeout, retry classification, exponential-backoff retry, + * circuit-breaker state machine, and the combined `resilientCall` helper. + */ +import { + withTimeout, + TimeoutError, + isRetryable, + HttpStatusError, + withRetry, + CircuitBreaker, + CircuitOpenError, + resilientCall, +} from './provider-resilience'; + +// ── helpers ────────────────────────────────────────────────────────────────── + +/** Resolves after `ms` milliseconds. */ +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +// ── withTimeout ─────────────────────────────────────────────────────────────── + +describe('withTimeout', () => { + it('resolves when the promise completes in time', async () => { + const result = await withTimeout(() => Promise.resolve(42), 500); + expect(result).toBe(42); + }); + + it('throws TimeoutError when the promise exceeds the limit', async () => { + await expect( + withTimeout(() => delay(200).then(() => 'late'), 50, 'test-op'), + ).rejects.toThrow(TimeoutError); + }); + + it('includes the label in the TimeoutError message', async () => { + await expect( + withTimeout(() => delay(200), 10, 'my-label'), + ).rejects.toThrow(/my-label timed out/); + }); + + it('re-throws non-timeout errors from the inner promise', async () => { + await expect( + withTimeout(() => Promise.reject(new Error('inner')), 500), + ).rejects.toThrow('inner'); + }); +}); + +// ── isRetryable ─────────────────────────────────────────────────────────────── + +describe('isRetryable', () => { + it('returns false for TimeoutError', () => { + expect(isRetryable(new TimeoutError('timed out'))).toBe(false); + }); + + it('returns false for 400 Bad Request', () => { + expect(isRetryable(new HttpStatusError(400))).toBe(false); + }); + + it('returns false for 404 Not Found', () => { + expect(isRetryable(new HttpStatusError(404))).toBe(false); + }); + + it('returns true for 429 Too Many Requests', () => { + expect(isRetryable(new HttpStatusError(429))).toBe(true); + }); + + it('returns true for 500 Internal Server Error', () => { + expect(isRetryable(new HttpStatusError(500))).toBe(true); + }); + + it('returns true for generic network Error', () => { + expect(isRetryable(new Error('ECONNRESET'))).toBe(true); + }); +}); + +// ── withRetry ───────────────────────────────────────────────────────────────── + +describe('withRetry', () => { + it('succeeds on the first attempt without retrying', async () => { + const fn = jest.fn().mockResolvedValue('ok'); + const result = await withRetry(fn, { maxAttempts: 3 }); + expect(result).toBe('ok'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('retries on transient error and succeeds on second attempt', async () => { + const fn = jest + .fn() + .mockRejectedValueOnce(new Error('network blip')) + .mockResolvedValue('ok'); + + const result = await withRetry(fn, { + maxAttempts: 3, + initialDelayMs: 0, + }); + expect(result).toBe('ok'); + expect(fn).toHaveBeenCalledTimes(2); + }); + + it('re-throws after maxAttempts transient failures', async () => { + const err = new Error('persistent failure'); + const fn = jest.fn().mockRejectedValue(err); + + await expect( + withRetry(fn, { maxAttempts: 3, initialDelayMs: 0 }), + ).rejects.toThrow('persistent failure'); + expect(fn).toHaveBeenCalledTimes(3); + }); + + it('does NOT retry non-retryable errors (e.g. 404)', async () => { + const err = new HttpStatusError(404); + const fn = jest.fn().mockRejectedValue(err); + + await expect( + withRetry(fn, { maxAttempts: 3, initialDelayMs: 0 }), + ).rejects.toBeInstanceOf(HttpStatusError); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('does NOT retry TimeoutError', async () => { + const err = new TimeoutError('timed out'); + const fn = jest.fn().mockRejectedValue(err); + + await expect( + withRetry(fn, { maxAttempts: 3, initialDelayMs: 0 }), + ).rejects.toBeInstanceOf(TimeoutError); + expect(fn).toHaveBeenCalledTimes(1); + }); +}); + +// ── CircuitBreaker ──────────────────────────────────────────────────────────── + +describe('CircuitBreaker', () => { + let cb: CircuitBreaker; + + beforeEach(() => { + cb = new CircuitBreaker('test', { + failureThreshold: 3, + recoveryTimeoutMs: 100, // short for tests + halfOpenProbes: 1, + }); + }); + + it('starts in CLOSED state', () => { + expect(cb.metrics.state).toBe('CLOSED'); + expect(cb.isOpen).toBe(false); + }); + + it('stays CLOSED after fewer failures than the threshold', async () => { + const fail = jest.fn().mockRejectedValue(new Error('err')); + for (let i = 0; i < 2; i++) { + await cb.execute(fail).catch(() => undefined); + } + expect(cb.metrics.state).toBe('CLOSED'); + }); + + it('opens after reaching the failure threshold', async () => { + const fail = jest.fn().mockRejectedValue(new Error('err')); + for (let i = 0; i < 3; i++) { + await cb.execute(fail).catch(() => undefined); + } + expect(cb.metrics.state).toBe('OPEN'); + expect(cb.isOpen).toBe(true); + }); + + it('rejects immediately with CircuitOpenError when OPEN', async () => { + const fail = jest.fn().mockRejectedValue(new Error('err')); + for (let i = 0; i < 3; i++) { + await cb.execute(fail).catch(() => undefined); + } + + const directCall = jest.fn().mockResolvedValue('should not run'); + await expect(cb.execute(directCall)).rejects.toBeInstanceOf(CircuitOpenError); + expect(directCall).not.toHaveBeenCalled(); + }); + + it('transitions to HALF_OPEN after the recovery timeout', async () => { + const fail = jest.fn().mockRejectedValue(new Error('err')); + for (let i = 0; i < 3; i++) { + await cb.execute(fail).catch(() => undefined); + } + expect(cb.metrics.state).toBe('OPEN'); + + // Wait for recovery timeout + await delay(150); + + // Execute a probe — this transitions to HALF_OPEN then lets the probe through + const success = jest.fn().mockResolvedValue('probe ok'); + await cb.execute(success); + expect(cb.metrics.state).toBe('CLOSED'); + }); + + it('closes after a successful probe in HALF_OPEN', async () => { + const fail = jest.fn().mockRejectedValue(new Error('err')); + for (let i = 0; i < 3; i++) { + await cb.execute(fail).catch(() => undefined); + } + await delay(150); + + await cb.execute(jest.fn().mockResolvedValue('ok')); + expect(cb.metrics.state).toBe('CLOSED'); + expect(cb.metrics.failures).toBe(0); + }); + + it('re-opens when the HALF_OPEN probe fails', async () => { + const fail = jest.fn().mockRejectedValue(new Error('err')); + for (let i = 0; i < 3; i++) { + await cb.execute(fail).catch(() => undefined); + } + await delay(150); + + await cb.execute(fail).catch(() => undefined); + expect(cb.metrics.state).toBe('OPEN'); + }); + + it('records success and failure counts', async () => { + const ok = jest.fn().mockResolvedValue(1); + const bad = jest.fn().mockRejectedValue(new Error('err')); + + await cb.execute(ok); + await cb.execute(ok); + await cb.execute(bad).catch(() => undefined); + + expect(cb.metrics.successes).toBe(2); + expect(cb.metrics.failures).toBe(1); + }); + + it('reset() returns the circuit to CLOSED with zeroed counters', async () => { + const fail = jest.fn().mockRejectedValue(new Error('err')); + for (let i = 0; i < 3; i++) { + await cb.execute(fail).catch(() => undefined); + } + expect(cb.metrics.state).toBe('OPEN'); + + cb.reset(); + expect(cb.metrics.state).toBe('CLOSED'); + expect(cb.metrics.failures).toBe(0); + expect(cb.metrics.successes).toBe(0); + }); +}); + +// ── resilientCall ───────────────────────────────────────────────────────────── + +describe('resilientCall', () => { + it('executes and returns the result when everything is healthy', async () => { + const cb = new CircuitBreaker('r-test', { failureThreshold: 5 }); + const fn = jest.fn().mockResolvedValue('result'); + + const value = await resilientCall(fn, cb, 1_000, {}, 'test'); + expect(value).toBe('result'); + }); + + it('retries on transient errors then succeeds', async () => { + const cb = new CircuitBreaker('r-test', { failureThreshold: 10 }); + const fn = jest + .fn() + .mockRejectedValueOnce(new HttpStatusError(503)) + .mockResolvedValue('recovered'); + + const result = await resilientCall(fn, cb, 1_000, { initialDelayMs: 0 }); + expect(result).toBe('recovered'); + expect(fn).toHaveBeenCalledTimes(2); + }); + + it('throws CircuitOpenError when the circuit is open', async () => { + const cb = new CircuitBreaker('r-test', { + failureThreshold: 1, + recoveryTimeoutMs: 60_000, + }); + const fail = jest.fn().mockRejectedValue(new Error('err')); + + // Trip the circuit + await cb.execute(fail).catch(() => undefined); + expect(cb.metrics.state).toBe('OPEN'); + + const fn = jest.fn().mockResolvedValue('should not run'); + await expect( + resilientCall(fn, cb, 1_000, { maxAttempts: 1 }), + ).rejects.toBeInstanceOf(CircuitOpenError); + expect(fn).not.toHaveBeenCalled(); + }); +}); diff --git a/BackendAcademy/src/notifications/providers/provider-resilience.ts b/BackendAcademy/src/notifications/providers/provider-resilience.ts new file mode 100644 index 000000000..a28f8d7d2 --- /dev/null +++ b/BackendAcademy/src/notifications/providers/provider-resilience.ts @@ -0,0 +1,332 @@ +/** + * Provider Resilience Utilities — Issue #674 + * + * Provides timeout wrapping, exponential-backoff retry, and a simple + * circuit-breaker for all notification providers so that a slow or + * failing external service cannot block batches or exhaust worker threads. + */ + +// ── Timeout ────────────────────────────────────────────────────────────────── + +/** + * Wraps a promise in a hard-timeout race. + * + * @param fn - A factory that returns the promise to execute. + * @param ms - Timeout in milliseconds. + * @param label - Descriptive label used in the rejection message. + * @returns The resolved value of `fn`, or throws a `TimeoutError`. + */ +export async function withTimeout( + fn: () => Promise, + ms: number, + label = 'operation', +): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => reject(new TimeoutError(`${label} timed out after ${ms} ms`)), + ms, + ); + + fn().then( + (value) => { + clearTimeout(timer); + resolve(value); + }, + (err: unknown) => { + clearTimeout(timer); + reject(err); + }, + ); + }); +} + +/** Thrown when a provider call exceeds its configured time limit. */ +export class TimeoutError extends Error { + constructor(message: string) { + super(message); + this.name = 'TimeoutError'; + } +} + +// ── Retry classification ───────────────────────────────────────────────────── + +/** + * Determines whether a given error should be retried. + * + * Transient failures (network errors, 429 / 5xx HTTP status) are retried. + * Permanent failures (4xx except 429, `TimeoutError`) are NOT retried + * because retrying will not help. + */ +export function isRetryable(err: unknown): boolean { + if (err instanceof TimeoutError) return false; + if (err instanceof HttpStatusError) { + // 429 Too Many Requests is retryable (rate-limited, back off and retry) + if (err.status === 429) return true; + // 4xx (except 429) are permanent client errors — no point retrying + if (err.status >= 400 && err.status < 500) return false; + // 5xx server errors are transient + return true; + } + // Network-level errors (no response) are transient + return true; +} + +/** Thrown to carry an HTTP status code through the retry logic. */ +export class HttpStatusError extends Error { + constructor( + public readonly status: number, + message?: string, + ) { + super(message ?? `HTTP ${status}`); + this.name = 'HttpStatusError'; + } +} + +// ── Exponential back-off retry ──────────────────────────────────────────────── + +export interface RetryOptions { + /** Maximum number of attempts (first call + retries). Default: 3 */ + maxAttempts?: number; + /** Initial back-off delay in milliseconds. Default: 200 */ + initialDelayMs?: number; + /** Multiplier applied to the delay after each failure. Default: 2 */ + backoffFactor?: number; + /** Upper bound on the calculated delay. Default: 5 000 */ + maxDelayMs?: number; +} + +const RETRY_DEFAULTS: Required = { + maxAttempts: 3, + initialDelayMs: 200, + backoffFactor: 2, + maxDelayMs: 5_000, +}; + +/** + * Retries `fn` with exponential back-off when the thrown error is + * classified as transient by `isRetryable`. + * + * Non-retryable errors are re-thrown immediately. + */ +export async function withRetry( + fn: () => Promise, + options: RetryOptions = {}, +): Promise { + const { maxAttempts, initialDelayMs, backoffFactor, maxDelayMs } = { + ...RETRY_DEFAULTS, + ...options, + }; + + let delay = initialDelayMs; + + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + try { + return await fn(); + } catch (err) { + const last = attempt === maxAttempts; + if (last || !isRetryable(err)) throw err; + + await sleep(delay); + delay = Math.min(delay * backoffFactor, maxDelayMs); + } + } + + // TypeScript path — never reached + throw new Error('Retry loop exhausted without result or error'); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +// ── Circuit breaker ─────────────────────────────────────────────────────────── + +export type CircuitState = 'CLOSED' | 'OPEN' | 'HALF_OPEN'; + +export interface CircuitBreakerOptions { + /** Number of consecutive failures before opening the circuit. Default: 5 */ + failureThreshold?: number; + /** How long (ms) the circuit stays OPEN before allowing a probe. Default: 30 000 */ + recoveryTimeoutMs?: number; + /** Maximum calls permitted in HALF_OPEN before deciding. Default: 1 */ + halfOpenProbes?: number; +} + +const CB_DEFAULTS: Required = { + failureThreshold: 5, + recoveryTimeoutMs: 30_000, + halfOpenProbes: 1, +}; + +export interface CircuitBreakerMetrics { + state: CircuitState; + failures: number; + successes: number; + lastFailureAt?: Date; + openedAt?: Date; +} + +/** + * A simple three-state circuit breaker. + * + * States: + * - CLOSED — normal operation; failures are counted. + * - OPEN — calls are rejected immediately; entered when the failure + * threshold is reached. + * - HALF_OPEN — a limited probe is allowed after `recoveryTimeoutMs`; + * a success closes the circuit, any failure reopens it. + */ +export class CircuitBreaker { + private state: CircuitState = 'CLOSED'; + private failures = 0; + private successes = 0; + private halfOpenProbeCount = 0; + private openedAt?: Date; + private lastFailureAt?: Date; + + private readonly opts: Required; + + constructor( + public readonly name: string, + options: CircuitBreakerOptions = {}, + ) { + this.opts = { ...CB_DEFAULTS, ...options }; + } + + /** Whether the circuit breaker is currently preventing calls. */ + get isOpen(): boolean { + return this.state === 'OPEN' || this.state === 'HALF_OPEN'; + } + + /** Current observable metrics snapshot. */ + get metrics(): CircuitBreakerMetrics { + return { + state: this.state, + failures: this.failures, + successes: this.successes, + lastFailureAt: this.lastFailureAt, + openedAt: this.openedAt, + }; + } + + /** + * Executes `fn` through the circuit breaker. + * + * - OPEN: throws `CircuitOpenError` immediately. + * - HALF_OPEN: allows a limited probe call; records the outcome. + * - CLOSED: executes normally; transitions to OPEN on threshold breach. + */ + async execute(fn: () => Promise): Promise { + if (this.state === 'OPEN') { + const elapsed = Date.now() - (this.openedAt?.getTime() ?? 0); + if (elapsed >= this.opts.recoveryTimeoutMs) { + this.transitionTo('HALF_OPEN'); + } else { + throw new CircuitOpenError( + `Circuit "${this.name}" is OPEN (${Math.round((this.opts.recoveryTimeoutMs - elapsed) / 1000)} s until probe)`, + ); + } + } + + if ( + this.state === 'HALF_OPEN' && + this.halfOpenProbeCount >= this.opts.halfOpenProbes + ) { + throw new CircuitOpenError( + `Circuit "${this.name}" is HALF_OPEN — probe already in-flight`, + ); + } + + if (this.state === 'HALF_OPEN') { + this.halfOpenProbeCount++; + } + + try { + const result = await fn(); + this.onSuccess(); + return result; + } catch (err) { + this.onFailure(); + throw err; + } + } + + /** Resets the circuit to CLOSED (e.g., for testing). */ + reset(): void { + this.transitionTo('CLOSED'); + this.failures = 0; + this.successes = 0; + this.halfOpenProbeCount = 0; + this.openedAt = undefined; + this.lastFailureAt = undefined; + } + + private onSuccess(): void { + this.successes++; + if (this.state === 'HALF_OPEN') { + this.transitionTo('CLOSED'); + this.failures = 0; + this.halfOpenProbeCount = 0; + } + } + + private onFailure(): void { + this.failures++; + this.lastFailureAt = new Date(); + + if (this.state === 'HALF_OPEN') { + // Probe failed — reopen immediately + this.transitionTo('OPEN'); + this.halfOpenProbeCount = 0; + return; + } + + if (this.failures >= this.opts.failureThreshold) { + this.transitionTo('OPEN'); + } + } + + private transitionTo(next: CircuitState): void { + this.state = next; + if (next === 'OPEN') { + this.openedAt = new Date(); + } + } +} + +/** Thrown when a call is rejected because the circuit is OPEN. */ +export class CircuitOpenError extends Error { + constructor(message: string) { + super(message); + this.name = 'CircuitOpenError'; + } +} + +// ── Combined helper ─────────────────────────────────────────────────────────── + +/** + * Composes timeout, retry, and circuit-breaker into a single call. + * + * Execution order: + * circuit breaker → retry loop → timeout per attempt + * + * This ensures: + * - No call is made when the circuit is open. + * - Each individual attempt has a bounded duration. + * - Transient failures trigger automatic back-off retries. + * - The circuit opens after too many consecutive failures. + */ +export async function resilientCall( + fn: () => Promise, + breaker: CircuitBreaker, + timeoutMs: number, + retryOpts: RetryOptions = {}, + label = 'provider call', +): Promise { + return breaker.execute(() => + withRetry( + () => withTimeout(fn, timeoutMs, label), + retryOpts, + ), + ); +} diff --git a/BackendAcademy/src/notifications/providers/push.provider.ts b/BackendAcademy/src/notifications/providers/push.provider.ts index cade79148..b874f4253 100644 --- a/BackendAcademy/src/notifications/providers/push.provider.ts +++ b/BackendAcademy/src/notifications/providers/push.provider.ts @@ -5,11 +5,36 @@ import { DeliveryContext, } from '../interfaces/notification-provider.interface'; import { Notification } from '../interfaces/notifications.interface'; +import { + CircuitBreaker, + CircuitBreakerMetrics, + resilientCall, + RetryOptions, + TimeoutError, + CircuitOpenError, + HttpStatusError, +} from './provider-resilience'; + +/** Per-attempt timeout for push delivery calls (ms). */ +const PUSH_TIMEOUT_MS = 3_000; + +/** Retry configuration for push delivery. */ +const PUSH_RETRY: RetryOptions = { + maxAttempts: 3, + initialDelayMs: 200, + backoffFactor: 2, + maxDelayMs: 4_000, +}; /** * Push notification delivery adapter. * * Handles sending push notifications to user devices via FCM/APNs. + * + * Resilience (Issue #674): + * - Per-attempt timeout of 3 s. + * - Exponential-backoff retry (up to 3 attempts) for transient failures. + * - Circuit breaker: opens after 5 consecutive failures; probes after 30 s. */ @Injectable() export class PushNotificationProvider implements INotificationProvider { @@ -17,34 +42,31 @@ export class PushNotificationProvider implements INotificationProvider { readonly providerName = 'Push Notification Provider'; private readonly logger = new Logger(PushNotificationProvider.name); + private readonly circuitBreaker = new CircuitBreaker('push', { + failureThreshold: 5, + recoveryTimeoutMs: 30_000, + halfOpenProbes: 1, + }); + + /** Expose circuit metrics for the health endpoint. */ + get circuitMetrics(): CircuitBreakerMetrics { + return this.circuitBreaker.metrics; + } + async send( notification: Notification, context: DeliveryContext, ): Promise { try { - // In production this would integrate with Firebase Cloud Messaging - // or Apple Push Notification service. - this.logger.log( - `[PUSH] To user: ${context.userId} | Title: "${notification.title}"`, + return await resilientCall( + () => this.doSend(notification, context), + this.circuitBreaker, + PUSH_TIMEOUT_MS, + PUSH_RETRY, + `push:${context.userId}`, ); - - // Simulate push delivery - await new Promise((resolve) => setTimeout(resolve, 100)); - - return { - success: true, - message: `Push delivered to user ${context.userId}`, - deliveredAt: new Date(), - }; - } catch (error) { - this.logger.error( - `[PUSH] Failed for user ${context.userId}: ${(error as Error).message}`, - ); - return { - success: false, - message: `Push delivery failed: ${(error as Error).message}`, - deliveredAt: new Date(), - }; + } catch (err) { + return this.buildErrorResult(err, context.userId); } } @@ -64,7 +86,57 @@ export class PushNotificationProvider implements INotificationProvider { } async healthCheck(): Promise { - this.logger.log('[PUSH] Health check OK'); - return true; + const { state } = this.circuitBreaker.metrics; + const circuitOk = state !== 'OPEN'; + this.logger.log( + `[PUSH] Health check: circuit=${state} healthy=${circuitOk}`, + ); + return circuitOk; + } + + // ── Internal delivery ───────────────────────────────────────── + + private async doSend( + notification: Notification, + context: DeliveryContext, + ): Promise { + // In production this would integrate with Firebase Cloud Messaging + // or Apple Push Notification service. + this.logger.log( + `[PUSH] To user: ${context.userId} | Title: "${notification.title}"`, + ); + + // Simulate push delivery + await new Promise((resolve) => setTimeout(resolve, 100)); + + return { + success: true, + message: `Push delivered to user ${context.userId}`, + deliveredAt: new Date(), + }; + } + + // ── Error handling ──────────────────────────────────────────── + + private buildErrorResult( + err: unknown, + userId: string, + ): DeliveryResult { + let message: string; + if (err instanceof CircuitOpenError) { + this.logger.warn(`[PUSH] Circuit open — skipping user ${userId}: ${err.message}`); + message = `Provider unavailable (circuit open): ${err.message}`; + } else if (err instanceof TimeoutError) { + this.logger.error(`[PUSH] Timeout for user ${userId}: ${err.message}`); + message = `Delivery timed out: ${err.message}`; + } else if (err instanceof HttpStatusError) { + this.logger.error(`[PUSH] HTTP ${err.status} for user ${userId}`); + message = `HTTP error ${err.status}`; + } else { + const msg = err instanceof Error ? err.message : String(err); + this.logger.error(`[PUSH] Failed for user ${userId}: ${msg}`); + message = `Push delivery failed: ${msg}`; + } + return { success: false, message, deliveredAt: new Date() }; } } diff --git a/BackendAcademy/src/social/social.controller.spec.ts b/BackendAcademy/src/social/social.controller.spec.ts index 1bf7d2ce7..2be07a5fd 100644 --- a/BackendAcademy/src/social/social.controller.spec.ts +++ b/BackendAcademy/src/social/social.controller.spec.ts @@ -1,3 +1,12 @@ +/** + * SocialController unit tests — Issue #686 + * + * Covers: + * - Feed / discovery endpoints pass requester context through to the service + * - Moderator role forwarded on moderate endpoint + * - Delete endpoint forwards requester context + */ +import { ForbiddenException } from '@nestjs/common'; import { SocialController } from './social.controller'; import { SocialService } from './social.service'; @@ -10,9 +19,11 @@ describe('SocialController', () => { controller = new SocialController(service); }); + // ── Discovery alias ───────────────────────────────────────────────────────── + it('should return the same feed from discovery alias', () => { const post = service.createPost('user-1', { content: 'Hello #rust' }); - service.moderatePost(post.id, 'moderator-1', { status: 'approved' }); + service.moderatePost(post.id, 'moderator-1', { status: 'approved' }, 'moderator'); const feed = controller.getFeed({}); const discovery = controller.getDiscovery({}); @@ -20,4 +31,58 @@ describe('SocialController', () => { expect(discovery).toEqual(feed); expect(discovery.posts[0].id).toBe(post.id); }); + + // ── Visibility via controller ─────────────────────────────────────────────── + + it('public feed (no requester) returns only approved posts', () => { + service.createPost('user-1', { content: 'Pending post' }); // pending + + const result = controller.getFeed({}); + expect(result.posts).toHaveLength(0); + }); + + it('non-moderator requesting pending feed throws ForbiddenException', () => { + expect(() => + controller.getFeed({ status: 'pending' }, 'user-1', 'user'), + ).toThrow(ForbiddenException); + }); + + it('moderator can request pending feed via controller', () => { + service.createPost('user-1', { content: 'Pending post' }); + + const result = controller.getFeed({ status: 'pending' }, 'mod-1', 'moderator'); + expect(result.posts).toHaveLength(1); + }); + + // ── Ownership via controller ──────────────────────────────────────────────── + + it('author can delete their own post via controller', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => controller.deletePost(post.id, 'user-1')).not.toThrow(); + }); + + it('non-owner cannot delete someone else\'s post via controller', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => controller.deletePost(post.id, 'user-2')).toThrow(ForbiddenException); + }); + + it('moderator can delete any post via controller', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => controller.deletePost(post.id, 'mod-1', 'moderator')).not.toThrow(); + }); + + // ── Moderate endpoint ─────────────────────────────────────────────────────── + + it('moderator can approve a post via controller', () => { + const post = service.createPost('user-1', { content: 'Test post' }); + const result = controller.moderatePost(post.id, 'mod-1', 'moderator', { status: 'approved' }); + expect(result.moderationStatus).toBe('approved'); + }); + + it('non-moderator cannot approve a post via controller', () => { + const post = service.createPost('user-1', { content: 'Test post' }); + expect(() => + controller.moderatePost(post.id, 'user-1', 'user', { status: 'approved' }), + ).toThrow(ForbiddenException); + }); }); diff --git a/BackendAcademy/src/social/social.controller.ts b/BackendAcademy/src/social/social.controller.ts index 1d75dc4d3..6cab2b75a 100644 --- a/BackendAcademy/src/social/social.controller.ts +++ b/BackendAcademy/src/social/social.controller.ts @@ -40,13 +40,21 @@ export class SocialController { } @Get('feed') - getFeed(@Query() dto: GetSocialFeedDto): SocialFeedResponse { - return this.socialService.getFeed(dto); + getFeed( + @Query() dto: GetSocialFeedDto, + @Query('requesterId') requesterId?: string, + @Query('requesterRole') requesterRole?: string, + ): SocialFeedResponse { + return this.socialService.getFeed(dto, requesterId, requesterRole); } @Get('discovery') - getDiscovery(@Query() dto: GetSocialFeedDto): SocialFeedResponse { - return this.socialService.getFeed(dto); + getDiscovery( + @Query() dto: GetSocialFeedDto, + @Query('requesterId') requesterId?: string, + @Query('requesterRole') requesterRole?: string, + ): SocialFeedResponse { + return this.socialService.getFeed(dto, requesterId, requesterRole); } @Get('posts/:postId') @@ -59,9 +67,10 @@ export class SocialController { moderatePost( @Param('postId') postId: string, @Query('moderatorId') moderatorId: string, + @Query('moderatorRole') moderatorRole: string, @Body() dto: UpdateModerationDto, ): SocialPost { - return this.socialService.moderatePost(postId, moderatorId, dto); + return this.socialService.moderatePost(postId, moderatorId, dto, moderatorRole); } @Post('posts/:postId/flag') @@ -96,8 +105,12 @@ export class SocialController { @Delete('posts/:postId') @HttpCode(HttpStatus.NO_CONTENT) - deletePost(@Param('postId') postId: string): void { - this.socialService.deletePost(postId); + deletePost( + @Param('postId') postId: string, + @Query('requesterId') requesterId?: string, + @Query('requesterRole') requesterRole?: string, + ): void { + this.socialService.deletePost(postId, requesterId, requesterRole); } @Post('users/:userId/follow/:targetUserId') @@ -178,9 +191,9 @@ export class SocialController { @HttpCode(HttpStatus.OK) getPostsByHashtag( @Param('tag') tag: string, - @Query('page') page = 1, + @Query('cursor') cursor?: string, @Query('limit') limit = 10, ): SocialFeedResponse { - return this.socialService.getPostsByHashtag(tag, Number(page), Number(limit)); + return this.socialService.getPostsByHashtag(tag, cursor, Number(limit)); } } \ No newline at end of file diff --git a/BackendAcademy/src/social/social.service.spec.ts b/BackendAcademy/src/social/social.service.spec.ts index ffd33ae24..8453700a4 100644 --- a/BackendAcademy/src/social/social.service.spec.ts +++ b/BackendAcademy/src/social/social.service.spec.ts @@ -1,5 +1,14 @@ +/** + * SocialService unit tests — Issue #686 + * + * Covers: + * - Moderation-status visibility (pending/flagged posts do NOT leak into public feeds) + * - Post ownership enforcement (only the author or a moderator may delete) + * - Moderator role gates on moderate action + * - Existing feed / follow / hashtag functionality + */ import { Test, TestingModule } from '@nestjs/testing'; -import { BadRequestException, NotFoundException } from '@nestjs/common'; +import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common'; import { SocialService } from './social.service'; import { CreateSocialPostDto } from './dto/create-social-post.dto'; import { UpdateModerationDto } from './dto/update-moderation.dto'; @@ -15,33 +24,27 @@ describe('SocialService', () => { service = module.get(SocialService); }); + // ── Basic feed ────────────────────────────────────────────────────────────── + it('should return only approved posts by default', () => { const firstPost = service.createPost('user-1', { content: 'First post' }); - const secondPost = service.createPost('user-2', { content: 'Second post' }); + service.createPost('user-2', { content: 'Second post' }); - service.moderatePost(firstPost.id, 'moderator-1', { - status: 'approved', - }); + service.moderatePost(firstPost.id, 'moderator-1', { status: 'approved' }, 'moderator'); const result = service.getFeed({}); expect(result.posts.length).toBe(1); expect(result.posts[0].id).toBe(firstPost.id); expect(result.total).toBe(1); - expect(result.page).toBe(1); - expect(result.limit).toBe(10); }); it('should support search filtering', () => { const firstPost = service.createPost('user-1', { content: 'Learning Rust is fun' }); const secondPost = service.createPost('user-2', { content: 'Another post' }); - service.moderatePost(firstPost.id, 'moderator-1', { - status: 'approved', - }); - service.moderatePost(secondPost.id, 'moderator-1', { - status: 'approved', - }); + service.moderatePost(firstPost.id, 'mod', { status: 'approved' }, 'moderator'); + service.moderatePost(secondPost.id, 'mod', { status: 'approved' }, 'moderator'); const result = service.getFeed({ search: 'rust' }); @@ -53,8 +56,8 @@ describe('SocialService', () => { const firstPost = service.createPost('user-1', { content: 'First post' }); const secondPost = service.createPost('user-2', { content: 'Second post' }); - service.moderatePost(firstPost.id, 'moderator-1', { status: 'approved' }); - service.moderatePost(secondPost.id, 'moderator-1', { status: 'approved' }); + service.moderatePost(firstPost.id, 'mod', { status: 'approved' }, 'moderator'); + service.moderatePost(secondPost.id, 'mod', { status: 'approved' }, 'moderator'); const result = service.getFeed({ userId: 'user-2' }); @@ -66,8 +69,8 @@ describe('SocialService', () => { const firstPost = service.createPost('user-1', { content: 'Welcome to #rust' }); const secondPost = service.createPost('user-2', { content: 'No hashtag here' }); - service.moderatePost(firstPost.id, 'moderator-1', { status: 'approved' }); - service.moderatePost(secondPost.id, 'moderator-1', { status: 'approved' }); + service.moderatePost(firstPost.id, 'mod', { status: 'approved' }, 'moderator'); + service.moderatePost(secondPost.id, 'mod', { status: 'approved' }, 'moderator'); const result = service.getFeed({ tag: 'rust' }); @@ -83,6 +86,144 @@ describe('SocialService', () => { expect(() => service.getFeed({ status: 'invalid' as any })).toThrow(BadRequestException); }); + // ── Visibility enforcement — Issue #686 ──────────────────────────────────── + + describe('Visibility enforcement', () => { + it('pending posts do NOT appear in the default public feed', () => { + service.createPost('user-1', { content: 'Pending post' }); + const result = service.getFeed({}); + expect(result.posts).toHaveLength(0); + }); + + it('flagged posts do NOT appear in the default public feed', () => { + const post = service.createPost('user-1', { content: 'Flagged post' }); + service.moderatePost(post.id, 'mod', { status: 'approved' }, 'moderator'); + service.flagPost(post.id, 'reporter'); + const result = service.getFeed({}); + expect(result.posts).toHaveLength(0); + }); + + it('rejected posts do NOT appear in the default public feed', () => { + const post = service.createPost('user-1', { content: 'Rejected post' }); + service.moderatePost(post.id, 'mod', { status: 'rejected' }, 'moderator'); + const result = service.getFeed({}); + expect(result.posts).toHaveLength(0); + }); + + it('non-moderator cannot request the pending feed', () => { + expect(() => + service.getFeed({ status: 'pending' }, 'user-1', 'user'), + ).toThrow(ForbiddenException); + }); + + it('non-moderator cannot request the flagged feed', () => { + expect(() => + service.getFeed({ status: 'flagged' }, 'user-1', 'user'), + ).toThrow(ForbiddenException); + }); + + it('moderator can request the pending feed', () => { + service.createPost('user-1', { content: 'Pending content' }); + const result = service.getFeed({ status: 'pending' }, 'mod-1', 'moderator'); + expect(result.posts).toHaveLength(1); + }); + + it('admin can request the pending feed', () => { + service.createPost('user-1', { content: 'Pending content' }); + const result = service.getFeed({ status: 'pending' }, 'admin-1', 'admin'); + expect(result.posts).toHaveLength(1); + }); + + it('moderator can request the flagged feed', () => { + const post = service.createPost('user-1', { content: 'Test' }); + service.moderatePost(post.id, 'mod', { status: 'approved' }, 'moderator'); + service.flagPost(post.id, 'reporter'); + + const result = service.getFeed({ status: 'flagged' }, 'mod-1', 'moderator'); + expect(result.posts).toHaveLength(1); + }); + + it('getFeed without status or role always returns only approved posts', () => { + const post = service.createPost('user-1', { content: 'Approved post' }); + service.createPost('user-2', { content: 'Pending post' }); + + service.moderatePost(post.id, 'mod', { status: 'approved' }, 'moderator'); + + const result = service.getFeed({}); + expect(result.posts).toHaveLength(1); + expect(result.posts[0].id).toBe(post.id); + }); + }); + + // ── Post ownership enforcement — Issue #686 ──────────────────────────────── + + describe('Post ownership enforcement', () => { + it('author can delete their own post', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => service.deletePost(post.id, 'user-1')).not.toThrow(); + expect(() => service.getPostById(post.id)).toThrow(NotFoundException); + }); + + it('another user cannot delete a post they do not own', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => service.deletePost(post.id, 'user-2')).toThrow(ForbiddenException); + }); + + it('moderator can delete any post regardless of ownership', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => service.deletePost(post.id, 'mod-1', 'moderator')).not.toThrow(); + }); + + it('admin can delete any post regardless of ownership', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => service.deletePost(post.id, 'admin-1', 'admin')).not.toThrow(); + }); + + it('deletePost without requesterId deletes unconditionally (legacy / internal)', () => { + const post = service.createPost('user-1', { content: 'My post' }); + expect(() => service.deletePost(post.id)).not.toThrow(); + }); + }); + + // ── Moderator role enforcement — Issue #686 ──────────────────────────────── + + describe('Moderator role enforcement on moderatePost', () => { + it('moderator can approve a post', () => { + const post = service.createPost('user-1', { content: 'Test post' }); + const moderated = service.moderatePost(post.id, 'mod-1', { status: 'approved' }, 'moderator'); + expect(moderated.moderationStatus).toBe('approved'); + expect(moderated.moderatedBy).toBe('mod-1'); + }); + + it('admin can approve a post', () => { + const post = service.createPost('user-1', { content: 'Test post' }); + const moderated = service.moderatePost(post.id, 'admin-1', { status: 'approved' }, 'admin'); + expect(moderated.moderationStatus).toBe('approved'); + }); + + it('moderator can reject a post', () => { + const post = service.createPost('user-1', { content: 'Test post' }); + const moderated = service.moderatePost(post.id, 'mod-1', { status: 'rejected', reason: 'spam' }, 'moderator'); + expect(moderated.moderationStatus).toBe('rejected'); + expect(moderated.moderationReason).toBe('spam'); + }); + + it('non-moderator user cannot moderate a post', () => { + const post = service.createPost('user-1', { content: 'Test post' }); + expect(() => + service.moderatePost(post.id, 'user-2', { status: 'approved' }, 'user'), + ).toThrow(ForbiddenException); + }); + + it('moderatePost without role still works (legacy / internal callers)', () => { + const post = service.createPost('user-1', { content: 'Test post' }); + const moderated = service.moderatePost(post.id, 'mod-1', { status: 'approved' }); + expect(moderated.moderationStatus).toBe('approved'); + }); + }); + + // ── Follow / Unfollow ─────────────────────────────────────────────────────── + it('should follow and unfollow a user', () => { const followResponse = service.followUser('user-1', 'user-2'); @@ -106,4 +247,16 @@ describe('SocialService', () => { it('should not allow unfollow when not following', () => { expect(() => service.unfollowUser('user-1', 'user-2')).toThrow(BadRequestException); }); + + // ── Moderation queue ──────────────────────────────────────────────────────── + + it('getModerationQueue returns pending and flagged posts', () => { + const p1 = service.createPost('user-1', { content: 'Post 1' }); // pending + const p2 = service.createPost('user-2', { content: 'Post 2' }); + service.moderatePost(p2.id, 'mod', { status: 'approved' }, 'moderator'); + service.flagPost(p2.id, 'reporter'); // approved → flagged + + const queue = service.getModerationQueue(); + expect(queue.map((p) => p.id)).toEqual(expect.arrayContaining([p1.id, p2.id])); + }); }); diff --git a/BackendAcademy/src/social/social.service.ts b/BackendAcademy/src/social/social.service.ts index 1f54903c9..2031fc73a 100644 --- a/BackendAcademy/src/social/social.service.ts +++ b/BackendAcademy/src/social/social.service.ts @@ -1,4 +1,4 @@ -import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common'; +import { BadRequestException, ForbiddenException, Injectable, NotFoundException } from '@nestjs/common'; import { CreateSocialPostDto } from './dto/create-social-post.dto'; import { GetSocialFeedDto } from './dto/get-social-feed.dto'; import { UpdateModerationDto } from './dto/update-moderation.dto'; @@ -13,6 +13,9 @@ import { Hashtag, HashtagListResponse } from './interfaces/hashtag.interface'; export { ModerationStatus, SocialPost, SocialFeedResponse, FollowResponse } from './interfaces/social-post.interface'; +/** Roles that are permitted to perform moderation actions. */ +const MODERATOR_ROLES = new Set(['moderator', 'admin']); + @Injectable() export class SocialService { private readonly posts = new Map(); @@ -43,11 +46,28 @@ export class SocialService { return post; } - getFeed(dto: GetSocialFeedDto): SocialFeedResponse { + getFeed(dto: GetSocialFeedDto, requesterId?: string, requesterRole?: string): SocialFeedResponse { const { limit = 10, status, search, userId, tag, cursor } = dto; - const normalizedStatus = status - ? this.normalizeStatus(status) - : 'approved'; + + // ── Visibility enforcement (Issue #686) ────────────────────────────── + // Determine which moderation status is being requested. + // Only moderators/admins may request non-approved statuses. + // Public callers always see only "approved" content. + const isModerator = requesterRole ? MODERATOR_ROLES.has(requesterRole) : false; + + let normalizedStatus: ModerationStatus; + if (status) { + normalizedStatus = this.normalizeStatus(status); + // Non-moderators may not request pending/flagged/rejected feeds + if (!isModerator && normalizedStatus !== 'approved') { + throw new ForbiddenException({ + error: 'FORBIDDEN', + message: 'Only moderators may access non-approved content feeds', + }); + } + } else { + normalizedStatus = 'approved'; + } let filteredPosts = Array.from(this.posts.values()).filter( (post) => post.moderationStatus === normalizedStatus, @@ -116,15 +136,30 @@ export class SocialService { return post; } + /** + * Moderates a post (approve, reject, or flag it). + * + * Role enforcement (Issue #686): only moderators and admins may perform + * moderation actions on posts. + */ moderatePost( postId: string, moderatorId: string, dto: UpdateModerationDto, + moderatorRole?: string, ): SocialPost { const normalizedPostId = this.normalizeId(postId, 'postId'); const normalizedModeratorId = this.normalizeUserId(moderatorId); const normalizedStatus = this.normalizeStatus(dto.status); + // Enforce moderator role if provided + if (moderatorRole !== undefined && !MODERATOR_ROLES.has(moderatorRole)) { + throw new ForbiddenException({ + error: 'FORBIDDEN', + message: 'Only moderators may perform moderation actions', + }); + } + const post = this.posts.get(normalizedPostId); if (!post) { @@ -144,16 +179,38 @@ export class SocialService { return post; } - deletePost(postId: string): void { + /** + * Deletes a post. + * + * Ownership enforcement (Issue #686): only the post's author or a + * moderator/admin may delete. Regular users cannot remove other users' posts. + */ + deletePost(postId: string, requesterId?: string, requesterRole?: string): void { const normalizedPostId = this.normalizeId(postId, 'postId'); - const deleted = this.posts.delete(normalizedPostId); + const post = this.posts.get(normalizedPostId); - if (!deleted) { + if (!post) { throw new NotFoundException({ error: 'POST_NOT_FOUND', message: `Post with ID ${normalizedPostId} not found`, }); } + + // If a requesterId is provided, enforce ownership unless the requester is a moderator + if (requesterId) { + const normalizedRequesterId = this.normalizeUserId(requesterId); + const isModerator = requesterRole ? MODERATOR_ROLES.has(requesterRole) : false; + const isAuthor = post.userId === normalizedRequesterId; + + if (!isAuthor && !isModerator) { + throw new ForbiddenException({ + error: 'FORBIDDEN', + message: 'You are not allowed to delete this post', + }); + } + } + + this.posts.delete(normalizedPostId); } flagPost(postId: string, userId: string): SocialPost {