feat: add SSO embed widget login strategy to bypass 429 rate limiting#345
Open
diegoscarabelli wants to merge 4 commits intocyberjunky:masterfrom
Open
feat: add SSO embed widget login strategy to bypass 429 rate limiting#345diegoscarabelli wants to merge 4 commits intocyberjunky:masterfrom
diegoscarabelli wants to merge 4 commits intocyberjunky:masterfrom
Conversation
The portal and mobile JSON API endpoints are subject to aggressive per-clientId rate limiting. This adds a fifth login strategy using the SSO embed widget HTML form flow (/sso/embed + /sso/signin) with curl_cffi TLS impersonation. This flow does not use a clientId parameter, so it is not subject to the same rate limiting. The strategy is tried first when curl_cffi is available, before falling back to the existing portal and mobile strategies. Closes cyberjunky#344
Contributor
WalkthroughAdded a new SSO web widget login strategy ( Changes
Sequence DiagramsequenceDiagram
participant Client
participant SSO_Embed as SSO /embed
participant SSO_Signin as SSO /signin
participant SSO_MFA as SSO /verifyMFA
participant DI as DI Token Exchange
Client->>SSO_Embed: GET /sso/embed (establish cookies)
SSO_Embed-->>Client: Set-Cookie
Client->>SSO_Signin: GET /sso/signin (fetch sign-in form)
SSO_Signin-->>Client: HTML + CSRF token
Client->>Client: Extract CSRF from hidden input
Client->>SSO_Signin: POST /sso/signin (email, password, _csrf, embed)
SSO_Signin-->>Client: Response page (may indicate MFA)
alt MFA required
Client->>SSO_MFA: POST /sso/verifyMFA/loginEnterMfaCode (mfa-code)
SSO_MFA-->>Client: MFA success page (contains ticket)
end
Client->>Client: Extract service ticket from response
Client->>DI: Exchange ticket for DI token
DI-->>Client: DI Token
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Adds a new widget+cffi login strategy that uses the Garmin SSO embed widget HTML form flow (/sso/embed + /sso/signin) with curl_cffi TLS impersonation, intended to bypass per-clientId 429 rate limiting affecting the existing portal/mobile strategies.
Changes:
- Introduces
widget+cffias the first attempted login strategy whencurl_cffiis available. - Implements widget-flow MFA handling via
_complete_mfa_widgetand wires it intoresume_login. - Adds regex-based HTML parsing for CSRF/title/ticket extraction.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prevents stale _widget_session from being picked up by a subsequent resume_login call if a different login strategy is used next.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@garminconnect/client.py`:
- Around line 364-370: The current check uses title_match/_TITLE_RE and raises
GarminConnectAuthenticationError with a generic message when title != "Success";
instead, examine the extracted title string for known failure values (e.g.,
"Invalid code", "Code expired", "Too many attempts") and map them to clearer,
specific errors or messages before raising—either raise distinct exceptions
(e.g., GarminConnectMFAInvalidCodeError) or include a descriptive message that
differentiates "invalid MFA code" vs. "unexpected server response"; update the
block that references title_match, title, and the
GarminConnectAuthenticationError to perform this lookup and raise the
appropriate, more informative exception.
- Around line 352-362: The POST to "/sso/verifyMFA/loginEnterMfaCode" (the
sess.post call in garminconnect/client.py that sends the MFA code) is missing a
timeout; add a timeout keyword argument to that sess.post invocation so the
request cannot hang indefinitely (use an existing attribute like
self._request_timeout if one exists, otherwise add a sensible default such as
timeout=30 and surface it as a configurable client option). Make sure to update
any constructor or config to allow setting the timeout if you introduce a new
attribute and keep the same parameter name when used elsewhere.
- Around line 285-295: The POST that submits credentials (the sess.post call
that posts to f"{sso_base}/signin" with signin_params and data including
"_csrf") must include an explicit timeout argument so it cannot hang
indefinitely; update that sess.post invocation to pass a numeric timeout (or
reuse a configured value like self.timeout or a module-level DEFAULT_TIMEOUT)
and ensure any related callers expect a requests timeout exception
(requests.exceptions.Timeout) if needed.
- Around line 269-277: Check the response and handle errors for the initial
cookie-establishment call sess.get(sso_embed, params=embed_params) and the
subsequent CSRF fetch r = sess.get(f"{sso_base}/signin", ...): add an explicit
timeout argument to both calls, wrap them in try/except to catch
requests.exceptions.RequestException, verify r.status_code (treat non-2xx as
failure) and surface a clear error or raise an exception that includes the URL,
status_code and the underlying exception message before proceeding to CSRF
extraction; update any logging or error text so failures point to
sess.get(sso_embed, ...) or the signin GET rather than a generic "could not find
CSRF token."
- Around line 325-328: The strict title equality check (if title != "Success")
can false-negative if the success page is localized or changed; update the logic
in the same block (where title and GarminConnectConnectionError are referenced)
to try a fallback detection: if title isn't exactly "Success", attempt to locate
the service ticket (e.g., check for a parsed variable like service_ticket, look
for a "ticket" query param in the redirect URL, or search the page HTML for a
service ticket token) and only raise GarminConnectConnectionError when neither
the title matches nor a valid service ticket is found; keep the original
exception message but include the fallback branch so presence of a service
ticket allows success to proceed.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f7389bd9-ccec-4f2a-826b-0ade911cb6cc
📒 Files selected for processing (1)
garminconnect/client.py
Address Copilot review feedback: - Check status codes on embed and sign-in GET requests - Add explicit timeout=30 to credentials POST and MFA POST
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
garminconnect/client.py:1151
resume_login(self, _client_state, mfa_code)still ignores the_client_stateargument and relies on internal attributes (_widget_session,_widget_signin_params,_widget_last_resp) to complete MFA. This makes the "state" value returned fromlogin(return_on_mfa=True)effectively unused for the widget flow and prevents resuming MFA on a fresh Client instance. Consider using_client_stateas the source of truth (or documenting/removing the parameter) so MFA resumption behaves consistently.
def put(self, _domain: str, path: str, **kwargs: Any) -> Any:
api = kwargs.pop("api", False)
resp = self._run_request("PUT", path, **kwargs)
if api:
return resp.json() if hasattr(resp, "json") else None
return resp
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
garminconnect/client.py (1)
280-294:⚠️ Potential issue | 🟡 MinorRaise on
/sso/signinGET failures before parsing CSRF.This branch still skips
r.okhandling, so a 403/5xx is reported as “could not find CSRF token” instead of the actual HTTP failure.Suggested fix
if r.status_code == 429: raise GarminConnectTooManyRequestsError( "Widget login returned 429 on sign-in page" ) + if not r.ok: + raise GarminConnectConnectionError( + f"Widget login: sign-in page returned HTTP {r.status_code}" + ) csrf_match = self._CSRF_RE.search(r.text) if not csrf_match: raise GarminConnectConnectionError( "Widget login: could not find CSRF token in sign-in page" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@garminconnect/client.py` around lines 280 - 294, The GET to /sso/signin currently parses the response body before verifying HTTP success, causing 403/5xx to be misreported as a missing CSRF; update the block that performs sess.get(...) to first check r.ok (or r.status_code) and raise a GarminConnectConnectionError (including r.status_code and a short message) for non-2xx responses (handle 429 as before using GarminConnectTooManyRequestsError), then proceed to run self._CSRF_RE.search(r.text) and raise the existing CSRF-related GarminConnectConnectionError only if the token is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@garminconnect/client.py`:
- Around line 329-331: The widget login call sites pass service_url=sso_embed to
Client._establish_session but _establish_session still redeems the fallback
ticket against the hard-coded MOBILE_SSO_SERVICE_URL, breaking the JWT fallback;
update _establish_session to use the passed service_url parameter when
performing the fallback/JWT redemption (instead of the constant
MOBILE_SSO_SERVICE_URL), ensure any internal variable/branch that builds the
DI/JWT redemption endpoint uses service_url, and leave callers like the ones
that call _complete_mfa_widget(...)/_establish_session(ticket, sess=sess,
service_url=sso_embed) unchanged so widget logins can properly fall back to
JWT_WEB.
- Around line 315-341: The widget branch currently treats any non-"Success"
title as a connection error which causes Client.login() to continue trying other
strategies; modify the post-MFA check in the widget handling (around
title_match, title, _widget_session, _widget_signin_params, _widget_last_resp,
and _complete_mfa_widget) so that known widget credential-rejection titles
(e.g., titles containing "Invalid", "Incorrect", "Bad credentials", or a
specific rejection phrase your tests show) raise
GarminConnectAuthenticationError immediately, while leaving the generic
GarminConnectConnectionError for other unexpected titles; ensure return_on_mfa
and prompt_mfa flows remain unchanged.
- Around line 327-333: After successful inline MFA completion in the prompt_mfa
path, clear the stored widget state to avoid reusing stale widget sessions:
after calling self._establish_session(ticket, sess=sess, service_url=sso_embed)
(inside the prompt_mfa branch) reset self._widget_session,
self._widget_signin_params, and self._widget_last_resp to None so subsequent
resume_login() calls won't pick up old widget state from the same Client
instance.
- Around line 365-384: The _complete_mfa_widget() flow parses the HTML title
without checking the HTTP response code, so transport errors (429/5xx) get
misclassified as invalid MFA; update the POST handling to first inspect
r.status_code (and treat non-2xx as transport failures) and raise a distinct
error or re-raise an HTTP/transport exception before attempting to parse the
title; ensure the change references the POST response variable r, the method
_complete_mfa_widget, and use GarminConnectAuthenticationError (or a new
Transport/HTTP error) to signal network/server failures rather than treating
them as "Widget MFA verification failed".
---
Duplicate comments:
In `@garminconnect/client.py`:
- Around line 280-294: The GET to /sso/signin currently parses the response body
before verifying HTTP success, causing 403/5xx to be misreported as a missing
CSRF; update the block that performs sess.get(...) to first check r.ok (or
r.status_code) and raise a GarminConnectConnectionError (including r.status_code
and a short message) for non-2xx responses (handle 429 as before using
GarminConnectTooManyRequestsError), then proceed to run
self._CSRF_RE.search(r.text) and raise the existing CSRF-related
GarminConnectConnectionError only if the token is absent.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 2c2d4463-46d6-471b-805f-ef60e8cbb4d0
📒 Files selected for processing (1)
garminconnect/client.py
- Detect credential failures in widget title and raise AuthenticationError instead of ConnectionError to prevent falling through to other strategies with bad credentials - Clear widget MFA state in the prompt_mfa inline path (not just resume_login) to prevent stale session reuse - Check HTTP status on MFA POST before parsing HTML to correctly classify 429/5xx as transport failures
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@garminconnect/client.py`:
- Around line 169-180: The widget MFA state (_widget_session and related
_widget_* attributes) must be cleared before starting a fresh login so
resume_login() won't incorrectly prefer the stale widget path; update the
beginning of login() (or just before resume_login() selection) to reset/clear
_widget_session, _widget_challenge, _widget_result (or any _widget_* attributes
used) to None/empty so that _complete_mfa_widget() is not chosen for new
portal/mobile MFA flows; ensure you reference and clear all widget-related
fields present on the Client instance so the new login proceeds through the
correct strategy selection.
- Around line 297-354: The response body title-based authentication heuristic is
being applied even for transport errors; before inspecting title/title_lower
(the block using title_match/title_lower), check r.ok (HTTP 2xx) and if not ok
raise a GarminConnectConnectionError (or re-raise appropriate transport error)
so server-side 4xx/5xx pages don't get misclassified as credential failures;
mirror the guard used in _complete_mfa_widget(), and keep the existing MFA
handling (references: _complete_mfa_widget, _establish_session, _widget_session,
_widget_signin_params, _widget_last_resp) intact.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 56cac49c-1646-4031-8d37-728a243cc166
📒 Files selected for processing (1)
garminconnect/client.py
Comment on lines
+169
to
+180
| 1. SSO embed widget with curl_cffi (HTML form, no clientId) | ||
| 2. Portal web flow with curl_cffi (desktop browser TLS + UA) | ||
| 3. Portal web flow with plain requests (desktop browser UA) | ||
| 4. Mobile SSO with curl_cffi (Android WebView TLS) | ||
| 5. Mobile SSO with plain requests (last resort) | ||
| """ | ||
| strategies: list[tuple[str, Any]] = [] | ||
|
|
||
| # SSO embed widget — uses /sso/embed + /sso/signin HTML form flow. | ||
| # No clientId parameter, so not subject to per-client rate limiting. | ||
| if HAS_CFFI: | ||
| strategies.append(("widget+cffi", self._widget_login_cffi)) |
Contributor
There was a problem hiding this comment.
Reset stale widget MFA state before starting a new login() attempt.
Line 1182 now gives _widget_session highest priority in resume_login(). If a widget MFA challenge is abandoned or fails, and the same Client later runs a fresh login() that reaches portal/mobile MFA, the old _widget_* attributes are still present and the new code will be routed through _complete_mfa_widget() instead of the current flow.
🛠️ Suggested fix
def login(
self,
email: str,
password: str,
prompt_mfa: Any = None,
return_on_mfa: bool = False,
) -> tuple[str | None, Any]:
"""Log in to Garmin Connect.
@@
4. Mobile SSO with curl_cffi (Android WebView TLS)
5. Mobile SSO with plain requests (last resort)
"""
+ for attr in ("_widget_session", "_widget_signin_params", "_widget_last_resp"):
+ if hasattr(self, attr):
+ delattr(self, attr)
+
strategies: list[tuple[str, Any]] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@garminconnect/client.py` around lines 169 - 180, The widget MFA state
(_widget_session and related _widget_* attributes) must be cleared before
starting a fresh login so resume_login() won't incorrectly prefer the stale
widget path; update the beginning of login() (or just before resume_login()
selection) to reset/clear _widget_session, _widget_challenge, _widget_result (or
any _widget_* attributes used) to None/empty so that _complete_mfa_widget() is
not chosen for new portal/mobile MFA flows; ensure you reference and clear all
widget-related fields present on the Client instance so the new login proceeds
through the correct strategy selection.
Comment on lines
+297
to
+354
| r = sess.post( | ||
| f"{sso_base}/signin", | ||
| params=signin_params, | ||
| headers={"Referer": r.url}, | ||
| data={ | ||
| "username": email, | ||
| "password": password, | ||
| "embed": "true", | ||
| "_csrf": csrf_match.group(1), | ||
| }, | ||
| timeout=30, | ||
| ) | ||
|
|
||
| if r.status_code == 429: | ||
| raise GarminConnectTooManyRequestsError( | ||
| "Widget login returned 429" | ||
| ) | ||
|
|
||
| title_match = self._TITLE_RE.search(r.text) | ||
| title = title_match.group(1) if title_match else "" | ||
|
|
||
| # Step 4: Handle MFA | ||
| if "MFA" in title: | ||
| self._widget_session = sess | ||
| self._widget_signin_params = signin_params | ||
| self._widget_last_resp = r | ||
|
|
||
| if return_on_mfa: | ||
| return "needs_mfa", sess | ||
|
|
||
| if prompt_mfa: | ||
| mfa_code = prompt_mfa() | ||
| ticket = self._complete_mfa_widget(mfa_code) | ||
| self._establish_session( | ||
| ticket, sess=sess, service_url=sso_embed | ||
| ) | ||
| del self._widget_session | ||
| del self._widget_signin_params | ||
| del self._widget_last_resp | ||
| return None, None | ||
| raise GarminConnectAuthenticationError( | ||
| "MFA Required but no prompt_mfa mechanism supplied" | ||
| ) | ||
|
|
||
| if title != "Success": | ||
| # Detect credential failures to prevent falling through | ||
| # to other strategies with bad credentials | ||
| title_lower = title.lower() | ||
| if any( | ||
| hint in title_lower | ||
| for hint in ("locked", "invalid", "error", "incorrect") | ||
| ): | ||
| raise GarminConnectAuthenticationError( | ||
| f"Widget login: authentication failed ('{title}')" | ||
| ) | ||
| raise GarminConnectConnectionError( | ||
| f"Widget login: unexpected title '{title}'" | ||
| ) |
Contributor
There was a problem hiding this comment.
Check the credential POST status before the title-based auth heuristic.
After Line 310, any non-429 4xx/5xx HTML page still flows into the title_lower check. A server-side error page titled "Error" or "Invalid request" will now raise GarminConnectAuthenticationError and stop the remaining strategies, even though the failure was transport-side. Mirror the r.ok guard from _complete_mfa_widget() here and only use the title hints on successful responses.
🛠️ Suggested fix
if r.status_code == 429:
raise GarminConnectTooManyRequestsError(
"Widget login returned 429"
)
+ if not r.ok:
+ raise GarminConnectConnectionError(
+ f"Widget login: credential POST returned HTTP {r.status_code}"
+ )
title_match = self._TITLE_RE.search(r.text)
title = title_match.group(1) if title_match else ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@garminconnect/client.py` around lines 297 - 354, The response body
title-based authentication heuristic is being applied even for transport errors;
before inspecting title/title_lower (the block using title_match/title_lower),
check r.ok (HTTP 2xx) and if not ok raise a GarminConnectConnectionError (or
re-raise appropriate transport error) so server-side 4xx/5xx pages don't get
misclassified as credential failures; mirror the guard used in
_complete_mfa_widget(), and keep the existing MFA handling (references:
_complete_mfa_widget, _establish_session, _widget_session,
_widget_signin_params, _widget_last_resp) intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a fifth login strategy (
widget+cffi) that uses the SSO embed widget HTML form flow (/sso/embed+/sso/signin) withcurl_cffiTLS impersonation. This bypasses the per-clientId 429 rate limiting that affects all four existing strategies.Closes #344
Problem
The portal and mobile login endpoints require a
clientIdparameter (GarminConnect,GCM_ANDROID_DARK). Garmin rate-limits these perclientId + account email. Once triggered, switching IP or User-Agent does not help. The official Garmin Connect app is unaffected because it uses a different rate limit bucket.How this works
The SSO embed widget flow is an HTML form-based login (CSRF token + POST with username/password). It does not use a
clientIdparameter, so it sits in a completely different rate limit bucket. Combined withcurl_cffiChrome TLS impersonation to pass Cloudflare, it reliably authenticates even when all other strategies return 429.The flow:
GET /sso/embed(establish cookies)GET /sso/signin(obtain CSRF token from HTML)POST /sso/signin(submit credentials via form data)POST /sso/verifyMFA/loginEnterMfaCodeThe service ticket feeds into the existing
_exchange_service_ticketfor DI token exchange. No changes needed downstream.Changes
import refor HTML parsing (CSRF token, title, ticket extraction)_widget_login_cffias a new login strategy (tried first whencurl_cffiis available)_complete_mfa_widgetfor MFA handling in the widget flowresume_loginto support widget MFA completioncurl_cffi)Testing
Tested manually against two Garmin accounts (one with MFA enabled, one without):
resume_loginpath verified for thereturn_on_mfa=TruecaseTrade-offs
The widget flow parses HTML (CSRF token from a hidden input, success/MFA status from
<title>tag, ticket from a URL in the response). This is more fragile than JSON APIs. However:Resolves #344
Summary by CodeRabbit
New Features
Improvements