Skip to content

fix: preserve auth cookies across Google redirects#274

Open
voidborne-d wants to merge 1 commit intoteng-lin:mainfrom
voidborne-d:fix/auth-cookie-jar-redirects
Open

fix: preserve auth cookies across Google redirects#274
voidborne-d wants to merge 1 commit intoteng-lin:mainfrom
voidborne-d:fix/auth-cookie-jar-redirects

Conversation

@voidborne-d
Copy link
Copy Markdown
Contributor

@voidborne-d voidborne-d commented Apr 13, 2026

Summary

  • switch auth bootstrap and core client setup from raw Cookie headers to httpx cookie jars
  • keep accounts.google.com cookies when loading storage state so Google auth redirects stay authenticated
  • add regression coverage for the new cookie handling paths

Testing

  • python3 -m pytest tests/unit/test_auth.py tests/unit/test_client.py -q
  • python3 -m pytest tests/unit/cli/test_session.py -q

Closes #273

Summary by CodeRabbit

  • Bug Fixes

    • Refactored HTTP cookie handling to use proper cookie jar management instead of manual header injection during authentication requests.
    • Extended supported authentication domains to include accounts.google.com for cookie extraction.
  • Tests

    • Added comprehensive test coverage validating cookie jar initialization and authentication header updates in the client core.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c820dda6-033f-457a-9981-c6f4ac6c05fa

📥 Commits

Reviewing files that changed from the base of the PR and between a997718 and 2e2e9db.

📒 Files selected for processing (4)
  • src/notebooklm/_core.py
  • src/notebooklm/auth.py
  • tests/unit/test_auth.py
  • tests/unit/test_client.py

📝 Walkthrough

Walkthrough

The PR fixes an authentication bug where raw HTTP Cookie headers were dropped during cross-domain redirects. Changes replace raw cookie header injection with httpx's native cookie jar mechanism in both ClientCore and fetch_tokens(), and expand allowed cookie domains to include accounts.google.com.

Changes

Cohort / File(s) Summary
Cookie jar adoption
src/notebooklm/_core.py, src/notebooklm/auth.py
Replaced raw Cookie header strings with httpx.Cookies cookie jar (cookies= parameter). Updated ClientCore.open() and fetch_tokens() to configure client with cookie jar; modified update_auth_headers() to clear and repopulate jar instead of mutating headers.
Cookie domain expansion
src/notebooklm/auth.py
Added accounts.google.com to ALLOWED_COOKIE_DOMAINS to preserve cross-domain redirect cookies (e.g., ACCOUNT_CHOOSER, LSID).
Test coverage
tests/unit/test_auth.py, tests/unit/test_client.py
Extended auth test fixture with ACCOUNT_CHOOSER cookie from accounts.google.com; refactored fetch_tokens test to validate cookie jar behavior; added new TestClientCoreCookies class verifying httpx client is initialized with cookie jar rather than raw headers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #245: Modifies auth-related cookie handling in src/notebooklm/auth.py (cookie domain and processing logic), directly overlapping with this PR's scope.

Poem

🐰 A cookie jar beats a header string!
Cross-domain hops now have wings,
No drops on redirects so sly—
The auth shall not fail, not I! 🍪✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing raw Cookie headers with httpx cookie jars to preserve cookies across Google authentication redirects.
Linked Issues check ✅ Passed All three required fixes from issue #273 are implemented: (1) replaced raw Cookie header with httpx.Cookies in fetch_tokens(), (2) same in ClientCore.open(), (3) expanded ALLOWED_COOKIE_DOMAINS to include accounts.google.com.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue #273. Updates to auth.py, _core.py, and new tests in test_auth.py and test_client.py are all within scope.
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the httpx client to use the built-in cookie jar instead of manual Cookie headers and expands the allowed cookie domains to include accounts.google.com. Review feedback suggests ensuring that session cookies updated by the server are synchronized back to the local state to maintain session continuity and recommends adopting granular timeouts for better network resilience.

Comment on lines +174 to 176
self._http_client.cookies.clear()
self._http_client.cookies.update(self.auth.cookies)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Clearing the cookie jar here will discard any session cookies or updates received from the server during previous requests (including the refresh request itself) that haven't been manually synced back to self.auth.cookies. Since self.auth.cookies is a simple dictionary, it doesn't automatically track changes in the httpx cookie jar. To maintain session continuity, consider updating self.auth.cookies from the jar before resetting, or simply updating the jar without clearing it.

Suggested change
self._http_client.cookies.clear()
self._http_client.cookies.update(self.auth.cookies)
self.auth.cookies.update(self._http_client.cookies)
self._http_client.cookies.clear()
self._http_client.cookies.update(self.auth.cookies)

Comment on lines +664 to 667
async with httpx.AsyncClient(cookies=cookies, follow_redirects=True, timeout=30.0) as client:
response = await client.get("https://notebooklm.google.com/")
response.raise_for_status()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The httpx.AsyncClient correctly handles cookies during redirects, but any new cookies set by the server are currently lost when the client is closed. Updating the cookies dictionary ensures these updates are preserved. Additionally, when using httpx.AsyncClient, configure granular timeouts with a shorter connect timeout and a longer read timeout to improve network resilience.

Suggested change
async with httpx.AsyncClient(cookies=cookies, follow_redirects=True, timeout=30.0) as client:
response = await client.get("https://notebooklm.google.com/")
response.raise_for_status()
async with httpx.AsyncClient(cookies=cookies, follow_redirects=True, timeout=httpx.Timeout(10.0, read=60.0)) as client:
response = await client.get("https://notebooklm.google.com/")
response.raise_for_status()
cookies.update(client.cookies)
References
  1. When using httpx.AsyncClient, configure granular timeouts with a shorter connect timeout and a longer read timeout (e.g., httpx.Timeout(10.0, read=60.0)) to improve network resilience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auth fails after ~10 min: raw Cookie header dropped on cross-domain redirects

1 participant