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

Requirements and suggestions related to MFA handling #41

Open
cyberjunky opened this issue Dec 29, 2023 · 14 comments · Fixed by #52
Open

Requirements and suggestions related to MFA handling #41

cyberjunky opened this issue Dec 29, 2023 · 14 comments · Fixed by #52

Comments

@cyberjunky
Copy link

I want to implement -a much needed and requested- MFA functionality to my Home-Assistant integration home-assistant-garmin_connect

Garth handles this perfectly from cli/python-garminconnect's example.py code but for this to work from GUI code I think I need some small changes to or extra methods added to garth to handle MFA.

I think about this approach (by looking at the current code briefly due to time shortage) :

  • The garth login method (or a seperate new one) should not handle mfa by itself but it should just return a value if MFA is requested so python-garminconnect can detect the need to query the MFA code from the user
  • Followed by a call to a new (or changed) handle_mfa method where I specify the user supplied MFA code as argument

What do you think? If you have suggestions or a better approach (which you often have), please let me know.

@matin
Copy link
Owner

matin commented Dec 29, 2023

I added support for a custom MFA handler on the custom_mfa branch.

Would something like that work?

@cyberjunky
Copy link
Author

This is a different/better approach, I will check if I can get this to work, thanks alot!
Will be this weekend...!

@cyberjunky
Copy link
Author

@matin Finally got some time to try get the 'external/custom' mfa to work with example.py (so I can adapt it to this afterwards) But i run into an issue, not sure if I fetched the repo branch correctly, or something is still missing in Login() somewhere.

This is the requirements-dev.txt, example.py and adapted garmin_connect package so far: https://github.com/cyberjunky/python-garminconnect/tree/mfa

I get this error when running example.py:


During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/ron/python-garminconnect/./example.py", line 845, in <module>
    api = init_api(email, password)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ron/python-garminconnect/./example.py", line 211, in init_api
    garmin.login()
  File "/home/ron/python-garminconnect/garminconnect/__init__.py", line 197, in login
    self.garth.login(self.username, self.password, prompt_mfa=self.prompt_mfa)
TypeError: Client.login() got an unexpected keyword argument 'prompt_mfa'

@matin
Copy link
Owner

matin commented Mar 16, 2024

I'm not sure you're on the right branch. Could you try the following in your requirements-dev.txt?

git+https://github.com/matin/garth.git@custom_mfa#egg=garth

@cyberjunky
Copy link
Author

I first needed to remove the old garth package it seems, and then use your url.

It works! I has some small bugs to fix, but then I just got my first MFA login from example.py using external MFA input code, this is a breakthrough!
Can you please add this feature to your main branch when you have some time?
Then I can implement it in my home assistant integration, this will help a lot of people!

@matin matin mentioned this issue Mar 17, 2024
@matin matin closed this as completed in #52 Mar 17, 2024
@matin
Copy link
Owner

matin commented Mar 17, 2024

Released as 0.4.45

@matin
Copy link
Owner

matin commented Mar 18, 2024

Great to hear this worked!

@cyberjunky
Copy link
Author

Thanks a lot!

@cyberjunky
Copy link
Author

@matin I run into an issue with the MFA lambda call, I hope it's due to my limited Python knowledge, or brain fog, but i have this:

api = Garmin(email=username, password=password, prompt_mfa=mycall)

Where mycall is an async function to get the MFA code
Is there a way around it, or should garmin_connect be converted to async code?
It's like this kind context and form code where the code should come from:

https://github.com/home-assistant/core/blob/dev/homeassistant/components/abode/config_flow.py#L136

Any help or pointers are welcome, thanks!

@matin
Copy link
Owner

matin commented Apr 27, 2024

I'll take a look

@matin
Copy link
Owner

matin commented Apr 27, 2024

It's because Garth isn't expecting a coroutine as a response.

Let me see what I can do.

@cyberjunky
Copy link
Author

@matin Did you manage to investigate/think about a possible solution?
Maybe its needed/good to port all/request part of Garth to async? (maybe even port garminconnect to async and merge a part?)
Any help is welcome, thanks in advance!

@matin
Copy link
Owner

matin commented May 18, 2024

@cyberjunky sorry this took me so long. I've been busier than usual the last few weeks.

I just published 0.4.46.

prompt_mfa can now be async.

The change was fairly simple.

Let me know if this works for you.

@matin matin reopened this May 18, 2024
@matin
Copy link
Owner

matin commented May 18, 2024

Btw, if you're looking to use garminconnect as async, it'd be easier to wrap it with asyncer than to rewrite it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants