Fix: updated POST Login headers to bypass new cloudflare block#226
Fix: updated POST Login headers to bypass new cloudflare block#226brunosantos wants to merge 3 commits intomatin:mainfrom
Conversation
spoofing user agent with new values to bypass new cloudflare blocks
WalkthroughBundle of mostly documentation updates plus behavioral defaults and small code changes: README/docs deprecation text removed and badges/quick-start added; telemetry defaults flipped to enabled; SSO User-Agent string updated; package version changed; deprecation warning removed; tests adjusted to new telemetry defaults. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Apply fix from matin/garth#226 — SSO sign-in pages require a desktop browser User-Agent to avoid Cloudflare blocking. The mobile app UA is kept for API calls only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
How can this work if old oauth endpoints are gone? Do you get oauth1/oauth2 tokens with this? |
|
I tested the lib by running: And got the oauth1/oauth2 tokens stored in the new folder and got activities back from the API calls. From what I can see it's not that Garmin removed the existing OAuth endpoints, but that they do a basic user agent validation. By impersonating as a browser instead of a native app this works. |
|
I'm also considering adding back the readme and other md files to the previous state (from this brunosantos@f99159a) given that the lib can work with a small change. But maybe it's beyond the scope of this PR and needs consent from @matin. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
README.md (3)
42-52: Remove import statements from code examples.Same issue as above - remove
import garthandfrom garth.exc import GarthException.📝 Suggested fix
```python -import garth -from garth.exc import GarthException garth.resume("~/.garth") try: garth.client.username except GarthException: # Session is expired. You'll need to log in again pass</details> As per coding guidelines: "Code examples in documentation don't need import statements (e.g., don't show `import garth`)". <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 42 - 52, Remove the top-level import lines shown in
the example so the snippet only demonstrates usage; specifically delete the
import garthandfrom garth.exc import GarthExceptionlines and leave the
example showing use of garth.resume, garth.client.username and the except
GarthException block unchanged so the documentation follows the guideline of
omitting import statements in code examples.</details> --- `62-66`: **Remove import statement from code example.** <details> <summary>📝 Suggested fix</summary> ```diff ```python -import garth # Session is automatically loaded garth.client.username</details> As per coding guidelines: "Code examples in documentation don't need import statements (e.g., don't show `import garth`)". <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 62 - 66, Remove the top-level import from the README
code example so it matches the guideline; specifically edit the snippet that
references garth.client.username to delete the "import garth" line and leave the
comment and usage (i.e., the example containing garth.client.username and the
"Session is automatically loaded" comment) unchanged.</details> --- `32-38`: **Remove import statements from code examples.** The code examples include `import garth` and `from getpass import getpass`, but the coding guidelines specify that code examples in documentation don't need import statements. <details> <summary>📝 Suggested fix</summary> ```diff ```python -import garth from getpass import getpass garth.login(input("Email: "), getpass("Password: ")) garth.save("~/.garth")</details> As per coding guidelines: "Code examples in documentation don't need import statements (e.g., don't show `import garth`)". <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 32 - 38, Remove the import lines from the README code
block: delete the "import garth" and "from getpass import getpass" statements
and leave the example calls (garth.login(...), getpass(...), garth.save(...)) so
the snippet shows usage without explicit imports.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@src/garth/telemetry.py:
- Around line 112-113: The change flips telemetry defaults to opt-out-breaking
behavior; revert the default booleans back to disabled by setting enabled: bool
= False and send_to_logfire: bool = False (or make them driven by an explicit
opt-in like an environment variable) so telemetry.Session creation (which
triggers print(f"Garth session: {self.telemetry.session_id}") in http.py) and
automatic Logfire sends do not occur at import time; update the configuration
initialization in telemetry.py (the enabled and send_to_logfire symbols) to
require explicit opt-in and ensure any env/config flag (e.g.,
GARTH_TELEMETRY_ENABLED) is respected when constructing the telemetry object.
Nitpick comments:
In@README.md:
- Around line 42-52: Remove the top-level import lines shown in the example so
the snippet only demonstrates usage; specifically delete theimport garthand
from garth.exc import GarthExceptionlines and leave the example showing use
of garth.resume, garth.client.username and the except GarthException block
unchanged so the documentation follows the guideline of omitting import
statements in code examples.- Around line 62-66: Remove the top-level import from the README code example so
it matches the guideline; specifically edit the snippet that references
garth.client.username to delete the "import garth" line and leave the comment
and usage (i.e., the example containing garth.client.username and the "Session
is automatically loaded" comment) unchanged.- Around line 32-38: Remove the import lines from the README code block: delete
the "import garth" and "from getpass import getpass" statements and leave the
example calls (garth.login(...), getpass(...), garth.save(...)) so the snippet
shows usage without explicit imports.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `7deee4ff-74ed-4bb9-9232-73c8ef00a6c3` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 96f6cd8383905eba90a3d9e39d888266714c8aaa and 19b8bca64b04eb04ead113b39b93a3c1b5667d3e. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `uv.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (9)</summary> * `README.md` * `docs/getting-started.md` * `docs/index.md` * `docs/telemetry.md` * `pyproject.toml` * `src/garth/__init__.py` * `src/garth/telemetry.py` * `src/garth/version.py` * `tests/test_telemetry.py` </details> <details> <summary>💤 Files with no reviewable changes (2)</summary> * src/garth/__init__.py * docs/getting-started.md </details> <details> <summary>✅ Files skipped from review due to trivial changes (4)</summary> * src/garth/version.py * pyproject.toml * docs/telemetry.md * docs/index.md </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| enabled: bool = True | ||
| send_to_logfire: bool = True |
There was a problem hiding this comment.
Breaking change: telemetry now enabled by default.
Changing defaults from False to True for both enabled and send_to_logfire is a significant behavioral change. Per the context snippet from http.py:67-68, this means:
- All users will see
print(f"Garth session: {self.telemetry.session_id}")output at import time - HTTP request/response data (sanitized) will be sent to Logfire by default
While the data is sanitized, this is an opt-out change that may surprise existing users. Consider:
- Documenting this prominently in release notes
- Keeping
enabled=Falseas default and requiring explicit opt-in for telemetry collection
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/garth/telemetry.py` around lines 112 - 113, The change flips telemetry
defaults to opt-out-breaking behavior; revert the default booleans back to
disabled by setting enabled: bool = False and send_to_logfire: bool = False (or
make them driven by an explicit opt-in like an environment variable) so
telemetry.Session creation (which triggers print(f"Garth session:
{self.telemetry.session_id}") in http.py) and automatic Logfire sends do not
occur at import time; update the configuration initialization in telemetry.py
(the enabled and send_to_logfire symbols) to require explicit opt-in and ensure
any env/config flag (e.g., GARTH_TELEMETRY_ENABLED) is respected when
constructing the telemetry object.
I'm actually still using the existing endpoints from way back when (garth from 1+ yrs ago) on my own iOS project. I've been waiting for it to not work, but it still does. I had 1 instance of 429 (I really did try a few times in succession) and it locked me out for a bit but then after 15+min or so it went back to working. |
3 participants
Why
Garmin has made some changes around Auth and broke Garth and cyberjunky/python-garminconnect#332. As a result matin has deprecated the Garth project (#222).
What
This PR change follows #222 (comment) and seems to work by spoofing the user agent with new values to bypass the new cloudflare block
Summary by CodeRabbit
Chores
Documentation
Telemetry
Misc