Auth: require email-verified by default for all app-data routes
Previously only 2 share-code routes required a verified email; every other authenticated route (residences, tasks, contractors, documents, notifications, subscription, users, uploads, media — ~70 routes) accepted an authenticated but UNVERIFIED user. This inverts the default to verified-by-default. - router.go: add a `verified` sub-group that applies RequireVerified() ONCE at the group level, and move all app-data route setups under it. Verification is now the default; new routes are gated automatically. The authenticated-only allow-list is just the sign-up surface (/auth/me, /auth/profile, /auth/account). Public stays: register, health, webhooks, lookups. - kratos_auth.go: fix a latent bug the gating exposed — the Redis session cache stored the verified flag for 24h, so a user who verified their email mid-session was still seen as unverified until the TTL expired (sign up -> verify -> create residence would 403). Now only a cached verified=true is trusted (verification is sticky); a cached verified=false re-resolves the live status from Kratos. - auth_safety_test.go: add RequireVerified unit tests (verified passes, unverified -> 403, no-user -> 401). Validated: API gating test (unverified->403, verified->200) + full iOS XCUITest suite green (211 passed) including the onboarding verify->use-immediately flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -9,6 +9,7 @@ import (
|
|||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"github.com/treytartt/honeydue-api/internal/apperrors"
|
||||||
"github.com/treytartt/honeydue-api/internal/config"
|
"github.com/treytartt/honeydue-api/internal/config"
|
||||||
"github.com/treytartt/honeydue-api/internal/models"
|
"github.com/treytartt/honeydue-api/internal/models"
|
||||||
)
|
)
|
||||||
@@ -117,3 +118,76 @@ func TestAdminAuth_QueryParamToken_Rejected(t *testing.T) {
|
|||||||
assert.Equal(t, http.StatusUnauthorized, rec.Code, "query param token must be rejected")
|
assert.Equal(t, http.StatusUnauthorized, rec.Code, "query param token must be rejected")
|
||||||
assert.Contains(t, rec.Body.String(), "Authorization required")
|
assert.Contains(t, rec.Body.String(), "Authorization required")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// requireVerifiedContext builds an Echo context primed as the Authenticate
|
||||||
|
// middleware would leave it: an auth_user and the verified flag.
|
||||||
|
func requireVerifiedContext(user *models.User, verified bool) (echo.Context, *httptest.ResponseRecorder) {
|
||||||
|
e := echo.New()
|
||||||
|
req := httptest.NewRequest(http.MethodGet, "/api/residences/", nil)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
c := e.NewContext(req, rec)
|
||||||
|
if user != nil {
|
||||||
|
c.Set(AuthUserKey, user)
|
||||||
|
}
|
||||||
|
c.Set(AuthVerifiedKey, verified)
|
||||||
|
return c, rec
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRequireVerified_VerifiedUser_Passes confirms a verified user reaches the
|
||||||
|
// wrapped handler. This is the default tier for all app-data routes now that
|
||||||
|
// RequireVerified is applied at the `verified` group level in the router.
|
||||||
|
func TestRequireVerified_VerifiedUser_Passes(t *testing.T) {
|
||||||
|
m := &KratosAuth{}
|
||||||
|
c, _ := requireVerifiedContext(&models.User{Username: "v"}, true)
|
||||||
|
|
||||||
|
reached := false
|
||||||
|
handler := m.RequireVerified()(func(c echo.Context) error {
|
||||||
|
reached = true
|
||||||
|
return c.NoContent(http.StatusOK)
|
||||||
|
})
|
||||||
|
|
||||||
|
err := handler(c)
|
||||||
|
assert.NoError(t, err)
|
||||||
|
assert.True(t, reached, "verified user should reach the handler")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRequireVerified_UnverifiedUser_403 is the core gating assertion for the
|
||||||
|
// new policy: an authenticated-but-unverified user is rejected with 403 on a
|
||||||
|
// data route, NOT allowed through.
|
||||||
|
func TestRequireVerified_UnverifiedUser_403(t *testing.T) {
|
||||||
|
m := &KratosAuth{}
|
||||||
|
c, _ := requireVerifiedContext(&models.User{Username: "u"}, false)
|
||||||
|
|
||||||
|
reached := false
|
||||||
|
handler := m.RequireVerified()(func(c echo.Context) error {
|
||||||
|
reached = true
|
||||||
|
return c.NoContent(http.StatusOK)
|
||||||
|
})
|
||||||
|
|
||||||
|
err := handler(c)
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.False(t, reached, "unverified user must NOT reach the handler")
|
||||||
|
|
||||||
|
var appErr *apperrors.AppError
|
||||||
|
require.ErrorAs(t, err, &appErr)
|
||||||
|
assert.Equal(t, http.StatusForbidden, appErr.Code)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRequireVerified_NoUser_401 confirms RequireVerified rejects an
|
||||||
|
// unauthenticated request with 401 (defense-in-depth even though Authenticate
|
||||||
|
// runs first in the router).
|
||||||
|
func TestRequireVerified_NoUser_401(t *testing.T) {
|
||||||
|
m := &KratosAuth{}
|
||||||
|
c, _ := requireVerifiedContext(nil, false)
|
||||||
|
|
||||||
|
handler := m.RequireVerified()(func(c echo.Context) error {
|
||||||
|
return c.NoContent(http.StatusOK)
|
||||||
|
})
|
||||||
|
|
||||||
|
err := handler(c)
|
||||||
|
require.Error(t, err)
|
||||||
|
|
||||||
|
var appErr *apperrors.AppError
|
||||||
|
require.ErrorAs(t, err, &appErr)
|
||||||
|
assert.Equal(t, http.StatusUnauthorized, appErr.Code)
|
||||||
|
}
|
||||||
|
|||||||
@@ -131,14 +131,20 @@ func (m *KratosAuth) resolve(c echo.Context) (*models.User, bool, string, error)
|
|||||||
cacheKey := kratosSessionPrefix + hashCredential(cred)
|
cacheKey := kratosSessionPrefix + hashCredential(cred)
|
||||||
if m.cache != nil {
|
if m.cache != nil {
|
||||||
if v, err := m.cache.GetString(ctx, cacheKey); err == nil && v != "" {
|
if v, err := m.cache.GetString(ctx, cacheKey); err == nil && v != "" {
|
||||||
if user, verified, ok := m.userFromCacheValue(ctx, v); ok {
|
// Only a cached `verified=true` is authoritative — email verification
|
||||||
// Sliding-window refresh: extend the TTL on every successful
|
// is sticky (it never reverts), so we can safely short-circuit.
|
||||||
// hit so active users don't get bounced when their original
|
// A cached `verified=false` is deliberately NOT trusted: the user may
|
||||||
// cache entry would have otherwise expired. Best-effort —
|
// have verified their email since this entry was written, and a stale
|
||||||
// failure to refresh just means the entry expires on the
|
// false would lock a just-verified user out of every verified-gated
|
||||||
// original schedule.
|
// route until the 24h TTL expired (e.g. sign up -> verify -> create a
|
||||||
|
// residence immediately). On a cached false we fall through and
|
||||||
|
// re-resolve the live status from Kratos /whoami below.
|
||||||
|
if user, verified, ok := m.userFromCacheValue(ctx, v); ok && verified {
|
||||||
|
// Sliding-window refresh: extend the TTL on every successful hit
|
||||||
|
// so active (verified) users aren't bounced when their entry
|
||||||
|
// would otherwise expire. Best-effort.
|
||||||
_ = m.cache.SetString(ctx, cacheKey, v, kratosSessionCacheTTL)
|
_ = m.cache.SetString(ctx, cacheKey, v, kratosSessionCacheTTL)
|
||||||
return user, verified, cred, nil
|
return user, true, cred, nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+33
-18
@@ -342,29 +342,44 @@ func SetupRouter(deps *Dependencies) *echo.Echo {
|
|||||||
// Subscription webhook routes (no auth - called by Apple/Google servers)
|
// Subscription webhook routes (no auth - called by Apple/Google servers)
|
||||||
setupWebhookRoutes(api, subscriptionWebhookHandler)
|
setupWebhookRoutes(api, subscriptionWebhookHandler)
|
||||||
|
|
||||||
// Protected routes (auth required)
|
// Authenticated routes (valid session required). This level is the
|
||||||
|
// sign-up / shell allow-list: an authenticated-but-UNVERIFIED user may
|
||||||
|
// call these (read their own user + verification status, complete their
|
||||||
|
// profile during sign-up). EVERYTHING else requires a verified email
|
||||||
|
// (see the `verified` sub-group below).
|
||||||
protected := api.Group("")
|
protected := api.Group("")
|
||||||
protected.Use(authMiddleware.Authenticate())
|
protected.Use(authMiddleware.Authenticate())
|
||||||
protected.Use(custommiddleware.TimezoneMiddleware())
|
protected.Use(custommiddleware.TimezoneMiddleware())
|
||||||
{
|
{
|
||||||
|
// Allow-list — authenticated, may be unverified.
|
||||||
setupProtectedAuthRoutes(protected, authHandler)
|
setupProtectedAuthRoutes(protected, authHandler)
|
||||||
setupResidenceRoutes(protected, residenceHandler, authMiddleware.RequireVerified())
|
|
||||||
setupTaskRoutes(protected, taskHandler)
|
// Verified routes (authenticated AND email-verified) — the DEFAULT
|
||||||
setupSuggestionRoutes(protected, suggestionHandler)
|
// for all app data and actions. RequireVerified is applied ONCE at
|
||||||
setupContractorRoutes(protected, contractorHandler)
|
// the group level so verification is the default and every route
|
||||||
setupDocumentRoutes(protected, documentHandler)
|
// added under here is gated automatically. (The previous per-route
|
||||||
setupNotificationRoutes(protected, notificationHandler)
|
// approach left ~70 routes unverified — see the LIVE auth audit.)
|
||||||
setupSubscriptionRoutes(protected, subscriptionHandler)
|
verified := protected.Group("")
|
||||||
setupUserRoutes(protected, userHandler)
|
verified.Use(authMiddleware.RequireVerified())
|
||||||
|
{
|
||||||
|
setupResidenceRoutes(verified, residenceHandler)
|
||||||
|
setupTaskRoutes(verified, taskHandler)
|
||||||
|
setupSuggestionRoutes(verified, suggestionHandler)
|
||||||
|
setupContractorRoutes(verified, contractorHandler)
|
||||||
|
setupDocumentRoutes(verified, documentHandler)
|
||||||
|
setupNotificationRoutes(verified, notificationHandler)
|
||||||
|
setupSubscriptionRoutes(verified, subscriptionHandler)
|
||||||
|
setupUserRoutes(verified, userHandler)
|
||||||
|
|
||||||
// Upload routes (only if storage service is configured)
|
// Upload routes (only if storage service is configured)
|
||||||
if uploadHandler != nil {
|
if uploadHandler != nil {
|
||||||
setupUploadRoutes(protected, uploadHandler)
|
setupUploadRoutes(verified, uploadHandler)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Media routes (authenticated media serving)
|
// Media routes (verified media serving)
|
||||||
if mediaHandler != nil {
|
if mediaHandler != nil {
|
||||||
setupMediaRoutes(protected, mediaHandler)
|
setupMediaRoutes(verified, mediaHandler)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -583,7 +598,7 @@ func setupPublicDataRoutes(api *echo.Group, residenceHandler *handlers.Residence
|
|||||||
}
|
}
|
||||||
|
|
||||||
// setupResidenceRoutes configures residence routes
|
// setupResidenceRoutes configures residence routes
|
||||||
func setupResidenceRoutes(api *echo.Group, residenceHandler *handlers.ResidenceHandler, requireVerified echo.MiddlewareFunc) {
|
func setupResidenceRoutes(api *echo.Group, residenceHandler *handlers.ResidenceHandler) {
|
||||||
residences := api.Group("/residences")
|
residences := api.Group("/residences")
|
||||||
{
|
{
|
||||||
residences.GET("/", residenceHandler.ListResidences)
|
residences.GET("/", residenceHandler.ListResidences)
|
||||||
@@ -598,11 +613,11 @@ func setupResidenceRoutes(api *echo.Group, residenceHandler *handlers.ResidenceH
|
|||||||
residences.DELETE("/:id/", residenceHandler.DeleteResidence)
|
residences.DELETE("/:id/", residenceHandler.DeleteResidence)
|
||||||
|
|
||||||
residences.GET("/:id/share-code/", residenceHandler.GetShareCode)
|
residences.GET("/:id/share-code/", residenceHandler.GetShareCode)
|
||||||
// Audit LIVE-L19: generating a residence share code requires a
|
// Verification is now enforced at the group level for ALL residence
|
||||||
// verified email — it blocks bad-faith unverified signups from
|
// routes (see the `verified` group in SetupRouter) — the previous
|
||||||
// minting share codes.
|
// per-route RequireVerified on just these two is no longer needed.
|
||||||
residences.POST("/:id/generate-share-code/", residenceHandler.GenerateShareCode, requireVerified)
|
residences.POST("/:id/generate-share-code/", residenceHandler.GenerateShareCode)
|
||||||
residences.POST("/:id/generate-share-package/", residenceHandler.GenerateSharePackage, requireVerified)
|
residences.POST("/:id/generate-share-package/", residenceHandler.GenerateSharePackage)
|
||||||
residences.POST("/:id/generate-tasks-report/", residenceHandler.GenerateTasksReport)
|
residences.POST("/:id/generate-tasks-report/", residenceHandler.GenerateTasksReport)
|
||||||
residences.GET("/:id/users/", residenceHandler.GetResidenceUsers)
|
residences.GET("/:id/users/", residenceHandler.GetResidenceUsers)
|
||||||
residences.DELETE("/:id/users/:user_id/", residenceHandler.RemoveResidenceUser)
|
residences.DELETE("/:id/users/:user_id/", residenceHandler.RemoveResidenceUser)
|
||||||
|
|||||||
Reference in New Issue
Block a user