sander1988 / Indego

Home Assistant Custom Component for Bosch Indego Lawn Mower
Apache License 2.0
96 stars 29 forks source link

Adds OAuth (Bosch SingleKey ID support) #173

Closed sander1988 closed 1 year ago

sander1988 commented 1 year ago
JeedHome44 commented 1 year ago

Hello, i'm french! sorry for my bad english ! i add your files in my HA with the Chrome extension and it's work perfectly :) Thank you so much !

LJvHeck commented 1 year ago

It works! Great work.

jm-73 commented 1 year ago

I have created a new branch for the Bosch Single Key authentication code. I really appreciate that you have made a new pull request to the new branch. In that way I can commit the code there and test the code. Then merge the (hopefully) working code with the main branch. Makes sense?

jm-73 commented 1 year ago

Another thing: before merging into master, someone have to clean the code and figure out the dependencies. As you have written the code now you have added another dependency to your own github repo. In my opinion the dependencies that this integration should have is pyIndego where we have coded all the logic to set up a session and comunicate with the bosch servers.

sander1988 commented 1 year ago

I have created a new branch for the Bosch Single Key authentication code. I really appreciate that you have made a new pull request to the new branch. In that way I can commit the code there and test the code. Then merge the (hopefully) working code with the main branch. Makes sense?

Yes that's fine. Note that your message crossed a commit I just did. I did some small changes in the pyIndego library and this HA component to detect certain scenarios where the Bosch servers are down. In that case we delay the next update a bit to prevent loops. Let first test this version 1 or 2 weeks as I just made this change.

sander1988 commented 1 year ago

Another thing: before merging into master, someone have to clean the code and figure out the dependencies. As you have written the code now you have added another dependency to your own github repo. In my opinion the dependencies that this integration should have is pyIndego where we have coded all the logic to set up a session and comunicate with the bosch servers.

Yes I will change the library repo URL back as soon as pyIndego has been merged. Anything else?

jm-73 commented 1 year ago

No, just keep up the good work!!! :-)

sander1988 commented 1 year ago

Let first test this version 1 or 2 weeks as I just made this change.

This is gonna take a bit longer unfortunately. My own mower is broken atm and back to Bosch for repairs :-(. I will continue testing when he's back from repairs.

intovision commented 1 year ago

Any chance we can get access to the branch via HACS? Just makes it easier to try out and switch versions. Thanks for the great work!

Pr0mises commented 1 year ago

I tested it for a while now, everything is working as expected.

After upgrading to HASS 2023.4.3 the intregration stopped working with this error log:

Error setting up entry Indego (Indego ID) for indego Traceback (most recent call last): File "/usr/src/homeassistant/homeassistant/config_entries.py", line 383, in async_setup result = await component.async_setup_entry(hass, self) File "/config/custom_components/indego/init.py", line 223, in async_setup_entry await indego_hub.update_generic_data_and_load_platforms(load_platforms) File "/config/custom_components/indego/init.py", line 369, in update_generic_data_and_load_platforms generic_data = await self._update_generic_data() File "/config/custom_components/indego/init.py", line 616, in _update_generic_data await self._indego_client.update_generic_data() File "/usr/local/lib/python3.10/site-packages/pyIndego/indego_async_client.py", line 262, in update_generic_data self._update_generic_data(await self.get(f"alms/{self.serial}")) File "/usr/local/lib/python3.10/site-packages/pyIndego/indego_async_client.py", line 543, in get return await self._request(method=Methods.GET, path=path, timeout=timeout) File "/usr/local/lib/python3.10/site-packages/pyIndego/indego_async_client.py", line 476, in _request self._token = await self._token_refresh_method() File "/config/custom_components/indego/init.py", line 328, in async_token_refresh await session.async_ensure_token_valid() File "/usr/src/homeassistant/homeassistant/helpers/config_entry_oauth2_flow.py", line 499, in async_ensure_token_valid new_token = await self.implementation.async_refresh_token(self.token) File "/usr/src/homeassistant/homeassistant/helpers/config_entry_oauth2_flow.py", line 92, in async_refresh_token new_token = await self._async_refresh_token(token) File "/usr/src/homeassistant/homeassistant/helpers/config_entry_oauth2_flow.py", line 182, in _async_refresh_token new_token = await self._token_request( File "/usr/src/homeassistant/homeassistant/helpers/config_entry_oauth2_flow.py", line 209, in _token_request resp.raise_for_status() File "/usr/local/lib/python3.10/site-packages/aiohttp/client_reqrep.py", line 1005, in raise_for_status raise ClientResponseError( aiohttp.client_exceptions.ClientResponseError: 400, message='Bad Request', url=URL('https://prodindego.b2clogin.com/prodindego.onmicrosoft.com/b2c_1a_signup_signin/oauth2/v2.0/token')

Upgrading to 2023.4.4 same result. After readding the integration (keep credentials) and using the addon it's working again. Not sure what happened, I couldn't get gather any other logs. Looks like HASS deleted the token on my end, therefore no problem with this integration just an information.

I'll check if the next release also deletes the token or not.

sander1988 commented 1 year ago

I tested it for a while now, everything is working as expected.

After upgrading to HASS 2023.4.3 the intregration stopped working with this error log:

@Pr0mises - Are you really sure it started after the upgrade? Did you restart HA before just before upgrade without getting this error?

I see the same error on some of my HA instances. But I have not upgraded. I think it's an issue with the token refresh based on the stack and error.

Pr0mises commented 1 year ago

@sander1988 no I didn't restart hass just before the upgrade, it has nothing to do with this push but how hass refreshes the token.

I think it's an issue with the token refresh based on the stack and error.

That's what I'm thinking as well. It's no issue at all just wanted to inform you. As you had the same error it's worth mentioning in the readme as an hint, so no new Issue are opened for this behavior as it's known.

sander1988 commented 1 year ago

@sander1988 no I didn't restart hass just before the upgrade, it has nothing to do with this push but how hass refreshes the token.

I think it's an issue with the token refresh based on the stack and error.

That's what I'm thinking as well. It's no issue at all just wanted to inform you. As you had the same error it's worth mentioning in the readme as an hint, so no new Issue are opened for this behavior as it's known.

Ok. I see the 400 Bad Request. I will add some more debug output to the logs to see if the refresh is really happening and monitor it for the next few days.

FYI: I'm referring to this error:

Token request failed with status=400, body={"error":"invalid_grant","error_description":"AADB2C90080: The provided grant has expired. Please re-authenticate and try again. ...
sander1988 commented 1 year ago

I think I see the why... I have not seen this before (to me it looks like a misconfiguration of the OAuth server): _Both the refresh and access token use the same expiry of 1 day (expiresin).

HA refreshes around 20 seconds (https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/config_entry_oauth2_flow.py#L42) before expiring. So there is a small window for the (daily) refresh to succeed otherwise you will run into this 400 Bad Request error.

I will try to figure out a way to mark the access token as expired a few hours earlier as a workaround for this issue.

Pr0mises commented 1 year ago

That would explain why I'm stuck at the start screen of indego connect every now and then. After restart the app I've to login again.

sander1988 commented 1 year ago

That would explain why I'm stuck at the start screen of indego connect every now and then. After restart the app I've to login again.

Yes exactly, I have the same issue with their iOS app.

I think I have workaround for this HA integration... a way to force refresh it every 12 hours. That works fine in this case as long as HomeAssistant isn't offline for more than 12 hours. If you are offline for longer than 12 hours then the user has to readd the integration. I expect that won't happen for most the users. This is the best I can do here as it's a workaround, the real fix has to be implemented by Bosch on their server side. I hope they do, as it also affects their apps.

Have running this workaround in test now. I expect to update the repo within a few days.

Pr0mises commented 1 year ago

That's awesome, if you need more testing you could upload your fix into another branch so I can check it out.

jm-73 commented 1 year ago

Will you please commit to the dev branch instead? I have asked you this before.

sander1988 commented 1 year ago

Will you please commit to the dev branch instead? I have asked you this before.

I have changed it. Note that I was under the assumption you can change this as I left allow edits from maintainers enabled. Maybe I'm wrong here, I use GitLab for 99% of my projects.

sander1988 commented 1 year ago

That's awesome, if you need more testing you could upload your fix into another branch so I can check it out.

Please give me a few more days to fix 2 issues:

  1. I have logs confirming what I expected regarding the refresh misconfiguration by Bosch (which results in the 400 Bad Request after a couple of days). I can implement a workaround now.
  2. I also found another issue where the state update fails and stops updating from that point (you have restart HA or reload the integration to get it working again). I will also fix that. This is caused by:
    2023-04-20 03:31:11.384 ERROR (MainThread) [homeassistant] Error doing job: Future exception was never retrieved
    Traceback (most recent call last):
    File "/usr/local/lib/python3.10/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
    TypeError: IndegoHub._create_refresh_state_task() takes 1 positional argument but 2 were given
Pr0mises commented 1 year ago

Please give me a few more days to fix 2 issues:

I already pulled your latest commits and encountered the same issues. If you need any help let me know.

sander1988 commented 1 year ago

I already pulled your latest commits and encountered the same issues.

That's not possible ;-) ... my latest commit is from 3 weeks ago so it doesn't contain the 400 Bad request workaround. But I expect to push something today.

sander1988 commented 1 year ago

I have pushed version 5.0.2 ; this update should fix both issues. All known issues are fixed now.

In case you are testing... Please temporary add these logging configuration lines to your HA config:

...
logger:
  logs:
    homeassistant.helpers.config_entry_oauth2_flow: debug
    custom_components.indego: debug
    pyIndego.indego_async_client: debug
...

It helps debugging in case we run into another API related error.

Let's monitor the cloud connection for another week.

LarsLautrup commented 1 year ago

I got everything up and running - and the integration works again... Thanks!!! Is there a way to get more updates from the Indego? As it is right now, it looks like the states of the enitites is about 10/15 minutes delayed.

warpbe commented 1 year ago

I installed this last version, and it works flawlessly