diff --git a/.env.dev b/.env.dev --- a/.env.dev +++ b/.env.dev @@ -72,12 +72,12 @@ # ============================================================================= # Jetstream WebSocket URL for real-time atProto events # # Production: Use Bluesky's public Jetstream (indexes entire network) -# JETSTREAM_URL=wss://jetstream2.us-east.bsky.network/subscribe?wantedCollections=app.bsky.actor.profile +# JETSTREAM_URL=wss://jetstream2.us-east.bsky.network/subscribe?wantedCollections=social.coves.actor.profile # # Local E2E Testing: Use local Jetstream (indexes only local PDS) # 1. Start local Jetstream: docker-compose --profile jetstream up pds jetstream # 2. Use this URL: -JETSTREAM_URL=ws://localhost:6008/subscribe?wantedCollections=app.bsky.actor.profile +JETSTREAM_URL=ws://localhost:6008/subscribe?wantedCollections=social.coves.actor.profile # Optional: Filter events to specific PDS # JETSTREAM_PDS_FILTER=http://localhost:3001 diff --git a/.env.dev.example b/.env.dev.example --- a/.env.dev.example +++ b/.env.dev.example @@ -61,7 +61,7 @@ # ============================================================================= # Jetstream Configuration # ============================================================================= # User profile indexing - wantedCollections filters to profile events only -JETSTREAM_URL=ws://localhost:6008/subscribe?wantedCollections=app.bsky.actor.profile +JETSTREAM_URL=ws://localhost:6008/subscribe?wantedCollections=social.coves.actor.profile # ============================================================================= # Identity Resolution diff --git a/.env.prod.example b/.env.prod.example --- a/.env.prod.example +++ b/.env.prod.example @@ -127,7 +127,7 @@ # ============================================================================= # Jetstream Configuration (Real-time Event Indexing) # ============================================================================= # User profile indexing -JETSTREAM_URL=wss://jetstream2.us-east.bsky.network/subscribe?wantedCollections=app.bsky.actor.profile +JETSTREAM_URL=wss://jetstream2.us-east.bsky.network/subscribe?wantedCollections=social.coves.actor.profile # Optional: Filter Jetstream events to specific PDS # JETSTREAM_PDS_FILTER=pds.coves.social diff --git a/cmd/server/main.go b/cmd/server/main.go --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -320,7 +320,7 @@ // Start Jetstream consumer for read-forward user indexing jetstreamURL := os.Getenv("JETSTREAM_URL") if jetstreamURL == "" { - jetstreamURL = "wss://jetstream2.us-east.bsky.network/subscribe?wantedCollections=app.bsky.actor.profile" + jetstreamURL = "wss://jetstream2.us-east.bsky.network/subscribe?wantedCollections=social.coves.actor.profile" } pdsFilter := os.Getenv("JETSTREAM_PDS_FILTER") // Optional: filter to specific PDS @@ -728,7 +728,7 @@ log.Println(" - Indexing: social.coves.community.comment CREATE/UPDATE/DELETE operations") log.Println(" - Updating: Post comment counts and comment reply counts atomically") // Register XRPC routes - routes.RegisterUserRoutes(r, userService, authMiddleware, blobService) + routes.RegisterUserRoutes(r, userService, authMiddleware, oauthClient.ClientApp) log.Println("User XRPC endpoints registered") log.Println(" - GET /xrpc/social.coves.actor.getprofile (public)") log.Println(" - POST /xrpc/social.coves.actor.signup (public)") diff --git a/internal/api/handlers/user/update_profile.go b/internal/api/handlers/user/update_profile.go --- a/internal/api/handlers/user/update_profile.go +++ b/internal/api/handlers/user/update_profile.go @@ -1,21 +1,27 @@ package user import ( - "bytes" "context" "encoding/json" "errors" "fmt" - "io" "log/slog" "net/http" - "time" "Coves/internal/api/middleware" - "Coves/internal/core/blobs" + "Coves/internal/atproto/pds" - oauthlib "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/auth/oauth" ) + +// CovesProfileCollection is the atProto collection for Coves user profiles. +// NOTE: This constant is intentionally duplicated in internal/atproto/jetstream/user_consumer.go +// to avoid circular dependencies between packages. Keep both definitions in sync. +const CovesProfileCollection = "social.coves.actor.profile" + +// PDSClientFactory creates PDS clients from session data. +// Used to allow injection of different auth mechanisms (OAuth for production, password for E2E tests). +type PDSClientFactory func(ctx context.Context, session *oauth.ClientSessionData) (pds.Client, error) const ( // MaxDisplayNameLength is the maximum allowed length for display names (per atProto lexicon) @@ -30,15 +36,6 @@ // MaxRequestBodySize is the maximum request body size (10MB to accommodate base64 overhead) MaxRequestBodySize = 10_000_000 ) -// pdsError represents an error returned from the PDS with a specific status code -type pdsError struct { - StatusCode int -} - -func (e *pdsError) Error() string { - return fmt.Sprintf("PDS returned error %d", e.StatusCode) -} - // UpdateProfileRequest represents the request body for updating a user profile type UpdateProfileRequest struct { DisplayName *string `json:"displayName,omitempty"` @@ -55,47 +52,52 @@ URI string `json:"uri"` CID string `json:"cid"` } -// userBlobOwner implements blobs.BlobOwner for users -// This allows us to use the blob service to upload blobs on behalf of users -type userBlobOwner struct { - pdsURL string - accessToken string +// UpdateProfileHandler handles POST /xrpc/social.coves.actor.updateProfile +// This endpoint allows authenticated users to update their Coves profile on their PDS. +// It validates inputs, uploads any provided blobs, and writes the profile record. +type UpdateProfileHandler struct { + oauthClient *oauth.ClientApp // For creating authenticated PDS clients (production) + pdsClientFactory PDSClientFactory // Optional: custom factory for testing } -// GetPDSURL returns the PDS URL for this user -func (u *userBlobOwner) GetPDSURL() string { - return u.pdsURL +// NewUpdateProfileHandler creates a new update profile handler. +// Panics if oauthClient is nil - use NewUpdateProfileHandlerWithFactory for testing. +func NewUpdateProfileHandler(oauthClient *oauth.ClientApp) *UpdateProfileHandler { + if oauthClient == nil { + panic("NewUpdateProfileHandler: oauthClient is required") + } + return &UpdateProfileHandler{ + oauthClient: oauthClient, + } } -// GetPDSAccessToken returns the access token for authenticating with the PDS -func (u *userBlobOwner) GetPDSAccessToken() string { - return u.accessToken -} - -// UpdateProfileHandler handles POST /xrpc/social.coves.actor.updateProfile -// This endpoint allows authenticated users to update their profile on their PDS. -// The handler: -// 1. Validates the user is authenticated via OAuth -// 2. Validates avatar/banner size and mime type constraints -// 3. Uploads any provided blobs to the user's PDS -// 4. Puts the profile record to the user's PDS via com.atproto.repo.putRecord -type UpdateProfileHandler struct { - blobService blobs.Service - httpClient *http.Client // For making PDS calls +// NewUpdateProfileHandlerWithFactory creates a new update profile handler with a custom PDS client factory. +// This is primarily for E2E testing with password-based authentication instead of OAuth. +// Panics if factory is nil. +func NewUpdateProfileHandlerWithFactory(factory PDSClientFactory) *UpdateProfileHandler { + if factory == nil { + panic("NewUpdateProfileHandlerWithFactory: factory is required") + } + return &UpdateProfileHandler{ + pdsClientFactory: factory, + } } -// NewUpdateProfileHandler creates a new update profile handler -func NewUpdateProfileHandler(blobService blobs.Service, httpClient *http.Client) *UpdateProfileHandler { - // Use default client if none provided - if httpClient == nil { - httpClient = &http.Client{ - Timeout: 30 * time.Second, - } +// getPDSClient creates a PDS client from an OAuth session. +// If a custom factory was provided (for testing), uses that. +// Otherwise, uses DPoP authentication via indigo's ClientApp for proper OAuth token handling. +func (h *UpdateProfileHandler) getPDSClient(ctx context.Context, session *oauth.ClientSessionData) (pds.Client, error) { + // Use custom factory if provided (e.g., for E2E testing with password auth) + if h.pdsClientFactory != nil { + return h.pdsClientFactory(ctx, session) } - return &UpdateProfileHandler{ - blobService: blobService, - httpClient: httpClient, + + // Production path: use OAuth with DPoP + if h.oauthClient == nil { + return nil, fmt.Errorf("OAuth client not configured") } + + return pds.NewFromOAuthSession(ctx, h.oauthClient, session) } // ServeHTTP handles the update profile request @@ -122,9 +124,7 @@ writeUpdateProfileError(w, http.StatusUnauthorized, "MissingSession", "Missing PDS credentials") return } - pdsURL := session.HostURL - accessToken := session.AccessToken - if pdsURL == "" || accessToken == "" { + if session.HostURL == "" { writeUpdateProfileError(w, http.StatusUnauthorized, "MissingCredentials", "Missing PDS credentials") return } @@ -186,12 +186,21 @@ return } } - // 4. Create blob owner for user (implements blobs.BlobOwner interface) - owner := &userBlobOwner{pdsURL: pdsURL, accessToken: accessToken} + // 4. Create PDS client (uses factory if provided, otherwise OAuth with DPoP) + pdsClient, err := h.getPDSClient(ctx, session) + if err != nil { + slog.Error("failed to create PDS client", + slog.String("did", userDID), + slog.String("error", err.Error()), + ) + writeUpdateProfileError(w, http.StatusUnauthorized, "SessionError", + "Failed to restore session. Please sign in again.") + return + } // 5. Build profile record profile := map[string]interface{}{ - "$type": "app.bsky.actor.profile", + "$type": CovesProfileCollection, } // Add displayName if provided @@ -206,13 +215,23 @@ } // 6. Upload avatar blob if provided if len(req.AvatarBlob) > 0 { - avatarRef, err := h.blobService.UploadBlob(ctx, owner, req.AvatarBlob, req.AvatarMimeType) + avatarRef, err := pdsClient.UploadBlob(ctx, req.AvatarBlob, req.AvatarMimeType) if err != nil { slog.Error("failed to upload avatar blob", slog.String("did", userDID), slog.String("error", err.Error()), ) - writeUpdateProfileError(w, http.StatusInternalServerError, "BlobUploadFailed", "Failed to upload avatar") + // Map specific PDS errors to user-friendly messages + switch { + case errors.Is(err, pds.ErrUnauthorized), errors.Is(err, pds.ErrForbidden): + writeUpdateProfileError(w, http.StatusUnauthorized, "AuthExpired", "Your session may have expired. Please re-authenticate.") + case errors.Is(err, pds.ErrRateLimited): + writeUpdateProfileError(w, http.StatusTooManyRequests, "RateLimited", "Too many requests. Please try again later.") + case errors.Is(err, pds.ErrPayloadTooLarge): + writeUpdateProfileError(w, http.StatusRequestEntityTooLarge, "AvatarTooLarge", "Avatar exceeds PDS size limit.") + default: + writeUpdateProfileError(w, http.StatusInternalServerError, "BlobUploadFailed", "Failed to upload avatar") + } return } if avatarRef == nil || avatarRef.Ref == nil || avatarRef.Type == "" { @@ -230,13 +249,23 @@ } // 7. Upload banner blob if provided if len(req.BannerBlob) > 0 { - bannerRef, err := h.blobService.UploadBlob(ctx, owner, req.BannerBlob, req.BannerMimeType) + bannerRef, err := pdsClient.UploadBlob(ctx, req.BannerBlob, req.BannerMimeType) if err != nil { slog.Error("failed to upload banner blob", slog.String("did", userDID), slog.String("error", err.Error()), ) - writeUpdateProfileError(w, http.StatusInternalServerError, "BlobUploadFailed", "Failed to upload banner") + // Map specific PDS errors to user-friendly messages + switch { + case errors.Is(err, pds.ErrUnauthorized), errors.Is(err, pds.ErrForbidden): + writeUpdateProfileError(w, http.StatusUnauthorized, "AuthExpired", "Your session may have expired. Please re-authenticate.") + case errors.Is(err, pds.ErrRateLimited): + writeUpdateProfileError(w, http.StatusTooManyRequests, "RateLimited", "Too many requests. Please try again later.") + case errors.Is(err, pds.ErrPayloadTooLarge): + writeUpdateProfileError(w, http.StatusRequestEntityTooLarge, "BannerTooLarge", "Banner exceeds PDS size limit.") + default: + writeUpdateProfileError(w, http.StatusInternalServerError, "BlobUploadFailed", "Failed to upload banner") + } return } if bannerRef == nil || bannerRef.Ref == nil || bannerRef.Type == "" { @@ -253,29 +282,24 @@ } } // 8. Put profile record to PDS using com.atproto.repo.putRecord - uri, cid, err := h.putProfileRecord(ctx, session, userDID, profile) + uri, cid, err := pdsClient.PutRecord(ctx, CovesProfileCollection, "self", profile, "") if err != nil { slog.Error("failed to put profile record to PDS", slog.String("did", userDID), - slog.String("pds_url", pdsURL), + slog.String("pds_url", session.HostURL), slog.String("error", err.Error()), ) - // Map PDS status codes to user-friendly messages - var pdsErr *pdsError - if errors.As(err, &pdsErr) { - switch pdsErr.StatusCode { - case http.StatusUnauthorized, http.StatusForbidden: - writeUpdateProfileError(w, http.StatusUnauthorized, "AuthExpired", "Your session may have expired. Please re-authenticate.") - return - case http.StatusTooManyRequests: - writeUpdateProfileError(w, http.StatusTooManyRequests, "RateLimited", "Too many requests. Please try again later.") - return - case http.StatusRequestEntityTooLarge: - writeUpdateProfileError(w, http.StatusBadRequest, "PayloadTooLarge", "Profile data exceeds PDS limits.") - return - } + // Map PDS errors to user-friendly messages + switch { + case errors.Is(err, pds.ErrUnauthorized), errors.Is(err, pds.ErrForbidden): + writeUpdateProfileError(w, http.StatusUnauthorized, "AuthExpired", "Your session may have expired. Please re-authenticate.") + case errors.Is(err, pds.ErrRateLimited): + writeUpdateProfileError(w, http.StatusTooManyRequests, "RateLimited", "Too many requests. Please try again later.") + case errors.Is(err, pds.ErrPayloadTooLarge): + writeUpdateProfileError(w, http.StatusRequestEntityTooLarge, "PayloadTooLarge", "Profile data exceeds PDS size limit.") + default: + writeUpdateProfileError(w, http.StatusInternalServerError, "PDSError", "Failed to update profile") } - writeUpdateProfileError(w, http.StatusInternalServerError, "PDSError", "Failed to update profile") return } @@ -301,86 +325,6 @@ slog.String("did", userDID), slog.String("error", writeErr.Error()), ) } -} - -// putProfileRecord calls com.atproto.repo.putRecord on the user's PDS -// This creates or updates the user's profile record at: -// at://{did}/app.bsky.actor.profile/self -func (h *UpdateProfileHandler) putProfileRecord(ctx context.Context, session *oauthlib.ClientSessionData, did string, profile map[string]interface{}) (string, string, error) { - pdsURL := session.HostURL - accessToken := session.AccessToken - - // Build the putRecord request body - putRecordReq := map[string]interface{}{ - "repo": did, - "collection": "app.bsky.actor.profile", - "rkey": "self", - "record": profile, - } - - reqBody, err := json.Marshal(putRecordReq) - if err != nil { - return "", "", fmt.Errorf("failed to marshal putRecord request: %w", err) - } - - // Build the endpoint URL - endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.putRecord", pdsURL) - - // Create the HTTP request - req, err := http.NewRequestWithContext(ctx, "POST", endpoint, bytes.NewReader(reqBody)) - if err != nil { - return "", "", fmt.Errorf("failed to create PDS request: %w", err) - } - - // Set headers - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+accessToken) - - // Execute the request - resp, err := h.httpClient.Do(req) - if err != nil { - return "", "", fmt.Errorf("PDS request failed: %w", err) - } - defer func() { - if closeErr := resp.Body.Close(); closeErr != nil { - slog.Warn("failed to close PDS response body", slog.String("error", closeErr.Error())) - } - }() - - // Read response body - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", "", fmt.Errorf("failed to read PDS response: %w", err) - } - - // Check for errors - if resp.StatusCode != http.StatusOK { - // Truncate error body for logging to prevent leaking sensitive data - bodyPreview := string(body) - if len(bodyPreview) > 200 { - bodyPreview = bodyPreview[:200] + "... (truncated)" - } - slog.Error("PDS putRecord failed", - slog.Int("status", resp.StatusCode), - slog.String("body", bodyPreview), - ) - return "", "", &pdsError{StatusCode: resp.StatusCode} - } - - // Parse the successful response - var result struct { - URI string `json:"uri"` - CID string `json:"cid"` - } - if err := json.Unmarshal(body, &result); err != nil { - return "", "", fmt.Errorf("failed to parse PDS response: %w", err) - } - - if result.URI == "" || result.CID == "" { - return "", "", fmt.Errorf("PDS response missing required fields (uri or cid)") - } - - return result.URI, result.CID, nil } // isValidImageMimeType checks if the MIME type is allowed for profile images diff --git a/internal/api/handlers/user/update_profile_test.go b/internal/api/handlers/user/update_profile_test.go --- a/internal/api/handlers/user/update_profile_test.go +++ b/internal/api/handlers/user/update_profile_test.go @@ -11,51 +11,78 @@ "strings" "testing" "Coves/internal/api/middleware" + "Coves/internal/atproto/pds" "Coves/internal/core/blobs" oauthlib "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/bluesky-social/indigo/atproto/syntax" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" ) -// MockBlobService is a mock implementation of blobs.Service for testing -type MockBlobService struct { - mock.Mock +// mockPDSClient implements pds.Client for testing error paths +type mockPDSClient struct { + uploadBlobError error + uploadBlobRef *blobs.BlobRef + putRecordError error + putRecordURI string + putRecordCID string +} + +func (m *mockPDSClient) CreateRecord(_ context.Context, _ string, _ string, _ any) (string, string, error) { + return "", "", nil +} + +func (m *mockPDSClient) DeleteRecord(_ context.Context, _ string, _ string) error { + return nil +} + +func (m *mockPDSClient) ListRecords(_ context.Context, _ string, _ int, _ string) (*pds.ListRecordsResponse, error) { + return nil, nil } -func (m *MockBlobService) UploadBlobFromURL(ctx context.Context, owner blobs.BlobOwner, imageURL string) (*blobs.BlobRef, error) { - args := m.Called(ctx, owner, imageURL) - if args.Get(0) == nil { - return nil, args.Error(1) +func (m *mockPDSClient) GetRecord(_ context.Context, _ string, _ string) (*pds.RecordResponse, error) { + return nil, nil +} + +func (m *mockPDSClient) PutRecord(_ context.Context, _ string, _ string, _ any, _ string) (string, string, error) { + if m.putRecordError != nil { + return "", "", m.putRecordError } - return args.Get(0).(*blobs.BlobRef), args.Error(1) + return m.putRecordURI, m.putRecordCID, nil } -func (m *MockBlobService) UploadBlob(ctx context.Context, owner blobs.BlobOwner, data []byte, mimeType string) (*blobs.BlobRef, error) { - args := m.Called(ctx, owner, data, mimeType) - if args.Get(0) == nil { - return nil, args.Error(1) +func (m *mockPDSClient) UploadBlob(_ context.Context, _ []byte, _ string) (*blobs.BlobRef, error) { + if m.uploadBlobError != nil { + return nil, m.uploadBlobError } - return args.Get(0).(*blobs.BlobRef), args.Error(1) + return m.uploadBlobRef, nil } -// MockPDSClient is a mock HTTP client for PDS interactions -type MockPDSClient struct { - mock.Mock +func (m *mockPDSClient) DID() string { + return "did:plc:test123" } -// mockRoundTripper implements http.RoundTripper for testing -type mockRoundTripper struct { - mock.Mock +func (m *mockPDSClient) HostURL() string { + return "https://test.pds.example" } -func (m *mockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - args := m.Called(req) - if args.Get(0) == nil { - return nil, args.Error(1) +// createMockFactory creates a PDSClientFactory that returns the given mock client +func createMockFactory(client pds.Client, err error) PDSClientFactory { + return func(_ context.Context, _ *oauthlib.ClientSessionData) (pds.Client, error) { + if err != nil { + return nil, err + } + return client, nil } - return args.Get(0).(*http.Response), args.Error(1) +} + +// createTestHandler creates a handler with a mock factory for testing validation paths +// that don't require actual PDS client operations +func createTestHandler() *UpdateProfileHandler { + // Use a factory that will never be called (tests exit before PDS client creation) + return NewUpdateProfileHandlerWithFactory(func(_ context.Context, _ *oauthlib.ClientSessionData) (pds.Client, error) { + return nil, errors.New("mock factory should not be called in validation tests") + }) } // createTestOAuthSession creates a test OAuth session for testing @@ -70,17 +97,15 @@ } } // setTestOAuthSession sets both user DID and OAuth session in context -func setTestOAuthSession(ctx context.Context, userDID string, session *oauthlib.ClientSessionData) context.Context { - ctx = middleware.SetTestUserDID(ctx, userDID) - ctx = context.WithValue(ctx, middleware.OAuthSessionKey, session) - ctx = context.WithValue(ctx, middleware.UserAccessToken, session.AccessToken) - return ctx +func setTestOAuthSession(req *http.Request, userDID string, session *oauthlib.ClientSessionData) *http.Request { + ctx := middleware.SetTestUserDID(req.Context(), userDID) + ctx = middleware.SetTestOAuthSession(ctx, session) + return req.WithContext(ctx) } // TestUpdateProfileHandler_Unauthenticated tests that unauthenticated requests return 401 func TestUpdateProfileHandler_Unauthenticated(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) + handler := createTestHandler() reqBody := UpdateProfileRequest{ DisplayName: strPtr("Test User"), @@ -96,14 +121,11 @@ handler.ServeHTTP(w, req) assert.Equal(t, http.StatusUnauthorized, w.Code) assert.Contains(t, w.Body.String(), "AuthRequired") - - mockBlobService.AssertNotCalled(t, "UploadBlob", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } // TestUpdateProfileHandler_MissingOAuthSession tests that missing OAuth session returns 401 func TestUpdateProfileHandler_MissingOAuthSession(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) + handler := createTestHandler() reqBody := UpdateProfileRequest{ DisplayName: strPtr("Test User"), @@ -126,16 +148,14 @@ } // TestUpdateProfileHandler_InvalidRequestBody tests that invalid JSON returns 400 func TestUpdateProfileHandler_InvalidRequestBody(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) + handler := createTestHandler() req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", strings.NewReader("not valid json")) req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -146,8 +166,7 @@ } // TestUpdateProfileHandler_AvatarSizeExceedsLimit tests that avatar over 1MB is rejected func TestUpdateProfileHandler_AvatarSizeExceedsLimit(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) + handler := createTestHandler() // Create avatar blob larger than 1MB (1,000,001 bytes) largeBlob := make([]byte, 1_000_001) @@ -164,22 +183,18 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) assert.Contains(t, w.Body.String(), "Avatar exceeds 1MB limit") - - mockBlobService.AssertNotCalled(t, "UploadBlob", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } // TestUpdateProfileHandler_BannerSizeExceedsLimit tests that banner over 2MB is rejected func TestUpdateProfileHandler_BannerSizeExceedsLimit(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) + handler := createTestHandler() // Create banner blob larger than 2MB (2,000,001 bytes) largeBlob := make([]byte, 2_000_001) @@ -196,22 +211,18 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) assert.Contains(t, w.Body.String(), "Banner exceeds 2MB limit") - - mockBlobService.AssertNotCalled(t, "UploadBlob", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } // TestUpdateProfileHandler_InvalidAvatarMimeType tests that invalid avatar mime type is rejected func TestUpdateProfileHandler_InvalidAvatarMimeType(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) + handler := createTestHandler() reqBody := UpdateProfileRequest{ DisplayName: strPtr("Test User"), @@ -225,8 +236,7 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -237,8 +247,7 @@ } // TestUpdateProfileHandler_InvalidBannerMimeType tests that invalid banner mime type is rejected func TestUpdateProfileHandler_InvalidBannerMimeType(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) + handler := createTestHandler() reqBody := UpdateProfileRequest{ DisplayName: strPtr("Test User"), @@ -252,8 +261,7 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -262,77 +270,36 @@ assert.Equal(t, http.StatusBadRequest, w.Code) assert.Contains(t, w.Body.String(), "Invalid banner mime type") } -// TestUpdateProfileHandler_ValidMimeTypes tests that all valid mime types are accepted -func TestUpdateProfileHandler_ValidMimeTypes(t *testing.T) { - validMimeTypes := []string{"image/png", "image/jpeg", "image/webp"} +// TestUpdateProfileHandler_MethodNotAllowed tests that non-POST methods are rejected +func TestUpdateProfileHandler_MethodNotAllowed(t *testing.T) { + handler := createTestHandler() - for _, mimeType := range validMimeTypes { - t.Run(mimeType, func(t *testing.T) { - mockBlobService := new(MockBlobService) - - // Set up mock PDS server for putRecord - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreicid123", - }) - })) - defer mockPDS.Close() - - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) - - avatarData := []byte("fake avatar image data") - expectedBlobRef := &blobs.BlobRef{ - Type: "blob", - Ref: map[string]string{"$link": "bafyreiabc123"}, - MimeType: mimeType, - Size: len(avatarData), - } - - mockBlobService.On("UploadBlob", mock.Anything, mock.Anything, avatarData, mimeType). - Return(expectedBlobRef, nil) - - reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Test User"), - AvatarBlob: avatarData, - AvatarMimeType: mimeType, - } - body, _ := json.Marshal(reqBody) + methods := []string{http.MethodGet, http.MethodPut, http.MethodDelete, http.MethodPatch} - req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") + for _, method := range methods { + t.Run(method, func(t *testing.T) { + req := httptest.NewRequest(method, "/xrpc/social.coves.actor.updateProfile", nil) testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL // Point to mock PDS - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - // Should succeed or fail at PDS call, not at validation - // We just verify the mime type validation passed - assert.NotEqual(t, http.StatusBadRequest, w.Code) - mockBlobService.AssertExpectations(t) + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) }) } } -// TestUpdateProfileHandler_AvatarBlobUploadFailure tests handling of blob upload failure -func TestUpdateProfileHandler_AvatarBlobUploadFailure(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) - - avatarData := []byte("fake avatar image data") - mockBlobService.On("UploadBlob", mock.Anything, mock.Anything, avatarData, "image/jpeg"). - Return(nil, errors.New("PDS upload failed")) +// TestUpdateProfileHandler_AvatarBlobWithoutMimeType tests that providing blob without mime type fails +func TestUpdateProfileHandler_AvatarBlobWithoutMimeType(t *testing.T) { + handler := createTestHandler() reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Test User"), - AvatarBlob: avatarData, - AvatarMimeType: "image/jpeg", + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), + // Missing AvatarMimeType } body, _ := json.Marshal(reqBody) @@ -341,31 +308,23 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "Failed to upload avatar") - - mockBlobService.AssertExpectations(t) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "mime type") } -// TestUpdateProfileHandler_BannerBlobUploadFailure tests handling of banner blob upload failure -func TestUpdateProfileHandler_BannerBlobUploadFailure(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) - - bannerData := []byte("fake banner image data") - mockBlobService.On("UploadBlob", mock.Anything, mock.Anything, bannerData, "image/png"). - Return(nil, errors.New("PDS upload failed")) +// TestUpdateProfileHandler_BannerBlobWithoutMimeType tests that providing banner without mime type fails +func TestUpdateProfileHandler_BannerBlobWithoutMimeType(t *testing.T) { + handler := createTestHandler() reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Test User"), - BannerBlob: bannerData, - BannerMimeType: "image/png", + DisplayName: strPtr("Test User"), + BannerBlob: []byte("fake image data"), + // Missing BannerMimeType } body, _ := json.Marshal(reqBody) @@ -374,51 +333,46 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "Failed to upload banner") - - mockBlobService.AssertExpectations(t) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "mime type") } -// TestUpdateProfileHandler_PartialUpdateDisplayNameOnly tests updating only displayName (no blobs) -func TestUpdateProfileHandler_PartialUpdateDisplayNameOnly(t *testing.T) { - mockBlobService := new(MockBlobService) +// TestUpdateProfileHandler_DisplayNameTooLong tests that displayName exceeding limit is rejected +func TestUpdateProfileHandler_DisplayNameTooLong(t *testing.T) { + handler := createTestHandler() - // Mock PDS server for putRecord - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Verify it's the right endpoint - assert.Equal(t, "/xrpc/com.atproto.repo.putRecord", r.URL.Path) + longName := strings.Repeat("a", MaxDisplayNameLength+1) + reqBody := UpdateProfileRequest{ + DisplayName: &longName, + } + body, _ := json.Marshal(reqBody) + + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") - // Parse request body - var putReq map[string]interface{} - json.NewDecoder(r.Body).Decode(&putReq) + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) - // Verify record structure - record, ok := putReq["record"].(map[string]interface{}) - assert.True(t, ok, "record should exist") - assert.Equal(t, "app.bsky.actor.profile", record["$type"]) - assert.Equal(t, "Updated Display Name", record["displayName"]) - assert.Nil(t, record["avatar"], "avatar should not be set") - assert.Nil(t, record["banner"], "banner should not be set") + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreicid123", - }) - })) - defer mockPDS.Close() + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "DisplayNameTooLong") +} - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// TestUpdateProfileHandler_BioTooLong tests that bio exceeding limit is rejected +func TestUpdateProfileHandler_BioTooLong(t *testing.T) { + handler := createTestHandler() + longBio := strings.Repeat("a", MaxBioLength+1) reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Updated Display Name"), + Bio: &longBio, } body, _ := json.Marshal(reqBody) @@ -427,49 +381,67 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "BioTooLong") +} - var response UpdateProfileResponse - json.Unmarshal(w.Body.Bytes(), &response) - assert.Contains(t, response.URI, "did:plc:testuser123") - assert.NotEmpty(t, response.CID) +// TestUpdateProfileHandler_MissingHostURL tests that missing PDS host URL returns error +func TestUpdateProfileHandler_MissingHostURL(t *testing.T) { + handler := createTestHandler() - // No blob uploads should have been called - mockBlobService.AssertNotCalled(t, "UploadBlob", mock.Anything, mock.Anything, mock.Anything, mock.Anything) -} + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + } + body, _ := json.Marshal(reqBody) -// TestUpdateProfileHandler_PartialUpdateBioOnly tests updating only bio (description) -func TestUpdateProfileHandler_PartialUpdateBioOnly(t *testing.T) { - mockBlobService := new(MockBlobService) + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") - // Mock PDS server for putRecord - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var putReq map[string]interface{} - json.NewDecoder(r.Body).Decode(&putReq) + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + session.HostURL = "" // Missing host URL + req = setTestOAuthSession(req, testDID, session) - record := putReq["record"].(map[string]interface{}) - assert.Equal(t, "This is my updated bio", record["description"]) - assert.Nil(t, record["displayName"], "displayName should not be set if not provided") + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreicid123", + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "Missing PDS credentials") +} + +// TestIsValidImageMimeType tests the mime type validation function +func TestIsValidImageMimeType(t *testing.T) { + validTypes := []string{"image/png", "image/jpeg", "image/webp"} + for _, mt := range validTypes { + t.Run("valid_"+mt, func(t *testing.T) { + assert.True(t, isValidImageMimeType(mt)) }) - })) - defer mockPDS.Close() + } + + invalidTypes := []string{"image/gif", "image/bmp", "application/pdf", "text/plain", "", "image/svg+xml"} + for _, mt := range invalidTypes { + t.Run("invalid_"+mt, func(t *testing.T) { + assert.False(t, isValidImageMimeType(mt)) + }) + } +} + +// ============================================================================ +// PDS Client Error Path Tests +// These tests use mock factories to test error handling for PDS operations +// ============================================================================ - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// TestUpdateProfileHandler_PDSClientCreationFails tests session restoration failure +func TestUpdateProfileHandler_PDSClientCreationFails(t *testing.T) { + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(nil, errors.New("session restoration failed"))) reqBody := UpdateProfileRequest{ - Bio: strPtr("This is my updated bio"), + DisplayName: strPtr("Test User"), } body, _ := json.Marshal(reqBody) @@ -478,70 +450,83 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) - mockBlobService.AssertNotCalled(t, "UploadBlob", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "SessionError") + assert.Contains(t, w.Body.String(), "Failed to restore session") } -// TestUpdateProfileHandler_FullUpdate tests updating displayName, bio, avatar, and banner -func TestUpdateProfileHandler_FullUpdate(t *testing.T) { - mockBlobService := new(MockBlobService) +// TestUpdateProfileHandler_AvatarUploadUnauthorized tests avatar upload auth error +func TestUpdateProfileHandler_AvatarUploadUnauthorized(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobError: pds.ErrUnauthorized, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), + AvatarMimeType: "image/jpeg", + } + body, _ := json.Marshal(reqBody) + + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - avatarData := []byte("avatar image data") - bannerData := []byte("banner image data") + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "AuthExpired") +} - avatarBlobRef := &blobs.BlobRef{ - Type: "blob", - Ref: map[string]string{"$link": "bafyreiavatarcid"}, - MimeType: "image/jpeg", - Size: len(avatarData), +// TestUpdateProfileHandler_AvatarUploadRateLimited tests avatar upload rate limiting +func TestUpdateProfileHandler_AvatarUploadRateLimited(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobError: pds.ErrRateLimited, } - bannerBlobRef := &blobs.BlobRef{ - Type: "blob", - Ref: map[string]string{"$link": "bafyreibannercid"}, - MimeType: "image/png", - Size: len(bannerData), + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), + AvatarMimeType: "image/jpeg", } + body, _ := json.Marshal(reqBody) - mockBlobService.On("UploadBlob", mock.Anything, mock.Anything, avatarData, "image/jpeg"). - Return(avatarBlobRef, nil) - mockBlobService.On("UploadBlob", mock.Anything, mock.Anything, bannerData, "image/png"). - Return(bannerBlobRef, nil) + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") - // Mock PDS server for putRecord - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var putReq map[string]interface{} - json.NewDecoder(r.Body).Decode(&putReq) + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) - record := putReq["record"].(map[string]interface{}) - assert.Equal(t, "Full Update User", record["displayName"]) - assert.Equal(t, "Updated bio with full profile", record["description"]) - assert.NotNil(t, record["avatar"], "avatar should be set") - assert.NotNil(t, record["banner"], "banner should be set") + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreifullcid", - }) - })) - defer mockPDS.Close() + assert.Equal(t, http.StatusTooManyRequests, w.Code) + assert.Contains(t, w.Body.String(), "RateLimited") +} - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// TestUpdateProfileHandler_AvatarUploadPayloadTooLarge tests avatar upload payload size error +func TestUpdateProfileHandler_AvatarUploadPayloadTooLarge(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobError: pds.ErrPayloadTooLarge, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Full Update User"), - Bio: strPtr("Updated bio with full profile"), - AvatarBlob: avatarData, + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), AvatarMimeType: "image/jpeg", - BannerBlob: bannerData, - BannerMimeType: "image/png", } body, _ := json.Marshal(reqBody) @@ -550,39 +535,66 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code) + assert.Contains(t, w.Body.String(), "AvatarTooLarge") +} - var response UpdateProfileResponse - json.Unmarshal(w.Body.Bytes(), &response) - assert.Contains(t, response.URI, "did:plc:testuser123") - assert.Equal(t, "bafyreifullcid", response.CID) +// TestUpdateProfileHandler_BannerUploadUnauthorized tests banner upload auth error +func TestUpdateProfileHandler_BannerUploadUnauthorized(t *testing.T) { + // First upload succeeds (avatar), second fails (banner) + callCount := 0 + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "blob", + Ref: map[string]string{"$link": "bafytest"}, + MimeType: "image/jpeg", + Size: 100, + }, + } + handler := NewUpdateProfileHandlerWithFactory(func(_ context.Context, _ *oauthlib.ClientSessionData) (pds.Client, error) { + // Return a mock that fails on second UploadBlob call + return &mockPDSClientWithCallCounter{ + mockPDSClient: mockClient, + callCount: &callCount, + failOnCall: 2, // Fail on banner upload + failError: pds.ErrUnauthorized, + }, nil + }) - mockBlobService.AssertExpectations(t) -} + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("avatar data"), + AvatarMimeType: "image/jpeg", + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", + } + body, _ := json.Marshal(reqBody) -// TestUpdateProfileHandler_PDSPutRecordFailure tests handling of PDS putRecord failure -func TestUpdateProfileHandler_PDSPutRecordFailure(t *testing.T) { - mockBlobService := new(MockBlobService) + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) - // Mock PDS server that returns an error - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusInternalServerError) - json.NewEncoder(w).Encode(map[string]interface{}{ - "error": "InternalError", - "message": "Failed to update record", - }) - })) - defer mockPDS.Close() + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "AuthExpired") +} + +// TestUpdateProfileHandler_PutRecordUnauthorized tests PutRecord auth error +func TestUpdateProfileHandler_PutRecordUnauthorized(t *testing.T) { + mockClient := &mockPDSClient{ + putRecordError: pds.ErrUnauthorized, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ DisplayName: strPtr("Test User"), @@ -594,50 +606,50 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "Failed to update profile") + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "AuthExpired") } -// TestUpdateProfileHandler_MethodNotAllowed tests that non-POST methods are rejected -func TestUpdateProfileHandler_MethodNotAllowed(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) +// TestUpdateProfileHandler_PutRecordRateLimited tests PutRecord rate limiting +func TestUpdateProfileHandler_PutRecordRateLimited(t *testing.T) { + mockClient := &mockPDSClient{ + putRecordError: pds.ErrRateLimited, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) - methods := []string{http.MethodGet, http.MethodPut, http.MethodDelete, http.MethodPatch} + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + } + body, _ := json.Marshal(reqBody) - for _, method := range methods { - t.Run(method, func(t *testing.T) { - req := httptest.NewRequest(method, "/xrpc/social.coves.actor.updateProfile", nil) + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") - testDID := "did:plc:testuser123" - session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusMethodNotAllowed, w.Code) - }) - } + assert.Equal(t, http.StatusTooManyRequests, w.Code) + assert.Contains(t, w.Body.String(), "RateLimited") } -// TestUpdateProfileHandler_AvatarBlobWithoutMimeType tests that providing blob without mime type fails -func TestUpdateProfileHandler_AvatarBlobWithoutMimeType(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) +// TestUpdateProfileHandler_PutRecordPayloadTooLarge tests PutRecord payload size error +func TestUpdateProfileHandler_PutRecordPayloadTooLarge(t *testing.T) { + mockClient := &mockPDSClient{ + putRecordError: pds.ErrPayloadTooLarge, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ DisplayName: strPtr("Test User"), - AvatarBlob: []byte("fake image data"), - // Missing AvatarMimeType } body, _ := json.Marshal(reqBody) @@ -646,25 +658,24 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Contains(t, w.Body.String(), "mime type") + assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code) + assert.Contains(t, w.Body.String(), "PayloadTooLarge") } -// TestUpdateProfileHandler_BannerBlobWithoutMimeType tests that providing banner without mime type fails -func TestUpdateProfileHandler_BannerBlobWithoutMimeType(t *testing.T) { - mockBlobService := new(MockBlobService) - handler := NewUpdateProfileHandler(mockBlobService, nil) +// TestUpdateProfileHandler_PutRecordForbidden tests PutRecord forbidden error +func TestUpdateProfileHandler_PutRecordForbidden(t *testing.T) { + mockClient := &mockPDSClient{ + putRecordError: pds.ErrForbidden, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ DisplayName: strPtr("Test User"), - BannerBlob: []byte("fake image data"), - // Missing BannerMimeType } body, _ := json.Marshal(reqBody) @@ -673,48 +684,67 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Contains(t, w.Body.String(), "mime type") + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "AuthExpired") } -// TestUpdateProfileHandler_UserBlobOwnerInterface tests that userBlobOwner correctly implements BlobOwner -func TestUpdateProfileHandler_UserBlobOwnerInterface(t *testing.T) { - owner := &userBlobOwner{ - pdsURL: "https://test.pds.example", - accessToken: "test-token-123", +// TestUpdateProfileHandler_Success tests successful profile update +func TestUpdateProfileHandler_Success(t *testing.T) { + mockClient := &mockPDSClient{ + putRecordURI: "at://did:plc:test123/social.coves.actor.profile/self", + putRecordCID: "bafyreifake", } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) - // Verify interface compliance - var _ blobs.BlobOwner = owner + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + Bio: strPtr("Hello world"), + } + body, _ := json.Marshal(reqBody) - assert.Equal(t, "https://test.pds.example", owner.GetPDSURL()) - assert.Equal(t, "test-token-123", owner.GetPDSAccessToken()) -} + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") -// TestUpdateProfileHandler_EmptyRequest tests that empty request body is handled -func TestUpdateProfileHandler_EmptyRequest(t *testing.T) { - mockBlobService := new(MockBlobService) + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) - // Mock PDS server - even empty update should work - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreicid123", - }) - })) - defer mockPDS.Close() + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + + var resp UpdateProfileResponse + err := json.Unmarshal(w.Body.Bytes(), &resp) + assert.NoError(t, err) + assert.Equal(t, "at://did:plc:test123/social.coves.actor.profile/self", resp.URI) + assert.Equal(t, "bafyreifake", resp.CID) +} - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// TestUpdateProfileHandler_SuccessWithAvatar tests successful profile update with avatar +func TestUpdateProfileHandler_SuccessWithAvatar(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "blob", + Ref: map[string]string{"$link": "bafyavatartest"}, + MimeType: "image/jpeg", + Size: 1000, + }, + putRecordURI: "at://did:plc:test123/social.coves.actor.profile/self", + putRecordCID: "bafyreifake", + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) - // Empty JSON object - reqBody := UpdateProfileRequest{} + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), + AvatarMimeType: "image/jpeg", + } body, _ := json.Marshal(reqBody) req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) @@ -722,36 +752,92 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - // Empty update is valid - just puts an empty profile record assert.Equal(t, http.StatusOK, w.Code) } -// TestUpdateProfileHandler_PDSURLFromSession tests that PDS URL is correctly extracted from OAuth session -func TestUpdateProfileHandler_PDSURLFromSession(t *testing.T) { - mockBlobService := new(MockBlobService) +// Helper function to create string pointers +func strPtr(s string) *string { + return &s +} + +// mockPDSClientWithCallCounter wraps mockPDSClient to track call count +type mockPDSClientWithCallCounter struct { + *mockPDSClient + callCount *int + failOnCall int + failError error +} - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Verify request was received at the mock PDS - assert.NotEmpty(t, r.URL.Path) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreicid123", - }) - })) - defer mockPDS.Close() +func (m *mockPDSClientWithCallCounter) UploadBlob(_ context.Context, _ []byte, _ string) (*blobs.BlobRef, error) { + *m.callCount++ + if *m.callCount == m.failOnCall { + return nil, m.failError + } + return m.mockPDSClient.uploadBlobRef, nil +} - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// ============================================================================ +// Constructor Panic Tests +// ============================================================================ + +// TestNewUpdateProfileHandler_NilOAuthClientPanics verifies that passing nil oauthClient panics +func TestNewUpdateProfileHandler_NilOAuthClientPanics(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected NewUpdateProfileHandler to panic with nil oauthClient, but it did not panic") + } else { + // Verify the panic message is as expected + panicMsg, ok := r.(string) + if !ok { + t.Errorf("Expected panic message to be a string, got %T", r) + return + } + assert.Contains(t, panicMsg, "oauthClient is required") + } + }() + + NewUpdateProfileHandler(nil) +} + +// TestNewUpdateProfileHandlerWithFactory_NilFactoryPanics verifies that passing nil factory panics +func TestNewUpdateProfileHandlerWithFactory_NilFactoryPanics(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected NewUpdateProfileHandlerWithFactory to panic with nil factory, but it did not panic") + } else { + // Verify the panic message is as expected + panicMsg, ok := r.(string) + if !ok { + t.Errorf("Expected panic message to be a string, got %T", r) + return + } + assert.Contains(t, panicMsg, "factory is required") + } + }() + + NewUpdateProfileHandlerWithFactory(nil) +} + +// ============================================================================ +// Invalid BlobRef Handling Tests +// ============================================================================ + +// TestUpdateProfileHandler_AvatarUploadReturnsNilRef tests handling of nil BlobRef from avatar upload +func TestUpdateProfileHandler_AvatarUploadReturnsNilRef(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: nil, // Nil BlobRef + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Test User"), + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), + AvatarMimeType: "image/jpeg", } body, _ := json.Marshal(reqBody) @@ -760,46 +846,65 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - // Use the mock server URL - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Contains(t, w.Body.String(), "BlobUploadFailed") + assert.Contains(t, w.Body.String(), "Invalid avatar blob reference") } -// TestUpdateProfileHandler_AvatarExactly1MB tests boundary condition - avatar exactly 1MB should be accepted -func TestUpdateProfileHandler_AvatarExactly1MB(t *testing.T) { - mockBlobService := new(MockBlobService) - - // Create avatar blob exactly 1MB (1,000,000 bytes) - avatarData := make([]byte, 1_000_000) +// TestUpdateProfileHandler_AvatarUploadReturnsNilRefField tests handling of BlobRef with nil Ref field +func TestUpdateProfileHandler_AvatarUploadReturnsNilRefField(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "blob", + Ref: nil, // Nil Ref field + MimeType: "image/jpeg", + Size: 100, + }, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) - expectedBlobRef := &blobs.BlobRef{ - Type: "blob", - Ref: map[string]string{"$link": "bafyreiabc123"}, - MimeType: "image/jpeg", - Size: len(avatarData), + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), + AvatarMimeType: "image/jpeg", } - mockBlobService.On("UploadBlob", mock.Anything, mock.Anything, avatarData, "image/jpeg"). - Return(expectedBlobRef, nil) + body, _ := json.Marshal(reqBody) + + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreicid123", - }) - })) - defer mockPDS.Close() + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Contains(t, w.Body.String(), "BlobUploadFailed") + assert.Contains(t, w.Body.String(), "Invalid avatar blob reference") +} - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// TestUpdateProfileHandler_AvatarUploadReturnsEmptyType tests handling of BlobRef with empty Type +func TestUpdateProfileHandler_AvatarUploadReturnsEmptyType(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "", // Empty Type + Ref: map[string]string{"$link": "bafytest"}, + MimeType: "image/jpeg", + Size: 100, + }, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ - AvatarBlob: avatarData, + DisplayName: strPtr("Test User"), + AvatarBlob: []byte("fake image data"), AvatarMimeType: "image/jpeg", } body, _ := json.Marshal(reqBody) @@ -809,47 +914,64 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) - mockBlobService.AssertExpectations(t) + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Contains(t, w.Body.String(), "BlobUploadFailed") + assert.Contains(t, w.Body.String(), "Invalid avatar blob reference") } -// TestUpdateProfileHandler_BannerExactly2MB tests boundary condition - banner exactly 2MB should be accepted -func TestUpdateProfileHandler_BannerExactly2MB(t *testing.T) { - mockBlobService := new(MockBlobService) +// TestUpdateProfileHandler_BannerUploadReturnsNilRef tests handling of nil BlobRef from banner upload +func TestUpdateProfileHandler_BannerUploadReturnsNilRef(t *testing.T) { + // We need the avatar upload to succeed and banner upload to return nil + callCount := 0 + handler := NewUpdateProfileHandlerWithFactory(func(_ context.Context, _ *oauthlib.ClientSessionData) (pds.Client, error) { + return &mockPDSClientWithNilBannerRef{ + callCount: &callCount, + }, nil + }) - // Create banner blob exactly 2MB (2,000,000 bytes) - bannerData := make([]byte, 2_000_000) - - expectedBlobRef := &blobs.BlobRef{ - Type: "blob", - Ref: map[string]string{"$link": "bafyreiabc123"}, - MimeType: "image/png", - Size: len(bannerData), + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", } - mockBlobService.On("UploadBlob", mock.Anything, mock.Anything, bannerData, "image/png"). - Return(expectedBlobRef, nil) + body, _ := json.Marshal(reqBody) - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "bafyreicid123", - }) - })) - defer mockPDS.Close() + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Contains(t, w.Body.String(), "BlobUploadFailed") + assert.Contains(t, w.Body.String(), "Invalid banner blob reference") +} - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// TestUpdateProfileHandler_BannerUploadReturnsNilRefField tests handling of BlobRef with nil Ref field for banner +func TestUpdateProfileHandler_BannerUploadReturnsNilRefField(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "blob", + Ref: nil, // Nil Ref field - will affect banner since no avatar in request + MimeType: "image/jpeg", + Size: 100, + }, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ - BannerBlob: bannerData, - BannerMimeType: "image/png", + DisplayName: strPtr("Test User"), + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", } body, _ := json.Marshal(reqBody) @@ -858,26 +980,32 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) - mockBlobService.AssertExpectations(t) + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Contains(t, w.Body.String(), "BlobUploadFailed") + assert.Contains(t, w.Body.String(), "Invalid banner blob reference") } -// TestUpdateProfileHandler_PDSNetworkError tests handling of network errors when calling PDS -func TestUpdateProfileHandler_PDSNetworkError(t *testing.T) { - mockBlobService := new(MockBlobService) - - // Create a handler with a client that will fail - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +// TestUpdateProfileHandler_BannerUploadReturnsEmptyType tests handling of BlobRef with empty Type for banner +func TestUpdateProfileHandler_BannerUploadReturnsEmptyType(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "", // Empty Type - will affect banner since no avatar in request + Ref: map[string]string{"$link": "bafytest"}, + MimeType: "image/jpeg", + Size: 100, + }, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Test User"), + DisplayName: strPtr("Test User"), + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", } body, _ := json.Marshal(reqBody) @@ -886,38 +1014,79 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - // Use an invalid URL that will fail connection - session.HostURL = "http://localhost:1" // Port 1 is typically refused - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "Failed to update profile") + assert.Contains(t, w.Body.String(), "BlobUploadFailed") + assert.Contains(t, w.Body.String(), "Invalid banner blob reference") } -// TestUpdateProfileHandler_ResponseFormat tests that response matches expected format -func TestUpdateProfileHandler_ResponseFormat(t *testing.T) { - mockBlobService := new(MockBlobService) +// mockPDSClientWithNilBannerRef returns nil for banner uploads (called when no avatar blob is present) +type mockPDSClientWithNilBannerRef struct { + callCount *int +} - expectedURI := "at://did:plc:testuser123/app.bsky.actor.profile/self" - expectedCID := "bafyreicid456" +func (m *mockPDSClientWithNilBannerRef) CreateRecord(_ context.Context, _ string, _ string, _ any) (string, string, error) { + return "", "", nil +} - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]interface{}{ - "uri": expectedURI, - "cid": expectedCID, - }) - })) - defer mockPDS.Close() +func (m *mockPDSClientWithNilBannerRef) DeleteRecord(_ context.Context, _ string, _ string) error { + return nil +} + +func (m *mockPDSClientWithNilBannerRef) ListRecords(_ context.Context, _ string, _ int, _ string) (*pds.ListRecordsResponse, error) { + return nil, nil +} + +func (m *mockPDSClientWithNilBannerRef) GetRecord(_ context.Context, _ string, _ string) (*pds.RecordResponse, error) { + return nil, nil +} + +func (m *mockPDSClientWithNilBannerRef) PutRecord(_ context.Context, _ string, _ string, _ any, _ string) (string, string, error) { + return "", "", nil +} + +func (m *mockPDSClientWithNilBannerRef) UploadBlob(_ context.Context, _ []byte, _ string) (*blobs.BlobRef, error) { + // Return nil to simulate invalid response + return nil, nil +} - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) +func (m *mockPDSClientWithNilBannerRef) DID() string { + return "did:plc:test123" +} + +func (m *mockPDSClientWithNilBannerRef) HostURL() string { + return "https://test.pds.example" +} + +// ============================================================================ +// Boundary Size Tests +// ============================================================================ + +// TestUpdateProfileHandler_AvatarExactlyAtMaxSize tests that avatar at exactly 1MB is accepted +func TestUpdateProfileHandler_AvatarExactlyAtMaxSize(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "blob", + Ref: map[string]string{"$link": "bafyavatartest"}, + MimeType: "image/jpeg", + Size: MaxAvatarBlobSize, + }, + putRecordURI: "at://did:plc:test123/social.coves.actor.profile/self", + putRecordCID: "bafyreifake", + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + // Create avatar blob at exactly 1MB (1,000,000 bytes) + avatarBlob := make([]byte, MaxAvatarBlobSize) reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Test User"), + DisplayName: strPtr("Test User"), + AvatarBlob: avatarBlob, + AvatarMimeType: "image/jpeg", } body, _ := json.Marshal(reqBody) @@ -926,110 +1095,213 @@ req.Header.Set("Content-Type", "application/json") testDID := "did:plc:testuser123" session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) + req = setTestOAuthSession(req, testDID, session) w := httptest.NewRecorder() handler.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) - var response UpdateProfileResponse - err := json.Unmarshal(w.Body.Bytes(), &response) + var resp UpdateProfileResponse + err := json.Unmarshal(w.Body.Bytes(), &resp) assert.NoError(t, err) - assert.Equal(t, expectedURI, response.URI) - assert.Equal(t, expectedCID, response.CID) + assert.NotEmpty(t, resp.URI) + assert.NotEmpty(t, resp.CID) } -// Helper function to create string pointers -func strPtr(s string) *string { - return &s +// TestUpdateProfileHandler_BannerExactlyAtMaxSize tests that banner at exactly 2MB is accepted +func TestUpdateProfileHandler_BannerExactlyAtMaxSize(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "blob", + Ref: map[string]string{"$link": "bafybannertest"}, + MimeType: "image/jpeg", + Size: MaxBannerBlobSize, + }, + putRecordURI: "at://did:plc:test123/social.coves.actor.profile/self", + putRecordCID: "bafyreifake", + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + // Create banner blob at exactly 2MB (2,000,000 bytes) + bannerBlob := make([]byte, MaxBannerBlobSize) + + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + BannerBlob: bannerBlob, + BannerMimeType: "image/jpeg", + } + body, _ := json.Marshal(reqBody) + + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + + var resp UpdateProfileResponse + err := json.Unmarshal(w.Body.Bytes(), &resp) + assert.NoError(t, err) + assert.NotEmpty(t, resp.URI) + assert.NotEmpty(t, resp.CID) } -// TestUserBlobOwner_ImplementsBlobOwnerInterface verifies interface compliance at compile time -func TestUserBlobOwner_ImplementsBlobOwnerInterface(t *testing.T) { - // This test ensures at compile time that userBlobOwner implements blobs.BlobOwner - var owner blobs.BlobOwner = &userBlobOwner{ - pdsURL: "https://test.example", - accessToken: "token", +// ============================================================================ +// Banner-Specific Error Path Tests +// ============================================================================ + +// TestUpdateProfileHandler_BannerUploadRateLimited tests banner upload rate limiting +func TestUpdateProfileHandler_BannerUploadRateLimited(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobError: pds.ErrRateLimited, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", } - assert.NotNil(t, owner) + body, _ := json.Marshal(reqBody) + + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusTooManyRequests, w.Code) + assert.Contains(t, w.Body.String(), "RateLimited") } -// TestUpdateProfileHandler_PDSReturnsEmptyURIOrCID tests handling when PDS returns 200 but with empty URI or CID -func TestUpdateProfileHandler_PDSReturnsEmptyURIOrCID(t *testing.T) { - testCases := []struct { - name string - response map[string]interface{} - }{ - { - name: "empty URI", - response: map[string]interface{}{ - "uri": "", - "cid": "bafyreicid123", - }, - }, - { - name: "empty CID", - response: map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - "cid": "", - }, - }, - { - name: "missing URI", - response: map[string]interface{}{ - "cid": "bafyreicid123", - }, - }, - { - name: "missing CID", - response: map[string]interface{}{ - "uri": "at://did:plc:testuser123/app.bsky.actor.profile/self", - }, - }, - { - name: "both empty", - response: map[string]interface{}{}, - }, +// TestUpdateProfileHandler_BannerUploadPayloadTooLarge tests banner upload payload size error +func TestUpdateProfileHandler_BannerUploadPayloadTooLarge(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobError: pds.ErrPayloadTooLarge, } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - mockBlobService := new(MockBlobService) + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", + } + body, _ := json.Marshal(reqBody) - // Mock PDS server that returns 200 but with empty/missing fields - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(tc.response) - })) - defer mockPDS.Close() + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") - handler := NewUpdateProfileHandler(mockBlobService, http.DefaultClient) + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) - reqBody := UpdateProfileRequest{ - DisplayName: strPtr("Test User"), - } - body, _ := json.Marshal(reqBody) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) - req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") + assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code) + assert.Contains(t, w.Body.String(), "BannerTooLarge") +} - testDID := "did:plc:testuser123" - session := createTestOAuthSession(testDID) - session.HostURL = mockPDS.URL - ctx := setTestOAuthSession(req.Context(), testDID, session) - req = req.WithContext(ctx) +// TestUpdateProfileHandler_BannerUploadForbidden tests banner upload forbidden error +func TestUpdateProfileHandler_BannerUploadForbidden(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobError: pds.ErrForbidden, + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", + } + body, _ := json.Marshal(reqBody) - // Should return an internal server error because URI/CID are required - assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "PDSError") - }) + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "AuthExpired") +} + +// TestUpdateProfileHandler_BannerUploadGenericError tests banner upload with generic error +func TestUpdateProfileHandler_BannerUploadGenericError(t *testing.T) { + mockClient := &mockPDSClient{ + uploadBlobError: errors.New("network error"), + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + reqBody := UpdateProfileRequest{ + DisplayName: strPtr("Test User"), + BannerBlob: []byte("banner data"), + BannerMimeType: "image/jpeg", } + body, _ := json.Marshal(reqBody) + + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Contains(t, w.Body.String(), "BlobUploadFailed") + assert.Contains(t, w.Body.String(), "Failed to upload banner") } +// ============================================================================ +// Empty Request Success Test +// ============================================================================ + +// TestUpdateProfileHandler_EmptyRequestSuccess tests that an empty request succeeds +// This verifies that when no fields are provided, the profile is still created/updated +// with just the $type field +func TestUpdateProfileHandler_EmptyRequestSuccess(t *testing.T) { + mockClient := &mockPDSClient{ + putRecordURI: "at://did:plc:test123/social.coves.actor.profile/self", + putRecordCID: "bafyreifake", + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + // Empty request - no fields set + reqBody := UpdateProfileRequest{} + body, _ := json.Marshal(reqBody) + + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + testDID := "did:plc:testuser123" + session := createTestOAuthSession(testDID) + req = setTestOAuthSession(req, testDID, session) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + + var resp UpdateProfileResponse + err := json.Unmarshal(w.Body.Bytes(), &resp) + assert.NoError(t, err) + assert.Equal(t, "at://did:plc:test123/social.coves.actor.profile/self", resp.URI) + assert.Equal(t, "bafyreifake", resp.CID) +} diff --git a/internal/api/middleware/auth.go b/internal/api/middleware/auth.go --- a/internal/api/middleware/auth.go +++ b/internal/api/middleware/auth.go @@ -301,6 +301,16 @@ func SetTestUserDID(ctx context.Context, userDID string) context.Context { return context.WithValue(ctx, UserDIDKey, userDID) } +// SetTestOAuthSession sets the OAuth session in the context for testing purposes +// This function should ONLY be used in tests to mock authenticated sessions +func SetTestOAuthSession(ctx context.Context, session *oauthlib.ClientSessionData) context.Context { + ctx = context.WithValue(ctx, OAuthSessionKey, session) + if session != nil { + ctx = context.WithValue(ctx, UserAccessToken, session.AccessToken) + } + return ctx +} + // extractBearerToken extracts the token from a Bearer Authorization header. // HTTP auth schemes are case-insensitive per RFC 7235, so "Bearer", "bearer", "BEARER" are all valid. // Returns the token and true if valid Bearer scheme, empty string and false otherwise. diff --git a/internal/api/routes/user.go b/internal/api/routes/user.go --- a/internal/api/routes/user.go +++ b/internal/api/routes/user.go @@ -3,7 +3,6 @@ import ( "Coves/internal/api/handlers/user" "Coves/internal/api/middleware" - "Coves/internal/core/blobs" "Coves/internal/core/users" "encoding/json" "errors" @@ -11,6 +10,7 @@ "log" "net/http" "strings" + "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/go-chi/chi/v5" ) @@ -26,9 +26,24 @@ userService: userService, } } +// UserRouteOptions contains optional configuration for user routes. +// Use this to inject test dependencies like custom PDS client factories. +type UserRouteOptions struct { + // PDSClientFactory overrides the default OAuth-based PDS client creation. + // If nil, uses OAuth with DPoP (production behavior). + // Set this in E2E tests to use password-based authentication. + PDSClientFactory user.PDSClientFactory +} + // RegisterUserRoutes registers user-related XRPC endpoints on the router // Implements social.coves.actor.* lexicon endpoints -func RegisterUserRoutes(r chi.Router, service users.UserService, authMiddleware *middleware.OAuthAuthMiddleware, blobService blobs.Service) { +func RegisterUserRoutes(r chi.Router, service users.UserService, authMiddleware *middleware.OAuthAuthMiddleware, oauthClient *oauth.ClientApp) { + RegisterUserRoutesWithOptions(r, service, authMiddleware, oauthClient, nil) +} + +// RegisterUserRoutesWithOptions registers user-related XRPC endpoints with optional configuration. +// Use opts to inject test dependencies like custom PDS client factories. +func RegisterUserRoutesWithOptions(r chi.Router, service users.UserService, authMiddleware *middleware.OAuthAuthMiddleware, oauthClient *oauth.ClientApp, opts *UserRouteOptions) { h := NewUserHandler(service) // social.coves.actor.getprofile - query endpoint (public) @@ -46,7 +61,14 @@ // social.coves.actor.updateProfile - procedure endpoint (authenticated) // Updates the authenticated user's profile on their PDS (avatar, banner, displayName, bio). // This writes directly to the user's PDS and the Jetstream consumer will index the change. - updateProfileHandler := user.NewUpdateProfileHandler(blobService, nil) + var updateProfileHandler *user.UpdateProfileHandler + if opts != nil && opts.PDSClientFactory != nil { + // Use custom factory (for E2E tests with password auth) + updateProfileHandler = user.NewUpdateProfileHandlerWithFactory(opts.PDSClientFactory) + } else { + // Use OAuth client for DPoP-authenticated PDS requests (production) + updateProfileHandler = user.NewUpdateProfileHandler(oauthClient) + } r.With(authMiddleware.RequireAuth).Post("/xrpc/social.coves.actor.updateProfile", updateProfileHandler.ServeHTTP) } diff --git a/internal/atproto/jetstream/user_consumer.go b/internal/atproto/jetstream/user_consumer.go --- a/internal/atproto/jetstream/user_consumer.go +++ b/internal/atproto/jetstream/user_consumer.go @@ -15,6 +15,11 @@ "github.com/gorilla/websocket" ) +// CovesProfileCollection is the atProto collection for Coves user profiles. +// NOTE: This constant is intentionally duplicated in internal/api/handlers/user/update_profile.go +// to avoid circular dependencies between packages. Keep both definitions in sync. +const CovesProfileCollection = "social.coves.actor.profile" + // SessionHandleUpdater is an interface for updating OAuth session handles // when identity changes occur. This keeps active sessions in sync with // the user's current handle. @@ -198,7 +203,7 @@ return fmt.Errorf("failed to parse event: %w", err) } // We're interested in identity events (handle updates), account events (new users), - // and commit events (profile updates from app.bsky.actor.profile) + // and commit events (profile updates from social.coves.actor.profile) switch event.Kind { case "identity": return c.handleIdentityEvent(ctx, &event) @@ -262,18 +267,26 @@ // CRITICAL: Purge BOTH old handle and DID from cache // Old handle: alice.bsky.social → did:plc:abc123 (must be removed) if purgeErr := c.identityResolver.Purge(ctx, existingUser.Handle); purgeErr != nil { - log.Printf("Warning: failed to purge old handle cache for %s: %v", existingUser.Handle, purgeErr) + slog.Error("CRITICAL: failed to purge old handle cache", + slog.String("handle", existingUser.Handle), + slog.String("error", purgeErr.Error())) } // DID: did:plc:abc123 → alice.bsky.social (must be removed) if purgeErr := c.identityResolver.Purge(ctx, did); purgeErr != nil { - log.Printf("Warning: failed to purge DID cache for %s: %v", did, purgeErr) + slog.Error("CRITICAL: failed to purge DID cache", + slog.String("did", did), + slog.String("error", purgeErr.Error())) } // Update OAuth session handles to keep mobile/web sessions in sync + // Failure here causes users to see stale handles in their active sessions if c.sessionHandleUpdater != nil { if sessionsUpdated, updateErr := c.sessionHandleUpdater.UpdateHandleByDID(ctx, did, handle); updateErr != nil { - log.Printf("Warning: failed to update OAuth session handles for %s: %v", did, updateErr) + slog.Error("failed to update OAuth session handles (users may see stale handle)", + slog.String("did", did), + slog.String("new_handle", handle), + slog.String("error", updateErr.Error())) } else if sessionsUpdated > 0 { log.Printf("Updated %d OAuth session(s) with new handle: %s", sessionsUpdated, handle) } @@ -304,16 +317,16 @@ return nil } // handleCommitEvent processes commit events for user profile updates -// Only handles app.bsky.actor.profile collection for users already in our database. -// This syncs profile data (displayName, bio, avatar, banner) from Bluesky profiles. +// Only handles social.coves.actor.profile collection for users already in our database. +// This syncs profile data (displayName, bio, avatar, banner) from Coves profiles. func (c *UserEventConsumer) handleCommitEvent(ctx context.Context, event *JetstreamEvent) error { if event.Commit == nil { - slog.Debug("received nil commit in handleCommitEvent", slog.String("did", event.Did)) + slog.Warn("received nil commit in handleCommitEvent (malformed event)", slog.String("did", event.Did)) return nil } - // Only handle app.bsky.actor.profile collection - if event.Commit.Collection != "app.bsky.actor.profile" { + // Only handle social.coves.actor.profile collection + if event.Commit.Collection != CovesProfileCollection { return nil } @@ -343,9 +356,9 @@ // handleProfileUpdate processes profile create/update operations // Extracts displayName, description (bio), avatar, and banner from the record func (c *UserEventConsumer) handleProfileUpdate(ctx context.Context, did string, commit *CommitEvent) error { if commit.Record == nil { - slog.Debug("received nil record in profile commit", + slog.Warn("received nil record in profile commit (profile update silently dropped)", slog.String("did", did), - slog.String("operation", string(commit.Operation))) + slog.String("operation", commit.Operation)) return nil } diff --git a/internal/atproto/jetstream/user_consumer_test.go b/internal/atproto/jetstream/user_consumer_test.go --- a/internal/atproto/jetstream/user_consumer_test.go +++ b/internal/atproto/jetstream/user_consumer_test.go @@ -131,7 +131,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "social.coves.post", // Not app.bsky.actor.profile + Collection: "social.coves.post", // Not CovesProfileCollection RKey: "post123", CID: "bafy123", Record: map[string]interface{}{ @@ -165,7 +165,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -204,7 +204,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -246,7 +246,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -288,7 +288,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -335,7 +335,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -382,7 +382,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -450,7 +450,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "delete", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", }, } @@ -499,7 +499,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev124", Operation: "update", // Update operation instead of create - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy456", Record: map[string]interface{}{ @@ -542,7 +542,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -597,7 +597,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: nil, // No record data @@ -628,7 +628,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ @@ -685,7 +685,7 @@ Kind: "commit", Commit: &CommitEvent{ Rev: "rev123", Operation: "create", - Collection: "app.bsky.actor.profile", + Collection: CovesProfileCollection, RKey: "self", CID: "bafy123", Record: map[string]interface{}{ diff --git a/internal/atproto/pds/client.go b/internal/atproto/pds/client.go --- a/internal/atproto/pds/client.go +++ b/internal/atproto/pds/client.go @@ -4,10 +4,14 @@ // authentication method (OAuth with DPoP or password-based Bearer tokens). package pds import ( + "bytes" "context" "errors" "fmt" + "Coves/internal/core/blobs" + + comatproto "github.com/bluesky-social/indigo/api/atproto" "github.com/bluesky-social/indigo/atproto/atclient" "github.com/bluesky-social/indigo/atproto/syntax" ) @@ -34,6 +38,13 @@ // PutRecord creates or updates a record with optional optimistic locking. // If swapRecord CID is provided, the operation fails if the current CID doesn't match. PutRecord(ctx context.Context, collection string, rkey string, record any, swapRecord string) (uri string, cid string, err error) + + // UploadBlob uploads binary data to the user's PDS repository. + // Returns a BlobRef that can be used in records. + // Note: The mimeType parameter is accepted for interface compatibility, but the PDS + // performs its own MIME type detection from the blob content. The returned BlobRef + // will contain the PDS-detected MIME type. + UploadBlob(ctx context.Context, data []byte, mimeType string) (*blobs.BlobRef, error) // DID returns the authenticated user's DID. DID() string @@ -95,6 +106,10 @@ case 404: return fmt.Errorf("%s: %w: %s", operation, ErrNotFound, apiErr.Message) case 409: return fmt.Errorf("%s: %w: %s", operation, ErrConflict, apiErr.Message) + case 413: + return fmt.Errorf("%s: %w: %s", operation, ErrPayloadTooLarge, apiErr.Message) + case 429: + return fmt.Errorf("%s: %w: %s", operation, ErrRateLimited, apiErr.Message) } } @@ -251,3 +266,18 @@ } return result.URI, result.CID, nil } + +// UploadBlob uploads binary data to the user's PDS repository. +func (c *client) UploadBlob(ctx context.Context, data []byte, mimeType string) (*blobs.BlobRef, error) { + result, err := comatproto.RepoUploadBlob(ctx, c.apiClient, bytes.NewReader(data)) + if err != nil { + return nil, wrapAPIError(err, "uploadBlob") + } + + return &blobs.BlobRef{ + Type: "blob", + Ref: map[string]string{"$link": result.Blob.Ref.String()}, + MimeType: result.Blob.MimeType, + Size: int(result.Blob.Size), + }, nil +} diff --git a/internal/atproto/pds/errors.go b/internal/atproto/pds/errors.go --- a/internal/atproto/pds/errors.go +++ b/internal/atproto/pds/errors.go @@ -18,8 +18,14 @@ // ErrBadRequest indicates the request was malformed or invalid (HTTP 400). ErrBadRequest = errors.New("bad request") - // ErrConflict indicates the record was modified by another operation (HTTP 409). - ErrConflict = errors.New("record was modified by another operation") + // ErrConflict indicates a conflict occurred, such as a record being modified by another operation (HTTP 409). + ErrConflict = errors.New("conflict") + + // ErrRateLimited indicates the request was rejected due to rate limiting (HTTP 429). + ErrRateLimited = errors.New("rate limited") + + // ErrPayloadTooLarge indicates the request payload exceeds PDS limits (HTTP 413). + ErrPayloadTooLarge = errors.New("payload too large") ) // IsAuthError returns true if the error is an authentication/authorization error. diff --git a/internal/core/comments/comment_write_service_test.go b/internal/core/comments/comment_write_service_test.go --- a/internal/core/comments/comment_write_service_test.go +++ b/internal/core/comments/comment_write_service_test.go @@ -2,6 +2,7 @@ package comments import ( "Coves/internal/atproto/pds" + "Coves/internal/core/blobs" "context" "errors" "fmt" @@ -128,6 +129,16 @@ uri := fmt.Sprintf("at://%s/%s/%s", m.did, collection, rkey) cid := fmt.Sprintf("bafytest%d", time.Now().UnixNano()) return uri, cid, nil +} + +func (m *mockPDSClient) UploadBlob(ctx context.Context, data []byte, mimeType string) (*blobs.BlobRef, error) { + // Return a mock blob reference - comments don't use blob uploads + return &blobs.BlobRef{ + Type: "blob", + Ref: map[string]string{"$link": fmt.Sprintf("bafymock%d", time.Now().UnixNano())}, + MimeType: mimeType, + Size: len(data), + }, nil } // mockPDSClientFactory creates mock PDS clients for testing diff --git a/tests/integration/helpers.go b/tests/integration/helpers.go --- a/tests/integration/helpers.go +++ b/tests/integration/helpers.go @@ -477,3 +477,19 @@ return pds.NewFromAccessToken(session.HostURL, session.AccountDID.String(), session.AccessToken) } } + +// UserProfilePasswordAuthPDSClientFactory creates a PDSClientFactory for user profile updates +// that uses password-based Bearer auth. This is for E2E tests that use createSession instead of OAuth. +// The factory extracts the access token and host URL from the session data. +func UserProfilePasswordAuthPDSClientFactory() func(ctx context.Context, session *oauthlib.ClientSessionData) (pds.Client, error) { + return func(ctx context.Context, session *oauthlib.ClientSessionData) (pds.Client, error) { + if session.AccessToken == "" { + return nil, fmt.Errorf("session has no access token") + } + if session.HostURL == "" { + return nil, fmt.Errorf("session has no host URL") + } + + return pds.NewFromAccessToken(session.HostURL, session.AccountDID.String(), session.AccessToken) + } +} diff --git a/tests/integration/user_profile_avatar_e2e_test.go b/tests/integration/user_profile_avatar_e2e_test.go --- a/tests/integration/user_profile_avatar_e2e_test.go +++ b/tests/integration/user_profile_avatar_e2e_test.go @@ -5,7 +5,6 @@ "Coves/internal/api/handlers/user" "Coves/internal/api/routes" "Coves/internal/atproto/identity" "Coves/internal/atproto/jetstream" - "Coves/internal/core/blobs" "Coves/internal/core/users" "Coves/internal/db/postgres" "bytes" @@ -54,7 +53,7 @@ } // TestUserProfileAvatarE2E_UpdateWithAvatar tests the full flow of updating a user profile with an avatar: // 1. User updates profile via Coves API (POST /xrpc/social.coves.actor.updateProfile) -// 2. Profile record is written to PDS (app.bsky.actor.profile) +// 2. Profile record is written to PDS (social.coves.actor.profile) // 3. Jetstream consumer receives and processes the event // 4. GetProfile returns the correct avatar URL func TestUserProfileAvatarE2E_UpdateWithAvatar(t *testing.T) { @@ -92,7 +91,7 @@ // Check if Jetstream is running pdsHostname := strings.TrimPrefix(pdsURL, "http://") pdsHostname = strings.TrimPrefix(pdsHostname, "https://") pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=app.bsky.actor.profile", pdsHostname) + jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) if connErr != nil { @@ -115,15 +114,16 @@ // Setup services userRepo := postgres.NewUserRepository(db) userService := users.NewUserService(userRepo, identityResolver, pdsURL) - blobService := blobs.NewBlobService(pdsURL) // Setup user consumer for processing Jetstream events userConsumer := jetstream.NewUserEventConsumer(userService, identityResolver, jetstreamURL, "") - // Setup HTTP server with all user routes + // Setup HTTP server with all user routes using password-based PDS client for E2E tests e2eAuth := NewE2EOAuthMiddleware() r := chi.NewRouter() - routes.RegisterUserRoutes(r, userService, e2eAuth.OAuthAuthMiddleware, blobService) + routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ + PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), + }) httpServer := httptest.NewServer(r) defer httpServer.Close() @@ -203,7 +203,7 @@ } // Only process profile update events for our user if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "app.bsky.actor.profile" && + event.Commit.Collection == "social.coves.actor.profile" && event.Did == userDID { eventChan <- &event } @@ -279,7 +279,7 @@ t.Logf(" Note: Identity event handling result: %v", handleErr) } // For profile updates, we need to manually process the commit event - // The consumer checks for app.bsky.actor.profile commit events + // The consumer checks for social.coves.actor.profile commit events if realEvent.Kind == "commit" && realEvent.Commit != nil { // Extract profile data from the event and update the user var displayNamePtr, bioPtr, avatarCIDPtr, bannerCIDPtr *string @@ -389,7 +389,7 @@ // Check if Jetstream is running pdsHostname := strings.TrimPrefix(pdsURL, "http://") pdsHostname = strings.TrimPrefix(pdsHostname, "https://") pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=app.bsky.actor.profile", pdsHostname) + jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) if connErr != nil { @@ -411,12 +411,13 @@ // Setup services userRepo := postgres.NewUserRepository(db) userService := users.NewUserService(userRepo, identityResolver, pdsURL) - blobService := blobs.NewBlobService(pdsURL) - // Setup HTTP server + // Setup HTTP server using password-based PDS client for E2E tests e2eAuth := NewE2EOAuthMiddleware() r := chi.NewRouter() - routes.RegisterUserRoutes(r, userService, e2eAuth.OAuthAuthMiddleware, blobService) + routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ + PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), + }) httpServer := httptest.NewServer(r) defer httpServer.Close() @@ -482,7 +483,7 @@ continue } if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "app.bsky.actor.profile" && + event.Commit.Collection == "social.coves.actor.profile" && event.Did == userDID { eventChan <- &event } @@ -624,7 +625,7 @@ // Check if Jetstream is running pdsHostname := strings.TrimPrefix(pdsURL, "http://") pdsHostname = strings.TrimPrefix(pdsHostname, "https://") pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=app.bsky.actor.profile", pdsHostname) + jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) if connErr != nil { @@ -646,12 +647,13 @@ // Setup services userRepo := postgres.NewUserRepository(db) userService := users.NewUserService(userRepo, identityResolver, pdsURL) - blobService := blobs.NewBlobService(pdsURL) - // Setup HTTP server + // Setup HTTP server using password-based PDS client for E2E tests e2eAuth := NewE2EOAuthMiddleware() r := chi.NewRouter() - routes.RegisterUserRoutes(r, userService, e2eAuth.OAuthAuthMiddleware, blobService) + routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ + PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), + }) httpServer := httptest.NewServer(r) defer httpServer.Close() @@ -703,7 +705,7 @@ continue } if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "app.bsky.actor.profile" && + event.Commit.Collection == "social.coves.actor.profile" && event.Did == userDID { eventChan <- &event } @@ -819,7 +821,7 @@ // Check if Jetstream is running pdsHostname := strings.TrimPrefix(pdsURL, "http://") pdsHostname = strings.TrimPrefix(pdsHostname, "https://") pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=app.bsky.actor.profile", pdsHostname) + jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) if connErr != nil { @@ -841,12 +843,13 @@ // Setup services userRepo := postgres.NewUserRepository(db) userService := users.NewUserService(userRepo, identityResolver, pdsURL) - blobService := blobs.NewBlobService(pdsURL) - // Setup HTTP server + // Setup HTTP server using password-based PDS client for E2E tests e2eAuth := NewE2EOAuthMiddleware() r := chi.NewRouter() - routes.RegisterUserRoutes(r, userService, e2eAuth.OAuthAuthMiddleware, blobService) + routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ + PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), + }) httpServer := httptest.NewServer(r) defer httpServer.Close() @@ -884,7 +887,7 @@ continue } if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "app.bsky.actor.profile" && + event.Commit.Collection == "social.coves.actor.profile" && event.Did == userDID { eventChan <- &event } diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -3,12 +3,13 @@ import ( "Coves/internal/api/routes" "Coves/internal/atproto/identity" - "Coves/internal/core/blobs" + "Coves/internal/atproto/pds" "Coves/internal/core/users" "Coves/internal/db/postgres" "context" "database/sql" "encoding/json" + "errors" "fmt" "io" "log" @@ -19,20 +20,20 @@ "strings" "testing" "time" + "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/go-chi/chi/v5" _ "github.com/lib/pq" "github.com/pressly/goose/v3" ) -// stubBlobService is a minimal blob service implementation for tests that don't need it -type stubBlobService struct{} - -func (s *stubBlobService) UploadBlobFromURL(ctx context.Context, owner blobs.BlobOwner, imageURL string) (*blobs.BlobRef, error) { - return nil, fmt.Errorf("stub blob service: UploadBlobFromURL not implemented") -} - -func (s *stubBlobService) UploadBlob(ctx context.Context, owner blobs.BlobOwner, data []byte, mimeType string) (*blobs.BlobRef, error) { - return nil, fmt.Errorf("stub blob service: UploadBlob not implemented") +// testUserRouteOptions returns route options with a dummy PDS client factory. +// Use this for tests that register user routes but don't actually call updateProfile. +func testUserRouteOptions() *routes.UserRouteOptions { + return &routes.UserRouteOptions{ + PDSClientFactory: func(ctx context.Context, session *oauth.ClientSessionData) (pds.Client, error) { + return nil, errors.New("not implemented - test does not use updateProfile") + }, + } } // TestMain controls test setup for the integration package. @@ -237,7 +238,7 @@ // Set up HTTP router with auth middleware r := chi.NewRouter() authMiddleware, _ := CreateTestOAuthMiddleware("did:plc:testuser") - routes.RegisterUserRoutes(r, userService, authMiddleware, &stubBlobService{}) + routes.RegisterUserRoutesWithOptions(r, userService, authMiddleware, nil, testUserRouteOptions()) // Test 1: Get profile by DID t.Run("Get Profile By DID", func(t *testing.T) { @@ -866,7 +867,7 @@ t.Run("HTTP endpoint returns 404 for non-existent DID", func(t *testing.T) { r := chi.NewRouter() authMiddleware, _ := CreateTestOAuthMiddleware("did:plc:testuser") - routes.RegisterUserRoutes(r, userService, authMiddleware, &stubBlobService{}) + routes.RegisterUserRoutesWithOptions(r, userService, authMiddleware, nil, testUserRouteOptions()) req := httptest.NewRequest("GET", "/xrpc/social.coves.actor.getprofile?actor=did:plc:nonexistentuser12345", nil) w := httptest.NewRecorder() @@ -916,7 +917,7 @@ // Set up HTTP router with auth middleware r := chi.NewRouter() authMiddleware, _ := CreateTestOAuthMiddleware("did:plc:testuser") - routes.RegisterUserRoutes(r, userService, authMiddleware, &stubBlobService{}) + routes.RegisterUserRoutesWithOptions(r, userService, authMiddleware, nil, testUserRouteOptions()) t.Run("Response includes stats object", func(t *testing.T) { req := httptest.NewRequest("GET", "/xrpc/social.coves.actor.getprofile?actor="+testDID, nil)