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

Media files are reachable unauthenticated #17972

Closed
Andres1357 opened this issue Nov 8, 2024 · 4 comments · Fixed by #17990
Closed

Media files are reachable unauthenticated #17972

Andres1357 opened this issue Nov 8, 2024 · 4 comments · Fixed by #17990
Assignees
Labels
severity: high Completely breaks certain functions, or substantially degrades performance application-wide status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@Andres1357
Copy link

Deployment Type

Self-hosted

Triage priority

N/A

NetBox Version

v4.1.6

Python Version

3.12

Steps to Reproduce

  1. Install NetBox v4.1.0 - 4.1.6 and ensure configuration variable LOGIN_REQUIRED = True.
  2. Add an image attachment to any object and get the URL to the image
    (https://yournetbox.com/media/image-attachments/yourimage.png)
  3. Attempt to access that URL while logged out of NetBox.

I've reproduced this in various NetBox versions (4.1.0, 4.1.1, 4.1.5, 4.1.6) both as standalone and in Docker.

It is reproducible using the Documents plugin as well as it also stores files in the media directory (https://yournetbox.com/media/netbox-documents/doc.pdf)

I've also reproduced it on https://netbox-demo.netboxlabs.com, though I can't confirm if that instance has LOGIN_REQUIRED = True.

Expected Behavior

It is my understanding that NetBox should not display the file and instead redirect to the login page if the variable LOGIN_REQUIRED = True. This was the behavior seen on NetBox v4.0.3.

Observed Behavior

NetBox displays the file just as if you were logged in.
image

@Andres1357 Andres1357 added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Nov 8, 2024
@Andres1357
Copy link
Author

I believe change #16580 may be the cause.

@Azmodeszer
Copy link
Contributor

Can confirm this.

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: high Completely breaks certain functions, or substantially degrades performance application-wide and removed status: needs triage This issue is awaiting triage by a maintainer labels Nov 12, 2024
@jeremystretch
Copy link
Member

We'll need to implement a custom wrapper view to inherit from ConditionalLoginRequiredMixin and ensure LOGIN_REQUIRED is enforced consistently for static media.

@jeremystretch jeremystretch self-assigned this Nov 12, 2024
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Nov 12, 2024
@Andres1357
Copy link
Author

Thanks Jeremy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: high Completely breaks certain functions, or substantially degrades performance application-wide status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
3 participants