fix(auth): delete the Kratos identity on account deletion
Account deletion removed all local data but left the Ory Kratos
identity intact — an orphaned identity that can still authenticate.
Close the gap:
- kratos.Client gains the admin API: NewClient(publicURL, adminURL)
and DeleteIdentity (DELETE /admin/identities/{id}; a 404 is treated
as success so a retry after a partial failure is idempotent).
- AuthService.DeleteAccount deletes the Kratos identity FIRST; if that
call fails it aborts before touching local data, so the operation is
retryable rather than partially applied.
- KRATOS_ADMIN_URL config (default http://kratos:4434) + router wiring.
- kratos NetworkPolicy split: the api pods may now reach the admin API
:4434 (Traefik still reaches only the public API :4433).
- kratos CORS: allow_credentials + OPTIONS so the web browser flows
(ory_kratos_session cookie) work; origins stay an explicit allowlist.
- Regression tests: identity teardown happens, and a Kratos failure
aborts the deletion instead of orphaning local data.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -31,9 +31,13 @@ data:
|
||||
- https://myhoneydue.com
|
||||
- https://app.myhoneydue.com
|
||||
- https://admin.myhoneydue.com
|
||||
allowed_methods: [GET, POST, PUT, PATCH, DELETE]
|
||||
allowed_methods: [GET, POST, PUT, PATCH, DELETE, OPTIONS]
|
||||
allowed_headers: [Authorization, Content-Type, X-Session-Token, Cookie]
|
||||
exposed_headers: [Content-Type, Set-Cookie]
|
||||
# Required: the web clients call Kratos browser flows with
|
||||
# credentials (the ory_kratos_session cookie). Safe here because
|
||||
# allowed_origins is an explicit list, never a wildcard.
|
||||
allow_credentials: true
|
||||
admin:
|
||||
base_url: http://kratos.honeydue.svc.cluster.local:4434/
|
||||
|
||||
|
||||
@@ -127,9 +127,10 @@ spec:
|
||||
port: 4434
|
||||
targetPort: 4434
|
||||
---
|
||||
# Ingress to Kratos: Traefik (the auth.myhoneydue.com IngressRoute) and the
|
||||
# honeyDue api pods (session whoami) may reach the public API :4433. The
|
||||
# admin API :4434 takes no cluster ingress — it is reachable only in-pod.
|
||||
# Ingress to Kratos. Traefik (the auth.myhoneydue.com IngressRoute) reaches
|
||||
# only the public API :4433. The honeyDue api pods reach the public API :4433
|
||||
# (session whoami) AND the admin API :4434 (identity deletion on account
|
||||
# close). The admin API :4434 takes no other cluster ingress.
|
||||
apiVersion: networking.k8s.io/v1
|
||||
kind: NetworkPolicy
|
||||
metadata:
|
||||
@@ -142,16 +143,24 @@ spec:
|
||||
policyTypes:
|
||||
- Ingress
|
||||
ingress:
|
||||
# Traefik ingress controller -> public API only.
|
||||
- from:
|
||||
- namespaceSelector:
|
||||
matchLabels:
|
||||
kubernetes.io/metadata.name: kube-system
|
||||
ports:
|
||||
- port: 4433
|
||||
protocol: TCP
|
||||
# honeyDue api pods -> public API (whoami) + admin API (identity deletion).
|
||||
- from:
|
||||
- podSelector:
|
||||
matchLabels:
|
||||
app.kubernetes.io/name: api
|
||||
ports:
|
||||
- port: 4433
|
||||
protocol: TCP
|
||||
- port: 4434
|
||||
protocol: TCP
|
||||
---
|
||||
# Kratos egress: DNS, the Neon Postgres database, SMTP, and HTTPS to the
|
||||
# OIDC providers (Apple/Google token + JWKS endpoints).
|
||||
|
||||
@@ -145,6 +145,10 @@ type SecurityConfig struct {
|
||||
// KratosPublicURL is the Ory Kratos public API base URL. The auth
|
||||
// middleware validates sessions against {KratosPublicURL}/sessions/whoami.
|
||||
KratosPublicURL string
|
||||
// KratosAdminURL is the Ory Kratos admin API base URL. Account deletion
|
||||
// removes the user's Kratos identity via
|
||||
// {KratosAdminURL}/admin/identities/{id}.
|
||||
KratosAdminURL string
|
||||
}
|
||||
|
||||
// StorageConfig holds file storage settings.
|
||||
@@ -308,6 +312,7 @@ func Load() (*Config, error) {
|
||||
TokenExpiryDays: viper.GetInt("TOKEN_EXPIRY_DAYS"),
|
||||
TokenRefreshDays: viper.GetInt("TOKEN_REFRESH_DAYS"),
|
||||
KratosPublicURL: viper.GetString("KRATOS_PUBLIC_URL"),
|
||||
KratosAdminURL: viper.GetString("KRATOS_ADMIN_URL"),
|
||||
},
|
||||
Storage: StorageConfig{
|
||||
UploadDir: viper.GetString("STORAGE_UPLOAD_DIR"),
|
||||
@@ -416,6 +421,7 @@ func setDefaults() {
|
||||
// Token expiry defaults
|
||||
viper.SetDefault("TOKEN_EXPIRY_DAYS", 90) // Tokens expire after 90 days
|
||||
viper.SetDefault("KRATOS_PUBLIC_URL", "http://kratos:4433") // Ory Kratos public API
|
||||
viper.SetDefault("KRATOS_ADMIN_URL", "http://kratos:4434") // Ory Kratos admin API
|
||||
viper.SetDefault("TOKEN_REFRESH_DAYS", 60) // Tokens can be refreshed after 60 days
|
||||
|
||||
// Storage defaults
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Package kratos is a thin client for the Ory Kratos public API. honeyDue
|
||||
// Package kratos is a thin client for the Ory Kratos APIs. honeyDue
|
||||
// delegates all identity concerns (credentials, sessions, verification,
|
||||
// recovery, social sign-in) to Kratos; this client only validates sessions.
|
||||
// recovery, social sign-in) to Kratos; this client validates sessions
|
||||
// against the public API and deletes identities via the admin API.
|
||||
package kratos
|
||||
|
||||
import (
|
||||
@@ -9,6 +10,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
@@ -17,17 +19,21 @@ import (
|
||||
// or inactive — the caller should respond 401.
|
||||
var ErrUnauthorized = errors.New("kratos: session invalid or inactive")
|
||||
|
||||
// Client talks to the Ory Kratos public API.
|
||||
// Client talks to the Ory Kratos public and admin APIs.
|
||||
type Client struct {
|
||||
publicURL string
|
||||
adminURL string
|
||||
http *http.Client
|
||||
}
|
||||
|
||||
// NewClient builds a Kratos client for the given public-API base URL
|
||||
// (e.g. http://kratos:4433 in-cluster).
|
||||
func NewClient(publicURL string) *Client {
|
||||
// NewClient builds a Kratos client. publicURL is the public-API base used for
|
||||
// session validation (e.g. http://kratos:4433 in-cluster); adminURL is the
|
||||
// admin-API base used for identity management (e.g. http://kratos:4434).
|
||||
// Either may be empty when the corresponding API is unused.
|
||||
func NewClient(publicURL, adminURL string) *Client {
|
||||
return &Client{
|
||||
publicURL: strings.TrimRight(publicURL, "/"),
|
||||
adminURL: strings.TrimRight(adminURL, "/"),
|
||||
http: &http.Client{Timeout: 5 * time.Second},
|
||||
}
|
||||
}
|
||||
@@ -105,3 +111,35 @@ func (c *Client) Whoami(ctx context.Context, sessionToken, cookie string) (*Sess
|
||||
}
|
||||
return &s, nil
|
||||
}
|
||||
|
||||
// DeleteIdentity permanently removes a Kratos identity by its UUID via the
|
||||
// admin API (DELETE /admin/identities/{id}). A 404 is treated as success —
|
||||
// the identity is already gone, which is the desired end state, so the call
|
||||
// is idempotent across retries. Called when a honeyDue account is deleted so
|
||||
// no orphaned, still-loginable identity is left behind.
|
||||
func (c *Client) DeleteIdentity(ctx context.Context, identityID string) error {
|
||||
if c.adminURL == "" {
|
||||
return errors.New("kratos: admin URL not configured")
|
||||
}
|
||||
if identityID == "" {
|
||||
return errors.New("kratos: empty identity id")
|
||||
}
|
||||
endpoint := c.adminURL + "/admin/identities/" + url.PathEscape(identityID)
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodDelete, endpoint, nil)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
resp, err := c.http.Do(req)
|
||||
if err != nil {
|
||||
return fmt.Errorf("kratos delete identity: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
switch resp.StatusCode {
|
||||
case http.StatusNoContent, http.StatusNotFound:
|
||||
return nil
|
||||
default:
|
||||
return fmt.Errorf("kratos delete identity: unexpected status %d", resp.StatusCode)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -246,8 +246,9 @@ func SetupRouter(deps *Dependencies) *echo.Echo {
|
||||
subscriptionWebhookHandler.SetCacheService(deps.Cache)
|
||||
|
||||
// Initialize Kratos auth middleware (replaces hand-rolled token auth).
|
||||
kratosClient := kratos.NewClient(cfg.Security.KratosPublicURL)
|
||||
kratosClient := kratos.NewClient(cfg.Security.KratosPublicURL, cfg.Security.KratosAdminURL)
|
||||
authMiddleware := custommiddleware.NewKratosAuth(kratosClient, deps.Cache, deps.DB)
|
||||
authService.SetKratosClient(kratosClient) // account deletion removes the Kratos identity
|
||||
|
||||
// Initialize audit service for security event logging
|
||||
auditService := services.NewAuditService(deps.DB)
|
||||
|
||||
@@ -3,12 +3,14 @@ package services
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
|
||||
"github.com/rs/zerolog/log"
|
||||
|
||||
"github.com/treytartt/honeydue-api/internal/apperrors"
|
||||
"github.com/treytartt/honeydue-api/internal/dto/requests"
|
||||
"github.com/treytartt/honeydue-api/internal/dto/responses"
|
||||
"github.com/treytartt/honeydue-api/internal/kratos"
|
||||
"github.com/treytartt/honeydue-api/internal/repositories"
|
||||
)
|
||||
|
||||
@@ -18,6 +20,7 @@ type AuthService struct {
|
||||
userRepo *repositories.UserRepository
|
||||
notificationRepo *repositories.NotificationRepository
|
||||
cache *CacheService
|
||||
kratos *kratos.Client
|
||||
}
|
||||
|
||||
// NewAuthService creates a new auth service.
|
||||
@@ -38,6 +41,13 @@ func (s *AuthService) SetCacheService(cache *CacheService) {
|
||||
s.cache = cache
|
||||
}
|
||||
|
||||
// SetKratosClient wires the Ory Kratos client so account deletion also
|
||||
// removes the user's Kratos identity. When nil (e.g. in tests) the Kratos
|
||||
// teardown is skipped.
|
||||
func (s *AuthService) SetKratosClient(k *kratos.Client) {
|
||||
s.kratos = k
|
||||
}
|
||||
|
||||
// GetCurrentUser returns the current authenticated user with profile.
|
||||
func (s *AuthService) GetCurrentUser(ctx context.Context, userID uint) (*responses.CurrentUserResponse, error) {
|
||||
user, err := s.userRepo.WithContext(ctx).FindByIDWithProfile(userID)
|
||||
@@ -102,12 +112,17 @@ func (s *AuthService) UpdateProfile(ctx context.Context, userID uint, req *reque
|
||||
// DeleteAccount deletes a user's account and all associated data.
|
||||
// Kratos owns credentials; confirmation string "DELETE" is required from the
|
||||
// caller since we can no longer verify a password here.
|
||||
//
|
||||
// The Kratos identity is deleted FIRST: Kratos owns the credentials, so if
|
||||
// that call fails the local data is left intact and an error is returned for
|
||||
// the caller to retry. Deleting local data first would risk an orphaned
|
||||
// Kratos identity that can still authenticate against a half-deleted account.
|
||||
func (s *AuthService) DeleteAccount(ctx context.Context, userID uint, _ *string, confirmation *string) ([]string, error) {
|
||||
if confirmation == nil || *confirmation != "DELETE" {
|
||||
return nil, apperrors.BadRequest("error.confirmation_required")
|
||||
}
|
||||
|
||||
_, err := s.userRepo.WithContext(ctx).FindByID(userID)
|
||||
user, err := s.userRepo.WithContext(ctx).FindByID(userID)
|
||||
if err != nil {
|
||||
if errors.Is(err, repositories.ErrUserNotFound) {
|
||||
return nil, apperrors.NotFound("error.user_not_found")
|
||||
@@ -115,6 +130,14 @@ func (s *AuthService) DeleteAccount(ctx context.Context, userID uint, _ *string,
|
||||
return nil, apperrors.Internal(err)
|
||||
}
|
||||
|
||||
// Tear down the Kratos identity before touching local data. DeleteIdentity
|
||||
// treats a 404 as success, so a retry after a partial failure is safe.
|
||||
if s.kratos != nil && user.KratosID != "" {
|
||||
if err := s.kratos.DeleteIdentity(ctx, user.KratosID); err != nil {
|
||||
return nil, apperrors.Internal(fmt.Errorf("delete kratos identity: %w", err))
|
||||
}
|
||||
}
|
||||
|
||||
var fileURLs []string
|
||||
txErr := s.userRepo.WithContext(ctx).Transaction(func(txRepo *repositories.UserRepository) error {
|
||||
urls, err := txRepo.DeleteUserCascade(userID)
|
||||
|
||||
@@ -3,6 +3,7 @@ package services
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -10,6 +11,8 @@ import (
|
||||
|
||||
"github.com/treytartt/honeydue-api/internal/config"
|
||||
"github.com/treytartt/honeydue-api/internal/dto/requests"
|
||||
"github.com/treytartt/honeydue-api/internal/kratos"
|
||||
"github.com/treytartt/honeydue-api/internal/models"
|
||||
"github.com/treytartt/honeydue-api/internal/repositories"
|
||||
"github.com/treytartt/honeydue-api/internal/testutil"
|
||||
)
|
||||
@@ -165,6 +168,60 @@ func TestAuthService_DeleteAccount_UserNotFound(t *testing.T) {
|
||||
testutil.AssertAppError(t, err, http.StatusNotFound, "error.user_not_found")
|
||||
}
|
||||
|
||||
// Regression: deleting an account must also delete the user's Kratos
|
||||
// identity, or an orphaned, still-loginable identity is left behind.
|
||||
func TestAuthService_DeleteAccount_DeletesKratosIdentity(t *testing.T) {
|
||||
service, userRepo := setupAuthService(t)
|
||||
db := userRepo.DB()
|
||||
|
||||
user := testutil.CreateTestUser(t, db, "kuser", "kuser@test.com", "")
|
||||
require.NoError(t, db.Model(&models.User{}).Where("id = ?", user.ID).
|
||||
Update("kratos_id", "kratos-uuid-123").Error)
|
||||
|
||||
var gotMethod, gotPath string
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
gotMethod, gotPath = r.Method, r.URL.Path
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
defer srv.Close()
|
||||
service.SetKratosClient(kratos.NewClient("", srv.URL))
|
||||
|
||||
confirmation := "DELETE"
|
||||
_, err := service.DeleteAccount(context.Background(), user.ID, nil, &confirmation)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, http.MethodDelete, gotMethod)
|
||||
assert.Equal(t, "/admin/identities/kratos-uuid-123", gotPath)
|
||||
|
||||
// Local user row is gone.
|
||||
_, err = userRepo.WithContext(context.Background()).FindByID(user.ID)
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
// Regression: if the Kratos identity delete fails, local data must NOT be
|
||||
// deleted — the operation must be retryable, not partially applied.
|
||||
func TestAuthService_DeleteAccount_KratosFailureAbortsDeletion(t *testing.T) {
|
||||
service, userRepo := setupAuthService(t)
|
||||
db := userRepo.DB()
|
||||
|
||||
user := testutil.CreateTestUser(t, db, "kuser2", "kuser2@test.com", "")
|
||||
require.NoError(t, db.Model(&models.User{}).Where("id = ?", user.ID).
|
||||
Update("kratos_id", "kratos-uuid-456").Error)
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
}))
|
||||
defer srv.Close()
|
||||
service.SetKratosClient(kratos.NewClient("", srv.URL))
|
||||
|
||||
confirmation := "DELETE"
|
||||
_, err := service.DeleteAccount(context.Background(), user.ID, nil, &confirmation)
|
||||
require.Error(t, err)
|
||||
|
||||
// Local user row must still exist.
|
||||
_, ferr := userRepo.WithContext(context.Background()).FindByID(user.ID)
|
||||
assert.NoError(t, ferr)
|
||||
}
|
||||
|
||||
// === SetNotificationRepository ===
|
||||
|
||||
func TestAuthService_SetNotificationRepository(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user