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

Improve documentation/add-clarification around the storage options and thier permission enforcement, including notice on roles view #3688

Closed
2 tasks done
brynmoorhouse opened this issue Aug 30, 2022 · 7 comments

Comments

@brynmoorhouse
Copy link

Attempted Debugging

  • I have read the debugging page

Searched GitHub Issues

  • I have searched GitHub for the issue.

Describe the Scenario

This is possibly me misunderstanding the feature, or it might be a bug, but I've enabled the following in my .env
STORAGE_TYPE=local_secure
I've verified that all images are being uploaded to storage/uploads (outside the public directory), but yet I can still enter the direct image URL in an incognito tab to view the image. I'd expect that I'd need to be logged in to view the image?

I did find issue #2998, which is the same scenario, apart from I have not set STORAGE_IMAGE_TYPE at all, which if I've read the docs correctly, means that it will use STORAGE_TYPE

Thanks,

Exact BookStack Version

v22.07.3

Log Content

No response

PHP Version

8.1

Hosting Environment

Debian 10, NGINx, PHP 8.1 fpm

@ssddanbrown
Copy link
Member

Hi @brynmoorhouse,
Do you have the "Allow public access" setting enabled in the "Settings > Features & Security" area of BookStack?

@brynmoorhouse
Copy link
Author

I do, but the page which the image is uploaded to isn't public.

@ssddanbrown
Copy link
Member

@brynmoorhouse Yeah, that'll be the cause. This image solution is only enforced when public access is disabled, since this uses the same auth control logic. This is mentioned in our security docs page but maybe we need to also mention this on our file uploads docs page.

@brynmoorhouse
Copy link
Author

I've just turned it off and it now requests login. This will work for us for now, but it would be good if we can have that as a feature request - the permissions show that the image access is controlled by the item it's uploaded to, but that doesn't seem to be the case here.
image

@ssddanbrown
Copy link
Member

the permissions show that the image access is controlled by the item it's uploaded to, but that doesn't seem to be the case here.

Yeah, this could do with clarifying I guess on this roles view to avoid insecure assumptions.
The view of images, within the image manager, will be controlled by items they're uploaded to.
Access to the actual image file, is not so controlled via the system, and depends by storage type as discussed above.

but it would be good if we can have that as a feature request

Sure, I haven't had this as a common request here (Can't find existing issue) but has been requested via some other channels.
It's about time we add this as an option, although there will be some caveats/usability concerns. I've started this off in #3693, to be part of the next release.

This issue will remain open, with an aim to clarify the available image security options. I'll update the title for this focus.

@ssddanbrown ssddanbrown changed the title local_secure doesn't seem to be working Improve documentation/add-clarification around the storage options and thier permission enforcement Sep 1, 2022
@ssddanbrown ssddanbrown changed the title Improve documentation/add-clarification around the storage options and thier permission enforcement Improve documentation/add-clarification around the storage options and thier permission enforcement, including notice on roles view Sep 1, 2022
ssddanbrown added a commit to BookStackApp/website that referenced this issue Sep 6, 2022
Also updated security docs to be leaner and reference the file-uploads
docs for specifics on upload options to reduce docs duplication.
Also reformatted storage options overview to be extra clear about the
different options.

Related to BookStackApp/BookStack#3688
@ssddanbrown ssddanbrown added this to the Next Feature Release milestone Sep 6, 2022
ssddanbrown added a commit that referenced this issue Sep 6, 2022
Added to clarify the role permission in scenarios where users may have
not read the docs site to understand image access control.

Related to #3688
@ssddanbrown
Copy link
Member

I've now added a notice to the roles view for clarification as part of d867294.
Permission-controlled images have been added as part of #3693.
The BookStack docs have been updated with clearer detail of the options as part of the work done in BookStackApp/website@5cacef6.

All of this will be part of the next feature release.
Thanks @brynmoorhouse for your input here.

@brynmoorhouse
Copy link
Author

Epic, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants