Skip to content
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.

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

[Feature] Save long lived Garmin Api token for users #585

Closed
philosowaffle opened this issue Dec 31, 2023 · 6 comments · Fixed by #622
Closed

[Feature] Save long lived Garmin Api token for users #585

philosowaffle opened this issue Dec 31, 2023 · 6 comments · Fixed by #622
Assignees
Labels
enhancement new feature / improvments WIP
Milestone

Comments

@philosowaffle
Copy link
Owner

philosowaffle commented Dec 31, 2023

Now that we're using Garth pattern for authentication (#524) it seems possible to save a long lived access token. This opens a few possibilities:

  1. Potentially do not need save users credentials with P2G, instead only supply them when we need to refresh the token
  2. MFA users could also now take advantage of background syncing

Explore these options and see what is feasible.

Requirements:

  1. P2G will use User Garmin credentials to get both an OAuth1 and OAuth2 token
    1. P2G will save the OAuth1 and OAuth2 tokens encrypted at rest
    2. OAuth1 token is expected to last 1year
    3. OAuth2 token is expected to last 24hr
    4. When the user changes their Garmin credentials, all saved tokens should be cleared
    5. When the user toggles 2FA enabled, all saved tokens should be cleared
  2. If OAuth2 token exists and is not expired, P2G will use this token for Garmin requests
  3. If OAuth2 token is missing or expired, P2G will attempt to refresh with OAuth1 token
  4. If OAuth1 token is missing or fails to refresh, then P2G will fall back to Garmin signin flow using user credentials
  5. During SignIn, if MFA is needed, the user will be prompted
  6. If Sync is enabled, the Sync service will wait until the user has completed MFA flow atleast once and an OAuth2 token has been stored
    1. GUI's - user should be prompted to go through MFA flow when they enable Sync setting
    2. Console - user should be prompted to go through MFA before starting background sync

Out of Scope:

  1. Fully removing dependency on users credentials - while this would be nice to do so that P2G did not ever save email/password, its a large lift to do this right now and requires re-thinking much of the p2g onboarding flow as well as handling scenarios when credentials do need to be asked for if tokens are lost or expired
  2. GitHub Action - GHA does not support anyway to get user input during runtime, nor does it support saving data to disk, meaning the existing console support for MFA will not work. And once OAuth tokens are obtained, they cannot be saved by P2G. In order to support MFA, I believe I'd need to create some kind of multi-step "initialization" workflows that allow the user to get their OAuth tokens and then manually save those as secrets to the repo.
@philosowaffle philosowaffle added enhancement new feature / improvments research needed needs more research by developer labels Dec 31, 2023
@philosowaffle philosowaffle self-assigned this Jan 27, 2024
@philosowaffle philosowaffle added WIP and removed research needed needs more research by developer labels Jan 27, 2024
@philosowaffle philosowaffle added this to the 4.2.0 milestone Jan 27, 2024
@philosowaffle
Copy link
Owner Author

Reference for how to refresh auth token. Apparently, we do not use the Refresh Token, instead save the OAuth1Token (which lasts up to a year) and use that to re-exchange for OAuth2 whenever needed.

@philosowaffle
Copy link
Owner Author

philosowaffle commented Jan 27, 2024

Feature todo:

  • UI Sync Settings - When Sync toggled on, check if MFA GarminAuth token needs to be initialized and prompt user
  • UI Garmin Auth Settings - Provide button to test credentials?

Testing todo:

  • Generally test if Token refresh logic is working
  • Console - Test background sync without MFA
  • Console - Test background sync with MFA
  • Console - Test regular sync without MFA
  • Console - Test regular sync with MFA
  • WebUI - Test background sync without MFA
  • WebUI - Test background sync with MFA
  • WebUI - Test regular sync without MFA
  • WebUI - Test regular sync with MFA
  • Windows GUI - Test regular sync without MFA
  • Windows GUI - Test regular sync with MFA

@philosowaffle
Copy link
Owner Author

Look at upgrading to latest Flurl library which adds first-class citizen support for saving cookie jars.

@philosowaffle
Copy link
Owner Author

Flurl is updated, but MFA flow now doesn't work. getting a 500 from Garmin on SendCredentials. Could be temporary issue. (garmin api is returning nre)

@geoffbon
Copy link
Contributor

geoffbon commented Oct 24, 2024

@philosowaffle Did this get completed and checked into main? I don't see it working in 4.2.0, but maybe wasn't included in the tag for that release?
Edit: sorry, just saw the 4.3.0 tag, no worries - will look forward to that.

@philosowaffle
Copy link
Owner Author

It's in main and has been available under the latest tag or pulling the latest build. I'll go ahead and publish 4.3.0 today so its also in a proper release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new feature / improvments WIP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants