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

Bug: QR code on landing page does not include protocol #239

Open
AaronDewes opened this issue Oct 22, 2022 · 6 comments
Open

Bug: QR code on landing page does not include protocol #239

AaronDewes opened this issue Oct 22, 2022 · 6 comments

Comments

@AaronDewes
Copy link

For some reason (it seems to be implemented), the QR code on the landing page fails to include the protocol.

lndhub.io (or self-hosted Lndhub) generates a QR Code like bluewallet:setlndhuburl?url=http%3A%2F%2Flndhub.io, which works in BlueWallet

ln.getalby.com (or self-hosted Lndhub.go) generates a QR code like bluewallet:setlndhuburl?url=%3A%2F%2Fln.getalby.com, which does not work in BlueWallet, because the protocol is missing.

@prusnak
Copy link
Contributor

prusnak commented Oct 23, 2022

c.Request().URL.Scheme always returns empty value -> golang/go#28940

So it's impossible to use this to distinguish between http and https (it makes sense, the Go process has no idea about what the final deployment is, maybe there is a NGINX in front which translates http to https).

@prusnak
Copy link
Contributor

prusnak commented Oct 23, 2022

Maybe the best fix is to use Config.Branding.Url instead of c.Request().URL.Scheme+c.Request().Host?

diff --git a/controllers/home.ctrl.go b/controllers/home.ctrl.go
index 5c94dfd..eda20ae 100644
--- a/controllers/home.ctrl.go
+++ b/controllers/home.ctrl.go
@@ -7,7 +7,6 @@ import (
        "html/template"
        "net/http"
        "net/url"
-       "strings"
 
        "github.com/getAlby/lndhub.go/lib/service"
        "github.com/labstack/echo/v4"
@@ -61,8 +60,7 @@ func Max(x, y int) int {
        return x
 }
 func (controller *HomeController) QR(c echo.Context) error {
-       customPath := strings.Replace(c.Request().URL.Path, "/qr", "", 1)
-       encoded := url.QueryEscape(fmt.Sprintf("%s://%s%s", c.Request().URL.Scheme, c.Request().Host, customPath))
+       encoded := url.QueryEscape(controller.svc.Config.Branding.Url + "/qr")
        url := fmt.Sprintf("bluewallet:setlndhuburl?url=%s", encoded)
        png, err := qrcode.Encode(url, qrcode.Medium, 256)
        if err != nil {

@AaronDewes
Copy link
Author

I'm developing @runcitadel, where we allow users to self-host Lndhub.go. Some users only want local access, and use http. Others use Tor for remote access. There's also Tailscale, which can be used for remote access too. All three are HTTP, not HTTPS, but different URLs.

Allowing this kind of dynamic access would be a lot harder with Config.Branding.Url.

Maybe you could make this configurable as a separate option? Config.Branding.QrProtocol -> Protocol to use, Config.Branding.QrHost -> Host to use (If not set, but protocol is, c.Request().Host could be used).

If these Config.Branding.Qr values are not set, Config.Branding.Url could be the fallback.

@prusnak
Copy link
Contributor

prusnak commented Oct 23, 2022

Allowing this kind of dynamic access would be a lot harder with Config.Branding.Url.

Branding is just a branding, nothing else, webserver can still be running on all 4 URLs (local http, remote https, tor http, tailscale http). Can you describe an use-case where a user wants to access the web interface from one URL and use the Bluewallet with another URL?

@AaronDewes
Copy link
Author

Branding is just a branding, nothing else, webserver can still be running on all 4 URLs (local http, remote https, tor http, tailscale http). Can you describe an use-case where a user wants to access the web interface from one URL and use the Bluewallet with another URL?

Branding is not dynamic though. The problem is that the QR code would always lead to the same page then, no matter where the user is accessing the web interface.

@bumi bumi added this to 🐝 Alby Nov 7, 2022
@bumi bumi moved this to Ready to be worked on in 🐝 Alby Nov 7, 2022
@bumi bumi removed this from 🐝 Alby Nov 7, 2022
@bumi bumi added this to 🐝 Alby Nov 7, 2022
@praeluceo
Copy link

Just encountered this issue on https://ln.getalby.com/ where a malformed URL is generated by the QR code on the page, in this case returning "://ln.getalby.com" rather than "https://ln.getalby.com".

Also, I understand there are issues around how a user may want to run LNDHub either over http, https, or tor. As one of those users trying to migrate, it would be worth pointing out in the documentation that some wallets (such as BlueWallet) will refuse to connect to an http hosted LNDHub without a good error message. And then to follow that with an nginx example or something similar on how to run LNDHub.go over HTTPS?

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

No branches or pull requests

3 participants