Support for raw content subdomain #52

Closed
hw wants to merge 1 commits from raw_content_subdomain_support into master
hw commented 1 year ago
Owner

Support for raw content delivery on dedicated subdomain. (Needs review)

Support for raw content delivery on dedicated subdomain. (Needs review)
hw added 1 commit 1 year ago
momar approved these changes 1 year ago
momar left a comment
Owner

Looks good so far, but basically this whole PR acts as a pretty basic reverse-proxy right now - why don't we just do that in HAProxy or nginx instead?

Looks good so far, but basically this whole PR acts as a pretty basic reverse-proxy right now - why don't we just do that in HAProxy or nginx instead?
if ($owner === "raw") {
$owner = strtolower(array_shift($request_url_parts));
$cors = true;
$ch = curl_init("http://localhost:3000" . $_SERVER["REQUEST_URI"]);
momar commented 1 year ago
Owner

Does this limit in any way the accessible pages? Like can I get the source of my user account page by going to raw.codeberg.org/momar?

Does this limit in any way the accessible pages? Like can I get the source of my user account page by going to raw.codeberg.org/momar?
hw commented 1 year ago
Poster
Owner

It recognizes permissions (hidden repos get 404 which is not the case currently).

It recognizes permissions (hidden repos get 404 which is not the case currently).
momar commented 1 year ago
Owner

Possibly a problem: it seems like people can get a valid CSRF token through that, as they seem to not be limited to an IP address!

I'd suggest to only map raw.codeberg.org/{username}/{repo}/{...} to codeberg.org/{username}/{repo}/raw/{...}, and only serve the response when it's 200 OK

Possibly a problem: it seems like people can get a valid CSRF token through that, as they seem to not be limited to an IP address! I'd suggest to only map `raw.codeberg.org/{username}/{repo}/{...}` to `codeberg.org/{username}/{repo}/raw/{...}`, and only serve the response when it's `200 OK`
hw commented 1 year ago
Poster
Owner

So we should strip the headers?

So we should strip the headers?
hw commented 1 year ago
Poster
Owner

(just like the cookie?)

(just like the cookie?)
momar commented 1 year ago
Owner

Nonono, the CSRF token is in the content of the response! We should explicitly only allow raw files from repositories (raw.codeberg.org/username/repo/branch/main/filename.txt), instead of any Gitea URL (like raw.codeberg.org/user/settings, even if cookies are stripped).

Nonono, the CSRF token is in the content of the response! We should explicitly *only* allow raw files from repositories (`raw.codeberg.org/username/repo/branch/main/filename.txt`), instead of any Gitea URL (like `raw.codeberg.org/user/settings`, even if cookies are stripped).
hw commented 1 year ago
Poster
Owner

never from .org I guess?

never from .org I guess?
momar commented 1 year ago
Owner

That doesn't matter - replace it with .page or whatever. IMO it should definitely only be possible to get files from repositories via the .page TLD, and make sure to never expose Gitea resources like the API, or HTML pages.

That doesn't matter - replace it with .page or whatever. IMO it should definitely only be possible to get files from repositories via the .page TLD, and make sure to never expose Gitea resources like the API, or HTML pages.
hw commented 1 year ago
Poster
Owner

agree

agree
momar requested changes 1 year ago
momar left a comment
Owner

Whoops, pressed the wrong button...

Whoops, pressed the wrong button...
Owner

Oh, and I think the issue that I have fixed in #50 (even though it was caused by the original plans of this PR) isn't fixed here yet?

Oh, and I think the issue that I have fixed in #50 (even though it was caused by the original plans of this PR) isn't fixed here yet?
Owner

Alright, I found out some more issues: The if ($owner === "raw") block doesn't die (so the rest of the script is executed as well), and HTML would be rendered, which is probably more confusing than useful. I fixed those issues in #50.

Also a question: shall this expose raw.codeberg.org or raw.codeberg.page?! because currently I think it uses the latter one.

Alright, I found out some more issues: The `if ($owner === "raw")` block doesn't die (so the rest of the script is executed as well), and HTML would be rendered, which is probably more confusing than useful. I fixed those issues in #50. Also a question: shall this expose raw.codeberg.org or raw.codeberg.page?! because currently I think it uses the latter one.
Owner

Please see Codeberg/build-deploy-gitea#52 before merging, I think that's a blocker!

Please see https://codeberg.org/Codeberg/build-deploy-gitea/pulls/52#issuecomment-183053 before merging, I think that's a blocker!
Owner

Thank you for merging #50! I guess with it containing the changes here, this PR can be closed.

Thank you for merging #50! I guess with it containing the changes here, this PR can be closed.
hw commented 1 year ago
Poster
Owner

Superseded by #50

Superseded by #50
hw closed this pull request 1 year ago

Reviewers

momar requested changes 1 year ago
This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Loading…
There is no content yet.