Requirements and suggestions related to MFA handling

[cyberjunky]

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]

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

Would something like that work?

[cyberjunky]

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

[cyberjunky]

@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]

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]

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]

Released as 0.4.45

[matin]

Great to hear this worked!

[cyberjunky]

Thanks a lot!

[cyberjunky]

@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]

I'll take a look

[matin]

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

Let me see what I can do.

[cyberjunky]

@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]

@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]

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

[RAYs3T]

Hey @matin I just tried to implement this, but with Home Assistant, this is a real struggle, when the MFA request happens in the same call as the initial login.

Would it be possible to refactor the login to split it into two seperate calls?

Like having the garth.login() take the password and username and NOT executing the handle_mfa() so in a later call we can just Invoke login_enter_mfa(mail_totp) or something which then creates the session finally?

In the home assistant config flow there are tight restrictions on co-routines and background jobs, so this is quite tricky to implement when the initial call has to wait for the input, as we can't just show a new dialog.

[matin]

what's the issue with the callback approach?

[RAYs3T]

When we call the login in the config flow the dialog has no ability to hide until this procedure call is done.

Therefore we can't show a new dialog and ask the user for the MFA, since the username+password dialog is still waiting for the feedback (return) of the login() call

So in the HA integration this starts with showing the form and processes the user input afterwards. But since this call can never finish, we can't ask for an MFA code. Also using the prompt_mfa() does not work, since we can't show a new dialog as long as the old one is not finished.

[RAYs3T]

@matin I've create a first draft (not tested yet). Maybe something like this could work? I will continue tomorrow and test it fully. https://github.com/matin/garth/pull/68

[matin]

got it. I'll take a look

[matin]

fixed in 0.5.1

added instructions on custom mfa handlers in the readme

let me know if this works for everyone or if there are further changes I can make

/cc @cyberjunky @RAYs3T

[RAYs3T]

Awesome, will give it a try later and report back.

[RAYs3T]

@matin I'm currently implementing this - I think this method signature has to be changed as well for the new return type? https://github.com/matin/garth/blob/922f3c305c71fb177c3bb5e3ca6697ed2e34424c/garth/http.py#L163

It still returns the tokens and not the new tuple.