diff --git a/control-plane/internal/server/middleware/did_auth.go b/control-plane/internal/server/middleware/did_auth.go index 35e642b06..05387ff26 100644 --- a/control-plane/internal/server/middleware/did_auth.go +++ b/control-plane/internal/server/middleware/did_auth.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "encoding/base64" "encoding/hex" + "errors" "fmt" "io" "net/http" @@ -15,6 +16,7 @@ import ( "time" "github.com/Agent-Field/agentfield/control-plane/internal/logger" + "github.com/Agent-Field/agentfield/control-plane/internal/services" "github.com/gin-gonic/gin" ) @@ -281,6 +283,14 @@ func DIDAuthMiddleware(didService DIDWebServiceInterface, config DIDAuthConfig) ) if err != nil { + if errors.Is(err, services.ErrDIDResolutionUnavailable) { + logger.Logger.Warn().Err(err).Str("caller_did", callerDID).Msg("DID resolution temporarily unavailable") + c.AbortWithStatusJSON(http.StatusServiceUnavailable, gin.H{ + "error": "did_resolution_unavailable", + "message": "DID resolution is temporarily unavailable, retry shortly", + }) + return + } logger.Logger.Warn().Err(err).Str("caller_did", callerDID).Msg("DID signature verification error") c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{ "error": "verification_error", diff --git a/control-plane/internal/server/middleware/did_auth_test.go b/control-plane/internal/server/middleware/did_auth_test.go index a09ff9c9c..aec1466cf 100644 --- a/control-plane/internal/server/middleware/did_auth_test.go +++ b/control-plane/internal/server/middleware/did_auth_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/Agent-Field/agentfield/control-plane/internal/services" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" ) @@ -202,6 +203,29 @@ func TestDIDAuth_VerificationError(t *testing.T) { assert.Contains(t, w.Body.String(), "verification_error") } +func TestDIDAuth_TransientResolutionError(t *testing.T) { + resetReplayCache() + svc := &mockDIDService{ + verifyFunc: func(_ context.Context, _ string, _ []byte, _ []byte) (bool, error) { + // A transient storage failure during resolution surfaces as the + // retryable sentinel rather than a plain verification failure. + return false, fmt.Errorf("failed to resolve DID: %w", services.ErrDIDResolutionUnavailable) + }, + } + router := newDIDAuthRouter(svc, DIDAuthConfig{Enabled: true}) + + req := httptest.NewRequest(http.MethodPost, "/execute/agent.func", strings.NewReader("{}")) + req.Header.Set("X-Caller-DID", "did:web:example.com:agents:test") + req.Header.Set("X-DID-Signature", uniqueSig()) + req.Header.Set("X-DID-Timestamp", fmt.Sprintf("%d", time.Now().Unix())) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + // Transient storage errors must be retryable, not reported as auth failure. + assert.Equal(t, http.StatusServiceUnavailable, w.Code) + assert.Contains(t, w.Body.String(), "did_resolution_unavailable") +} + func TestDIDAuth_InvalidSignature(t *testing.T) { resetReplayCache() svc := &mockDIDService{ diff --git a/control-plane/internal/services/did_web_service.go b/control-plane/internal/services/did_web_service.go index 0810bc9f2..36ae60da7 100644 --- a/control-plane/internal/services/did_web_service.go +++ b/control-plane/internal/services/did_web_service.go @@ -5,14 +5,22 @@ import ( "crypto/ed25519" "encoding/base64" "encoding/json" + "errors" "fmt" "strings" "time" "github.com/Agent-Field/agentfield/control-plane/internal/logger" + "github.com/Agent-Field/agentfield/control-plane/internal/storage" "github.com/Agent-Field/agentfield/control-plane/pkg/types" ) +// ErrDIDResolutionUnavailable indicates a DID could not be resolved because of +// a transient storage failure rather than a genuine missing or revoked record. +// Callers (e.g. the DID auth middleware) should treat it as retryable and +// return a 503 instead of a 401. +var ErrDIDResolutionUnavailable = errors.New("DID resolution temporarily unavailable") + // DIDWebService handles did:web generation, storage, and resolution. type DIDWebService struct { domain string @@ -129,11 +137,17 @@ func (s *DIDWebService) ResolveDID(ctx context.Context, did string) (*types.DIDR // Get the DID document record record, err := s.storage.GetDIDDocument(ctx, did) if err != nil { - return &types.DIDResolutionResult{ - DIDResolutionMetadata: types.DIDResolutionMetadata{ - Error: "notFound", - }, - }, nil + // A genuine missing record resolves to "notFound"; any other storage + // failure is transient and must propagate so callers can retry rather + // than mistake a DB blip for an unknown DID. + if errors.Is(err, storage.ErrDIDDocumentNotFound) { + return &types.DIDResolutionResult{ + DIDResolutionMetadata: types.DIDResolutionMetadata{ + Error: "notFound", + }, + }, nil + } + return nil, fmt.Errorf("%w: %w", ErrDIDResolutionUnavailable, err) } // Check if revoked @@ -195,10 +209,9 @@ func (s *DIDWebService) RevokeDID(ctx context.Context, did string) error { func (s *DIDWebService) IsDIDRevoked(ctx context.Context, did string) bool { record, err := s.storage.GetDIDDocument(ctx, did) if err != nil { - // Check if this is a "not found" error vs a real storage failure. - // Not found means the DID was never registered — treat as not revoked. - // Any other error (DB timeout, connection failure) — fail closed. - if strings.Contains(err.Error(), "not found") || strings.Contains(err.Error(), "no rows") { + // Not found means the DID was never registered - treat as not revoked. + // Any other error (DB timeout, connection failure) - fail closed. + if errors.Is(err, storage.ErrDIDDocumentNotFound) { return false } logger.Logger.Warn().Err(err).Str("did", did).Msg("Storage error checking DID revocation, failing closed") diff --git a/control-plane/internal/services/did_web_service_additional_test.go b/control-plane/internal/services/did_web_service_additional_test.go index c720afe42..bb2c95855 100644 --- a/control-plane/internal/services/did_web_service_additional_test.go +++ b/control-plane/internal/services/did_web_service_additional_test.go @@ -7,9 +7,11 @@ import ( "encoding/base64" "encoding/json" "errors" + "fmt" "testing" "time" + storagepkg "github.com/Agent-Field/agentfield/control-plane/internal/storage" "github.com/Agent-Field/agentfield/control-plane/pkg/types" "github.com/stretchr/testify/require" ) @@ -73,7 +75,7 @@ func TestDIDWebServiceDocumentLifecycleAndResolution(t *testing.T) { require.Equal(t, "example.com", service.GetDomain()) }) - t.Run("storage failures and malformed payloads return DID resolution metadata", func(t *testing.T) { + t.Run("genuine missing, revoked and malformed records map to resolution metadata; transient errors propagate", func(t *testing.T) { now := time.Now().UTC() service := NewDIDWebService("example.com", nil, &didWebStorageWithErrors{ didWebStorageStub: didWebStorageStub{ @@ -95,7 +97,10 @@ func TestDIDWebServiceDocumentLifecycleAndResolution(t *testing.T) { }, }, errByDID: map[string]error{ - "did:web:example.com:agents:missing": errors.New("not found"), + // A genuine missing record surfaces the storage not-found sentinel. + "did:web:example.com:agents:missing": fmt.Errorf("%w: did:web:example.com:agents:missing", storagepkg.ErrDIDDocumentNotFound), + // A transient failure is a different, non-sentinel error. + "did:web:example.com:agents:flaky": errors.New("connection reset by peer"), }, }, }) @@ -104,6 +109,13 @@ func TestDIDWebServiceDocumentLifecycleAndResolution(t *testing.T) { require.NoError(t, err) require.Equal(t, "notFound", missing.DIDResolutionMetadata.Error) + // Transient storage errors must not be masked as notFound; they propagate + // as a retryable resolution error so the middleware can answer 503. + flaky, err := service.ResolveDID(context.Background(), "did:web:example.com:agents:flaky") + require.Error(t, err) + require.Nil(t, flaky) + require.ErrorIs(t, err, ErrDIDResolutionUnavailable) + revoked, err := service.ResolveDID(context.Background(), "did:web:example.com:agents:revoked") require.NoError(t, err) require.Equal(t, "deactivated", revoked.DIDResolutionMetadata.Error) @@ -184,7 +196,7 @@ func TestDIDWebServiceGetOrCreateAndRevocationPaths(t *testing.T) { }, }, errByDID: map[string]error{ - "did:web:example.com:agents:missing": errors.New("not found"), + "did:web:example.com:agents:missing": fmt.Errorf("%w: did:web:example.com:agents:missing", storagepkg.ErrDIDDocumentNotFound), "did:web:example.com:agents:error": errors.New("database unavailable"), }, }, diff --git a/control-plane/internal/services/did_web_service_test.go b/control-plane/internal/services/did_web_service_test.go index 8b5a369ae..d1fd55bea 100644 --- a/control-plane/internal/services/did_web_service_test.go +++ b/control-plane/internal/services/did_web_service_test.go @@ -5,9 +5,10 @@ package services import ( "context" - "errors" + "fmt" "testing" + "github.com/Agent-Field/agentfield/control-plane/internal/storage" "github.com/Agent-Field/agentfield/control-plane/pkg/types" "github.com/stretchr/testify/require" ) @@ -32,7 +33,7 @@ func (s *didWebStorageStub) GetDIDDocument(_ context.Context, did string) (*type if record, ok := s.docsByDID[did]; ok { return record, nil } - return nil, errors.New("not found") + return nil, fmt.Errorf("%w: %s", storage.ErrDIDDocumentNotFound, did) } func (s *didWebStorageStub) GetDIDDocumentByAgentID(_ context.Context, agentID string) (*types.DIDDocumentRecord, error) { @@ -41,7 +42,7 @@ func (s *didWebStorageStub) GetDIDDocumentByAgentID(_ context.Context, agentID s return record, nil } } - return nil, errors.New("not found") + return nil, fmt.Errorf("%w: %s", storage.ErrDIDDocumentNotFound, agentID) } func (s *didWebStorageStub) RevokeDIDDocument(_ context.Context, _ string) error { diff --git a/control-plane/internal/storage/local_did.go b/control-plane/internal/storage/local_did.go index 92de0ef36..155e97ca6 100644 --- a/control-plane/internal/storage/local_did.go +++ b/control-plane/internal/storage/local_did.go @@ -24,6 +24,11 @@ var ErrDIDAllocationConflict = errors.New("DID allocation conflict") // ErrDIDNodeAlreadyRegistered means another active identity owns the node or component name. var ErrDIDNodeAlreadyRegistered = errors.New("DID node or component already registered") +// ErrDIDDocumentNotFound means no DID document exists for the requested DID. +// It lets callers distinguish a genuine missing record from a transient +// storage failure, so a momentary DB error is not reported as "not found". +var ErrDIDDocumentNotFound = errors.New("DID document not found") + const didPostgresLockTimeout = "15s" func isSQLiteContention(err error) bool { diff --git a/control-plane/internal/storage/local_events.go b/control-plane/internal/storage/local_events.go index 27f8a781a..ee0168608 100644 --- a/control-plane/internal/storage/local_events.go +++ b/control-plane/internal/storage/local_events.go @@ -743,7 +743,7 @@ func (ls *LocalStorage) GetDIDDocument(ctx context.Context, did string) (*types. ) if err != nil { if err == sql.ErrNoRows { - return nil, fmt.Errorf("DID document not found: %s", did) + return nil, fmt.Errorf("%w: %s", ErrDIDDocumentNotFound, did) } return nil, fmt.Errorf("failed to get DID document: %w", err) }