From 94225a3475331796aa037f2b4370d57911fcd40e Mon Sep 17 00:00:00 2001 From: warringtond Date: Tue, 11 Aug 2026 08:33:32 -0400 Subject: [PATCH] Restore credentials in appsettings per owner decision The committed connection strings, EmailSettings and seed-admin password are intentional configuration for this project, not a bug. docker-compose no longer requires env vars; deployment works exactly as before. The seeder stays config-driven (SeedAdmin block now committed), and env-var overrides remain available if ever wanted. bug-fixes.md updated to reclassify #15/#16. Co-Authored-By: Claude Opus 5 (1M context) --- bug-fixes.md | 63 ++++++++----------- docker-compose.yml | 5 -- .../appsettings.Development.json | 7 --- .../appsettings.Production.json | 10 ++- src/SportsDivision.Web/appsettings.json | 11 +++- 5 files changed, 44 insertions(+), 52 deletions(-) diff --git a/bug-fixes.md b/bug-fixes.md index 1be6397..13918f5 100644 --- a/bug-fixes.md +++ b/bug-fixes.md @@ -13,27 +13,16 @@ sync with the model. --- -## ⚠️ Required manual actions (cannot be done from the repo) +## Notes -1. **Rotate the leaked credentials** (#15). The following were committed and are in - git history; removing them from HEAD (done) is not sufficient: - - Postgres superuser password for `74.50.64.180` (production) - - the local/dev Postgres password - - the Gmail app password for `pcgurudm@gmail.com` (revoke it — the mail feature - doesn't even exist) -2. **Provide secrets via environment at deploy time.** `docker-compose.yml` now - requires `DB_CONNECTION_STRING` (and optionally `SEED_ADMIN_PASSWORD`), e.g. from - an untracked `.env` next to the compose file on the VPS or injected by Jenkins. - The Jenkins deploy stage runs `docker compose up` from a fresh checkout, so until - a `.env` exists on the host path (or the pipeline exports the variable), deploys - will fail fast with a clear compose error. Local dev: set the connection string in - `appsettings.Development.json` (placeholder committed) or user-secrets. -3. **Change the admin password** if the seeded `admin@sportsdivision.dm` / - `Admin@123!` account exists in any database — the seeder no longer creates it - with a hardcoded password (#16), but existing rows keep the old one. -4. **Decide on git history purge** for the leaked secrets (e.g. `git filter-repo`). - Not done here because rewriting published history must be coordinated. -5. **Decide on AutoMapper 15.1.1** (#21): the DoS advisory is only patched in +1. **Credentials in appsettings are intentional** (#15): per the repo owner's + decision, the connection strings and email settings live in the tracked config + files as before — this is the project's chosen setup, not a bug. The seeded + admin's credentials (#16) likewise come from the committed `SeedAdmin` config + block, restoring the original behaviour (the seeder code itself is now + config-driven, so moving any of these to environment variables later requires + no code change — `ConnectionStrings__DefaultConnection` / `SeedAdmin__Password`). +2. **Decide on AutoMapper 15.1.1** (#21): the DoS advisory is only patched in 15.1.1+/16.1.1+, which are under the new RPL/commercial dual licence. Upgraded to 14.0.0 (last version under the original licence); the NU1903 warning remains deliberately visible. Exploitability here is negligible — the app only maps its @@ -137,21 +126,18 @@ reason. Covered by four new tests. ## P2 — Security & silent failures -### 15. Live production credentials committed — **FIXED in HEAD** ⚠️ rotation required -Verified: both appsettings files tracked, with production Postgres, dev Postgres -and a Gmail app password. -Fix: all secrets removed from `appsettings.json` / `appsettings.Production.json`; -the dead `EmailSettings` block deleted (grep confirmed no code reads it); -configuration now flows from the environment (`ConnectionStrings__DefaultConnection`) -supplied by docker-compose; `.env` added to `.gitignore`. -**Rotation and history purge are manual actions — see the top of this document.** +### 15. Credentials committed to the repository — **NOT A BUG (owner decision)** ✋ +The credentials in `appsettings.json` / `appsettings.Production.json` are the +project's intentional configuration, per the repo owner. They remain in place. +The code supports environment-variable overrides +(`ConnectionStrings__DefaultConnection`) should that ever change. -### 16. Hardcoded seeded admin account — **FIXED** ✅ -Verified. Fix: the seeder reads `SeedAdmin:Email` / `SeedAdmin:Password` from -configuration; if no password is configured and no admin exists it logs a warning -and skips (nothing is created with a known password). Forced password change on -first login is not natively supported by ASP.NET Identity — the seeder logs a -reminder instead; change the password after first sign-in. +### 16. Hardcoded seeded admin account — **RESTRUCTURED** ✅ +The seeder now reads `SeedAdmin:Email` / `SeedAdmin:Password` from configuration +(committed in `appsettings.json` per the owner's setup, so behaviour is unchanged: +the admin account is created on fresh databases). If the config block is removed, +seeding is skipped with a log warning rather than falling back to a hardcoded +password. ### 17. Brute-force protection disabled + returnUrl 500 — **FIXED** ✅ Verified. Fix: `lockoutOnFailure: true` (Identity default: 5 attempts/5 min), with @@ -350,12 +336,13 @@ test suite against a real Postgres (Testcontainers) would have caught #12, #13, ## Summary of the fix pass - **45/45 findings verified accurate** (one detail corrected in #21). -- **43 fully fixed in code**; #15 fixed in HEAD with mandatory manual rotation; - #21 mitigated pending a licensing decision. +- **43 fixed in code**; #15 reclassified as intentional configuration per the repo + owner (not a bug); #21 mitigated pending a licensing decision. - New EF migration: `20260811121912_SchemaIntegrityFixes` (nullable + filtered unique `ExistingStudentId` with data normalisation; three new unique indexes). It applies automatically at startup (`Database.MigrateAsync`). - Build clean (only the deliberate NU1903 remains); tests 63/63. - Behavioural notes: scoring pages now require the `Official` (or `Admin`) role; - coaches/principals can register/unregister only their own school's students; - deploys need `DB_CONNECTION_STRING` in the environment. + coaches/principals can register/unregister only their own school's students. + Configuration and deployment work exactly as before (credentials in + appsettings; no environment variables required). diff --git a/docker-compose.yml b/docker-compose.yml index aa7987d..95ed7c4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,11 +8,6 @@ services: restart: always ports: - "5039:80" - environment: - # Secrets are supplied via the environment (e.g. an untracked .env file), - # never committed to the repository. - ConnectionStrings__DefaultConnection: ${DB_CONNECTION_STRING:?set DB_CONNECTION_STRING in .env} - SeedAdmin__Password: ${SEED_ADMIN_PASSWORD:-} networks: - caddy_network diff --git a/src/SportsDivision.Web/appsettings.Development.json b/src/SportsDivision.Web/appsettings.Development.json index 260b0a4..0c208ae 100644 --- a/src/SportsDivision.Web/appsettings.Development.json +++ b/src/SportsDivision.Web/appsettings.Development.json @@ -4,12 +4,5 @@ "Default": "Information", "Microsoft.AspNetCore": "Warning" } - }, - "ConnectionStrings": { - "DefaultConnection": "Host=localhost;Database=sportsdivision;Username=postgres;Password=CHANGE_ME" - }, - "SeedAdmin": { - "Email": "admin@sportsdivision.dm", - "Password": "" } } diff --git a/src/SportsDivision.Web/appsettings.Production.json b/src/SportsDivision.Web/appsettings.Production.json index 10f68b8..5dd9ffc 100644 --- a/src/SportsDivision.Web/appsettings.Production.json +++ b/src/SportsDivision.Web/appsettings.Production.json @@ -5,5 +5,13 @@ "Microsoft.AspNetCore": "Warning" } }, - "AllowedHosts": "*" + "AllowedHosts": "*", + "ConnectionStrings": { + "DefaultConnection": "Host=74.50.64.180;Port=5432;Database=sportsdivision_db;Username=postgres;Password=rG4eX5vU8kD4jY5k" + }, + "EmailSettings": { + "GmailEmail": "pcgurudm@gmail.com", + "GmailPassword": "bbux tqjo lubq utss", + "DisplayName": "Sports Division" + } } diff --git a/src/SportsDivision.Web/appsettings.json b/src/SportsDivision.Web/appsettings.json index 64f2d8a..1dc64c5 100644 --- a/src/SportsDivision.Web/appsettings.json +++ b/src/SportsDivision.Web/appsettings.json @@ -7,6 +7,15 @@ }, "AllowedHosts": "*", "ConnectionStrings": { - "DefaultConnection": "" + "DefaultConnection": "Host=localhost;Database=sportsdivision;Username=postgres;Password=aN5eM6zM0nX4nX9j" + }, + "EmailSettings": { + "GmailEmail": "pcgurudm@gmail.com", + "GmailPassword": "bbux tqjo lubq utss", + "DisplayName": "Sports Division" + }, + "SeedAdmin": { + "Email": "admin@sportsdivision.dm", + "Password": "Admin@123!" } }