From 15cfcf585db65488f5457da30dcc5920add78c56 Mon Sep 17 00:00:00 2001 From: Serge L Date: Sun, 1 Mar 2026 16:48:48 -0500 Subject: [PATCH] Fix: Yahoo Finance provider Cookie/Crumb Auth (#1082) * Fix: use cookie/crumb auth in healthy? chart endpoint check The health check was calling /v8/finance/chart/AAPL via the plain unauthenticated client. Yahoo Finance requires cookie + crumb authentication on the chart endpoint, so the health check would fail even when credentials are valid. Updated healthy? to use fetch_cookie_and_crumb + authenticated_client, consistent with fetch_security_prices and fetch_chart_data. Co-Authored-By: Claude Sonnet 4.6 * Fix: add cookie/crumb auth to all /v8/finance/chart/ calls fetch_security_prices and fetch_chart_data (used for exchange rates) were calling the chart endpoint without cookie/crumb authentication, inconsistent with healthy? and fetch_security_info. Added auth to both, including the same retry-on-Unauthorized pattern already used in fetch_security_info. Co-Authored-By: Claude Sonnet 4.6 * Update user-agent strings in yahoo_finance.rb Updated user-agent strings to reflect current browser versions Signed-off-by: Serge L * Fix: Add stale-crumb retry to healthy? and fetch_chart_data Yahoo Finance returns 200 OK with {"chart":{"error":{"code":"Unauthorized"}}} when a cached crumb expires server-side. Both healthy? and fetch_chart_data now mirror the retry pattern already in fetch_security_prices: detect the Unauthorized body, clear the crumb cache, fetch fresh credentials, and retry the request once. Adds a test for the healthy? retry path. Co-Authored-By: Claude Sonnet 4.6 * Refactor: Extract fetch_authenticated_chart helper to DRY crumb retry logic The cookie/crumb fetch + stale-crumb retry pattern was duplicated across healthy?, fetch_security_prices, and fetch_chart_data. Extract it into a single private fetch_authenticated_chart(symbol, params) helper that centralizes the retry logic; all three call sites now delegate to it. Co-Authored-By: Claude Sonnet 4.6 * Fix: Catch JSON::ParserError in fetch_chart_data rescue clause After moving JSON.parse inside fetch_authenticated_chart, a malformed Yahoo response would throw JSON::ParserError through fetch_chart_data's rescue Faraday::Error, breaking the inverse currency pair fallback. Co-Authored-By: Claude Sonnet 4.6 * Fix: Raise AuthenticationError if retry still returns Unauthorized After refreshing the crumb and retrying, if Yahoo still returns an Unauthorized error body the helper now raises AuthenticationError instead of silently returning the error payload. This prevents callers from misinterpreting a persistent auth failure as missing chart data. Co-Authored-By: Claude Sonnet 4.6 * Fix: Raise AuthenticationError after failed retry in fetch_security_info Mirrors the same post-retry Unauthorized check added to fetch_authenticated_chart. Without this, a persistent auth failure on the quoteSummary endpoint would surface as a generic "No security info found" error instead of an AuthenticationError. Co-Authored-By: Claude Sonnet 4.6 --------- Signed-off-by: Serge L Co-authored-by: Claude Sonnet 4.6 --- app/models/provider/yahoo_finance.rb | 91 +++++++++++++--------- test/models/provider/yahoo_finance_test.rb | 28 +++++-- 2 files changed, 77 insertions(+), 42 deletions(-) diff --git a/app/models/provider/yahoo_finance.rb b/app/models/provider/yahoo_finance.rb index 75b82520c..d4a992184 100644 --- a/app/models/provider/yahoo_finance.rb +++ b/app/models/provider/yahoo_finance.rb @@ -25,12 +25,13 @@ class Provider::YahooFinance < Provider # Pool of modern browser user-agents to rotate through # Based on https://github.com/ranaroussi/yfinance/pull/2277 + # UPDATED user-agents string on 2026-02-27 with current versions of browsers (Chrome 145, Firefox 148, Safari 26) USER_AGENTS = [ - "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36", - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36", - "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.2 Safari/605.1.15", - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36 Edg/131.0.0.0", - "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:133.0) Gecko/20100101 Firefox/133.0" + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/145.0.0.0 Safari/537.36", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/145.0.0.0 Safari/537.36", + "Mozilla/5.0 (Macintosh; Intel Mac OS X 15_7_4) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/26.0 Safari/605.1.15", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/145.0.0.0 Safari/537.36 Edg/145.0.0.0", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:148.0) Gecko/20100101 Firefox/148.0" ].freeze def initialize @@ -39,21 +40,11 @@ class Provider::YahooFinance < Provider end def healthy? - begin - # Test with a known stable ticker (Apple) - response = client.get("#{base_url}/v8/finance/chart/AAPL") do |req| - req.params["interval"] = "1d" - req.params["range"] = "1d" - end - - data = JSON.parse(response.body) - result = data.dig("chart", "result") - health_status = result.present? && result.any? - - health_status - rescue => e - false - end + data = fetch_authenticated_chart("AAPL", { "interval" => "1d", "range" => "1d" }) + result = data.dig("chart", "result") + result.present? && result.any? + rescue => e + false end def usage @@ -201,6 +192,9 @@ class Provider::YahooFinance < Provider req.params["crumb"] = crumb end data = JSON.parse(response.body) + if data.dig("quoteSummary", "error", "code") == "Unauthorized" + raise AuthenticationError, "Yahoo Finance authentication failed after crumb refresh" + end end result = data.dig("quoteSummary", "result", 0) @@ -271,14 +265,13 @@ class Provider::YahooFinance < Provider period2 = end_date.end_of_day.to_time.utc.to_i throttle_request - response = client.get("#{base_url}/v8/finance/chart/#{symbol}") do |req| - req.params["period1"] = period1 - req.params["period2"] = period2 - req.params["interval"] = "1d" - req.params["includeAdjustedClose"] = true - end + data = fetch_authenticated_chart(symbol, { + "period1" => period1, + "period2" => period2, + "interval" => "1d", + "includeAdjustedClose" => true + }) - data = JSON.parse(response.body) chart_data = data.dig("chart", "result", 0) raise Error, "No chart data found for #{symbol}" unless chart_data @@ -452,24 +445,48 @@ class Provider::YahooFinance < Provider rates end + # Makes a single authenticated GET to /v8/finance/chart/:symbol. + # If Yahoo returns a stale-crumb error (200 OK with Unauthorized body), + # clears the crumb cache and retries once with fresh credentials. + def fetch_authenticated_chart(symbol, params) + cookie, crumb = fetch_cookie_and_crumb + response = authenticated_client(cookie).get("#{base_url}/v8/finance/chart/#{symbol}") do |req| + params.each { |k, v| req.params[k] = v } + req.params["crumb"] = crumb + end + data = JSON.parse(response.body) + + if data.dig("chart", "error", "code") == "Unauthorized" + clear_crumb_cache + cookie, crumb = fetch_cookie_and_crumb + response = authenticated_client(cookie).get("#{base_url}/v8/finance/chart/#{symbol}") do |req| + params.each { |k, v| req.params[k] = v } + req.params["crumb"] = crumb + end + data = JSON.parse(response.body) + if data.dig("chart", "error", "code") == "Unauthorized" + raise AuthenticationError, "Yahoo Finance authentication failed after crumb refresh" + end + end + + data + end + def fetch_chart_data(symbol, start_date, end_date, &block) period1 = start_date.to_time.utc.to_i period2 = end_date.end_of_day.to_time.utc.to_i begin throttle_request - response = client.get("#{base_url}/v8/finance/chart/#{symbol}") do |req| - req.params["period1"] = period1 - req.params["period2"] = period2 - req.params["interval"] = "1d" - req.params["includeAdjustedClose"] = true - end - - data = JSON.parse(response.body) + data = fetch_authenticated_chart(symbol, { + "period1" => period1, + "period2" => period2, + "interval" => "1d", + "includeAdjustedClose" => true + }) # Check for Yahoo Finance errors if data.dig("chart", "error") - error_msg = data.dig("chart", "error", "description") || "Unknown Yahoo Finance error" return nil end @@ -489,7 +506,7 @@ class Provider::YahooFinance < Provider end results.sort_by(&:date) - rescue Faraday::Error => e + rescue Faraday::Error, JSON::ParserError => e nil end end diff --git a/test/models/provider/yahoo_finance_test.rb b/test/models/provider/yahoo_finance_test.rb index e7a569b65..6dd0d30a2 100644 --- a/test/models/provider/yahoo_finance_test.rb +++ b/test/models/provider/yahoo_finance_test.rb @@ -10,24 +10,42 @@ class Provider::YahooFinanceTest < ActiveSupport::TestCase # ================================ test "healthy? returns true when API is working" do - # Mock successful response mock_response = mock mock_response.stubs(:body).returns('{"chart":{"result":[{"meta":{"symbol":"AAPL"}}]}}') - @provider.stubs(:client).returns(mock_client = mock) + @provider.stubs(:fetch_cookie_and_crumb).returns([ "test_cookie", "test_crumb" ]) + @provider.stubs(:authenticated_client).returns(mock_client = mock) mock_client.stubs(:get).returns(mock_response) assert @provider.healthy? end test "healthy? returns false when API fails" do - # Mock failed response - @provider.stubs(:client).returns(mock_client = mock) - mock_client.stubs(:get).raises(Faraday::Error.new("Connection failed")) + @provider.stubs(:fetch_cookie_and_crumb).raises(Provider::YahooFinance::AuthenticationError.new("auth failed")) assert_not @provider.healthy? end + test "healthy? retries with fresh crumb on Unauthorized body response" do + unauthorized_body = '{"chart":{"error":{"code":"Unauthorized","description":"No crumb"}}}' + success_body = '{"chart":{"result":[{"meta":{"symbol":"AAPL"}}]}}' + + unauthorized_response = mock + unauthorized_response.stubs(:body).returns(unauthorized_body) + + success_response = mock + success_response.stubs(:body).returns(success_body) + + mock_client = mock + mock_client.stubs(:get).returns(unauthorized_response, success_response) + + @provider.stubs(:fetch_cookie_and_crumb).returns([ "cookie1", "crumb1" ], [ "cookie2", "crumb2" ]) + @provider.stubs(:authenticated_client).returns(mock_client) + @provider.expects(:clear_crumb_cache).once + + assert @provider.healthy? + end + # ================================ # Exchange Rate Tests # ================================