Add /portal/sso/embed step to fix intermittent 401s by matin · Pull Request #209 · matin/garth

matin · 2026-03-18T09:55:30Z

Summary

Add the missing /portal/sso/embed step to the SSO login flow, matching what the iOS Garmin Connect app does. This sets the Cloudflare load balancer cookie (__cflb) that pins subsequent requests to the same backend server.

Without this step, the preauthorized call can hit a different backend that doesn't have the SSO session, causing intermittent 401 "Client app validation failed" errors. Telemetry data confirmed that non-MFA sessions had a much higher failure rate (15/48 = 31%) compared to MFA sessions (3/340 = <1%).

Thanks to @NiklasCa for reporting and providing session IDs that helped diagnose this.

Test plan

163 tests pass (VCR cassettes updated with new interaction)
Manual non-MFA login + API call verified against real API
CodeRabbit review clean

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved Single Sign-On reliability and backend routing during login to ensure consistent authentication.
Tests
- Added a safeguard to disable telemetry at import time during test runs to avoid unexpected test-side effects.
Chores
- Bumped package version to 0.7.3.

The iOS app visits /portal/sso/embed after login (and MFA if applicable) before calling preauthorized. This sets the Cloudflare load balancer cookie (__cflb) that pins subsequent requests to the same backend. Without it, preauthorized can hit a different backend that doesn't have the session, causing intermittent 401 errors. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

coderabbitai · 2026-03-18T09:56:37Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 09645a1a-3692-420c-903f-b6fe3d2a7f02

📥 Commits

Reviewing files that changed from the base of the PR and between 864478d and c436508.

📒 Files selected for processing (2)

src/garth/sso.py
tests/conftest.py

Walkthrough

Adds a pre-token GET to the SSO embed endpoint to establish Cloudflare LB session and copies cookies from a provided parent session; also bumps package version and prevents telemetry during test imports.

Changes

Cohort / File(s)	Summary
SSO session & cookie handling `src/garth/sso.py`	In `GarminOAuth1Session.__init__` copy cookies from `parent` when provided. In `_complete_login` perform a GET to `sso/portal/sso/embed` before exchanging/obtaining OAuth1/OAuth2 tokens to validate SSO session and set Cloudflare LB cookie.
Version bump `src/garth/version.py`	Updated `__version__` from `"0.7.2"` to `"0.7.3"`.
Test import-time telemetry guard `tests/conftest.py`	Set `GARTH_TELEMETRY_ENABLED="false"` before importing `garth` to prevent module-level telemetry/client initialization during test import.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client (garth)
    participant Parent as Parent Session
    participant CF as Cloudflare LB
    participant SSO as SSO Embed (`sso/portal/sso/embed`)
    participant OAuth as OAuth endpoints

    Client->>Parent: (optional) copy cookies
    Note right of Client: GarminOAuth1Session.__init__
    Client->>SSO: GET sso/portal/sso/embed
    SSO->>CF: set LB cookie (response)
    CF->>Client: LB cookie set in response
    Client->>OAuth: proceed to obtain OAuth1/OAuth2 tokens
    OAuth-->>Client: tokens

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Add /portal/sso/embed step to fix intermittent 401s #209: Also modifies src/garth/sso.py to add the pre-token GET to sso/portal/sso/embed to set the Cloudflare LB cookie.
Never hit real Garmin API in tests #208: Also updates tests/conftest.py to disable telemetry at import time for tests.

Suggested reviewers

felipao-mx

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add /portal/sso/embed step to fix intermittent 401s' directly matches the main change: adding the /portal/sso/embed endpoint call to the login flow to resolve intermittent 401 errors.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch add-missing-sso-steps

📝 Coding Plan

Generate coding plan for human review comments

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.}

codecov · 2026-03-18T09:56:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.97%. Comparing base (51d821f) to head (c436508).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #209   +/-   ##
=======================================
  Coverage   99.97%   99.97%           
=======================================
  Files          68       68           
  Lines        3555     3558    +3     
=======================================
+ Hits         3554     3557    +3     
  Misses          1        1

Flag	Coverage Δ
unittests	`99.97% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

semicolon-spec · 2026-03-18T09:59:56Z

tried this PR and for me it still fails, not sure how usefull the session id is: Garth session: 0a7123f8f34646c2

The module-level Client() in http.py runs at import time and configures logfire when telemetry is enabled (the new default). Logfire's background token verification requests interfere with VCR cassette matching. Setting the env var before import prevents this. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/garth/sso.py`:
- Around line 221-223: The Cloudflare LB cookie set by the embed request on
client.sess isn't propagated to the OAuth1 session; update
GarminOAuth1Session.__init__ to copy cookies from the parent/session that
performed client.get("sso", "/portal/sso/embed") into the OAuth1 session's
cookie jar (e.g., clone or update self.cookies from the provided parent/session
object) so subsequent OAuth1 requests use the same LB cookie for backend
pinning.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b3392c8a-1587-4c12-8fd1-742b6ab75f51

📥 Commits

Reviewing files that changed from the base of the PR and between 51d821f and 864478d.

⛔ Files ignored due to path filters (6)

tests/cassettes/test_login_command.yaml is excluded by !tests/**/cassettes/**
tests/cassettes/test_login_return_on_mfa.yaml is excluded by !tests/**/cassettes/**
tests/cassettes/test_login_success.yaml is excluded by !tests/**/cassettes/**
tests/cassettes/test_login_success_mfa.yaml is excluded by !tests/**/cassettes/**
tests/cassettes/test_login_success_mfa_async.yaml is excluded by !tests/**/cassettes/**
tests/cassettes/test_resume_login.yaml is excluded by !tests/**/cassettes/**

📒 Files selected for processing (2)

src/garth/sso.py
src/garth/version.py

src/garth/sso.py

The /portal/sso/embed step sets the Cloudflare LB cookie on client.sess, but GarminOAuth1Session wasn't copying cookies from the parent session. The preauthorized call would hit a different backend without the session context. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

matin · 2026-03-18T10:04:00Z

@semicolon-spec try now

NiklasCa · 2026-03-18T10:05:41Z

tried this PR and for me it still fails, not sure how usefull the session id is: Garth session: 0a7123f8f34646c2

Yes, still failing (Garth session: 5c5d62a9179c483f)
However, different error now:
garth.exc.GarthHTTPError: Error in request: 403 Client Error: Forbidden for url: https://sso.garmin.com/portal/sso/embed

semicolon-spec · 2026-03-18T10:07:07Z

still failing, Garth session: 397ea6f140c14c2d

matin and others added 2 commits March 18, 2026 02:51

Bump version to 0.7.3

864478d

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

coderabbitai bot reviewed Mar 18, 2026

View reviewed changes

src/garth/sso.py Show resolved Hide resolved

matin merged commit 868af84 into main Mar 18, 2026
25 checks passed

matin deleted the add-missing-sso-steps branch March 18, 2026 10:05

matin mentioned this pull request Mar 18, 2026

Did Garmin change authentication API? cyberjunky/python-garminconnect#332

Open

This was referenced Mar 18, 2026

Make /portal/sso/embed best-effort to handle Cloudflare challenges #210

Merged

Use consistent browser headers on all SSO endpoints #211

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add /portal/sso/embed step to fix intermittent 401s#209

Add /portal/sso/embed step to fix intermittent 401s#209
matin merged 4 commits intomainfrom
add-missing-sso-steps

matin commented Mar 18, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 18, 2026 •

edited

Loading

Review failed

❌ Failed checks (1 warning)

Uh oh!

codecov bot commented Mar 18, 2026 •

edited

Loading

Uh oh!

semicolon-spec commented Mar 18, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

matin commented Mar 18, 2026

Uh oh!

Uh oh!

NiklasCa commented Mar 18, 2026

Uh oh!

semicolon-spec commented Mar 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

matin commented Mar 18, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

codecov bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

semicolon-spec commented Mar 18, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matin commented Mar 18, 2026

Uh oh!

Uh oh!

NiklasCa commented Mar 18, 2026

Uh oh!

semicolon-spec commented Mar 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

matin commented Mar 18, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 18, 2026 •

edited

Loading

codecov bot commented Mar 18, 2026 •

edited

Loading