Codeberg-Infrastructure/build-deploy-forgejo
12
26
Fork 25
Code Issues 6 Pull requests 1 Releases Wiki Activity

Support for raw content subdomain #52

Closed
hw wants to merge 1 commit from raw_content_subdomain_support into master
pull from: raw_content_subdomain_support
merge into: Codeberg-Infrastructure:master
Conversation 7 Commits 1 Files changed 1 +17 -2
Image
Image hw commented 2021-02-21 16:35:41 +01:00
Member
Copy link

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

Support for raw content delivery on dedicated subdomain. (Needs review)
Image momar approved these changes 2021-02-21 17:49:00 +01:00
Image momar left a comment
Member
Copy link

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?
var/www/pages/index.php
@ -47,3 +47,2 @@
if ($owner === "raw") {
$owner = strtolower(array_shift($request_url_parts));
$cors = true;
$ch = curl_init("http://localhost:3000" . $_SERVER["REQUEST_URI"]);
Image momar commented 2021-02-21 17:42:54 +01:00
Member
Copy link

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?
Image hw commented 2021-02-21 19:17:35 +01:00
Author
Member
Copy link

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).
Image momar commented 2021-03-14 13:20:08 +01:00
Member
Copy link

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`
Image hw commented 2021-03-14 21:48:10 +01:00
Author
Member
Copy link

So we should strip the headers?

So we should strip the headers?
Image hw commented 2021-03-14 21:48:57 +01:00
Author
Member
Copy link

(just like the cookie?)

(just like the cookie?)
Image momar commented 2021-03-15 11:29:08 +01:00
Member
Copy link

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).
Image hw commented 2021-03-16 13:35:22 +01:00
Author
Member
Copy link

never from .org I guess?

never from .org I guess?
Image momar commented 2021-03-16 15:54:01 +01:00
Member
Copy link

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.
Image hw commented 2021-03-17 10:49:54 +01:00
Author
Member
Copy link

agree

agree
Image momar requested changes 2021-02-21 17:49:20 +01:00
Image momar left a comment
Member
Copy link

Whoops, pressed the wrong button...

Whoops, pressed the wrong button...
Image
Image momar commented 2021-02-21 17:59:16 +01:00
Member
Copy link

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?
Image
Image momar commented 2021-03-14 12:38:27 +01:00
Member
Copy link

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.
Image
Image momar commented 2021-03-14 13:20:33 +01:00
Member
Copy link

Please see Codeberg/build-deploy-gitea#52 (comment) 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!
Image
Image momar commented 2021-03-17 11:08:11 +01:00
Member
Copy link

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.
Image
Image hw commented 2021-03-17 11:10:02 +01:00
Author
Member
Copy link

Superseded by #50

Superseded by #50
👍 1
Image hw closed this pull request 2021-03-17 11:10:05 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Imagemomar
Labels
Clear labels   
inadvertently closed

Closed because of updated branch names - might still be valid!

  
Kind: Breaking

   
Kind: Bug

   
Kind: Enhancement

   
Kind: Feature

   
Kind: Maintenance

   
Kind: Question

   
Kind: Security

   
Kind: Testing

   
Priority: Critical

The priority is critical

  
Priority: High

The priority is high

  
Priority: Low

The priority is low

  
Priority: Medium

The priority is medium

  
Status: Blocked

   
Status: Completed

Work is complete

  
Status: Help wanted

   
Status: In progress

Work is in progress

  
Status: Needs feedback

Feedback is needed

  
Status: Stale
No labels
inadvertently closed
Kind: Breaking
Kind: Bug
Kind: Enhancement
Kind: Feature
Kind: Maintenance
Kind: Question
Kind: Security
Kind: Testing
Priority: Critical
Priority: High
Priority: Low
Priority: Medium
Status: Blocked
Status: Completed
Status: Help wanted
Status: In progress
Status: Needs feedback
Status: Stale
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Image Image
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Codeberg-Infrastructure/build-deploy-forgejo!52
No description provided.