fix(api): route Auth field becomes mandatory + readonly/user roles get access to read endpoints (P0)

[cristim]

Summary

P0/critical for any non-admin role. A user signed in as readonly (or user) saw "admin access required" on every page — Dashboard, Recommendations, Plans, Planned Purchases, Convertible RIs, Reshape Recommendations, Exchange History, Settings. The entire app was admin-gated.

Root cause

AuthLevel had AuthAdmin = iota at the zero value with a "secure by default" rationale:

const (
    AuthAdmin AuthLevel = iota  // 0 — any Route without an explicit Auth field gets this
    AuthUser                    // 1
    AuthPublic                  // 2
)

Every new read-only endpoint was registered without setting Auth: and silently inherited AuthAdmin via the zero value. The whole app's read surface became admin-only without anyone noticing — because admins (the only role testing it) saw nothing wrong.

Fix — two layers

1. Auth becomes a required Route field

authUnset is now the iota zero. NewRouter panics at startup if any route was registered without an explicit AuthAdmin / AuthUser / AuthPublic. The doc comment that called the old pattern "secure by default" is gone — it was the opposite of secure for non-admin roles.

2. Every existing route now declares its Auth level

• 37 routes had Auth: added explicitly (mostly AuthAdmin, mechanical pass).

• 24 routes flipped to Auth: AuthUser so signed-in non-admin users can use the app — every GET that feeds a dashboard page:

  Endpoint group	Routes
  Dashboard	/api/dashboard/summary, /api/dashboard/upcoming
  Settings	/api/config GET, /api/config/service/{p} GET
  Recommendations	/api/recommendations, .../freshness, .../{id}/detail
  Plans	/api/plans GET, /api/plans/{id} GET, .../{id}/accounts GET
  Planned purchases	/api/purchases/planned GET, /api/purchases/{id} GET
  History	/api/history, .../analytics, .../breakdown
  Accounts	/api/accounts GET, .../{id} GET, .../{id}/service-overrides GET
  Groups	/api/groups GET, /api/groups/{id} GET
  RI exchange	.../instances, .../utilization, .../reshape-recommendations, .../config GET, .../history

Mutating endpoints (POST/PUT/PATCH/DELETE) keep AuthAdmin pending the finer-grained per-role write-permission work (handler-level requirePermission checks already exist for the few endpoints that use them — a follow-up will widen that to plans/purchases/etc.).

Test plan

• TestNewRouter_PanicsOnUnsetAuth — startup-validation loop replayed on a fixture route with Auth omitted; asserts the panic fires.
• TestRoute_AllRegisteredHaveExplicitAuth — walks the live route table and asserts every entry declares an explicit Auth level. Catches a forgotten field in CI even if the Lambda never starts locally. Two independent safety nets.
• go test github.com/LeanerCloud/CUDly/internal/api — passes.
• go test ./... — full repo suite, no regressions.
• End-to-end on the deployed env after merge: signed-in readonly user sees the same pages an admin sees (read-only, no mutating buttons).

Out of scope (separate follow-ups)

• Finer-grained per-role write permissions (readonly = read-only; user = read + write plans; admin = full). Handler-level requirePermission(action, resource) exists for a few endpoints — needs to spread to plans / purchases / RI-exchange / etc.
• Decide which admin-management endpoints (/api/users, /api/registrations, /api/accounts writes) should ever loosen to non-admin. Default in this PR is: they stay admin-only.
• Audit requirePermission callers to make sure no AuthUser route accidentally surfaces data outside the caller's allowed_accounts grant.

Summary by CodeRabbit

• Bug Fixes

  • Enforced explicit authentication level for every API route; startup now fails fast if any route is left unset.
  • Strengthened runtime routing safety to refuse requests when an unexpected auth configuration is encountered.

• Tests

  • Added tests verifying mandatory auth on routes and that the registered route table has explicit auth set.

[cristim]

@coderabbitai review

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dbbb7248-77dd-4cc9-a2e3-c49aecd7ab5d

📥 Commits

Reviewing files that changed from the base of the PR and between 237b111 and a752658.

📒 Files selected for processing (2)

• internal/api/router.go
• internal/api/router_auth_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/api/router.go

---

📝 Walkthrough

Walkthrough

All API routes must declare an explicit AuthLevel. An internal authUnset zero value was added; NewRouter panics if any route leaves Auth unset. registerRoutes now sets Auth per endpoint, runtime dispatch rejects unknown auth values, and tests verify the enforcement.

Changes

Explicit Authentication Enforcement

Layer / File(s)	Summary
AuthLevel contract and startup validation internal/api/router.go	Introduces internal authUnset zero value; updates Route.Auth docs to REQUIRED; adds validateRoutes and NewRouter startup panic when any route leaves Auth unset.
Route registration with explicit authentication internal/api/router.go	Updates registerRoutes entries across multiple endpoint groups to explicitly set Auth: AuthAdmin, AuthUser, or AuthPublic as appropriate.
Runtime authentication dispatch internal/api/router.go	Updates Router.Route docs and adds a defensive default branch to refuse dispatch with a 500 internal routing error for unknown/unset auth values.
Test coverage for authentication enforcement internal/api/router_auth_test.go	Adds TestValidateRoutes_PanicsOnUnsetAuth, TestValidateRoutes_AcceptsAllExplicitLevels, and TestRoute_AllRegisteredHaveExplicitAuth to validate startup panic behavior and that all registered routes have explicit Auth.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

severity/high, impact/many

Poem

🐰
Every route now wears a tag so bright,
No silent gaps in the authing night.
authUnset hid the zero's sly groove,
Startup checks make sure each route moves.
Hops of safety, neat and light.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: making the route Auth field mandatory and granting user/readonly roles access to read endpoints. It is specific, concise, and directly related to the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/route-auth-read-endpoints

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/api/router_auth_test.go`:
- Around line 15-28: The test TestNewRouter_PanicsOnUnsetAuth duplicates
NewRouter's validation loop; extract that route-auth check into a single helper
(e.g., validateRoutes or validateRouteAuth) and have NewRouter call that helper,
then update the test to call the helper (or NewRouter directly) to assert the
panic instead of replaying the loop inline; reference the Router type, NewRouter
function, and the route.Auth/authUnset check so you remove the duplicated loop
and centralize validation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e83c4588-d255-4c85-9026-33333cba3b0c

📥 Commits

Reviewing files that changed from the base of the PR and between 44365bb and 237b111.

📒 Files selected for processing (2)

• internal/api/router.go
• internal/api/router_auth_test.go

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.