diff --git a/internal/atproto/oauth/client.go b/internal/atproto/oauth/client.go new file mode 100644 index 0000000..47904fa --- /dev/null +++ b/internal/atproto/oauth/client.go @@ -0,0 +1,198 @@ +package oauth + +import ( + "encoding/base64" + "fmt" + "net/url" + "time" + + "github.com/bluesky-social/indigo/atproto/atcrypto" + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/identity" +) + +// OAuthClient wraps indigo's OAuth ClientApp with Coves-specific configuration +type OAuthClient struct { + ClientApp *oauth.ClientApp + Config *OAuthConfig + SealSecret []byte // For sealing mobile tokens +} + +// OAuthConfig holds Coves OAuth client configuration +type OAuthConfig struct { + PublicURL string + ClientSecret string + ClientKID string + SealSecret string + PLCURL string + Scopes []string + SessionTTL time.Duration + SealedTokenTTL time.Duration + DevMode bool + AllowPrivateIPs bool +} + +// NewOAuthClient creates a new OAuth client for Coves +func NewOAuthClient(config *OAuthConfig, store oauth.ClientAuthStore) (*OAuthClient, error) { + if config == nil { + return nil, fmt.Errorf("config is required") + } + + // Validate seal secret + var sealSecret []byte + if config.SealSecret != "" { + decoded, err := base64.StdEncoding.DecodeString(config.SealSecret) + if err != nil { + return nil, fmt.Errorf("failed to decode seal secret: %w", err) + } + if len(decoded) != 32 { + return nil, fmt.Errorf("seal secret must be 32 bytes, got %d", len(decoded)) + } + sealSecret = decoded + } + + // Validate scopes + if len(config.Scopes) == 0 { + return nil, fmt.Errorf("scopes are required") + } + hasAtproto := false + for _, scope := range config.Scopes { + if scope == "atproto" { + hasAtproto = true + break + } + } + if !hasAtproto { + return nil, fmt.Errorf("scopes must include 'atproto'") + } + + // Set default TTL values if not specified + // Per atproto OAuth spec: + // - Public clients: 2-week (14 day) maximum session lifetime + // - Confidential clients: 180-day maximum session lifetime + if config.SessionTTL == 0 { + config.SessionTTL = 7 * 24 * time.Hour // 7 days default + } + if config.SealedTokenTTL == 0 { + config.SealedTokenTTL = 14 * 24 * time.Hour // 14 days (public client limit) + } + + // Create indigo client config + var clientConfig oauth.ClientConfig + if config.DevMode { + // Dev mode: localhost with HTTP + callbackURL := "http://localhost:3000/oauth/callback" + clientConfig = oauth.NewLocalhostConfig(callbackURL, config.Scopes) + } else { + // Production mode: HTTPS with client secret + callbackURL := config.PublicURL + "/oauth/callback" + clientConfig = oauth.NewPublicConfig(config.PublicURL, callbackURL, config.Scopes) + + // Set up confidential client if client secret is provided + if config.ClientSecret != "" && config.ClientKID != "" { + privKey, err := atcrypto.ParsePrivateMultibase(config.ClientSecret) + if err != nil { + return nil, fmt.Errorf("failed to parse client secret: %w", err) + } + + if err := clientConfig.SetClientSecret(privKey, config.ClientKID); err != nil { + return nil, fmt.Errorf("failed to set client secret: %w", err) + } + } + } + + // Set user agent + clientConfig.UserAgent = "Coves/1.0" + + // Create the indigo OAuth ClientApp + clientApp := oauth.NewClientApp(&clientConfig, store) + + // Override the default HTTP client with our SSRF-safe client + // This protects against SSRF attacks via malicious PDS URLs, DID documents, and JWKS URIs + clientApp.Client = NewSSRFSafeHTTPClient(config.AllowPrivateIPs) + + // Override the directory if a custom PLC URL is configured + // This is necessary for local development with a local PLC directory + if config.PLCURL != "" { + // Use SSRF-safe HTTP client for PLC directory requests + httpClient := NewSSRFSafeHTTPClient(config.AllowPrivateIPs) + baseDir := &identity.BaseDirectory{ + PLCURL: config.PLCURL, + HTTPClient: *httpClient, + UserAgent: "Coves/1.0", + } + // Wrap in cache directory for better performance + // Use pointer since CacheDirectory methods have pointer receivers + cacheDir := identity.NewCacheDirectory(baseDir, 100_000, time.Hour*24, time.Minute*2, time.Minute*5) + clientApp.Dir = &cacheDir + } + + return &OAuthClient{ + ClientApp: clientApp, + Config: config, + SealSecret: sealSecret, + }, nil +} + +// ClientMetadata returns the OAuth client metadata document +func (c *OAuthClient) ClientMetadata() oauth.ClientMetadata { + metadata := c.ClientApp.Config.ClientMetadata() + + // Add additional metadata for Coves + metadata.ClientName = strPtr("Coves") + if !c.Config.DevMode { + metadata.ClientURI = strPtr(c.Config.PublicURL) + } + + // For confidential clients, set JWKS URI + if c.ClientApp.Config.IsConfidential() && !c.Config.DevMode { + jwksURI := c.Config.PublicURL + "/.well-known/oauth-jwks.json" + metadata.JWKSURI = &jwksURI + } + + return metadata +} + +// PublicJWKS returns the public JWKS for this client (for confidential clients) +func (c *OAuthClient) PublicJWKS() oauth.JWKS { + return c.ClientApp.Config.PublicJWKS() +} + +// IsConfidential returns true if this is a confidential OAuth client +func (c *OAuthClient) IsConfidential() bool { + return c.ClientApp.Config.IsConfidential() +} + +// strPtr is a helper to get a pointer to a string +func strPtr(s string) *string { + return &s +} + +// ValidateCallbackURL validates that a callback URL matches the expected callback URL +func (c *OAuthClient) ValidateCallbackURL(callbackURL string) error { + expectedCallback := c.ClientApp.Config.CallbackURL + + // Parse both URLs + expected, err := url.Parse(expectedCallback) + if err != nil { + return fmt.Errorf("invalid expected callback URL: %w", err) + } + + actual, err := url.Parse(callbackURL) + if err != nil { + return fmt.Errorf("invalid callback URL: %w", err) + } + + // Compare scheme, host, and path (ignore query params) + if expected.Scheme != actual.Scheme { + return fmt.Errorf("callback URL scheme mismatch: expected %s, got %s", expected.Scheme, actual.Scheme) + } + if expected.Host != actual.Host { + return fmt.Errorf("callback URL host mismatch: expected %s, got %s", expected.Host, actual.Host) + } + if expected.Path != actual.Path { + return fmt.Errorf("callback URL path mismatch: expected %s, got %s", expected.Path, actual.Path) + } + + return nil +} diff --git a/internal/atproto/oauth/handlers.go b/internal/atproto/oauth/handlers.go new file mode 100644 index 0000000..0706779 --- /dev/null +++ b/internal/atproto/oauth/handlers.go @@ -0,0 +1,709 @@ +package oauth + +import ( + "context" + "encoding/json" + "fmt" + "log/slog" + "net/http" + "net/url" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" +) + +// MobileOAuthStore interface for mobile-specific OAuth operations +// This extends the base OAuth store with mobile CSRF tracking +type MobileOAuthStore interface { + SaveMobileOAuthData(ctx context.Context, state string, data MobileOAuthData) error + GetMobileOAuthData(ctx context.Context, state string) (*MobileOAuthData, error) +} + +// OAuthHandler handles OAuth-related HTTP endpoints +type OAuthHandler struct { + client *OAuthClient + store oauth.ClientAuthStore + mobileStore MobileOAuthStore // For server-side CSRF validation +} + +// NewOAuthHandler creates a new OAuth handler +func NewOAuthHandler(client *OAuthClient, store oauth.ClientAuthStore) *OAuthHandler { + handler := &OAuthHandler{ + client: client, + store: store, + } + + // Check if the store implements MobileOAuthStore for server-side CSRF + if mobileStore, ok := store.(MobileOAuthStore); ok { + handler.mobileStore = mobileStore + } + + return handler +} + +// HandleClientMetadata serves the OAuth client metadata document +// GET /oauth/client-metadata.json +func (h *OAuthHandler) HandleClientMetadata(w http.ResponseWriter, r *http.Request) { + metadata := h.client.ClientMetadata() + + // For confidential clients in production, set JWKS URI based on request host + if h.client.IsConfidential() && !h.client.Config.DevMode { + jwksURI := fmt.Sprintf("https://%s/oauth/jwks.json", r.Host) + metadata.JWKSURI = &jwksURI + } + + // Validate metadata before returning (skip in dev mode - localhost doesn't need https validation) + if !h.client.Config.DevMode { + if err := metadata.Validate(h.client.ClientApp.Config.ClientID); err != nil { + slog.Error("client metadata validation failed", "error", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(metadata); err != nil { + slog.Error("failed to encode client metadata", "error", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } +} + +// HandleJWKS serves the public JWKS for confidential clients +// GET /oauth/jwks.json +func (h *OAuthHandler) HandleJWKS(w http.ResponseWriter, r *http.Request) { + jwks := h.client.PublicJWKS() + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(jwks); err != nil { + slog.Error("failed to encode JWKS", "error", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } +} + +// HandleLogin starts the OAuth flow (web version) +// GET /oauth/login?handle=user.bsky.social +func (h *OAuthHandler) HandleLogin(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Get handle or DID from query params + identifier := r.URL.Query().Get("handle") + if identifier == "" { + identifier = r.URL.Query().Get("did") + } + if identifier == "" { + http.Error(w, "missing handle or did parameter", http.StatusBadRequest) + return + } + + // Start OAuth flow + redirectURL, err := h.client.ClientApp.StartAuthFlow(ctx, identifier) + if err != nil { + slog.Error("failed to start OAuth flow", "error", err, "identifier", identifier) + http.Error(w, fmt.Sprintf("failed to start OAuth flow: %v", err), http.StatusBadRequest) + return + } + + // Log OAuth flow initiation (sanitized - no full URL to avoid leaking state) + slog.Info("redirecting to PDS for OAuth", "identifier", identifier) + + // Redirect to PDS + http.Redirect(w, r, redirectURL, http.StatusFound) +} + +// HandleMobileLogin starts the OAuth flow for mobile apps +// GET /oauth/mobile/login?handle=user.bsky.social&redirect_uri=coves-app://callback +func (h *OAuthHandler) HandleMobileLogin(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Get handle or DID from query params + identifier := r.URL.Query().Get("handle") + if identifier == "" { + identifier = r.URL.Query().Get("did") + } + if identifier == "" { + http.Error(w, "missing handle or did parameter", http.StatusBadRequest) + return + } + + // Get mobile redirect URI (deep link) + mobileRedirectURI := r.URL.Query().Get("redirect_uri") + if mobileRedirectURI == "" { + http.Error(w, "missing redirect_uri parameter", http.StatusBadRequest) + return + } + + // SECURITY FIX 1: Validate redirect_uri against allowlist + if !isAllowedMobileRedirectURI(mobileRedirectURI) { + slog.Warn("rejected unauthorized mobile redirect URI", "scheme", extractScheme(mobileRedirectURI)) + http.Error(w, "invalid redirect_uri: scheme not allowed", http.StatusBadRequest) + return + } + + // SECURITY: Verify store is properly configured for mobile OAuth + // A plain PostgresOAuthStore implements MobileOAuthStore (has Save/GetMobileOAuthData), + // but without the MobileAwareStoreWrapper, SaveMobileOAuthData is never called during + // StartAuthFlow, so server-side CSRF data is never stored. This causes mobile callbacks + // to silently fall back to web flow. Fail fast here instead of silent breakage. + if _, ok := h.store.(MobileAwareClientStore); !ok { + slog.Error("mobile OAuth not supported: store is not wrapped with MobileAwareStoreWrapper", + "store_type", fmt.Sprintf("%T", h.store)) + http.Error(w, "mobile OAuth not configured on this server", http.StatusInternalServerError) + return + } + + // SECURITY FIX 2: Generate CSRF token + csrfToken, err := generateCSRFToken() + if err != nil { + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + // SECURITY FIX 4: Store CSRF server-side tied to OAuth state + // Add mobile data to context so the store wrapper can capture it when + // SaveAuthRequestInfo is called by indigo's StartAuthFlow. + // This is necessary because PAR redirects don't include the state in the URL, + // so we can't extract it after StartAuthFlow returns. + mobileCtx := ContextWithMobileFlowData(ctx, MobileOAuthData{ + CSRFToken: csrfToken, + RedirectURI: mobileRedirectURI, + }) + + // Start OAuth flow (the store wrapper will save mobile data when auth request is saved) + redirectURL, err := h.client.ClientApp.StartAuthFlow(mobileCtx, identifier) + if err != nil { + slog.Error("failed to start OAuth flow", "error", err, "identifier", identifier) + http.Error(w, fmt.Sprintf("failed to start OAuth flow: %v", err), http.StatusBadRequest) + return + } + + // Log mobile OAuth flow initiation (sanitized - no full URLs or sensitive params) + slog.Info("redirecting to PDS for mobile OAuth", "identifier", identifier) + + // SECURITY FIX 2: Store CSRF token in cookie + http.SetCookie(w, &http.Cookie{ + Name: "oauth_csrf", + Value: csrfToken, + Path: "/oauth", + MaxAge: 600, // 10 minutes + HttpOnly: true, + Secure: !h.client.Config.DevMode, + SameSite: http.SameSiteLaxMode, + }) + + // SECURITY FIX 3: Generate binding token to tie CSRF token + mobile redirect to this OAuth flow + // This prevents session fixation attacks where an attacker plants a mobile_redirect_uri + // cookie, then the user does a web login, and credentials get sent to attacker's deep link. + // The binding includes the CSRF token so we validate its VALUE (not just presence) on callback. + mobileBinding := generateMobileRedirectBinding(csrfToken, mobileRedirectURI) + + // Set cookie with mobile redirect URI for use in callback + http.SetCookie(w, &http.Cookie{ + Name: "mobile_redirect_uri", + Value: url.QueryEscape(mobileRedirectURI), + Path: "/oauth", + HttpOnly: true, + Secure: !h.client.Config.DevMode, + SameSite: http.SameSiteLaxMode, + MaxAge: 600, // 10 minutes + }) + + // Set binding cookie to validate mobile redirect in callback + http.SetCookie(w, &http.Cookie{ + Name: "mobile_redirect_binding", + Value: mobileBinding, + Path: "/oauth", + HttpOnly: true, + Secure: !h.client.Config.DevMode, + SameSite: http.SameSiteLaxMode, + MaxAge: 600, // 10 minutes + }) + + // Redirect to PDS + http.Redirect(w, r, redirectURL, http.StatusFound) +} + +// HandleCallback handles the OAuth callback from the PDS +// GET /oauth/callback?code=...&state=...&iss=... +func (h *OAuthHandler) HandleCallback(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // IMPORTANT: Look up mobile CSRF data BEFORE ProcessCallback + // ProcessCallback deletes the oauth_requests row, so we must retrieve mobile data first. + // We store it in a local variable for validation after ProcessCallback completes. + var serverMobileData *MobileOAuthData + var mobileDataLookupErr error + oauthState := r.URL.Query().Get("state") + + // Check if this might be a mobile callback (mobile_redirect_uri cookie present) + // We do a preliminary check here to decide if we need to fetch mobile data + mobileRedirectCookie, _ := r.Cookie("mobile_redirect_uri") + isMobileFlow := mobileRedirectCookie != nil && mobileRedirectCookie.Value != "" + + if isMobileFlow && h.mobileStore != nil && oauthState != "" { + // Fetch mobile data BEFORE ProcessCallback deletes the row + serverMobileData, mobileDataLookupErr = h.mobileStore.GetMobileOAuthData(ctx, oauthState) + // We'll handle errors after ProcessCallback - for now just capture the result + } + + // Process the callback (this deletes the oauth_requests row) + sessData, err := h.client.ClientApp.ProcessCallback(ctx, r.URL.Query()) + if err != nil { + slog.Error("failed to process OAuth callback", "error", err) + http.Error(w, fmt.Sprintf("OAuth callback failed: %v", err), http.StatusBadRequest) + return + } + + // Ensure sessData is not nil before using it + if sessData == nil { + slog.Error("OAuth callback returned nil session data") + http.Error(w, "OAuth callback failed: no session data", http.StatusInternalServerError) + return + } + + // Bidirectional handle verification: ensure the DID actually controls a valid handle + // This prevents impersonation via compromised PDS that issues tokens with invalid handle mappings + // Per AT Protocol spec: "Bidirectional verification required; confirm DID document claims handle" + if h.client.ClientApp.Dir != nil { + ident, err := h.client.ClientApp.Dir.LookupDID(ctx, sessData.AccountDID) + if err != nil { + // Directory lookup failed - this is a hard error for security + slog.Error("OAuth callback: DID lookup failed during handle verification", + "did", sessData.AccountDID, "error", err) + http.Error(w, "Handle verification failed", http.StatusUnauthorized) + return + } + + // Check if the handle is the special "handle.invalid" value + // This indicates that bidirectional verification failed (DID->handle->DID roundtrip failed) + if ident.Handle.String() == "handle.invalid" { + slog.Warn("OAuth callback: bidirectional handle verification failed", + "did", sessData.AccountDID, + "handle", "handle.invalid", + "reason", "DID document claims a handle that doesn't resolve back to this DID") + http.Error(w, "Handle verification failed: DID/handle mismatch", http.StatusUnauthorized) + return + } + + // Success: handle is valid and bidirectionally verified + slog.Info("OAuth callback successful", "did", sessData.AccountDID, "handle", ident.Handle) + } else { + // No directory client available - log warning but proceed + // This should only happen in testing scenarios + slog.Warn("OAuth callback: directory client not available, skipping handle verification", + "did", sessData.AccountDID) + slog.Info("OAuth callback successful (no handle verification)", "did", sessData.AccountDID) + } + + // Check if this is a mobile callback (check for mobile_redirect_uri cookie) + mobileRedirect, err := r.Cookie("mobile_redirect_uri") + if err == nil && mobileRedirect.Value != "" { + // SECURITY FIX 2: Validate CSRF token for mobile callback + csrfCookie, err := r.Cookie("oauth_csrf") + if err != nil { + slog.Warn("mobile callback missing CSRF token") + clearMobileCookies(w) + http.Error(w, "invalid request: missing CSRF token", http.StatusForbidden) + return + } + + // SECURITY FIX 3: Validate mobile redirect binding + // This prevents session fixation attacks where an attacker plants a mobile_redirect_uri + // cookie, then the user does a web login, and credentials get sent to attacker's deep link + bindingCookie, err := r.Cookie("mobile_redirect_binding") + if err != nil { + slog.Warn("mobile callback missing redirect binding - possible attack attempt") + clearMobileCookies(w) + http.Error(w, "invalid request: missing redirect binding", http.StatusForbidden) + return + } + + // Decode the mobile redirect URI to validate binding + mobileRedirectURI, err := url.QueryUnescape(mobileRedirect.Value) + if err != nil { + slog.Error("failed to decode mobile redirect URI", "error", err) + clearMobileCookies(w) + http.Error(w, "invalid mobile redirect URI", http.StatusBadRequest) + return + } + + // Validate that the binding matches both the CSRF token AND redirect URI + // This is the actual CSRF validation - we verify the token VALUE by checking + // that hash(csrfToken + redirectURI) == binding. This prevents: + // 1. CSRF attacks: attacker can't forge binding without knowing CSRF token + // 2. Session fixation: cookies must all originate from the same /oauth/mobile/login request + if !validateMobileRedirectBinding(csrfCookie.Value, mobileRedirectURI, bindingCookie.Value) { + slog.Warn("mobile redirect binding/CSRF validation failed - possible attack attempt", + "expected_scheme", extractScheme(mobileRedirectURI)) + clearMobileCookies(w) + // Fail closed: treat as web flow instead of mobile + h.handleWebCallback(w, r, sessData) + return + } + + // SECURITY FIX 4: Validate CSRF cookie against server-side state + // This compares the cookie CSRF against a value tied to the OAuth state parameter + // (which comes back through the OAuth response), satisfying the requirement to + // validate against server-side state rather than only against other cookies. + // + // CRITICAL: If mobile cookies are present but server-side mobile data is MISSING, + // this indicates a potential attack where: + // 1. Attacker did a WEB OAuth flow (no mobile data stored) + // 2. Attacker planted mobile cookies via cross-site /oauth/mobile/login + // 3. Attacker sends victim to callback with attacker's web-flow state/code + // We MUST fail closed and use web flow when server-side mobile data is missing. + // + // NOTE: serverMobileData was fetched BEFORE ProcessCallback (which deletes the row) + // at the top of this function. We use the pre-fetched result here. + if h.mobileStore != nil && oauthState != "" { + if mobileDataLookupErr != nil { + // Database error - fail closed, use web flow + slog.Warn("failed to retrieve server-side mobile OAuth data - using web flow", + "error", mobileDataLookupErr, "state", oauthState) + clearMobileCookies(w) + h.handleWebCallback(w, r, sessData) + return + } + if serverMobileData == nil { + // No server-side mobile data for this state - this OAuth flow was NOT started + // via /oauth/mobile/login. Mobile cookies are likely attacker-planted. + // Fail closed: clear cookies and use web flow. + slog.Warn("mobile cookies present but no server-side mobile data for OAuth state - "+ + "possible cross-flow attack, using web flow", "state", oauthState) + clearMobileCookies(w) + h.handleWebCallback(w, r, sessData) + return + } + // Server-side mobile data exists - validate it matches cookies + if !constantTimeCompare(csrfCookie.Value, serverMobileData.CSRFToken) { + slog.Warn("mobile callback CSRF mismatch: cookie differs from server-side state", + "state", oauthState) + clearMobileCookies(w) + h.handleWebCallback(w, r, sessData) + return + } + if serverMobileData.RedirectURI != mobileRedirectURI { + slog.Warn("mobile callback redirect URI mismatch: cookie differs from server-side state", + "cookie_uri", extractScheme(mobileRedirectURI), + "server_uri", extractScheme(serverMobileData.RedirectURI)) + clearMobileCookies(w) + h.handleWebCallback(w, r, sessData) + return + } + slog.Debug("server-side CSRF validation passed", "state", oauthState) + } else if h.mobileStore != nil { + // mobileStore exists but no state in query - shouldn't happen with valid OAuth + slog.Warn("mobile cookies present but no OAuth state in callback - using web flow") + clearMobileCookies(w) + h.handleWebCallback(w, r, sessData) + return + } + // Note: if h.mobileStore is nil (e.g., in tests), we fall back to cookie-only validation + + // All security checks passed - proceed with mobile flow + // Mobile flow: seal the session and redirect to deep link + h.handleMobileCallback(w, r, sessData, mobileRedirect.Value, csrfCookie.Value) + return + } + + // Web flow: set session cookie + h.handleWebCallback(w, r, sessData) +} + +// handleWebCallback handles the web OAuth callback flow +func (h *OAuthHandler) handleWebCallback(w http.ResponseWriter, r *http.Request, sessData *oauth.ClientSessionData) { + // Use sealed tokens for web flow (same as mobile) per atProto OAuth spec: + // "Access and refresh tokens should never be copied or shared across end devices. + // They should not be stored in session cookies." + + // Seal the session data using AES-GCM encryption + sealedToken, err := h.client.SealSession( + sessData.AccountDID.String(), + sessData.SessionID, + h.client.Config.SealedTokenTTL, + ) + if err != nil { + slog.Error("failed to seal session for web", "error", err) + http.Error(w, "failed to create session", http.StatusInternalServerError) + return + } + + http.SetCookie(w, &http.Cookie{ + Name: "coves_session", + Value: sealedToken, + Path: "/", + HttpOnly: true, + Secure: !h.client.Config.DevMode, + SameSite: http.SameSiteLaxMode, + MaxAge: int(h.client.Config.SealedTokenTTL.Seconds()), + }) + + // Clear all mobile cookies if they exist (defense in depth) + clearMobileCookies(w) + + // Redirect to home or app + redirectURL := "/" + if !h.client.Config.DevMode { + redirectURL = h.client.Config.PublicURL + "/" + } + http.Redirect(w, r, redirectURL, http.StatusFound) +} + +// handleMobileCallback handles the mobile OAuth callback flow +func (h *OAuthHandler) handleMobileCallback(w http.ResponseWriter, r *http.Request, sessData *oauth.ClientSessionData, mobileRedirectURIEncoded, csrfToken string) { + // Decode the mobile redirect URI + mobileRedirectURI, err := url.QueryUnescape(mobileRedirectURIEncoded) + if err != nil { + slog.Error("failed to decode mobile redirect URI", "error", err) + http.Error(w, "invalid mobile redirect URI", http.StatusBadRequest) + return + } + + // SECURITY FIX 1: Re-validate redirect URI against allowlist + if !isAllowedMobileRedirectURI(mobileRedirectURI) { + slog.Error("mobile callback attempted with unauthorized redirect URI", "scheme", extractScheme(mobileRedirectURI)) + http.Error(w, "invalid redirect URI", http.StatusBadRequest) + return + } + + // Seal the session data for mobile + sealedToken, err := h.client.SealSession( + sessData.AccountDID.String(), + sessData.SessionID, + h.client.Config.SealedTokenTTL, + ) + if err != nil { + slog.Error("failed to seal session data", "error", err) + http.Error(w, "failed to create session token", http.StatusInternalServerError) + return + } + + // Get account handle for convenience + handle := "" + if ident, err := h.client.ClientApp.Dir.LookupDID(r.Context(), sessData.AccountDID); err == nil { + handle = ident.Handle.String() + } + + // Clear all mobile cookies to prevent reuse (defense in depth) + clearMobileCookies(w) + + // Build deep link with sealed token + deepLink := fmt.Sprintf("%s?token=%s&did=%s&session_id=%s", + mobileRedirectURI, + url.QueryEscape(sealedToken), + url.QueryEscape(sessData.AccountDID.String()), + url.QueryEscape(sessData.SessionID), + ) + if handle != "" { + deepLink += "&handle=" + url.QueryEscape(handle) + } + + // Log mobile redirect (sanitized - no token or session ID to avoid leaking credentials) + slog.Info("redirecting to mobile app", "did", sessData.AccountDID, "handle", handle) + + // Redirect to mobile app deep link + http.Redirect(w, r, deepLink, http.StatusFound) +} + +// HandleLogout revokes the session and clears cookies +// POST /oauth/logout +func (h *OAuthHandler) HandleLogout(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Get session from cookie (now sealed) + cookie, err := r.Cookie("coves_session") + if err != nil { + // No session, just return success + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{"status": "logged_out"}) + return + } + + // Unseal the session token + sealed, err := h.client.UnsealSession(cookie.Value) + if err != nil { + // Invalid session, clear cookie and return + h.clearSessionCookie(w) + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{"status": "logged_out"}) + return + } + + // Parse DID + did, err := syntax.ParseDID(sealed.DID) + if err != nil { + // Invalid DID, clear cookie and return + h.clearSessionCookie(w) + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{"status": "logged_out"}) + return + } + + // Revoke session on auth server + if err := h.client.ClientApp.Logout(ctx, did, sealed.SessionID); err != nil { + slog.Error("failed to revoke session on auth server", "error", err, "did", did) + // Continue anyway to clear local session + } + + // Clear session cookie + h.clearSessionCookie(w) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{"status": "logged_out"}) +} + +// HandleRefresh refreshes the session token (for mobile) +// POST /oauth/refresh +// Body: {"did": "did:plc:...", "session_id": "...", "sealed_token": "..."} +func (h *OAuthHandler) HandleRefresh(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + var req struct { + DID string `json:"did"` + SessionID string `json:"session_id"` + SealedToken string `json:"sealed_token,omitempty"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + http.Error(w, "invalid request body", http.StatusBadRequest) + return + } + + // SECURITY: Require sealed_token for proof of possession + // Without this, anyone who knows a DID + session_id can steal credentials + if req.SealedToken == "" { + slog.Warn("refresh: missing sealed_token", "did", req.DID) + http.Error(w, "sealed_token required for refresh", http.StatusUnauthorized) + return + } + + // SECURITY: Unseal and validate the token + unsealed, err := h.client.UnsealSession(req.SealedToken) + if err != nil { + slog.Warn("refresh: invalid sealed token", "error", err) + http.Error(w, "Invalid or expired token", http.StatusUnauthorized) + return + } + + // SECURITY: Verify the unsealed token matches the claimed DID + if unsealed.DID != req.DID { + slog.Warn("refresh: DID mismatch", "token_did", unsealed.DID, "claimed_did", req.DID) + http.Error(w, "Token DID mismatch", http.StatusUnauthorized) + return + } + + // SECURITY: Verify the unsealed token matches the claimed session_id + if unsealed.SessionID != req.SessionID { + slog.Warn("refresh: session_id mismatch", "token_session", unsealed.SessionID, "claimed_session", req.SessionID) + http.Error(w, "Token session mismatch", http.StatusUnauthorized) + return + } + + // Parse DID after validation + did, err := syntax.ParseDID(req.DID) + if err != nil { + http.Error(w, "invalid DID", http.StatusBadRequest) + return + } + + // Resume session (now authenticated via sealed token) + sess, err := h.client.ClientApp.ResumeSession(ctx, did, req.SessionID) + if err != nil { + slog.Error("failed to resume session", "error", err, "did", did, "session_id", req.SessionID) + http.Error(w, "session not found", http.StatusUnauthorized) + return + } + + // Refresh tokens + newAccessToken, err := sess.RefreshTokens(ctx) + if err != nil { + slog.Error("failed to refresh tokens", "error", err, "did", did) + http.Error(w, "failed to refresh tokens", http.StatusUnauthorized) + return + } + + // Create new sealed token for mobile + sealedToken, err := h.client.SealSession( + sess.Data.AccountDID.String(), + sess.Data.SessionID, + h.client.Config.SealedTokenTTL, + ) + if err != nil { + slog.Error("failed to seal new session data", "error", err) + http.Error(w, "failed to create session token", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "access_token": newAccessToken, + "sealed_token": sealedToken, + }) +} + +// clearSessionCookie clears the session cookie +func (h *OAuthHandler) clearSessionCookie(w http.ResponseWriter) { + http.SetCookie(w, &http.Cookie{ + Name: "coves_session", + Value: "", + Path: "/", + MaxAge: -1, + }) +} + +// GetSessionFromRequest extracts session data from an HTTP request +func (h *OAuthHandler) GetSessionFromRequest(r *http.Request) (*oauth.ClientSessionData, error) { + // Try to get session from cookie (web) - now using sealed tokens + cookie, err := r.Cookie("coves_session") + if err == nil && cookie.Value != "" { + // Unseal the token to get DID and session ID + sealed, err := h.client.UnsealSession(cookie.Value) + if err == nil { + did, err := syntax.ParseDID(sealed.DID) + if err == nil { + return h.store.GetSession(r.Context(), did, sealed.SessionID) + } + } + } + + // Try to get session from Authorization header (mobile) + authHeader := r.Header.Get("Authorization") + if authHeader != "" { + // Expected format: "Bearer " + const prefix = "Bearer " + if len(authHeader) > len(prefix) && authHeader[:len(prefix)] == prefix { + sealedToken := authHeader[len(prefix):] + sealed, err := h.client.UnsealSession(sealedToken) + if err != nil { + return nil, fmt.Errorf("invalid sealed token: %w", err) + } + did, err := syntax.ParseDID(sealed.DID) + if err != nil { + return nil, fmt.Errorf("invalid DID in sealed token: %w", err) + } + return h.store.GetSession(r.Context(), did, sealed.SessionID) + } + } + + return nil, fmt.Errorf("no session found") +} + +// HandleProtectedResourceMetadata returns OAuth protected resource metadata +// per RFC 9449 and atproto OAuth spec. This endpoint allows third-party OAuth +// clients to discover which authorization server to use for this resource. +// Spec: https://datatracker.ietf.org/doc/html/rfc9449#section-5 +func (h *OAuthHandler) HandleProtectedResourceMetadata(w http.ResponseWriter, r *http.Request) { + metadata := map[string]interface{}{ + "resource": h.client.Config.PublicURL, + "authorization_servers": []string{"https://bsky.social"}, + } + + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Cache-Control", "public, max-age=3600") + if err := json.NewEncoder(w).Encode(metadata); err != nil { + slog.Error("failed to encode protected resource metadata", "error", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } +} diff --git a/internal/atproto/oauth/handlers_security.go b/internal/atproto/oauth/handlers_security.go new file mode 100644 index 0000000..8ab49d5 --- /dev/null +++ b/internal/atproto/oauth/handlers_security.go @@ -0,0 +1,126 @@ +package oauth + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "log/slog" + "net/http" + "net/url" +) + +// allowedMobileRedirectURIs contains the EXACT allowed redirect URIs for mobile apps. +// SECURITY: Only Universal Links (HTTPS) are allowed - cryptographically bound to app. +// +// Universal Links provide strong security guarantees: +// - iOS: Verified via /.well-known/apple-app-site-association +// - Android: Verified via /.well-known/assetlinks.json +// - System verifies domain ownership before routing to app +// - Prevents malicious apps from intercepting OAuth callbacks +// +// Custom URL schemes (coves-app://, coves://) are NOT allowed because: +// - Any app can register the same scheme and intercept tokens +// - No cryptographic binding to app identity +// - Token theft is trivial for malicious apps +// +// See: https://atproto.com/specs/oauth#mobile-clients +var allowedMobileRedirectURIs = map[string]bool{ + // Universal Links only - cryptographically bound to app + "https://coves.social/app/oauth/callback": true, +} + +// isAllowedMobileRedirectURI validates that the redirect URI is in the exact allowlist. +// SECURITY: Exact URI matching prevents token theft by rogue apps that register the same scheme. +// +// Custom URL schemes are NOT cryptographically bound to apps: +// - Any app on the device can register "coves-app://" or "coves://" +// - A malicious app can intercept deep links intended for Coves +// - Without exact URI matching, the attacker receives the sealed token +// +// This function performs EXACT matching (not scheme-only) as a security measure. +// For production, migrate to Universal Links (iOS) or App Links (Android). +func isAllowedMobileRedirectURI(redirectURI string) bool { + // Normalize and check exact match + return allowedMobileRedirectURIs[redirectURI] +} + +// extractScheme extracts the scheme from a URI for logging purposes +func extractScheme(uri string) string { + if u, err := url.Parse(uri); err == nil && u.Scheme != "" { + return u.Scheme + } + return "invalid" +} + +// generateCSRFToken generates a cryptographically secure CSRF token +func generateCSRFToken() (string, error) { + csrfToken := make([]byte, 32) + if _, err := rand.Read(csrfToken); err != nil { + slog.Error("failed to generate CSRF token", "error", err) + return "", err + } + return base64.URLEncoding.EncodeToString(csrfToken), nil +} + +// generateMobileRedirectBinding generates a cryptographically secure binding token +// that ties the CSRF token and mobile redirect URI to this specific OAuth flow. +// SECURITY: This prevents multiple attack vectors: +// 1. Session fixation: attacker plants mobile_redirect_uri cookie, user does web login +// 2. CSRF bypass: attacker manipulates cookies without knowing the CSRF token +// 3. Cookie replay: binding validates both CSRF and redirect URI together +// +// The binding is hash(csrfToken + "|" + mobileRedirectURI) which ensures: +// - CSRF token value is verified (not just presence) +// - Redirect URI is tied to the specific CSRF token that started the flow +// - Cannot forge binding without knowing both values +func generateMobileRedirectBinding(csrfToken, mobileRedirectURI string) string { + // Combine CSRF token and redirect URI with separator to prevent length extension + combined := csrfToken + "|" + mobileRedirectURI + hash := sha256.Sum256([]byte(combined)) + // Use first 16 bytes (128 bits) for the binding - sufficient for this purpose + return base64.URLEncoding.EncodeToString(hash[:16]) +} + +// validateMobileRedirectBinding validates that the CSRF token and mobile redirect URI +// together match the binding token, preventing CSRF attacks and cross-flow token theft. +// This implements a proper double-submit cookie pattern where the CSRF token value +// (not just presence) is cryptographically verified. +func validateMobileRedirectBinding(csrfToken, mobileRedirectURI, binding string) bool { + expectedBinding := generateMobileRedirectBinding(csrfToken, mobileRedirectURI) + // Constant-time comparison to prevent timing attacks + return constantTimeCompare(expectedBinding, binding) +} + +// constantTimeCompare performs a constant-time string comparison to prevent timing attacks +func constantTimeCompare(a, b string) bool { + if len(a) != len(b) { + return false + } + var result byte + for i := 0; i < len(a); i++ { + result |= a[i] ^ b[i] + } + return result == 0 +} + +// clearMobileCookies clears all mobile-related cookies to prevent reuse +func clearMobileCookies(w http.ResponseWriter) { + http.SetCookie(w, &http.Cookie{ + Name: "mobile_redirect_uri", + Value: "", + Path: "/oauth", + MaxAge: -1, + }) + http.SetCookie(w, &http.Cookie{ + Name: "mobile_redirect_binding", + Value: "", + Path: "/oauth", + MaxAge: -1, + }) + http.SetCookie(w, &http.Cookie{ + Name: "oauth_csrf", + Value: "", + Path: "/oauth", + MaxAge: -1, + }) +} diff --git a/internal/atproto/oauth/handlers_security_test.go b/internal/atproto/oauth/handlers_security_test.go new file mode 100644 index 0000000..e64f89e --- /dev/null +++ b/internal/atproto/oauth/handlers_security_test.go @@ -0,0 +1,477 @@ +package oauth + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestIsAllowedMobileRedirectURI tests the mobile redirect URI allowlist with EXACT URI matching +// Only Universal Links (HTTPS) are allowed - custom schemes are blocked for security +func TestIsAllowedMobileRedirectURI(t *testing.T) { + tests := []struct { + name string + uri string + expected bool + }{ + { + name: "allowed - Universal Link", + uri: "https://coves.social/app/oauth/callback", + expected: true, + }, + { + name: "rejected - custom scheme coves-app (vulnerable to interception)", + uri: "coves-app://oauth/callback", + expected: false, + }, + { + name: "rejected - custom scheme coves (vulnerable to interception)", + uri: "coves://oauth/callback", + expected: false, + }, + { + name: "rejected - evil scheme", + uri: "evil://callback", + expected: false, + }, + { + name: "rejected - http (not secure)", + uri: "http://example.com/callback", + expected: false, + }, + { + name: "rejected - https different domain", + uri: "https://example.com/callback", + expected: false, + }, + { + name: "rejected - https coves.social wrong path", + uri: "https://coves.social/wrong/path", + expected: false, + }, + { + name: "rejected - invalid URI", + uri: "not a uri", + expected: false, + }, + { + name: "rejected - empty string", + uri: "", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isAllowedMobileRedirectURI(tt.uri) + assert.Equal(t, tt.expected, result, + "isAllowedMobileRedirectURI(%q) = %v, want %v", tt.uri, result, tt.expected) + }) + } +} + +// TestExtractScheme tests the scheme extraction function +func TestExtractScheme(t *testing.T) { + tests := []struct { + name string + uri string + expected string + }{ + { + name: "https scheme", + uri: "https://coves.social/app/oauth/callback", + expected: "https", + }, + { + name: "custom scheme", + uri: "coves-app://callback", + expected: "coves-app", + }, + { + name: "invalid URI", + uri: "not a uri", + expected: "invalid", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractScheme(tt.uri) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestGenerateCSRFToken tests CSRF token generation +func TestGenerateCSRFToken(t *testing.T) { + // Generate two tokens and verify they are different (randomness check) + token1, err1 := generateCSRFToken() + require.NoError(t, err1) + require.NotEmpty(t, token1) + + token2, err2 := generateCSRFToken() + require.NoError(t, err2) + require.NotEmpty(t, token2) + + assert.NotEqual(t, token1, token2, "CSRF tokens should be unique") + + // Verify token is base64 encoded (should decode without error) + assert.Greater(t, len(token1), 40, "CSRF token should be reasonably long (32 bytes base64 encoded)") +} + +// TestHandleMobileLogin_RedirectURIValidation tests that HandleMobileLogin validates redirect URIs +func TestHandleMobileLogin_RedirectURIValidation(t *testing.T) { + // Note: This is a unit test for the validation logic only. + // Full integration tests with OAuth flow are in tests/integration/oauth_e2e_test.go + + tests := []struct { + name string + redirectURI string + expectedLog string + expectedStatus int + }{ + { + name: "allowed - Universal Link", + redirectURI: "https://coves.social/app/oauth/callback", + expectedStatus: http.StatusBadRequest, // Will fail at StartAuthFlow (no OAuth client setup) + }, + { + name: "rejected - custom scheme coves-app (insecure)", + redirectURI: "coves-app://oauth/callback", + expectedStatus: http.StatusBadRequest, + expectedLog: "rejected unauthorized mobile redirect URI", + }, + { + name: "rejected evil scheme", + redirectURI: "evil://callback", + expectedStatus: http.StatusBadRequest, + expectedLog: "rejected unauthorized mobile redirect URI", + }, + { + name: "rejected http", + redirectURI: "http://evil.com/callback", + expectedStatus: http.StatusBadRequest, + expectedLog: "scheme not allowed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the validation function directly + result := isAllowedMobileRedirectURI(tt.redirectURI) + if tt.expectedLog != "" { + assert.False(t, result, "Should reject %s", tt.redirectURI) + } + }) + } +} + +// TestHandleCallback_CSRFValidation tests that HandleCallback validates CSRF tokens for mobile flow +func TestHandleCallback_CSRFValidation(t *testing.T) { + // This is a conceptual test structure. Full implementation would require: + // 1. Mock OAuthClient + // 2. Mock OAuth store + // 3. Simulated OAuth callback with cookies + + t.Run("mobile callback requires CSRF token", func(t *testing.T) { + // Setup: Create request with mobile_redirect_uri cookie but NO oauth_csrf cookie + req := httptest.NewRequest("GET", "/oauth/callback?code=test&state=test", nil) + req.AddCookie(&http.Cookie{ + Name: "mobile_redirect_uri", + Value: "https%3A%2F%2Fcoves.social%2Fapp%2Foauth%2Fcallback", + }) + // Missing: oauth_csrf cookie + + // This would be rejected with 403 Forbidden in the actual handler + // (Full test in integration tests with real OAuth flow) + + assert.NotNil(t, req) // Placeholder assertion + }) + + t.Run("mobile callback with valid CSRF token", func(t *testing.T) { + // Setup: Create request with both cookies + req := httptest.NewRequest("GET", "/oauth/callback?code=test&state=test", nil) + req.AddCookie(&http.Cookie{ + Name: "mobile_redirect_uri", + Value: "https%3A%2F%2Fcoves.social%2Fapp%2Foauth%2Fcallback", + }) + req.AddCookie(&http.Cookie{ + Name: "oauth_csrf", + Value: "valid-csrf-token", + }) + + // This would be accepted (assuming valid OAuth code/state) + // (Full test in integration tests with real OAuth flow) + + assert.NotNil(t, req) // Placeholder assertion + }) +} + +// TestHandleMobileCallback_RevalidatesRedirectURI tests that handleMobileCallback re-validates the redirect URI +func TestHandleMobileCallback_RevalidatesRedirectURI(t *testing.T) { + // This is a critical security test: even if an attacker somehow bypasses the initial check, + // the callback handler should re-validate the redirect URI before redirecting. + + tests := []struct { + name string + redirectURI string + shouldPass bool + }{ + { + name: "allowed - Universal Link", + redirectURI: "https://coves.social/app/oauth/callback", + shouldPass: true, + }, + { + name: "blocked - custom scheme (insecure)", + redirectURI: "coves-app://oauth/callback", + shouldPass: false, + }, + { + name: "blocked - evil scheme", + redirectURI: "evil://callback", + shouldPass: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isAllowedMobileRedirectURI(tt.redirectURI) + assert.Equal(t, tt.shouldPass, result) + }) + } +} + +// TestGenerateMobileRedirectBinding tests the binding token generation +// The binding now includes the CSRF token for proper double-submit validation +func TestGenerateMobileRedirectBinding(t *testing.T) { + csrfToken := "test-csrf-token-12345" + tests := []struct { + name string + redirectURI string + }{ + { + name: "Universal Link", + redirectURI: "https://coves.social/app/oauth/callback", + }, + { + name: "different path", + redirectURI: "https://coves.social/different/path", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + binding1 := generateMobileRedirectBinding(csrfToken, tt.redirectURI) + binding2 := generateMobileRedirectBinding(csrfToken, tt.redirectURI) + + // Same CSRF token + URI should produce same binding (deterministic) + assert.Equal(t, binding1, binding2, "binding should be deterministic for same inputs") + + // Binding should not be empty + assert.NotEmpty(t, binding1, "binding should not be empty") + + // Binding should be base64 encoded (should decode without error) + assert.Greater(t, len(binding1), 20, "binding should be reasonably long") + }) + } + + // Different URIs should produce different bindings + binding1 := generateMobileRedirectBinding(csrfToken, "https://coves.social/app/oauth/callback") + binding2 := generateMobileRedirectBinding(csrfToken, "https://coves.social/different/path") + assert.NotEqual(t, binding1, binding2, "different URIs should produce different bindings") + + // Different CSRF tokens should produce different bindings + binding3 := generateMobileRedirectBinding("different-csrf-token", "https://coves.social/app/oauth/callback") + assert.NotEqual(t, binding1, binding3, "different CSRF tokens should produce different bindings") +} + +// TestValidateMobileRedirectBinding tests the binding validation +// Now validates both CSRF token and redirect URI together (double-submit pattern) +func TestValidateMobileRedirectBinding(t *testing.T) { + csrfToken := "test-csrf-token-for-validation" + redirectURI := "https://coves.social/app/oauth/callback" + validBinding := generateMobileRedirectBinding(csrfToken, redirectURI) + + tests := []struct { + name string + csrfToken string + redirectURI string + binding string + shouldPass bool + }{ + { + name: "valid - correct CSRF token and redirect URI", + csrfToken: csrfToken, + redirectURI: redirectURI, + binding: validBinding, + shouldPass: true, + }, + { + name: "invalid - wrong redirect URI", + csrfToken: csrfToken, + redirectURI: "https://coves.social/different/path", + binding: validBinding, + shouldPass: false, + }, + { + name: "invalid - wrong CSRF token", + csrfToken: "wrong-csrf-token", + redirectURI: redirectURI, + binding: validBinding, + shouldPass: false, + }, + { + name: "invalid - random binding", + csrfToken: csrfToken, + redirectURI: redirectURI, + binding: "random-invalid-binding", + shouldPass: false, + }, + { + name: "invalid - empty binding", + csrfToken: csrfToken, + redirectURI: redirectURI, + binding: "", + shouldPass: false, + }, + { + name: "invalid - empty CSRF token", + csrfToken: "", + redirectURI: redirectURI, + binding: validBinding, + shouldPass: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := validateMobileRedirectBinding(tt.csrfToken, tt.redirectURI, tt.binding) + assert.Equal(t, tt.shouldPass, result) + }) + } +} + +// TestSessionFixationAttackPrevention tests that the binding prevents session fixation +func TestSessionFixationAttackPrevention(t *testing.T) { + // Simulate attack scenario: + // 1. Attacker plants a cookie for evil://steal with binding for evil://steal + // 2. User does a web login (no mobile_redirect_binding cookie) + // 3. Callback should NOT redirect to evil://steal + + attackerCSRF := "attacker-csrf-token" + attackerRedirectURI := "evil://steal" + attackerBinding := generateMobileRedirectBinding(attackerCSRF, attackerRedirectURI) + + // Later, user's legitimate mobile login + userCSRF := "user-csrf-token" + userRedirectURI := "https://coves.social/app/oauth/callback" + userBinding := generateMobileRedirectBinding(userCSRF, userRedirectURI) + + // The attacker's binding should NOT validate for the user's redirect URI + assert.False(t, validateMobileRedirectBinding(userCSRF, userRedirectURI, attackerBinding), + "attacker's binding should not validate for user's CSRF token and redirect URI") + + // The user's binding should validate for the user's CSRF token and redirect URI + assert.True(t, validateMobileRedirectBinding(userCSRF, userRedirectURI, userBinding), + "user's binding should validate for user's CSRF token and redirect URI") + + // Cross-validation should fail + assert.False(t, validateMobileRedirectBinding(attackerCSRF, attackerRedirectURI, userBinding), + "user's binding should not validate for attacker's CSRF token and redirect URI") +} + +// TestCSRFTokenValidation tests that CSRF token VALUE is validated, not just presence +func TestCSRFTokenValidation(t *testing.T) { + // This test verifies the fix for the P1 security issue: + // "The callback never validates the token... the csrfToken argument is ignored entirely" + // + // The fix ensures that the CSRF token VALUE is cryptographically bound to the + // binding token, so changing the CSRF token will invalidate the binding. + + t.Run("CSRF token value must match", func(t *testing.T) { + originalCSRF := "original-csrf-token-from-login" + redirectURI := "https://coves.social/app/oauth/callback" + binding := generateMobileRedirectBinding(originalCSRF, redirectURI) + + // Original CSRF token should validate + assert.True(t, validateMobileRedirectBinding(originalCSRF, redirectURI, binding), + "original CSRF token should validate") + + // Different CSRF token should NOT validate (this is the key security fix) + differentCSRF := "attacker-forged-csrf-token" + assert.False(t, validateMobileRedirectBinding(differentCSRF, redirectURI, binding), + "different CSRF token should NOT validate - this is the security fix") + }) + + t.Run("attacker cannot forge binding without CSRF token", func(t *testing.T) { + // Attacker knows the redirect URI but not the CSRF token + redirectURI := "https://coves.social/app/oauth/callback" + victimCSRF := "victim-secret-csrf-token" + victimBinding := generateMobileRedirectBinding(victimCSRF, redirectURI) + + // Attacker tries various CSRF tokens to forge the binding + attackerGuesses := []string{ + "", + "guess1", + "attacker-csrf", + redirectURI, // trying the redirect URI as CSRF + } + + for _, guess := range attackerGuesses { + assert.False(t, validateMobileRedirectBinding(guess, redirectURI, victimBinding), + "attacker's CSRF guess %q should not validate", guess) + } + }) +} + +// TestConstantTimeCompare tests the timing-safe comparison function +func TestConstantTimeCompare(t *testing.T) { + tests := []struct { + name string + a string + b string + expected bool + }{ + { + name: "equal strings", + a: "abc123", + b: "abc123", + expected: true, + }, + { + name: "different strings same length", + a: "abc123", + b: "xyz789", + expected: false, + }, + { + name: "different lengths", + a: "short", + b: "longer", + expected: false, + }, + { + name: "empty strings", + a: "", + b: "", + expected: true, + }, + { + name: "one empty", + a: "abc", + b: "", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := constantTimeCompare(tt.a, tt.b) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/internal/atproto/oauth/handlers_test.go b/internal/atproto/oauth/handlers_test.go new file mode 100644 index 0000000..51c0a79 --- /dev/null +++ b/internal/atproto/oauth/handlers_test.go @@ -0,0 +1,279 @@ +package oauth + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHandleClientMetadata tests the client metadata endpoint +func TestHandleClientMetadata(t *testing.T) { + // Create a test OAuth client configuration + config := &OAuthConfig{ + PublicURL: "https://coves.social", + Scopes: []string{"atproto"}, + DevMode: false, + AllowPrivateIPs: false, + SealSecret: "MTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", // base64 encoded 32 bytes + } + + // Create OAuth client with memory store + client, err := NewOAuthClient(config, oauth.NewMemStore()) + require.NoError(t, err) + + // Create handler + handler := NewOAuthHandler(client, oauth.NewMemStore()) + + // Create test request + req := httptest.NewRequest(http.MethodGet, "/oauth/client-metadata.json", nil) + req.Host = "coves.social" + rec := httptest.NewRecorder() + + // Call handler + handler.HandleClientMetadata(rec, req) + + // Check response + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "application/json", rec.Header().Get("Content-Type")) + + // Parse response + var metadata oauth.ClientMetadata + err = json.NewDecoder(rec.Body).Decode(&metadata) + require.NoError(t, err) + + // Validate metadata + assert.Equal(t, "https://coves.social", metadata.ClientID) + assert.Contains(t, metadata.RedirectURIs, "https://coves.social/oauth/callback") + assert.Contains(t, metadata.GrantTypes, "authorization_code") + assert.Contains(t, metadata.GrantTypes, "refresh_token") + assert.True(t, metadata.DPoPBoundAccessTokens) + assert.Contains(t, metadata.Scope, "atproto") +} + +// TestHandleJWKS tests the JWKS endpoint +func TestHandleJWKS(t *testing.T) { + // Create a test OAuth client configuration (public client, no keys) + config := &OAuthConfig{ + PublicURL: "https://coves.social", + Scopes: []string{"atproto"}, + DevMode: false, + AllowPrivateIPs: false, + SealSecret: "MTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", + } + + client, err := NewOAuthClient(config, oauth.NewMemStore()) + require.NoError(t, err) + + handler := NewOAuthHandler(client, oauth.NewMemStore()) + + // Create test request + req := httptest.NewRequest(http.MethodGet, "/oauth/jwks.json", nil) + rec := httptest.NewRecorder() + + // Call handler + handler.HandleJWKS(rec, req) + + // Check response + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "application/json", rec.Header().Get("Content-Type")) + + // Parse response + var jwks oauth.JWKS + err = json.NewDecoder(rec.Body).Decode(&jwks) + require.NoError(t, err) + + // Public client should have empty JWKS + assert.NotNil(t, jwks.Keys) + assert.Equal(t, 0, len(jwks.Keys)) +} + +// TestHandleLogin tests the login endpoint +func TestHandleLogin(t *testing.T) { + config := &OAuthConfig{ + PublicURL: "https://coves.social", + Scopes: []string{"atproto"}, + DevMode: true, // Use dev mode to avoid real PDS calls + AllowPrivateIPs: true, // Allow private IPs in dev mode + SealSecret: "MTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", + } + + client, err := NewOAuthClient(config, oauth.NewMemStore()) + require.NoError(t, err) + + handler := NewOAuthHandler(client, oauth.NewMemStore()) + + t.Run("missing identifier", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/oauth/login", nil) + rec := httptest.NewRecorder() + + handler.HandleLogin(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + }) + + t.Run("with handle parameter", func(t *testing.T) { + // This test would need a mock PDS server to fully test + // For now, we just verify the endpoint accepts the parameter + req := httptest.NewRequest(http.MethodGet, "/oauth/login?handle=user.bsky.social", nil) + rec := httptest.NewRecorder() + + handler.HandleLogin(rec, req) + + // In dev mode or with a real PDS, this would redirect + // Without a mock, it will fail to resolve the handle + // We're just testing that the handler processes the request + assert.NotEqual(t, http.StatusOK, rec.Code) // Should redirect or error + }) +} + +// TestHandleMobileLogin tests the mobile login endpoint +func TestHandleMobileLogin(t *testing.T) { + config := &OAuthConfig{ + PublicURL: "https://coves.social", + Scopes: []string{"atproto"}, + DevMode: true, + AllowPrivateIPs: true, + SealSecret: "MTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", + } + + client, err := NewOAuthClient(config, oauth.NewMemStore()) + require.NoError(t, err) + + handler := NewOAuthHandler(client, oauth.NewMemStore()) + + t.Run("missing redirect_uri", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/oauth/mobile/login?handle=user.bsky.social", nil) + rec := httptest.NewRecorder() + + handler.HandleMobileLogin(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "redirect_uri") + }) + + t.Run("invalid redirect_uri (https)", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/oauth/mobile/login?handle=user.bsky.social&redirect_uri=https://example.com", nil) + rec := httptest.NewRecorder() + + handler.HandleMobileLogin(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "invalid redirect_uri") + }) + + t.Run("invalid redirect_uri (wrong path)", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/oauth/mobile/login?handle=user.bsky.social&redirect_uri=coves-app://callback", nil) + rec := httptest.NewRecorder() + + handler.HandleMobileLogin(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "invalid redirect_uri") + }) + + t.Run("valid mobile redirect_uri (Universal Link)", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/oauth/mobile/login?handle=user.bsky.social&redirect_uri=https://coves.social/app/oauth/callback", nil) + rec := httptest.NewRecorder() + + handler.HandleMobileLogin(rec, req) + + // Should fail to resolve handle but accept the parameters + // Check that cookie was set + cookies := rec.Result().Cookies() + var found bool + for _, cookie := range cookies { + if cookie.Name == "mobile_redirect_uri" { + found = true + break + } + } + // May or may not set cookie depending on error handling + _ = found + }) +} + +// TestParseSessionToken tests that we no longer use parseSessionToken +// (removed in favor of sealed tokens) +func TestParseSessionToken(t *testing.T) { + // This test is deprecated - we now use sealed tokens instead of plain "did:sessionID" format + // See TestSealAndUnsealSessionData for the new approach + t.Skip("parseSessionToken removed - we now use sealed tokens for security") +} + +// TestIsMobileRedirectURI tests mobile redirect URI validation with EXACT URI matching +// Only Universal Links (HTTPS) are allowed - custom schemes are blocked for security +func TestIsMobileRedirectURI(t *testing.T) { + tests := []struct { + uri string + expected bool + }{ + {"https://coves.social/app/oauth/callback", true}, // Universal Link - allowed + {"coves-app://oauth/callback", false}, // Custom scheme - blocked (insecure) + {"coves://oauth/callback", false}, // Custom scheme - blocked (insecure) + {"coves-app://callback", false}, // Custom scheme - blocked + {"coves://oauth", false}, // Custom scheme - blocked + {"myapp://oauth", false}, // Not in allowlist + {"https://example.com", false}, // Wrong domain + {"http://localhost", false}, // HTTP not allowed + {"", false}, + {"not-a-uri", false}, + } + + for _, tt := range tests { + t.Run(tt.uri, func(t *testing.T) { + result := isAllowedMobileRedirectURI(tt.uri) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestSealAndUnsealSessionData tests session data sealing/unsealing +func TestSealAndUnsealSessionData(t *testing.T) { + config := &OAuthConfig{ + PublicURL: "https://coves.social", + Scopes: []string{"atproto"}, + DevMode: false, + AllowPrivateIPs: false, + SealSecret: "MTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", + } + + client, err := NewOAuthClient(config, oauth.NewMemStore()) + require.NoError(t, err) + + // Create test DID + did, err := testDID() + require.NoError(t, err) + + sessionID := "test-session-123" + + // Seal the session using the client method + sealed, err := client.SealSession(did.String(), sessionID, 24*time.Hour) + require.NoError(t, err) + assert.NotEmpty(t, sealed) + + // Unseal the session using the client method + unsealed, err := client.UnsealSession(sealed) + require.NoError(t, err) + require.NotNil(t, unsealed) + + // Verify data matches + assert.Equal(t, did.String(), unsealed.DID) + assert.Equal(t, sessionID, unsealed.SessionID) + assert.Greater(t, unsealed.ExpiresAt, int64(0)) +} + +// testDID creates a test DID for testing +func testDID() (*syntax.DID, error) { + did, err := syntax.ParseDID("did:plc:test123abc456def789") + if err != nil { + return nil, err + } + return &did, nil +} diff --git a/internal/atproto/oauth/seal.go b/internal/atproto/oauth/seal.go new file mode 100644 index 0000000..c5149d4 --- /dev/null +++ b/internal/atproto/oauth/seal.go @@ -0,0 +1,152 @@ +package oauth + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "encoding/base64" + "encoding/json" + "fmt" + "time" +) + +// SealedSession represents the data sealed in a mobile session token +type SealedSession struct { + DID string `json:"did"` // User's DID + SessionID string `json:"sid"` // Session identifier + ExpiresAt int64 `json:"exp"` // Unix timestamp when token expires +} + +// SealSession creates an encrypted token containing session information. +// The token is encrypted using AES-256-GCM and encoded as base64url. +// +// Token format: base64url(nonce || ciphertext || tag) +// - nonce: 12 bytes (GCM standard nonce size) +// - ciphertext: encrypted JSON payload +// - tag: 16 bytes (GCM authentication tag) +// +// The sealed token can be safely given to mobile clients and used as +// a reference to the server-side session without exposing sensitive data. +func (c *OAuthClient) SealSession(did, sessionID string, ttl time.Duration) (string, error) { + if len(c.SealSecret) == 0 { + return "", fmt.Errorf("seal secret not configured") + } + + if did == "" { + return "", fmt.Errorf("DID is required") + } + + if sessionID == "" { + return "", fmt.Errorf("session ID is required") + } + + // Create the session data + expiresAt := time.Now().Add(ttl).Unix() + session := SealedSession{ + DID: did, + SessionID: sessionID, + ExpiresAt: expiresAt, + } + + // Marshal to JSON + plaintext, err := json.Marshal(session) + if err != nil { + return "", fmt.Errorf("failed to marshal session: %w", err) + } + + // Create AES cipher + block, err := aes.NewCipher(c.SealSecret) + if err != nil { + return "", fmt.Errorf("failed to create cipher: %w", err) + } + + // Create GCM mode + gcm, err := cipher.NewGCM(block) + if err != nil { + return "", fmt.Errorf("failed to create GCM: %w", err) + } + + // Generate random nonce + nonce := make([]byte, gcm.NonceSize()) + if _, err := rand.Read(nonce); err != nil { + return "", fmt.Errorf("failed to generate nonce: %w", err) + } + + // Encrypt and authenticate + // GCM.Seal appends the ciphertext and tag to the nonce + ciphertext := gcm.Seal(nonce, nonce, plaintext, nil) + + // Encode as base64url (no padding) + token := base64.RawURLEncoding.EncodeToString(ciphertext) + + return token, nil +} + +// UnsealSession decrypts and validates a sealed session token. +// Returns the session information if the token is valid and not expired. +func (c *OAuthClient) UnsealSession(token string) (*SealedSession, error) { + if len(c.SealSecret) == 0 { + return nil, fmt.Errorf("seal secret not configured") + } + + if token == "" { + return nil, fmt.Errorf("token is required") + } + + // Decode from base64url + ciphertext, err := base64.RawURLEncoding.DecodeString(token) + if err != nil { + return nil, fmt.Errorf("invalid token encoding: %w", err) + } + + // Create AES cipher + block, err := aes.NewCipher(c.SealSecret) + if err != nil { + return nil, fmt.Errorf("failed to create cipher: %w", err) + } + + // Create GCM mode + gcm, err := cipher.NewGCM(block) + if err != nil { + return nil, fmt.Errorf("failed to create GCM: %w", err) + } + + // Verify minimum size (nonce + tag) + nonceSize := gcm.NonceSize() + if len(ciphertext) < nonceSize { + return nil, fmt.Errorf("invalid token: too short") + } + + // Extract nonce and ciphertext + nonce := ciphertext[:nonceSize] + ciphertextData := ciphertext[nonceSize:] + + // Decrypt and authenticate + plaintext, err := gcm.Open(nil, nonce, ciphertextData, nil) + if err != nil { + return nil, fmt.Errorf("failed to decrypt token: %w", err) + } + + // Unmarshal JSON + var session SealedSession + if err := json.Unmarshal(plaintext, &session); err != nil { + return nil, fmt.Errorf("failed to unmarshal session: %w", err) + } + + // Validate required fields + if session.DID == "" { + return nil, fmt.Errorf("invalid session: missing DID") + } + + if session.SessionID == "" { + return nil, fmt.Errorf("invalid session: missing session ID") + } + + // Check expiration + now := time.Now().Unix() + if session.ExpiresAt <= now { + return nil, fmt.Errorf("token expired at %v", time.Unix(session.ExpiresAt, 0)) + } + + return &session, nil +} diff --git a/internal/atproto/oauth/seal_test.go b/internal/atproto/oauth/seal_test.go new file mode 100644 index 0000000..f845d16 --- /dev/null +++ b/internal/atproto/oauth/seal_test.go @@ -0,0 +1,331 @@ +package oauth + +import ( + "crypto/rand" + "encoding/base64" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// generateSealSecret generates a random 32-byte seal secret for testing +func generateSealSecret() []byte { + secret := make([]byte, 32) + if _, err := rand.Read(secret); err != nil { + panic(err) + } + return secret +} + +func TestSealSession_RoundTrip(t *testing.T) { + // Create client with seal secret + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 1 * time.Hour + + // Seal the session + token, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + require.NotEmpty(t, token) + + // Token should be base64url encoded + _, err = base64.RawURLEncoding.DecodeString(token) + require.NoError(t, err, "token should be valid base64url") + + // Unseal the session + session, err := client.UnsealSession(token) + require.NoError(t, err) + require.NotNil(t, session) + + // Verify data + assert.Equal(t, did, session.DID) + assert.Equal(t, sessionID, session.SessionID) + + // Verify expiration is approximately correct (within 1 second) + expectedExpiry := time.Now().Add(ttl).Unix() + assert.InDelta(t, expectedExpiry, session.ExpiresAt, 1.0) +} + +func TestSealSession_ExpirationValidation(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 2 * time.Second // Short TTL (must be >= 1 second due to Unix timestamp granularity) + + // Seal the session + token, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + // Should work immediately + session, err := client.UnsealSession(token) + require.NoError(t, err) + assert.Equal(t, did, session.DID) + + // Wait well past expiration + time.Sleep(2500 * time.Millisecond) + + // Should fail after expiration + session, err = client.UnsealSession(token) + assert.Error(t, err) + assert.Nil(t, session) + assert.Contains(t, err.Error(), "token expired") +} + +func TestSealSession_TamperedTokenDetection(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 1 * time.Hour + + // Seal the session + token, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + // Tamper with the token by modifying one character + tampered := token[:len(token)-5] + "XXXX" + token[len(token)-1:] + + // Should fail to unseal tampered token + session, err := client.UnsealSession(tampered) + assert.Error(t, err) + assert.Nil(t, session) + assert.Contains(t, err.Error(), "failed to decrypt token") +} + +func TestSealSession_InvalidTokenFormats(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + tests := []struct { + name string + token string + }{ + { + name: "empty token", + token: "", + }, + { + name: "invalid base64", + token: "not-valid-base64!@#$", + }, + { + name: "too short", + token: base64.RawURLEncoding.EncodeToString([]byte("short")), + }, + { + name: "random bytes", + token: base64.RawURLEncoding.EncodeToString(make([]byte, 50)), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + session, err := client.UnsealSession(tt.token) + assert.Error(t, err) + assert.Nil(t, session) + }) + } +} + +func TestSealSession_DifferentSecrets(t *testing.T) { + // Create two clients with different secrets + client1 := &OAuthClient{ + SealSecret: generateSealSecret(), + } + client2 := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 1 * time.Hour + + // Seal with client1 + token, err := client1.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + // Try to unseal with client2 (different secret) + session, err := client2.UnsealSession(token) + assert.Error(t, err) + assert.Nil(t, session) + assert.Contains(t, err.Error(), "failed to decrypt token") +} + +func TestSealSession_NoSecretConfigured(t *testing.T) { + client := &OAuthClient{ + SealSecret: nil, + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 1 * time.Hour + + // Should fail to seal without secret + token, err := client.SealSession(did, sessionID, ttl) + assert.Error(t, err) + assert.Empty(t, token) + assert.Contains(t, err.Error(), "seal secret not configured") + + // Should fail to unseal without secret + session, err := client.UnsealSession("dummy-token") + assert.Error(t, err) + assert.Nil(t, session) + assert.Contains(t, err.Error(), "seal secret not configured") +} + +func TestSealSession_MissingRequiredFields(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + ttl := 1 * time.Hour + + tests := []struct { + name string + did string + sessionID string + errorMsg string + }{ + { + name: "missing DID", + did: "", + sessionID: "session-123", + errorMsg: "DID is required", + }, + { + name: "missing session ID", + did: "did:plc:abc123", + sessionID: "", + errorMsg: "session ID is required", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + token, err := client.SealSession(tt.did, tt.sessionID, ttl) + assert.Error(t, err) + assert.Empty(t, token) + assert.Contains(t, err.Error(), tt.errorMsg) + }) + } +} + +func TestSealSession_UniquenessPerCall(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 1 * time.Hour + + // Seal the same session twice + token1, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + token2, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + // Tokens should be different (different nonces) + assert.NotEqual(t, token1, token2, "tokens should be unique due to different nonces") + + // But both should unseal to the same session data + session1, err := client.UnsealSession(token1) + require.NoError(t, err) + + session2, err := client.UnsealSession(token2) + require.NoError(t, err) + + assert.Equal(t, session1.DID, session2.DID) + assert.Equal(t, session1.SessionID, session2.SessionID) +} + +func TestSealSession_LongDIDAndSessionID(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + // Test with very long DID and session ID + did := "did:plc:" + strings.Repeat("a", 200) + sessionID := "session-" + strings.Repeat("x", 200) + ttl := 1 * time.Hour + + // Should work with long values + token, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + session, err := client.UnsealSession(token) + require.NoError(t, err) + assert.Equal(t, did, session.DID) + assert.Equal(t, sessionID, session.SessionID) +} + +func TestSealSession_URLSafeEncoding(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 1 * time.Hour + + // Seal multiple times to get different nonces + for i := 0; i < 100; i++ { + token, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + // Token should not contain URL-unsafe characters + assert.NotContains(t, token, "+", "token should not contain '+'") + assert.NotContains(t, token, "/", "token should not contain '/'") + assert.NotContains(t, token, "=", "token should not contain '='") + + // Should unseal successfully + session, err := client.UnsealSession(token) + require.NoError(t, err) + assert.Equal(t, did, session.DID) + } +} + +func TestSealSession_ConcurrentAccess(t *testing.T) { + client := &OAuthClient{ + SealSecret: generateSealSecret(), + } + + did := "did:plc:abc123" + sessionID := "session-xyz" + ttl := 1 * time.Hour + + // Run concurrent seal/unseal operations + done := make(chan bool) + for i := 0; i < 10; i++ { + go func() { + for j := 0; j < 100; j++ { + token, err := client.SealSession(did, sessionID, ttl) + require.NoError(t, err) + + session, err := client.UnsealSession(token) + require.NoError(t, err) + assert.Equal(t, did, session.DID) + } + done <- true + }() + } + + // Wait for all goroutines + for i := 0; i < 10; i++ { + <-done + } +} diff --git a/internal/atproto/oauth/store.go b/internal/atproto/oauth/store.go new file mode 100644 index 0000000..4bf6617 --- /dev/null +++ b/internal/atproto/oauth/store.go @@ -0,0 +1,614 @@ +package oauth + +import ( + "context" + "database/sql" + "errors" + "fmt" + "log/slog" + "net/url" + "strings" + "time" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/lib/pq" +) + +var ( + ErrSessionNotFound = errors.New("oauth session not found") + ErrAuthRequestNotFound = errors.New("oauth auth request not found") +) + +// PostgresOAuthStore implements oauth.ClientAuthStore interface using PostgreSQL +type PostgresOAuthStore struct { + db *sql.DB + sessionTTL time.Duration +} + +// NewPostgresOAuthStore creates a new PostgreSQL-backed OAuth store +func NewPostgresOAuthStore(db *sql.DB, sessionTTL time.Duration) oauth.ClientAuthStore { + if sessionTTL == 0 { + sessionTTL = 7 * 24 * time.Hour // Default to 7 days + } + return &PostgresOAuthStore{ + db: db, + sessionTTL: sessionTTL, + } +} + +// GetSession retrieves a session by DID and session ID +func (s *PostgresOAuthStore) GetSession(ctx context.Context, did syntax.DID, sessionID string) (*oauth.ClientSessionData, error) { + query := ` + SELECT + did, session_id, host_url, auth_server_iss, + auth_server_token_endpoint, auth_server_revocation_endpoint, + scopes, access_token, refresh_token, + dpop_authserver_nonce, dpop_pds_nonce, dpop_private_key_multibase + FROM oauth_sessions + WHERE did = $1 AND session_id = $2 AND expires_at > NOW() + ` + + var session oauth.ClientSessionData + var authServerIss, authServerTokenEndpoint, authServerRevocationEndpoint sql.NullString + var hostURL, dpopPrivateKeyMultibase sql.NullString + var scopes pq.StringArray + var dpopAuthServerNonce, dpopHostNonce sql.NullString + + err := s.db.QueryRowContext(ctx, query, did.String(), sessionID).Scan( + &session.AccountDID, + &session.SessionID, + &hostURL, + &authServerIss, + &authServerTokenEndpoint, + &authServerRevocationEndpoint, + &scopes, + &session.AccessToken, + &session.RefreshToken, + &dpopAuthServerNonce, + &dpopHostNonce, + &dpopPrivateKeyMultibase, + ) + + if err == sql.ErrNoRows { + return nil, ErrSessionNotFound + } + if err != nil { + return nil, fmt.Errorf("failed to get session: %w", err) + } + + // Convert nullable fields + if hostURL.Valid { + session.HostURL = hostURL.String + } + if authServerIss.Valid { + session.AuthServerURL = authServerIss.String + } + if authServerTokenEndpoint.Valid { + session.AuthServerTokenEndpoint = authServerTokenEndpoint.String + } + if authServerRevocationEndpoint.Valid { + session.AuthServerRevocationEndpoint = authServerRevocationEndpoint.String + } + if dpopAuthServerNonce.Valid { + session.DPoPAuthServerNonce = dpopAuthServerNonce.String + } + if dpopHostNonce.Valid { + session.DPoPHostNonce = dpopHostNonce.String + } + if dpopPrivateKeyMultibase.Valid { + session.DPoPPrivateKeyMultibase = dpopPrivateKeyMultibase.String + } + session.Scopes = scopes + + return &session, nil +} + +// SaveSession saves or updates a session (upsert operation) +func (s *PostgresOAuthStore) SaveSession(ctx context.Context, sess oauth.ClientSessionData) error { + // Input validation per atProto OAuth security requirements + + // Validate DID format + if _, err := syntax.ParseDID(sess.AccountDID.String()); err != nil { + return fmt.Errorf("invalid DID format: %w", err) + } + + // Validate token lengths (max 10000 chars to prevent memory issues) + const maxTokenLength = 10000 + if len(sess.AccessToken) > maxTokenLength { + return fmt.Errorf("access_token exceeds maximum length of %d characters", maxTokenLength) + } + if len(sess.RefreshToken) > maxTokenLength { + return fmt.Errorf("refresh_token exceeds maximum length of %d characters", maxTokenLength) + } + + // Validate session ID is not empty + if sess.SessionID == "" { + return fmt.Errorf("session_id cannot be empty") + } + + // Validate URLs if provided + if sess.HostURL != "" { + if _, err := url.Parse(sess.HostURL); err != nil { + return fmt.Errorf("invalid host_url: %w", err) + } + } + if sess.AuthServerURL != "" { + if _, err := url.Parse(sess.AuthServerURL); err != nil { + return fmt.Errorf("invalid auth_server URL: %w", err) + } + } + if sess.AuthServerTokenEndpoint != "" { + if _, err := url.Parse(sess.AuthServerTokenEndpoint); err != nil { + return fmt.Errorf("invalid auth_server_token_endpoint: %w", err) + } + } + if sess.AuthServerRevocationEndpoint != "" { + if _, err := url.Parse(sess.AuthServerRevocationEndpoint); err != nil { + return fmt.Errorf("invalid auth_server_revocation_endpoint: %w", err) + } + } + + query := ` + INSERT INTO oauth_sessions ( + did, session_id, handle, pds_url, host_url, + access_token, refresh_token, + dpop_private_jwk, dpop_private_key_multibase, + dpop_authserver_nonce, dpop_pds_nonce, + auth_server_iss, auth_server_token_endpoint, auth_server_revocation_endpoint, + scopes, expires_at, created_at, updated_at + ) VALUES ( + $1, $2, $3, $4, $5, + $6, $7, + NULL, $8, + $9, $10, + $11, $12, $13, + $14, $15, NOW(), NOW() + ) + ON CONFLICT (did, session_id) DO UPDATE SET + handle = EXCLUDED.handle, + pds_url = EXCLUDED.pds_url, + host_url = EXCLUDED.host_url, + access_token = EXCLUDED.access_token, + refresh_token = EXCLUDED.refresh_token, + dpop_private_key_multibase = EXCLUDED.dpop_private_key_multibase, + dpop_authserver_nonce = EXCLUDED.dpop_authserver_nonce, + dpop_pds_nonce = EXCLUDED.dpop_pds_nonce, + auth_server_iss = EXCLUDED.auth_server_iss, + auth_server_token_endpoint = EXCLUDED.auth_server_token_endpoint, + auth_server_revocation_endpoint = EXCLUDED.auth_server_revocation_endpoint, + scopes = EXCLUDED.scopes, + expires_at = EXCLUDED.expires_at, + updated_at = NOW() + ` + + // Calculate token expiration using configured TTL + expiresAt := time.Now().Add(s.sessionTTL) + + // Convert empty strings to NULL for optional fields + var authServerRevocationEndpoint sql.NullString + if sess.AuthServerRevocationEndpoint != "" { + authServerRevocationEndpoint.String = sess.AuthServerRevocationEndpoint + authServerRevocationEndpoint.Valid = true + } + + // Extract handle from DID (placeholder - in real implementation, resolve from identity) + // For now, use DID as handle since we don't have the handle in ClientSessionData + handle := sess.AccountDID.String() + + // Use HostURL as PDS URL + pdsURL := sess.HostURL + if pdsURL == "" { + pdsURL = sess.AuthServerURL // Fallback to auth server URL + } + + _, err := s.db.ExecContext( + ctx, query, + sess.AccountDID.String(), + sess.SessionID, + handle, + pdsURL, + sess.HostURL, + sess.AccessToken, + sess.RefreshToken, + sess.DPoPPrivateKeyMultibase, + sess.DPoPAuthServerNonce, + sess.DPoPHostNonce, + sess.AuthServerURL, + sess.AuthServerTokenEndpoint, + authServerRevocationEndpoint, + pq.Array(sess.Scopes), + expiresAt, + ) + if err != nil { + return fmt.Errorf("failed to save session: %w", err) + } + + return nil +} + +// DeleteSession deletes a session by DID and session ID +func (s *PostgresOAuthStore) DeleteSession(ctx context.Context, did syntax.DID, sessionID string) error { + query := `DELETE FROM oauth_sessions WHERE did = $1 AND session_id = $2` + + result, err := s.db.ExecContext(ctx, query, did.String(), sessionID) + if err != nil { + return fmt.Errorf("failed to delete session: %w", err) + } + + rows, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("failed to get rows affected: %w", err) + } + + if rows == 0 { + return ErrSessionNotFound + } + + return nil +} + +// GetAuthRequestInfo retrieves auth request information by state +func (s *PostgresOAuthStore) GetAuthRequestInfo(ctx context.Context, state string) (*oauth.AuthRequestData, error) { + query := ` + SELECT + state, did, handle, pds_url, pkce_verifier, + dpop_private_key_multibase, dpop_authserver_nonce, + auth_server_iss, request_uri, + auth_server_token_endpoint, auth_server_revocation_endpoint, + scopes, created_at + FROM oauth_requests + WHERE state = $1 + ` + + var info oauth.AuthRequestData + var did, handle, pdsURL sql.NullString + var dpopPrivateKeyMultibase, dpopAuthServerNonce sql.NullString + var requestURI, authServerTokenEndpoint, authServerRevocationEndpoint sql.NullString + var scopes pq.StringArray + var createdAt time.Time + + err := s.db.QueryRowContext(ctx, query, state).Scan( + &info.State, + &did, + &handle, + &pdsURL, + &info.PKCEVerifier, + &dpopPrivateKeyMultibase, + &dpopAuthServerNonce, + &info.AuthServerURL, + &requestURI, + &authServerTokenEndpoint, + &authServerRevocationEndpoint, + &scopes, + &createdAt, + ) + + if err == sql.ErrNoRows { + return nil, ErrAuthRequestNotFound + } + if err != nil { + return nil, fmt.Errorf("failed to get auth request info: %w", err) + } + + // Parse DID if present + if did.Valid && did.String != "" { + parsedDID, err := syntax.ParseDID(did.String) + if err != nil { + return nil, fmt.Errorf("failed to parse DID: %w", err) + } + info.AccountDID = &parsedDID + } + + // Convert nullable fields + if dpopPrivateKeyMultibase.Valid { + info.DPoPPrivateKeyMultibase = dpopPrivateKeyMultibase.String + } + if dpopAuthServerNonce.Valid { + info.DPoPAuthServerNonce = dpopAuthServerNonce.String + } + if requestURI.Valid { + info.RequestURI = requestURI.String + } + if authServerTokenEndpoint.Valid { + info.AuthServerTokenEndpoint = authServerTokenEndpoint.String + } + if authServerRevocationEndpoint.Valid { + info.AuthServerRevocationEndpoint = authServerRevocationEndpoint.String + } + info.Scopes = scopes + + return &info, nil +} + +// SaveAuthRequestInfo saves auth request information (create only, not upsert) +func (s *PostgresOAuthStore) SaveAuthRequestInfo(ctx context.Context, info oauth.AuthRequestData) error { + query := ` + INSERT INTO oauth_requests ( + state, did, handle, pds_url, pkce_verifier, + dpop_private_key_multibase, dpop_authserver_nonce, + auth_server_iss, request_uri, + auth_server_token_endpoint, auth_server_revocation_endpoint, + scopes, return_url, created_at + ) VALUES ( + $1, $2, $3, $4, $5, + $6, $7, + $8, $9, + $10, $11, + $12, NULL, NOW() + ) + ` + + // Extract DID string if present + var didStr sql.NullString + if info.AccountDID != nil { + didStr.String = info.AccountDID.String() + didStr.Valid = true + } + + // Convert empty strings to NULL for optional fields + var authServerRevocationEndpoint sql.NullString + if info.AuthServerRevocationEndpoint != "" { + authServerRevocationEndpoint.String = info.AuthServerRevocationEndpoint + authServerRevocationEndpoint.Valid = true + } + + // Placeholder values for handle and pds_url (not in AuthRequestData) + // In production, these would be resolved during the auth flow + handle := "" + pdsURL := "" + if info.AccountDID != nil { + handle = info.AccountDID.String() // Temporary placeholder + pdsURL = info.AuthServerURL // Temporary placeholder + } + + _, err := s.db.ExecContext( + ctx, query, + info.State, + didStr, + handle, + pdsURL, + info.PKCEVerifier, + info.DPoPPrivateKeyMultibase, + info.DPoPAuthServerNonce, + info.AuthServerURL, + info.RequestURI, + info.AuthServerTokenEndpoint, + authServerRevocationEndpoint, + pq.Array(info.Scopes), + ) + if err != nil { + // Check for duplicate state + if strings.Contains(err.Error(), "duplicate key") && strings.Contains(err.Error(), "oauth_requests_state_key") { + return fmt.Errorf("auth request with state already exists: %s", info.State) + } + return fmt.Errorf("failed to save auth request info: %w", err) + } + + return nil +} + +// DeleteAuthRequestInfo deletes auth request information by state +func (s *PostgresOAuthStore) DeleteAuthRequestInfo(ctx context.Context, state string) error { + query := `DELETE FROM oauth_requests WHERE state = $1` + + result, err := s.db.ExecContext(ctx, query, state) + if err != nil { + return fmt.Errorf("failed to delete auth request info: %w", err) + } + + rows, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("failed to get rows affected: %w", err) + } + + if rows == 0 { + return ErrAuthRequestNotFound + } + + return nil +} + +// CleanupExpiredSessions removes sessions that have expired +// Should be called periodically (e.g., via cron job) +func (s *PostgresOAuthStore) CleanupExpiredSessions(ctx context.Context) (int64, error) { + query := `DELETE FROM oauth_sessions WHERE expires_at < NOW()` + + result, err := s.db.ExecContext(ctx, query) + if err != nil { + return 0, fmt.Errorf("failed to cleanup expired sessions: %w", err) + } + + rows, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("failed to get rows affected: %w", err) + } + + return rows, nil +} + +// CleanupExpiredAuthRequests removes auth requests older than 30 minutes +// Should be called periodically (e.g., via cron job) +func (s *PostgresOAuthStore) CleanupExpiredAuthRequests(ctx context.Context) (int64, error) { + query := `DELETE FROM oauth_requests WHERE created_at < NOW() - INTERVAL '30 minutes'` + + result, err := s.db.ExecContext(ctx, query) + if err != nil { + return 0, fmt.Errorf("failed to cleanup expired auth requests: %w", err) + } + + rows, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("failed to get rows affected: %w", err) + } + + return rows, nil +} + +// MobileOAuthData holds mobile-specific OAuth flow data +type MobileOAuthData struct { + CSRFToken string + RedirectURI string +} + +// mobileFlowContextKey is the context key for mobile flow data +type mobileFlowContextKey struct{} + +// ContextWithMobileFlowData adds mobile flow data to a context. +// This is used by HandleMobileLogin to pass mobile data to the store wrapper, +// which will save it when SaveAuthRequestInfo is called by indigo. +func ContextWithMobileFlowData(ctx context.Context, data MobileOAuthData) context.Context { + return context.WithValue(ctx, mobileFlowContextKey{}, data) +} + +// getMobileFlowDataFromContext retrieves mobile flow data from context, if present +func getMobileFlowDataFromContext(ctx context.Context) (MobileOAuthData, bool) { + data, ok := ctx.Value(mobileFlowContextKey{}).(MobileOAuthData) + return data, ok +} + +// MobileAwareClientStore is a marker interface that indicates a store is properly +// configured for mobile OAuth flows. Only stores that intercept SaveAuthRequestInfo +// to save mobile CSRF data should implement this interface. +// This prevents silent mobile OAuth breakage when a plain PostgresOAuthStore is used. +type MobileAwareClientStore interface { + IsMobileAware() bool +} + +// MobileAwareStoreWrapper wraps a ClientAuthStore to automatically save mobile +// CSRF data when SaveAuthRequestInfo is called during a mobile OAuth flow. +// This is necessary because indigo's StartAuthFlow doesn't expose the OAuth state, +// so we intercept the SaveAuthRequestInfo call to capture it. +type MobileAwareStoreWrapper struct { + oauth.ClientAuthStore + mobileStore MobileOAuthStore +} + +// IsMobileAware implements MobileAwareClientStore, indicating this store +// properly saves mobile CSRF data during OAuth flow initiation. +func (w *MobileAwareStoreWrapper) IsMobileAware() bool { + return true +} + +// NewMobileAwareStoreWrapper creates a wrapper that intercepts SaveAuthRequestInfo +// to also save mobile CSRF data when present in context. +func NewMobileAwareStoreWrapper(store oauth.ClientAuthStore) *MobileAwareStoreWrapper { + wrapper := &MobileAwareStoreWrapper{ + ClientAuthStore: store, + } + // Check if the underlying store implements MobileOAuthStore + if ms, ok := store.(MobileOAuthStore); ok { + wrapper.mobileStore = ms + } + return wrapper +} + +// SaveAuthRequestInfo saves the auth request and also saves mobile CSRF data +// if mobile flow data is present in the context. +func (w *MobileAwareStoreWrapper) SaveAuthRequestInfo(ctx context.Context, info oauth.AuthRequestData) error { + // First, save the auth request to the underlying store + if err := w.ClientAuthStore.SaveAuthRequestInfo(ctx, info); err != nil { + return err + } + + // Check if this is a mobile flow (mobile data in context) + if mobileData, ok := getMobileFlowDataFromContext(ctx); ok && w.mobileStore != nil { + // Save mobile CSRF data tied to this OAuth state + // IMPORTANT: If this fails, we MUST propagate the error. Otherwise: + // 1. No server-side CSRF record is stored + // 2. Every mobile callback will "fail closed" to web flow + // 3. Mobile sign-in silently breaks with no indication + // Failing loudly here lets the user retry rather than being confused + // about why they're getting a web flow instead of mobile. + if err := w.mobileStore.SaveMobileOAuthData(ctx, info.State, mobileData); err != nil { + slog.Error("failed to save mobile CSRF data - mobile login will fail", + "state", info.State, "error", err) + return fmt.Errorf("failed to save mobile OAuth data: %w", err) + } + } + + return nil +} + +// GetMobileOAuthData implements MobileOAuthStore interface by delegating to underlying store +func (w *MobileAwareStoreWrapper) GetMobileOAuthData(ctx context.Context, state string) (*MobileOAuthData, error) { + if w.mobileStore != nil { + return w.mobileStore.GetMobileOAuthData(ctx, state) + } + return nil, nil +} + +// SaveMobileOAuthData implements MobileOAuthStore interface by delegating to underlying store +func (w *MobileAwareStoreWrapper) SaveMobileOAuthData(ctx context.Context, state string, data MobileOAuthData) error { + if w.mobileStore != nil { + return w.mobileStore.SaveMobileOAuthData(ctx, state, data) + } + return nil +} + +// UnwrapPostgresStore returns the underlying PostgresOAuthStore if present. +// This is useful for accessing cleanup methods that aren't part of the interface. +func (w *MobileAwareStoreWrapper) UnwrapPostgresStore() *PostgresOAuthStore { + if ps, ok := w.ClientAuthStore.(*PostgresOAuthStore); ok { + return ps + } + return nil +} + +// SaveMobileOAuthData stores mobile CSRF data tied to an OAuth state +// This ties the CSRF token to the OAuth flow via the state parameter, +// which comes back through the OAuth response for server-side validation. +func (s *PostgresOAuthStore) SaveMobileOAuthData(ctx context.Context, state string, data MobileOAuthData) error { + query := ` + UPDATE oauth_requests + SET mobile_csrf_token = $2, mobile_redirect_uri = $3 + WHERE state = $1 + ` + + result, err := s.db.ExecContext(ctx, query, state, data.CSRFToken, data.RedirectURI) + if err != nil { + return fmt.Errorf("failed to save mobile OAuth data: %w", err) + } + + rows, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("failed to get rows affected: %w", err) + } + + if rows == 0 { + return ErrAuthRequestNotFound + } + + return nil +} + +// GetMobileOAuthData retrieves mobile CSRF data by OAuth state +// This is called during callback to compare the server-side CSRF token +// (retrieved by state from the OAuth response) against the cookie CSRF. +func (s *PostgresOAuthStore) GetMobileOAuthData(ctx context.Context, state string) (*MobileOAuthData, error) { + query := ` + SELECT mobile_csrf_token, mobile_redirect_uri + FROM oauth_requests + WHERE state = $1 + ` + + var csrfToken, redirectURI sql.NullString + err := s.db.QueryRowContext(ctx, query, state).Scan(&csrfToken, &redirectURI) + + if err == sql.ErrNoRows { + return nil, ErrAuthRequestNotFound + } + if err != nil { + return nil, fmt.Errorf("failed to get mobile OAuth data: %w", err) + } + + // Return nil if no mobile data was stored (this was a web flow) + if !csrfToken.Valid { + return nil, nil + } + + return &MobileOAuthData{ + CSRFToken: csrfToken.String, + RedirectURI: redirectURI.String, + }, nil +} diff --git a/internal/atproto/oauth/store_test.go b/internal/atproto/oauth/store_test.go new file mode 100644 index 0000000..0fb8231 --- /dev/null +++ b/internal/atproto/oauth/store_test.go @@ -0,0 +1,522 @@ +package oauth + +import ( + "context" + "database/sql" + "os" + "testing" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" + _ "github.com/lib/pq" + "github.com/pressly/goose/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setupTestDB creates a test database connection and runs migrations +func setupTestDB(t *testing.T) *sql.DB { + dsn := os.Getenv("TEST_DATABASE_URL") + if dsn == "" { + dsn = "postgres://test_user:test_password@localhost:5434/coves_test?sslmode=disable" + } + + db, err := sql.Open("postgres", dsn) + require.NoError(t, err, "Failed to connect to test database") + + // Run migrations + require.NoError(t, goose.Up(db, "../../db/migrations"), "Failed to run migrations") + + return db +} + +// cleanupOAuth removes all test OAuth data from the database +func cleanupOAuth(t *testing.T, db *sql.DB) { + _, err := db.Exec("DELETE FROM oauth_sessions WHERE did LIKE 'did:plc:test%'") + require.NoError(t, err, "Failed to cleanup oauth_sessions") + + _, err = db.Exec("DELETE FROM oauth_requests WHERE state LIKE 'test%'") + require.NoError(t, err, "Failed to cleanup oauth_requests") +} + +func TestPostgresOAuthStore_SaveAndGetSession(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + did, err := syntax.ParseDID("did:plc:test123abc") + require.NoError(t, err) + + session := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "session123", + HostURL: "https://pds.example.com", + AuthServerURL: "https://auth.example.com", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + AuthServerRevocationEndpoint: "https://auth.example.com/oauth/revoke", + Scopes: []string{"atproto"}, + AccessToken: "at_test_token_abc123", + RefreshToken: "rt_test_token_xyz789", + DPoPAuthServerNonce: "nonce_auth_123", + DPoPHostNonce: "nonce_host_456", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + + // Save session + err = store.SaveSession(ctx, session) + assert.NoError(t, err) + + // Retrieve session + retrieved, err := store.GetSession(ctx, did, "session123") + assert.NoError(t, err) + assert.NotNil(t, retrieved) + assert.Equal(t, session.AccountDID.String(), retrieved.AccountDID.String()) + assert.Equal(t, session.SessionID, retrieved.SessionID) + assert.Equal(t, session.HostURL, retrieved.HostURL) + assert.Equal(t, session.AuthServerURL, retrieved.AuthServerURL) + assert.Equal(t, session.AuthServerTokenEndpoint, retrieved.AuthServerTokenEndpoint) + assert.Equal(t, session.AccessToken, retrieved.AccessToken) + assert.Equal(t, session.RefreshToken, retrieved.RefreshToken) + assert.Equal(t, session.DPoPAuthServerNonce, retrieved.DPoPAuthServerNonce) + assert.Equal(t, session.DPoPHostNonce, retrieved.DPoPHostNonce) + assert.Equal(t, session.DPoPPrivateKeyMultibase, retrieved.DPoPPrivateKeyMultibase) + assert.Equal(t, session.Scopes, retrieved.Scopes) +} + +func TestPostgresOAuthStore_SaveSession_Upsert(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + did, err := syntax.ParseDID("did:plc:testupsert") + require.NoError(t, err) + + // Initial session + session1 := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "session_upsert", + HostURL: "https://pds1.example.com", + AuthServerURL: "https://auth1.example.com", + AuthServerTokenEndpoint: "https://auth1.example.com/oauth/token", + Scopes: []string{"atproto"}, + AccessToken: "old_access_token", + RefreshToken: "old_refresh_token", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + + err = store.SaveSession(ctx, session1) + require.NoError(t, err) + + // Updated session (same DID and session ID) + session2 := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "session_upsert", + HostURL: "https://pds2.example.com", + AuthServerURL: "https://auth2.example.com", + AuthServerTokenEndpoint: "https://auth2.example.com/oauth/token", + Scopes: []string{"atproto", "transition:generic"}, + AccessToken: "new_access_token", + RefreshToken: "new_refresh_token", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktX", + } + + // Save again - should update + err = store.SaveSession(ctx, session2) + assert.NoError(t, err) + + // Retrieve should get updated values + retrieved, err := store.GetSession(ctx, did, "session_upsert") + assert.NoError(t, err) + assert.Equal(t, "new_access_token", retrieved.AccessToken) + assert.Equal(t, "new_refresh_token", retrieved.RefreshToken) + assert.Equal(t, "https://pds2.example.com", retrieved.HostURL) + assert.Equal(t, []string{"atproto", "transition:generic"}, retrieved.Scopes) +} + +func TestPostgresOAuthStore_GetSession_NotFound(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + did, err := syntax.ParseDID("did:plc:nonexistent") + require.NoError(t, err) + + _, err = store.GetSession(ctx, did, "nonexistent_session") + assert.ErrorIs(t, err, ErrSessionNotFound) +} + +func TestPostgresOAuthStore_DeleteSession(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + did, err := syntax.ParseDID("did:plc:testdelete") + require.NoError(t, err) + + session := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "session_delete", + HostURL: "https://pds.example.com", + AuthServerURL: "https://auth.example.com", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + Scopes: []string{"atproto"}, + AccessToken: "test_token", + RefreshToken: "test_refresh", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + + // Save session + err = store.SaveSession(ctx, session) + require.NoError(t, err) + + // Delete session + err = store.DeleteSession(ctx, did, "session_delete") + assert.NoError(t, err) + + // Verify session is gone + _, err = store.GetSession(ctx, did, "session_delete") + assert.ErrorIs(t, err, ErrSessionNotFound) +} + +func TestPostgresOAuthStore_DeleteSession_NotFound(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + did, err := syntax.ParseDID("did:plc:nonexistent") + require.NoError(t, err) + + err = store.DeleteSession(ctx, did, "nonexistent_session") + assert.ErrorIs(t, err, ErrSessionNotFound) +} + +func TestPostgresOAuthStore_SaveAndGetAuthRequestInfo(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + did, err := syntax.ParseDID("did:plc:testrequest") + require.NoError(t, err) + + info := oauth.AuthRequestData{ + State: "test_state_abc123", + AuthServerURL: "https://auth.example.com", + AccountDID: &did, + Scopes: []string{"atproto"}, + RequestURI: "urn:ietf:params:oauth:request_uri:abc123", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + AuthServerRevocationEndpoint: "https://auth.example.com/oauth/revoke", + PKCEVerifier: "verifier_xyz789", + DPoPAuthServerNonce: "nonce_abc", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + + // Save auth request info + err = store.SaveAuthRequestInfo(ctx, info) + assert.NoError(t, err) + + // Retrieve auth request info + retrieved, err := store.GetAuthRequestInfo(ctx, "test_state_abc123") + assert.NoError(t, err) + assert.NotNil(t, retrieved) + assert.Equal(t, info.State, retrieved.State) + assert.Equal(t, info.AuthServerURL, retrieved.AuthServerURL) + assert.NotNil(t, retrieved.AccountDID) + assert.Equal(t, info.AccountDID.String(), retrieved.AccountDID.String()) + assert.Equal(t, info.Scopes, retrieved.Scopes) + assert.Equal(t, info.RequestURI, retrieved.RequestURI) + assert.Equal(t, info.AuthServerTokenEndpoint, retrieved.AuthServerTokenEndpoint) + assert.Equal(t, info.PKCEVerifier, retrieved.PKCEVerifier) + assert.Equal(t, info.DPoPAuthServerNonce, retrieved.DPoPAuthServerNonce) + assert.Equal(t, info.DPoPPrivateKeyMultibase, retrieved.DPoPPrivateKeyMultibase) +} + +func TestPostgresOAuthStore_SaveAuthRequestInfo_NoDID(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + info := oauth.AuthRequestData{ + State: "test_state_nodid", + AuthServerURL: "https://auth.example.com", + AccountDID: nil, // No DID provided + Scopes: []string{"atproto"}, + RequestURI: "urn:ietf:params:oauth:request_uri:nodid", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + PKCEVerifier: "verifier_nodid", + DPoPAuthServerNonce: "nonce_nodid", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + + // Save auth request info without DID + err := store.SaveAuthRequestInfo(ctx, info) + assert.NoError(t, err) + + // Retrieve and verify DID is nil + retrieved, err := store.GetAuthRequestInfo(ctx, "test_state_nodid") + assert.NoError(t, err) + assert.Nil(t, retrieved.AccountDID) + assert.Equal(t, info.State, retrieved.State) +} + +func TestPostgresOAuthStore_GetAuthRequestInfo_NotFound(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + _, err := store.GetAuthRequestInfo(ctx, "nonexistent_state") + assert.ErrorIs(t, err, ErrAuthRequestNotFound) +} + +func TestPostgresOAuthStore_DeleteAuthRequestInfo(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + info := oauth.AuthRequestData{ + State: "test_state_delete", + AuthServerURL: "https://auth.example.com", + Scopes: []string{"atproto"}, + RequestURI: "urn:ietf:params:oauth:request_uri:delete", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + PKCEVerifier: "verifier_delete", + DPoPAuthServerNonce: "nonce_delete", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + + // Save auth request info + err := store.SaveAuthRequestInfo(ctx, info) + require.NoError(t, err) + + // Delete auth request info + err = store.DeleteAuthRequestInfo(ctx, "test_state_delete") + assert.NoError(t, err) + + // Verify it's gone + _, err = store.GetAuthRequestInfo(ctx, "test_state_delete") + assert.ErrorIs(t, err, ErrAuthRequestNotFound) +} + +func TestPostgresOAuthStore_DeleteAuthRequestInfo_NotFound(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + err := store.DeleteAuthRequestInfo(ctx, "nonexistent_state") + assert.ErrorIs(t, err, ErrAuthRequestNotFound) +} + +func TestPostgresOAuthStore_CleanupExpiredSessions(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + storeInterface := NewPostgresOAuthStore(db, 0) // Use default TTL + store, ok := storeInterface.(*PostgresOAuthStore) + require.True(t, ok, "store should be *PostgresOAuthStore") + ctx := context.Background() + + did1, err := syntax.ParseDID("did:plc:testexpired1") + require.NoError(t, err) + did2, err := syntax.ParseDID("did:plc:testexpired2") + require.NoError(t, err) + + // Create an expired session (manually insert with past expiration) + _, err = db.ExecContext(ctx, ` + INSERT INTO oauth_sessions ( + did, session_id, handle, pds_url, host_url, + access_token, refresh_token, + dpop_private_key_multibase, auth_server_iss, + auth_server_token_endpoint, scopes, + expires_at, created_at + ) VALUES ( + $1, $2, $3, $4, $5, + $6, $7, + $8, $9, + $10, $11, + NOW() - INTERVAL '1 day', NOW() + ) + `, did1.String(), "expired_session", "test.handle", "https://pds.example.com", "https://pds.example.com", + "expired_token", "expired_refresh", + "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", "https://auth.example.com", + "https://auth.example.com/oauth/token", `{"atproto"}`) + require.NoError(t, err) + + // Create a valid session + validSession := oauth.ClientSessionData{ + AccountDID: did2, + SessionID: "valid_session", + HostURL: "https://pds.example.com", + AuthServerURL: "https://auth.example.com", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + Scopes: []string{"atproto"}, + AccessToken: "valid_token", + RefreshToken: "valid_refresh", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + err = store.SaveSession(ctx, validSession) + require.NoError(t, err) + + // Cleanup expired sessions + count, err := store.CleanupExpiredSessions(ctx) + assert.NoError(t, err) + assert.Equal(t, int64(1), count, "Should delete 1 expired session") + + // Verify expired session is gone + _, err = store.GetSession(ctx, did1, "expired_session") + assert.ErrorIs(t, err, ErrSessionNotFound) + + // Verify valid session still exists + _, err = store.GetSession(ctx, did2, "valid_session") + assert.NoError(t, err) +} + +func TestPostgresOAuthStore_CleanupExpiredAuthRequests(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + storeInterface := NewPostgresOAuthStore(db, 0) + pgStore, ok := storeInterface.(*PostgresOAuthStore) + require.True(t, ok, "store should be *PostgresOAuthStore") + store := oauth.ClientAuthStore(pgStore) + ctx := context.Background() + + // Create an old auth request (manually insert with old timestamp) + _, err := db.ExecContext(ctx, ` + INSERT INTO oauth_requests ( + state, did, handle, pds_url, pkce_verifier, + dpop_private_key_multibase, dpop_authserver_nonce, + auth_server_iss, request_uri, + auth_server_token_endpoint, scopes, + created_at + ) VALUES ( + $1, $2, $3, $4, $5, + $6, $7, + $8, $9, + $10, $11, + NOW() - INTERVAL '1 hour' + ) + `, "test_old_state", "did:plc:testold", "test.handle", "https://pds.example.com", + "old_verifier", "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + "nonce_old", "https://auth.example.com", "urn:ietf:params:oauth:request_uri:old", + "https://auth.example.com/oauth/token", `{"atproto"}`) + require.NoError(t, err) + + // Create a recent auth request + recentInfo := oauth.AuthRequestData{ + State: "test_recent_state", + AuthServerURL: "https://auth.example.com", + Scopes: []string{"atproto"}, + RequestURI: "urn:ietf:params:oauth:request_uri:recent", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + PKCEVerifier: "recent_verifier", + DPoPAuthServerNonce: "nonce_recent", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + err = store.SaveAuthRequestInfo(ctx, recentInfo) + require.NoError(t, err) + + // Cleanup expired auth requests (older than 30 minutes) + count, err := pgStore.CleanupExpiredAuthRequests(ctx) + assert.NoError(t, err) + assert.Equal(t, int64(1), count, "Should delete 1 expired auth request") + + // Verify old request is gone + _, err = store.GetAuthRequestInfo(ctx, "test_old_state") + assert.ErrorIs(t, err, ErrAuthRequestNotFound) + + // Verify recent request still exists + _, err = store.GetAuthRequestInfo(ctx, "test_recent_state") + assert.NoError(t, err) +} + +func TestPostgresOAuthStore_MultipleSessions(t *testing.T) { + db := setupTestDB(t) + defer func() { _ = db.Close() }() + defer cleanupOAuth(t, db) + + store := NewPostgresOAuthStore(db, 0) // Use default TTL + ctx := context.Background() + + did, err := syntax.ParseDID("did:plc:testmulti") + require.NoError(t, err) + + // Create multiple sessions for the same DID + session1 := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "browser1", + HostURL: "https://pds.example.com", + AuthServerURL: "https://auth.example.com", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + Scopes: []string{"atproto"}, + AccessToken: "token_browser1", + RefreshToken: "refresh_browser1", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + + session2 := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "mobile_app", + HostURL: "https://pds.example.com", + AuthServerURL: "https://auth.example.com", + AuthServerTokenEndpoint: "https://auth.example.com/oauth/token", + Scopes: []string{"atproto"}, + AccessToken: "token_mobile", + RefreshToken: "refresh_mobile", + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktX", + } + + // Save both sessions + err = store.SaveSession(ctx, session1) + require.NoError(t, err) + err = store.SaveSession(ctx, session2) + require.NoError(t, err) + + // Retrieve both sessions + retrieved1, err := store.GetSession(ctx, did, "browser1") + assert.NoError(t, err) + assert.Equal(t, "token_browser1", retrieved1.AccessToken) + + retrieved2, err := store.GetSession(ctx, did, "mobile_app") + assert.NoError(t, err) + assert.Equal(t, "token_mobile", retrieved2.AccessToken) + + // Delete one session + err = store.DeleteSession(ctx, did, "browser1") + assert.NoError(t, err) + + // Verify only browser1 is deleted + _, err = store.GetSession(ctx, did, "browser1") + assert.ErrorIs(t, err, ErrSessionNotFound) + + // mobile_app should still exist + _, err = store.GetSession(ctx, did, "mobile_app") + assert.NoError(t, err) +} diff --git a/internal/atproto/oauth/transport.go b/internal/atproto/oauth/transport.go new file mode 100644 index 0000000..a04def1 --- /dev/null +++ b/internal/atproto/oauth/transport.go @@ -0,0 +1,99 @@ +package oauth + +import ( + "fmt" + "net" + "net/http" + "time" +) + +// ssrfSafeTransport wraps http.Transport to prevent SSRF attacks +type ssrfSafeTransport struct { + base *http.Transport + allowPrivate bool // For dev/testing only +} + +// isPrivateIP checks if an IP is in a private/reserved range +func isPrivateIP(ip net.IP) bool { + if ip == nil { + return false + } + + // Check for loopback + if ip.IsLoopback() { + return true + } + + // Check for link-local + if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { + return true + } + + // Check for private ranges + privateRanges := []string{ + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "169.254.0.0/16", + "::1/128", + "fc00::/7", + "fe80::/10", + } + + for _, cidr := range privateRanges { + _, network, err := net.ParseCIDR(cidr) + if err == nil && network.Contains(ip) { + return true + } + } + + return false +} + +func (t *ssrfSafeTransport) RoundTrip(req *http.Request) (*http.Response, error) { + host := req.URL.Hostname() + + // Resolve hostname to IP + ips, err := net.LookupIP(host) + if err != nil { + return nil, fmt.Errorf("failed to resolve host: %w", err) + } + + // Check all resolved IPs + if !t.allowPrivate { + for _, ip := range ips { + if isPrivateIP(ip) { + return nil, fmt.Errorf("SSRF blocked: %s resolves to private IP %s", host, ip) + } + } + } + + return t.base.RoundTrip(req) +} + +// NewSSRFSafeHTTPClient creates an HTTP client with SSRF protections +func NewSSRFSafeHTTPClient(allowPrivate bool) *http.Client { + transport := &ssrfSafeTransport{ + base: &http.Transport{ + DialContext: (&net.Dialer{ + Timeout: 10 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + }, + allowPrivate: allowPrivate, + } + + return &http.Client{ + Timeout: 15 * time.Second, + Transport: transport, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 5 { + return fmt.Errorf("too many redirects") + } + return nil + }, + } +} diff --git a/internal/atproto/oauth/transport_test.go b/internal/atproto/oauth/transport_test.go new file mode 100644 index 0000000..032d222 --- /dev/null +++ b/internal/atproto/oauth/transport_test.go @@ -0,0 +1,132 @@ +package oauth + +import ( + "net" + "net/http" + "testing" +) + +func TestIsPrivateIP(t *testing.T) { + tests := []struct { + name string + ip string + expected bool + }{ + // Loopback addresses + {"IPv4 loopback", "127.0.0.1", true}, + {"IPv6 loopback", "::1", true}, + + // Private IPv4 ranges + {"Private 10.x.x.x", "10.0.0.1", true}, + {"Private 10.x.x.x edge", "10.255.255.255", true}, + {"Private 172.16.x.x", "172.16.0.1", true}, + {"Private 172.31.x.x edge", "172.31.255.255", true}, + {"Private 192.168.x.x", "192.168.1.1", true}, + {"Private 192.168.x.x edge", "192.168.255.255", true}, + + // Link-local addresses + {"Link-local IPv4", "169.254.1.1", true}, + {"Link-local IPv6", "fe80::1", true}, + + // IPv6 private ranges + {"IPv6 unique local fc00", "fc00::1", true}, + {"IPv6 unique local fd00", "fd00::1", true}, + + // Public addresses + {"Public IP 1.1.1.1", "1.1.1.1", false}, + {"Public IP 8.8.8.8", "8.8.8.8", false}, + {"Public IP 172.15.0.1", "172.15.0.1", false}, // Just before 172.16/12 + {"Public IP 172.32.0.1", "172.32.0.1", false}, // Just after 172.31/12 + {"Public IP 11.0.0.1", "11.0.0.1", false}, // Just after 10/8 + {"Public IPv6", "2001:4860:4860::8888", false}, // Google DNS + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ip := net.ParseIP(tt.ip) + if ip == nil { + t.Fatalf("Failed to parse IP: %s", tt.ip) + } + + result := isPrivateIP(ip) + if result != tt.expected { + t.Errorf("isPrivateIP(%s) = %v, expected %v", tt.ip, result, tt.expected) + } + }) + } +} + +func TestIsPrivateIP_NilIP(t *testing.T) { + result := isPrivateIP(nil) + if result != false { + t.Errorf("isPrivateIP(nil) = %v, expected false", result) + } +} + +func TestNewSSRFSafeHTTPClient(t *testing.T) { + tests := []struct { + name string + allowPrivate bool + }{ + {"Production client (no private IPs)", false}, + {"Development client (allow private IPs)", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := NewSSRFSafeHTTPClient(tt.allowPrivate) + + if client == nil { + t.Fatal("NewSSRFSafeHTTPClient returned nil") + } + + if client.Timeout == 0 { + t.Error("Expected timeout to be set") + } + + if client.Transport == nil { + t.Error("Expected transport to be set") + } + + transport, ok := client.Transport.(*ssrfSafeTransport) + if !ok { + t.Error("Expected ssrfSafeTransport") + } + + if transport.allowPrivate != tt.allowPrivate { + t.Errorf("Expected allowPrivate=%v, got %v", tt.allowPrivate, transport.allowPrivate) + } + }) + } +} + +func TestSSRFSafeHTTPClient_RedirectLimit(t *testing.T) { + client := NewSSRFSafeHTTPClient(false) + + // Simulate checking redirect limit + if client.CheckRedirect == nil { + t.Fatal("Expected CheckRedirect to be set") + } + + // Test redirect limit (5 redirects) + var via []*http.Request + for i := 0; i < 5; i++ { + req := &http.Request{} + via = append(via, req) + } + + err := client.CheckRedirect(nil, via) + if err == nil { + t.Error("Expected error for too many redirects") + } + if err.Error() != "too many redirects" { + t.Errorf("Expected 'too many redirects' error, got: %v", err) + } + + // Test within limit (4 redirects) + via = via[:4] + err = client.CheckRedirect(nil, via) + if err != nil { + t.Errorf("Expected no error for 4 redirects, got: %v", err) + } +} diff --git a/internal/db/migrations/019_update_oauth_for_indigo.sql b/internal/db/migrations/019_update_oauth_for_indigo.sql new file mode 100644 index 0000000..6a4dff7 --- /dev/null +++ b/internal/db/migrations/019_update_oauth_for_indigo.sql @@ -0,0 +1,124 @@ +-- +goose Up +-- Update OAuth tables to match indigo's ClientAuthStore interface requirements +-- This migration adds columns needed for OAuth client sessions and auth requests + +-- Update oauth_requests table +-- Add columns for request URI, auth server endpoints, scopes, and DPoP key +ALTER TABLE oauth_requests + ADD COLUMN request_uri TEXT, + ADD COLUMN auth_server_token_endpoint TEXT, + ADD COLUMN auth_server_revocation_endpoint TEXT, + ADD COLUMN scopes TEXT[], + ADD COLUMN dpop_private_key_multibase TEXT; + +-- Make original dpop_private_jwk nullable (we now use dpop_private_key_multibase) +ALTER TABLE oauth_requests ALTER COLUMN dpop_private_jwk DROP NOT NULL; + +-- Make did nullable (indigo's AuthRequestData.AccountDID is a pointer - optional) +ALTER TABLE oauth_requests ALTER COLUMN did DROP NOT NULL; + +-- Make handle and pds_url nullable too (derived from DID resolution, not always available at auth request time) +ALTER TABLE oauth_requests ALTER COLUMN handle DROP NOT NULL; +ALTER TABLE oauth_requests ALTER COLUMN pds_url DROP NOT NULL; + +-- Update existing oauth_requests data +-- Convert dpop_private_jwk (JSONB) to multibase format if needed +-- Note: This will leave the multibase column NULL for now since conversion requires crypto logic +-- The application will need to handle NULL values or regenerate keys on next auth flow +UPDATE oauth_requests +SET + auth_server_token_endpoint = auth_server_iss || '/oauth/token', + scopes = ARRAY['atproto']::TEXT[] +WHERE auth_server_token_endpoint IS NULL; + +-- Add indexes for new columns +CREATE INDEX idx_oauth_requests_request_uri ON oauth_requests(request_uri) WHERE request_uri IS NOT NULL; + +-- Update oauth_sessions table +-- Add session_id column (will become part of composite key) +ALTER TABLE oauth_sessions + ADD COLUMN session_id TEXT, + ADD COLUMN host_url TEXT, + ADD COLUMN auth_server_token_endpoint TEXT, + ADD COLUMN auth_server_revocation_endpoint TEXT, + ADD COLUMN scopes TEXT[], + ADD COLUMN dpop_private_key_multibase TEXT; + +-- Make original dpop_private_jwk nullable (we now use dpop_private_key_multibase) +ALTER TABLE oauth_sessions ALTER COLUMN dpop_private_jwk DROP NOT NULL; + +-- Populate session_id for existing sessions (use DID as default for single-session per account) +-- In production, you may want to generate unique session IDs +UPDATE oauth_sessions +SET + session_id = 'default', + host_url = pds_url, + auth_server_token_endpoint = auth_server_iss || '/oauth/token', + scopes = ARRAY['atproto']::TEXT[] +WHERE session_id IS NULL; + +-- Make session_id NOT NULL after populating existing data +ALTER TABLE oauth_sessions + ALTER COLUMN session_id SET NOT NULL; + +-- Drop old unique constraint on did only +ALTER TABLE oauth_sessions + DROP CONSTRAINT IF EXISTS oauth_sessions_did_key; + +-- Create new composite unique constraint for (did, session_id) +-- This allows multiple sessions per account +-- Note: UNIQUE constraint automatically creates an index, so no separate index needed +ALTER TABLE oauth_sessions + ADD CONSTRAINT oauth_sessions_did_session_id_key UNIQUE (did, session_id); + +-- Add comment explaining the schema change +COMMENT ON COLUMN oauth_sessions.session_id IS 'Session identifier to support multiple concurrent sessions per account'; +COMMENT ON CONSTRAINT oauth_sessions_did_session_id_key ON oauth_sessions IS 'Composite key allowing multiple sessions per DID'; + +-- +goose Down +-- Rollback: Remove added columns and restore original unique constraint + +-- oauth_sessions rollback +-- Drop composite unique constraint (this also drops the associated index) +ALTER TABLE oauth_sessions + DROP CONSTRAINT IF EXISTS oauth_sessions_did_session_id_key; + +-- Delete all but the most recent session per DID before restoring unique constraint +-- This ensures the UNIQUE (did) constraint can be added without conflicts +DELETE FROM oauth_sessions a +USING oauth_sessions b +WHERE a.did = b.did + AND a.created_at < b.created_at; + +-- Restore old unique constraint +ALTER TABLE oauth_sessions + ADD CONSTRAINT oauth_sessions_did_key UNIQUE (did); + +-- Restore NOT NULL constraint on dpop_private_jwk +ALTER TABLE oauth_sessions + ALTER COLUMN dpop_private_jwk SET NOT NULL; + +ALTER TABLE oauth_sessions + DROP COLUMN IF EXISTS dpop_private_key_multibase, + DROP COLUMN IF EXISTS scopes, + DROP COLUMN IF EXISTS auth_server_revocation_endpoint, + DROP COLUMN IF EXISTS auth_server_token_endpoint, + DROP COLUMN IF EXISTS host_url, + DROP COLUMN IF EXISTS session_id; + +-- oauth_requests rollback +DROP INDEX IF EXISTS idx_oauth_requests_request_uri; + +-- Restore NOT NULL constraints +ALTER TABLE oauth_requests + ALTER COLUMN dpop_private_jwk SET NOT NULL, + ALTER COLUMN did SET NOT NULL, + ALTER COLUMN handle SET NOT NULL, + ALTER COLUMN pds_url SET NOT NULL; + +ALTER TABLE oauth_requests + DROP COLUMN IF EXISTS dpop_private_key_multibase, + DROP COLUMN IF EXISTS scopes, + DROP COLUMN IF EXISTS auth_server_revocation_endpoint, + DROP COLUMN IF EXISTS auth_server_token_endpoint, + DROP COLUMN IF EXISTS request_uri; diff --git a/internal/db/migrations/020_add_mobile_oauth_csrf.sql b/internal/db/migrations/020_add_mobile_oauth_csrf.sql new file mode 100644 index 0000000..b7bec65 --- /dev/null +++ b/internal/db/migrations/020_add_mobile_oauth_csrf.sql @@ -0,0 +1,23 @@ +-- +goose Up +-- Add columns for mobile OAuth CSRF protection with server-side state +-- This ties the CSRF token to the OAuth state, allowing validation against +-- a value that comes back through the OAuth response (the state parameter) +-- rather than only validating cookies against each other. + +ALTER TABLE oauth_requests + ADD COLUMN mobile_csrf_token TEXT, + ADD COLUMN mobile_redirect_uri TEXT; + +-- Index for quick lookup of mobile data when callback is received +CREATE INDEX idx_oauth_requests_mobile_csrf ON oauth_requests(state) + WHERE mobile_csrf_token IS NOT NULL; + +COMMENT ON COLUMN oauth_requests.mobile_csrf_token IS 'CSRF token for mobile OAuth flows, validated against cookie on callback'; +COMMENT ON COLUMN oauth_requests.mobile_redirect_uri IS 'Mobile redirect URI (Universal Link) for this OAuth flow'; + +-- +goose Down +DROP INDEX IF EXISTS idx_oauth_requests_mobile_csrf; + +ALTER TABLE oauth_requests + DROP COLUMN IF EXISTS mobile_redirect_uri, + DROP COLUMN IF EXISTS mobile_csrf_token;