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

CORS support for external web clients? #3109

Closed
4 tasks done
pfgithub opened this issue Jun 14, 2023 · 22 comments
Closed
4 tasks done

CORS support for external web clients? #3109

pfgithub opened this issue Jun 14, 2023 · 22 comments
Labels
enhancement New feature or request

Comments

@pfgithub
Copy link
Contributor

  • Did you check to see if this issue already exists?
  • Is this only a single feature request? Do not put multiple feature requests in one issue.
  • Is this a question or discussion? Don't use this, use https://lemmy.ml/c/lemmy_support.
  • Is this a UI / front end issue? Use the lemmy-ui repo.

Describe the feature request below

I have a reddit web client (threadclient) that I would like to add lemmy support to. Currently it implements reddit support and some mastodon support by sending requests directly to websites, as both reddit and mastodon allow cross-origin access from any domain. This prevents the need for a proxy server to send requests through.

Assuming there are no security implications to this, it should be a pretty simple change:

// src/lib.rs
-    let cors_config = if cfg!(debug_assertions) {
-      Cors::permissive()
-    } else {
-      Cors::default()
-    };
+    let cors_config = Cors::default()
+      .allow_any_origin()
+      .allow_any_method()
+      .allow_any_header()
+      .max_age(3600);

I belive Cors::permissive() should be okay to use since authentication isn't passed on when sending a fetch request with {credentials: "include"}, but the documentation says not to use it in production.

@pfgithub pfgithub added the enhancement New feature or request label Jun 14, 2023
@AlexanderMaxRanabel
Copy link

Open a pull request for it. they more likely to look into it if its a pull request i suppose

@cinderisles
Copy link

see my comment here. it should help

@auouymous
Copy link

@cinderisles That environment variable only helps those who are accessing the API on their own instance. We have web clients that need access to the API of all lemmy instances.

Fleeing from a platform that is restricting its API to one that has no public API at all doesn't make sense.

@cinderisles
Copy link

cinderisles commented Jun 21, 2023

@auouymous in that case wouldn't it be up to each server admin to allow that? For example, if beehaw.org wanted to allow external clients from any origin, they would have to set something like LEMMY_CORS_ORIGIN=*

unless you're saying that you want it to be * by default. in that case, I think having the environment variable LEMMY_CORS_ORIGIN at all is pointless, unless some admin wanted to be restrictive for some reason

Fleeing from a platform that is restricting its API to one that has no public API at all doesn't make sense.

lemmy is just server software. it's up to the individual admins to decide whether they want to be like reddit or not. to be as open as possible, they should set that env var to *. and maybe consider having an API key. there are security concerns to allowing any origin

@pfgithub
Copy link
Contributor Author

pfgithub commented Jun 21, 2023

there are security concerns to allowing any origin

What are the security concerns? Reddit and twitch allow cross-origin, and mastodon defaults to allowing it. The main thing to watch out for is allowing impersonation, and as long as authentication is implemented properly (which it seems to be), that's not possible. The other thing it allows is websites to potentially ddos but that seems difficult

@cinderisles
Copy link

cinderisles commented Jun 21, 2023

none really if you have other ways to block certain origins. im just repeating what every cors guide says 😅

it makes sense to me to have it default to *

@dullbananas
Copy link
Collaborator

#3191 adds ability to set allowed origin

@auouymous
Copy link

there are security concerns to allowing any origin

Only an insecure API can have security concerns. The user's lemmy instance cookies are not sent along with API requests, because that would be huge security issue, and browsers fixed that hole a long time ago. Any actual API security vulnerabilities can be exploited outside of the browser, which isn't protected by CORS origin.

The only actual issue is the user giving a third party website their login credentials so it can authenticate with the API. I don't know if lemmy allows that or if its API is only authenticated with an oauth token, in which case there is no issue. It is currently possible to create a proxy API which routes all data to and from the instance through the third party server, but it is not possible for the user's browser to communicate directly with the instance. However, that privacy invading proxy is likely to get banned when it sends thousands of requests a day to an instance.

What percentage of server owners will know about LEMMY_CORS_ORIGIN=* when released and will actually enable it? The malicious third party programs/apps don't need to request permission to access the API of each instance, why should web clients have to?

The other thing it allows is websites to potentially ddos

DDOS can happen against the instance's website too, cloudflare can handle both. And third party programs, which are allowed, can DDOS the API just as easily as a web client.

@aeharding
Copy link

aeharding commented Jun 23, 2023

FWIW, Mastodon always allows * for CORS.

It's how the Elk client is made possible: https://github.com/elk-zone/elk

And I think it's actually more secure, because it means there doesn't have to be an intermediary server to proxy all requests to in order to get around CORS (especially for 3rd party web clients)


EDIT: To be clear, I am building a web client for Lemmy called wefwef. https://github.com/aeharding/wefwef/

Currently, I have to proxy all requests to an intermediary server I control. This is not just extra work, but has security and privacy ramifications.

So please, check out how Mastodon does it (you can inspect requests with Elk) and check out how it can directly connect to any Mastodon server, since they all allow CORS *.

TLDR I think it's actually more secure to have CORS * for all Lemmy instances, because it allows 3rd party web clients to directly connect to a Lemmy instance.

@tgxn
Copy link
Contributor

tgxn commented Jun 24, 2023

TLDR I think it's actually more secure to have CORS * for all Lemmy instances, because it allows 3rd party web clients to directly connect to a Lemmy instance.

Agreed! I was hoping to have client-side "login to lemmy" on my site, but because of CORS; that's not possible, and instead we're left with less secure options like proxying requests like in your case.

@Zetaphor
Copy link

Zetaphor commented Jun 27, 2023

Agreed, not defaulting this to being open is going to severely limit the capabilities of web developers to create custom clients or API integrations. I'm happy to see #3191 was merged so that this can be resolved (modifying nginx config isn't enough), but leaving it defaulted to localhost only is still going to require every developer to get explicit opt-in support from instance admins, or take on the extra burden/security liability of hosting a proxy.

@ayan4m1, @dessalines, @Nutomic, can we get your thoughts on this?

Would you be open to a PR that defaults it to open, and then exposing the option further through documentation or an admin panel setting with a clear explanation?

@ayan4m1
Copy link
Contributor

ayan4m1 commented Jun 29, 2023

Defaulting it to * is fine by me as long as I can still override it with an environment variable.

@Zetaphor
Copy link

I've submitted a backend PR in #3408

@aeharding
Copy link

I love seeing this move forward! wefwef has grown a ton in 48 hours and we're running up against rate limit issues with some the of the largest instances. CORS * support would fully resolve that (rate limiting per client IP instead of entire wefwef.app userbase).

https://lemmy.world/post/801298

@diamondburned
Copy link
Contributor

I've made PR #3421 that fixes this issue.

@Zetaphor
Copy link

Zetaphor commented Jul 1, 2023

I've made PR #3421 that fixes this issue.

Thanks @diamondburned, your implementation appears to be better informed and tested than mine.

I've closed my PR in favor of yours, looking forward to getting this resolved!

@diamondburned
Copy link
Contributor

#3421 has been merged, so I think it's safe to close this issue.

@grndctrl
Copy link

grndctrl commented Jul 8, 2023

@diamondburned Actually, testing this (0.18.1 is live at lemmy.world) weirdly gives the error that multiple values have been set.
Screenshot 2023-07-08 at 16 33 27

@auouymous
Copy link

@grndctrl It is a problem with lemmy.world, because lemmy.ml works fine.

@grndctrl
Copy link

grndctrl commented Jul 8, 2023

@auouymous I see! Well then, @diamondburned sorry I bothered you about nothing :) case closed.

@diamondburned
Copy link
Contributor

Worth giving a heads up that instances with existing workarounds for CORS will likely break, especially if the webserver is configured to add a header rather than to set the header. It's up to each instance to undo their workarounds.

@aeharding
Copy link

This can probably be closed, CORS working great with Voyager the last couple days!

The only outstanding CORS issue we've encountered is #3567

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests