petergardfjall / garminexport

Garmin Connect activity exporter and backup tool
Apache License 2.0
507 stars 83 forks source link

Migrate to new auth flow and use Garth, fixes #103 #104

Closed SimonBaars closed 1 year ago

SimonBaars commented 1 year ago

For full context, see the relevant issue thread: https://github.com/pe-st/garmin-connect-export/issues/95

TLDR: Garmin shut down the old authentication flow. The new authentication flow has many security measures in place to script the login. The solution described in issue #102 works and we can use the Garth library to implement an OAuth flow, which should be more robust than the original method.

The OAuth flow also required using the new endpoints (which could be migrated by simply removing the proxy).

Thanks to @geraudloup for investigating this with me, and @app4g for the support!

matin commented 1 year ago

@SimonBaars love this!

Why not use garth.connectapi() for the requests though?

garth.connectapi() routes to connectapi.garmin.com vs connect.garmin.com (and rely on setting the backend). The method also handles auto-renew of the OAuth2 Bearer token and special cases like a 204 response.

Take a look at how Garth is being used in garminconnect

I'm the maintainer of Garth. Let me know how I can be helpful.

matin commented 1 year ago

If you're looking to download an activity, you can use garth.client.download()

SimonBaars commented 1 year ago

Hey @matin, good to have you around! FYI, I implemented Garth in a couple of affected repos:

I agree that using those Garth features as well would be better, but for now I wanted to stick as close as possible to the original implementation to not make it a too great diff at once (and also not too dependent on Garth, in case any of the maintainers would prefer a different direction later). We can always make it a separate effort 😄

SimonBaars commented 1 year ago

I changed the PR title to reflect that the main goal is to fix auth.

matin commented 1 year ago

Fair point, and thanks for the work on the migrations!

petergardfjall commented 1 year ago

Thanks for the contribution. I need to understand the implications better though. In particular, https://github.com/petergardfjall/garminexport/issues/102#issuecomment-1742000911.

matin commented 1 year ago

Thanks for the contribution. I need to understand the implications better though. In particular, #102 (comment).

Responded in #102

petergardfjall commented 1 year ago

Garmin shut down the old authentication flow.

The "old authentication flow" (the one used by the web browser) is not shut down, but Garmin Connect uses Cloudflare's bot protection to prevent scripted access. That's why trying to replicate browser requests with curl gives a 403 (Forbidden) response. Cloudflare recognizes (by the TLS handshake fingerprint) that the request did not come from a web browser.

One solution that resolves the present 403 (Forbidden) issue on my end is to use curl_cffi, a requests replacement that relies on https://github.com/lwthiker/curl-impersonate to perform requests that Cloudflare believes are coming from a web browser.

With that, the "old" (browser-based) authentication flow that garminexport has always relied on seems to work fine. I have a separate PR up for this: https://github.com/petergardfjall/garminexport/pull/105

My main objection against garth is that it, by default, makes use of copied Garmin Connect app credentials and I'm not sure about the legal implications of using those.

So for the time being I feel quite hesitant to merge this PR.

SimonBaars commented 1 year ago

@petergardfjall Interesting, when I tried using the old login portal manually via Chrome I couldn't get a session either. It gave me a SESSION cookie instead of the SESSIONID cookie I needed for the requests.

Were you able to get a working SESSIONID cookie? And then the rest of the script worked fine?

app4g commented 1 year ago

@SimonBaars if you look at my comment at https://github.com/pe-st/garmin-connect-export/issues/95#issuecomment-1736800945, when I login manually via chrome, I Also did get a SESSIONID cookie, but it was further down the flow rather than earlier.

petergardfjall commented 1 year ago

For reasons stated here I will not merge this PR.

Appreciate the contribution and discussion though.