OttPeterR / addon-babybuddy

BabyBuddy, wrapped into a Home Assistant addon
35 stars 13 forks source link

Fix login QR code; reverse proxy sub_path configuration #47

Closed MrApplejuice closed 1 year ago

MrApplejuice commented 1 year ago

Hey there!

I am maintaining an Android-App for Baby Buddy and I am trying to add QR-code based login support right now. The feature is done, however, while testing the feature a bit, I found that it does not actually work if babybuddy is not quite setup correctly - or as expected. I would like to help fixing this particular issue, but I am uncertain on which level, because I am not familiar with homeassitant ingress, which is why I am trying to reach out to you to make a "plan-of-attack" for this bug

The problem

If you select the option "Add a device" in the options-menu:

image

You will reach a page like this one:

image

The QR-code encodes a json-structure with mainly two components:

You can inspect the QR-code contents using the browser's inspect-element function. The containing div contains the contents as a plain-text data-attribute:

image

So far so good, I also know that this works for my manual "live setup" of babybuddy which uses a reverse-proxy configuration using apache. My reverse-proxy configuration uses a sub-path of a url, something like: example.com/babybuddy. Apache is configured to remove the /babybuddy-prefix of the reqeust path, then adds X-Forwarded-Host: example.com to the query headers to tell django what the actual server name is.

For baby buddy, I needed to customize the configuration a bit, adding:

The result is that qr-code on the add-device page contains the correct URL:`http://example.com/babybuddy.

The home assistant addon

The question now becomes - how to fix this of the addon?

When testing the QR-code on the home assistant addon, I get this:

As can be seen, the entire ingress-sub path is omitted...

I have no clue what headers are being set by the ingress-service in homeasisstant and find it also somewhat hard to inspect what is happening. However, this seems to be fixable by some sort of reverse-proxy configuration. I think that this could be solved in one go, if the FORCE_SCRIPT_PATH value was set to the right value. However, it is also possible that the actual code that generates the QR-code needs to be adapted somehow:

However, I do not see any mistakes in there, I think we need to tell django its actual absolute host-url (including the sub_path) somehow.

Do you know if it possible to get the root-path from the ingress-server somehow so that I can try to modify the django settings so that babybuddy can construct absolute urls to its pages?

Thanks for your help in advance!

MrApplejuice commented 1 year ago

I did a bit of research myself. I think a plugin can query the Home Assistant-API somehow to query its own ingress configuration and read the public-facing URL that way back from home assistant.

However, right now I am having trouble getting access to those api-endpoints. So I cannot test if this is a viable option, yet.

cdubz commented 1 year ago

Yeah I'm also not very familiar with HA but I would expect that in a proxy situation like this the proper external domain would need to be obtained from something like an API. It likely cannot be determined directly by Baby Buddy itself.

MrApplejuice commented 1 year ago

Hi cbudz! Yeah, I was considering that possibility myself. However, we could do a frontend solution in those circumstances. I am currently still probing around in home-assistant and keeping my hopes up that way.

However, a more flexible solution could be a scheme like this:

image

This would than also work for other reverse-proxy situations, but I find this a quite bit more messy because it relies on client-side javascript code to do the verification and if necessary query an updated QR code... meh. Generally, I like static websites a little bit more here but this could scale better for other integrations.

Opinions?

MrApplejuice commented 1 year ago

Solution for Home Assistant

I got it! The home assistant ingress server exposes the/a public facing URL in a barely documented header it adds to forwarded requests:

HTTP_X_HASS_SOURCE: 'core.ingress'
HTTP_X_INGRESS_PATH: '/api/hassio_ingress/cdxHLIks8AqDO2BrOSoU-X7VGeEdYPu89wdSeLiWk-s'
HTTP_X_FORWARDED_HOST: '192.168.0.122:8123'

These three headers allow to:

  1. Detect what is doing the forwarding: 'core.ingress'
  2. Construction of the proper public-facing URL.

Issue with Django is, that it does not have any clue what "HTTP_X_INGRESS_PATH" is. It tries instead to use a more static configuration for the purpose of constructing sub-path URLs, namely SCRIPT_PATH (option in the WSGI-headers, populated by tools like gunicorn) and in settings.py FORCE_SCRIPT_PATH.

Something "custom" would need to be created to interpret HTTP_X_INGRESS_PATH.

Solution

@cdubz - since you are in here as well, I would like to ask you for your opinion on this as well!

Since the homeassistant-addon is quite popular from what I can tell, we could make babybuddy "home assistant aware". This would include:

This would be the "static" home assistant solution. I am a bit torn between the more flexible, but messy frontend-solution proposed before this post, or the home assistant-ingress URL header based solution...

cdubz commented 1 year ago

I'm OK with making Baby Buddy "Home Assistant aware" given the user base. Happy to take a PR for this 👍

MrApplejuice commented 1 year ago

Okay then... thank you for the input. I will try to get this working then. Kind of an interesting integration issue trying to test this on three very different platforms ("regular server"/home assistant ingress <-> Android) :stuck_out_tongue:

cdubz commented 1 year ago

Haha yeah. I appreciate you digging in to it so thoroughly!

MrApplejuice commented 1 year ago

Meh... I think I got it working, but cannot test because the alpine-linux images for building addons are borked. Someone did update libcrypto3 in the alpine-repositories, but forgot to build it for the ARM-systems.

image

Problem is, that the homeassistant base images now need to be updated so that the dependency configurations are correct again (an point to libcrypto3.1.1). However, since because the library was not updated for ARM in the alpine repositories, the home assistant image builds are broken:

https://github.com/hassio-addons/addon-base/pull/245

Glorious mess... I think I have to wait until someone comes along and fixes the alpine packages :-(

Haha yeah. I appreciate you digging in to it so thoroughly!

Yeah, really no problem. This project seems to bring a lot of utility to people in general, so contributing to the ecosystem is kind of fun!!!

OttPeterR commented 1 year ago

Thanks for all the time you're putting into this, I'm on holiday for a bit longer so I can't look into it yet but you've run into the libcrypto issue thats plagued me for a while. All you can do is wait 🤷‍♂️ Anyways thanks again

MrApplejuice commented 1 year ago

Hey @OttPeterR - no worries, enjoy your holidays! And congrats to the seemingly pretty popular home-assistant integration. I most say: Now that I tested it, it seems a big deal more convenient to host baby buddy this way than doing a custom host. I can see the appeal!

Anyhow, yeah, the libcrypto-issue, I think I traced it to the alpine repository. The base image update automation for the homeassistant-addons should automatically pick these up, but since the package repo is bugged, no new version is published yet. One could spend a few hours fixing this in a custom way, but then this is a "custom fix". I do not feel leaving the beaten path for this right now :D

I will keep on watching:

When either one of those two are fixed, I can give it another try!

MrApplejuice commented 1 year ago

Hey, look at that! The watched PR was merged! That should mean that this can be continued... will try to do that in the next few days!

MrApplejuice commented 1 year ago

Good news! I have the "trinity" working. Not sure how about the stability yet but here is the cookbook for documentation purposes:

  1. BabyBuddy needs to be modified using a middleware so it can interpret the X-Ingress-Path provided by home assistant
  2. BabyBuddy QR-Code login generation needs to be modified to expose the ingress_token for authentication purposes
  3. HomeAssistant Addon needs to be modified to enable the BabyBuddy middleware from 1)
    • I have a local hack to get this done, needs polishing and I need to create a PR
  4. BabyBuddy for Android needs to be adapter to store the now exposes ingress_token from 2) and use that as a second-layer for authentication.

I think that I can translate all of these into PRs soon now. Took so long to get the integration tests done, but I think that is it for now!

OttPeterR commented 1 year ago

this'll be resolved when I release the v2.0.2 in a few days after I finish that up

MrApplejuice commented 1 year ago

Woohoo! Testing time it is then. Thanks for finalizing the release, eager to try everything out - so many moving parts with this one :laughing: