Access-Control-Allow-Origin #267
Conversation
Co-authored-by: Aaron Dewes <[email protected]>
01.md
Outdated
| @@ -35,6 +35,10 @@ Once `LNURL` is decoded: | |||
|
|
|||
| Neither status codes or any HTTP Header has any meaning. Servers may use whatever they want. Clients should ignore them (and be careful when using libraries that treat responses differently based on headers and status codes) and just parse the response body as JSON, then interpret it accordingly. | |||
|
|
|||
| ## CORS Browser Compatibility | |||
|
|
|||
| Application endpoints or Reverse Proxy directives that are accessed when using the LNURL must respond to `OPTIONS` requests on that endpoint. In their replies to `GET` and `OPTIONS`, they must set the header `Access-Control-Allow-Origin: *` for LNURL's to work with browser-based wallets, services and other webapps. If they set the `Access-Control-Allow-Methods` header, they must set in such a way that `GET` requests are permitted. | |||
There was a problem hiding this comment.
| Application endpoints or Reverse Proxy directives that are accessed when using the LNURL must respond to `OPTIONS` requests on that endpoint. In their replies to `GET` and `OPTIONS`, they must set the header `Access-Control-Allow-Origin: *` for LNURL's to work with browser-based wallets, services and other webapps. If they set the `Access-Control-Allow-Methods` header, they must set in such a way that `GET` requests are permitted. | |
| Application endpoints that are accessed when using the LNURL must respond to `OPTIONS` requests on that endpoint. In their replies to `GET` and `OPTIONS`, they must set the header `Access-Control-Allow-Origin: *` for LNURL's to work with browser-based wallets, services and other webapps. If they set the `Access-Control-Allow-Methods` header, they must set in such a way that `GET` requests are permitted. |
I still disagree with the reference to reverse proxies. A reverse proxy is something completely separate and controlled by the user or platform, not by the application developer.
A reverse proxy is not always part of the application stack, and not under the control of an app developer. For example, BTCPay, which supports LNUrl, can be run on StartOS, Umbrel, ... which may already include a reverse proxy.
Requiring this for apps themselves means apps are correct without users externally modifying something about the app's responses.
If we impose this rule on reverse proxies, a lot of people are going to set Access-Control-Allow-Origin: * for all routes, which will make apps insecure.
Clearly putting the responsibility on the app developer is much better imo.
There was a problem hiding this comment.
That's a very niche subset of reverse proxies and afaik they function more as gateways than reverse proxies, an umbrel user exposing services to the internet would need to reconfigure the reverse proxy anyway for things like SSL and so on... I also explicitly mentioned the directive for the endpoint, of which there would be many in an box like that for various apps
There was a problem hiding this comment.
Still, these proxies are controlled by the user and not the application developer, who is responsible for implementing the spec.
There was a problem hiding this comment.
By exposing an umbrel etc to the internet the user is now a service provider, the box is the application including the customized reverse proxy configuration
It's services that need these fixes, not users.
There was a problem hiding this comment.
By exposing an umbrel etc to the internet the user is now a service provider, the box is the application including the customized reverse proxy configuration
It's services that need these fixes, not users.
Every other part of the spec is implemented by app developers, not users. All of these services have implemented LNURL. No user configured the LNURL handling in their reverse proxy or has to read the spec. Application & library developers have to know them, however.
The application can do this, and the reverse proxy is not needed for such functionality, so it should be part of the app.
Saying "our app may support LNURL" is incredibly bad as a developer and complicates setup for users unnecessarily.
There was a problem hiding this comment.
If we're being precise then those are apps, not services
I agree that apps should implement this, but the reality is that many apps are unlikely to be updated for this and are harder for the service provider to update themselves vs. their reverse proxy configuration
I think the spec should talk about apps, and then service providers can fix invalid apps if they choose so, but that should be outside of the spec.
There was a problem hiding this comment.
Apps are geared towards services, services which are run by operators that may be trying to figure out why teir app isn't working in certian cases- including searching on the CORS error from their browser
A reference to app directives gives both sides the information they need, and doesn't imply app developers shouldn't do it directly
There was a problem hiding this comment.
But it may lead app developers to put the responsibility onto users. Not mentioning reverse proxies makes it clear app developers must do it.
There was a problem hiding this comment.
The responsibility is already on operators of broken services to fix them, a bad app developer or unmaintained app will deviate no matter whats added to the spec and so verbiage is warranted for both configurations
It's also fair for an app to simply document the external requirement just as it might SSL
There was a problem hiding this comment.
a bad app developer or unmaintained app will deviate no matter whats added to the spec and so verbiage is warranted for both configurations
Then the app is just broken. The spec is written for correct apps, not for broken apps.
It's also fair for an app to simply document the external requirement just as it might SSL
CORS is something completely different from SSL. For CORS, an app should always handle it itself, because the app developers know what CORS rules are safe.
|
Why are we adding I suggest we remove this. |
This is necessary for the LNURL to work/be resolved from web browsers. |
|
I was mistaken, LNURL resolution is considered a "simple" request that does not require options. |
| @@ -35,6 +35,10 @@ Once `LNURL` is decoded: | |||
|
|
|||
| Neither status codes or any HTTP Header has any meaning. Servers may use whatever they want. Clients should ignore them (and be careful when using libraries that treat responses differently based on headers and status codes) and just parse the response body as JSON, then interpret it accordingly. | |||
|
|
|||
| ## CORS Browser Compatibility | |||
|
|
|||
| Application endpoints (or their defined Reverse Proxy directives) accessed when using the LNURL must respond to `GET` having set the header `Access-Control-Allow-Origin: *` for LNURL's to work with browser-based wallets, services and other webapps. If they set the `Access-Control-Allow-Methods` header, they must set in such a way that `GET` requests are permitted. | |||
There was a problem hiding this comment.
| Application endpoints (or their defined Reverse Proxy directives) accessed when using the LNURL must respond to `GET` having set the header `Access-Control-Allow-Origin: *` for LNURL's to work with browser-based wallets, services and other webapps. If they set the `Access-Control-Allow-Methods` header, they must set in such a way that `GET` requests are permitted. | |
| Application endpoints that need to be accessed when interacting with an LNURL must set [CORS headers](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS) in such a way that they are reachable from any web origin. |
Based on feedback from @hsjoberg and @fiatjaf, maybe just this sentence would be best.
No someone else apart from me and @shocknet-justin should probably also decide if we should include reverse proxies, because we don't seem to be able to agree here.
There was a problem hiding this comment.
I think we should at least keep the Access-Control-Allow-Origin: * or/and a link to the MDN describing CORS, to make it easier & more straightforward for devs to actually implement.
The issues is that incorrectly configured CORS is not immediately noticeable by a dev who uses non webapp based wallet (majority of wallets currently), I am worried that if this is not straightforward to implement (e.g. devs having to research what this CORS even means) they might just skip through it thinking their LN service is working all good.
Maybe it'd make sense to have a simple online tool (webapp) for checking whether LNURLs adhere to the spec (including CORS)
There was a problem hiding this comment.
I think we should remove the mention of proxies to keep the text shorter.
Isn't it already obvious for proxies that CORS header would be required? The proxy is really an "application endpoint" here if I understand it correctly.
There was a problem hiding this comment.
Feel like we're bikeshedding on the proxy text. On one hand it's not taking up much space, on the other it also feels obvious to me.
I could go either way, but I lean towards keeping it as is.
There was a problem hiding this comment.
Great! Two more people disagreeing 🙃
There was a problem hiding this comment.
I think it's obvious and an unnecessary implementation detail to mention.
|
Long needed guidance. Thanks for writing this. ACK |
|
another one... plz merge |
#259