Skip to content

feat(auth): add fresh login and logout flow #270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
voidborne-d wants to merge 3 commits into
base: main
Choose a base branch
from
+312 −4
Open

feat(auth): add fresh login and logout flow #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cca664d
feat(auth): add fresh login and logout flow
voidborne-d Apr 12, 2026
2f810e0
fix auth cleanup edge cases
voidborne Apr 12, 2026
f3a5fe0
fix(auth): recover page after login handoff
voidborne Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 117 additions & 3 deletions src/notebooklm/cli/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import json
import logging
import os
import shutil
import subprocess
import sys
import time
Expand All @@ -35,6 +36,7 @@
from ..paths import (
get_browser_profile_dir,
get_context_path,
get_home_dir,
get_path_info,
get_storage_path,
)
Expand Down Expand Up @@ -340,6 +342,45 @@ def _ensure_chromium_installed() -> None:
)


def _clear_auth_files(storage_path: Path, browser_profile: Path) -> list[Path]:
"""Delete auth files/directories for the active profile.

Also checks legacy default-profile paths when the resolved paths are profile-based.
Returns the paths that were actually removed.
"""
paths_to_remove = [storage_path, browser_profile]

legacy_storage = get_home_dir() / "storage_state.json"
legacy_browser = get_home_dir() / "browser_profile"
for legacy_path in (legacy_storage, legacy_browser):
if legacy_path not in paths_to_remove:
paths_to_remove.append(legacy_path)
Comment on lines +345 to +357
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Limit legacy cleanup to the default profile.

_clear_auth_files() always appends the legacy home-root auth paths, so login --fresh / auth logout on a non-default profile also wipes the legacy default profile's session. That breaks profile isolation while the command text says it only affects the active profile.

Proposed fix 
-def _clear_auth_files(storage_path: Path, browser_profile: Path) -> list[Path]:
+def _clear_auth_files(
+    storage_path: Path, browser_profile: Path, *, clear_legacy_default: bool = False
+) -> list[Path]:
     """Delete auth files/directories for the active profile.

     Also checks legacy default-profile paths when the resolved paths are profile-based.
     Returns the paths that were actually removed.
     """
     paths_to_remove = [storage_path, browser_profile]

-    legacy_storage = get_home_dir() / "storage_state.json"
-    legacy_browser = get_home_dir() / "browser_profile"
-    for legacy_path in (legacy_storage, legacy_browser):
-        if legacy_path not in paths_to_remove:
-            paths_to_remove.append(legacy_path)
+    if clear_legacy_default:
+        home_dir = get_home_dir()
+        for legacy_path in (home_dir / "storage_state.json", home_dir / "browser_profile"):
+            if legacy_path not in paths_to_remove:
+                paths_to_remove.append(legacy_path)

Update the two call sites to pass clear_legacy_default=True only when the resolved profile is "default".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/notebooklm/cli/session.py` around lines 345 - 357, _clear_auth_files
currently always appends legacy home-root auth paths (legacy_storage and
legacy_browser) which causes logout/--fresh on non-default profiles to wipe the
default profile; change _clear_auth_files(signature) to accept a boolean
parameter clear_legacy_default (default False) and only append legacy_storage
and legacy_browser when clear_legacy_default is True, then update the two call
sites that invoke _clear_auth_files to pass clear_legacy_default=True only when
the resolved profile name equals "default" so legacy cleanup is limited to the
default profile.


removed: list[Path] = []
for path in paths_to_remove:
if not path.exists():
continue
if path.is_symlink():
path.unlink()
elif path.is_dir():
shutil.rmtree(path)
else:
path.unlink(missing_ok=True)
Comment on lines +360 to +368
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle broken symlinks before the exists() guard.

A dangling symlink makes Path.exists() return False, so this loop skips it instead of unlinking it. That leaves stale storage_state.json / browser_profile entries behind, and the later login setup can fail with a pre-existing path.

Proposed fix 
     removed: list[Path] = []
     for path in paths_to_remove:
-        if not path.exists():
-            continue
         if path.is_symlink():
             path.unlink()
+            removed.append(path)
+            continue
+        if not path.exists():
+            continue
         elif path.is_dir():
             shutil.rmtree(path)
         else:
             path.unlink(missing_ok=True)
         removed.append(path)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/notebooklm/cli/session.py` around lines 360 - 368, The loop over
paths_to_remove incorrectly skips broken symlinks because Path.exists() is False
for dangling links; change the existence guard to detect symlinks too (e.g., use
path.lexists() or test (path.exists() or path.is_symlink())) so dangling
symlinks are unlinked; update the loop around paths_to_remove (the block that
calls path.unlink(), shutil.rmtree(path), and path.unlink(missing_ok=True)) to
first consider path.is_symlink() or use lexists() before continuing.

removed.append(path)

return removed


def _get_recovered_page(context: Any, current_page: Any) -> Any:
"""Recover a live page when the original Playwright page was closed."""
pages = [page for page in context.pages if not page.is_closed()]
if pages:
return pages[0]
if current_page and not current_page.is_closed():
return current_page
return context.new_page()


def register_session_commands(cli):
"""Register session commands on the main CLI group."""

Expand Down Expand Up @@ -368,7 +409,12 @@ def register_session_commands(cli):
"Requires: pip install 'notebooklm[cookies]'"
),
)
def login(storage, browser, browser_cookies):
@click.option(
"--fresh",
is_flag=True,
help="Delete saved browser session before login so you can choose a different account.",
)
def login(storage, browser, browser_cookies, fresh):
"""Log in to NotebookLM via browser.

Opens a browser window for Google login. After logging in,
Expand Down Expand Up @@ -399,6 +445,20 @@ def login(storage, browser, browser_cookies):

storage_path = Path(storage) if storage else get_storage_path()
browser_profile = get_browser_profile_dir()

if fresh:
try:
removed = _clear_auth_files(storage_path, browser_profile)
except OSError as e:
console.print(
"[red]Could not clear the saved browser session.[/red]\n"
"Close any running NotebookLM/Chromium windows and try again.\n"
f"Details: {e}"
)
raise SystemExit(1) from e
if removed:
console.print("[yellow]Cleared saved browser session for a fresh login.[/yellow]")

if sys.platform == "win32":
# On Windows < Python 3.13, mode= is ignored by mkdir(). On
# Python 3.13+, mode= applies Windows ACLs that can be overly
Expand Down Expand Up @@ -463,12 +523,31 @@ def login(storage, browser, browser_cookies):
break
except PlaywrightError as exc:
error_str = str(exc)
is_target_closed = (
"target page, context or browser has been closed" in error_str.lower()
)
is_retryable = any(
code in error_str for code in RETRYABLE_CONNECTION_ERRORS
)

# Check if we should retry
if is_retryable and attempt < LOGIN_MAX_RETRIES:
if is_target_closed and attempt < LOGIN_MAX_RETRIES:
page = _get_recovered_page(context, page)
backoff_seconds = attempt
console.print(
f"[yellow]Browser page was replaced during login "
f"(attempt {attempt}/{LOGIN_MAX_RETRIES}). "
f"Retrying in {backoff_seconds}s...[/yellow]"
)
time.sleep(backoff_seconds)
elif is_target_closed:
logger.error("Login page kept closing during account switch")
console.print(
"[red]The login page kept closing while switching accounts.[/red]\n"
"Retry with 'notebooklm login --fresh' to start from a clean browser session."
)
raise SystemExit(1) from None
elif is_retryable and attempt < LOGIN_MAX_RETRIES:
# Retryable error with attempts remaining: retry
backoff_seconds = attempt # Linear backoff: 1s, 2s
logger.debug(
Expand Down Expand Up @@ -499,6 +578,7 @@ def login(storage, browser, browser_cookies):
console.print("3. Press [bold]ENTER[/bold] here to save and close\n")

input("[Press ENTER when logged in] ")
page = _get_recovered_page(context, page)

# Force .google.com cookies for regional users (e.g. UK lands on
# .google.co.uk). Use "commit" to resolve once response headers
Expand All @@ -508,7 +588,11 @@ def login(storage, browser, browser_cookies):
try:
page.goto(url, wait_until="commit")
except PlaywrightError as exc:
if "Navigation interrupted" not in str(exc):
error_str = str(exc).lower()
if "target page, context or browser has been closed" in error_str:
page = _get_recovered_page(context, page)
page.goto(url, wait_until="commit")
elif "navigation interrupted" not in error_str:
raise

current_url = page.url
Expand Down Expand Up @@ -948,3 +1032,33 @@ def format_cookie_name(name: str) -> str:
console.print(
"\n[yellow]Cookies may be expired. Run 'notebooklm login' to refresh.[/yellow]"
)

@auth_group.command("logout")
def auth_logout():
"""Clear saved authentication state for the active profile."""
has_inline_auth = bool(os.environ.get("NOTEBOOKLM_AUTH_JSON"))
storage_path = get_storage_path()
browser_profile = get_browser_profile_dir()

try:
removed = _clear_auth_files(storage_path, browser_profile)
except OSError as e:
console.print(
"[red]Could not remove saved auth files.[/red]\n"
"Close any running browser windows for this profile and try again.\n"
f"Details: {e}"
)
raise SystemExit(1) from e

if removed:
console.print(
"[green]Logged out. Removed saved browser session and auth state.[/green]"
)
else:
console.print("[yellow]No saved auth state found for the active profile.[/yellow]")

if has_inline_auth:
console.print(
"[yellow]NOTEBOOKLM_AUTH_JSON is still set, so inline auth remains active.[/yellow]\n"
"Unset NOTEBOOKLM_AUTH_JSON (or restart your shell) to fully log out."
)
Loading
Loading