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.

Sign up for GitHub

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

Access-Control-Allow-Origin #267

Open
wants to merge 4 commits into
base: luds
Choose a base branch
from
Open

Access-Control-Allow-Origin #267

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d3f40d
Access-Control-Allow-Origin
shocknet-justin Jun 16, 2024
90014dd
Apply suggestions from code review
shocknet-justin Jun 17, 2024
4273c0e
Update 01.md
shocknet-justin Jun 17, 2024
59ba7da
Update 01.md
shocknet-justin Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions 01.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, these proxies are controlled by the user and not the application developer, who is responsible for implementing the spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it may lead app developers to put the responsibility onto users. Not mentioning reverse proxies makes it clear app developers must do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


## Decoding examples

In Scala:
Expand Down