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

Add support for other file storage drivers (Flysystem) #11120

Open
asmecher opened this issue Mar 18, 2025 · 2 comments
Open

Add support for other file storage drivers (Flysystem) #11120

asmecher opened this issue Mar 18, 2025 · 2 comments

Comments

@asmecher
Copy link
Member

asmecher commented Mar 18, 2025

OJS currently supports file interactions with a mixture of Flysystem's local adapter and legacy PHP functions (wrapped with homebrew FileManager classes).

There are two separate file storage areas involved:

  • The files directory (files_dir in config.inc.php). This is a private storage area that should never be accessed through any other mechanism than PHP scripts.
  • The public directory. This exists inside the web root; OJS/OMP/OPS uses it for storing CSS, images, and other content that can be accessed directly through the web server.

Eventually it would be beneficial to convert both of these over to consistently using Flysystem and eliminate the old FileManager classes as much as possible. This would potentially allow the use of other Flysystem adapters, which could be configured via config.inc.php.

I'm ambivalent about the public files directory; having PHP scripts write to web-accessible areas is an opportunity for security hazards. I would support a move to homogenize the two file storage areas and serve everything there via PHP.

Related issues:

@almereyda
Copy link

In a decoupled scenario with Flysystem writing to S3 instead of a local host, the web server delivering the assets can be a different one. This needs to be taken into account for the load-balancing strategy of a given service. Additionally it is considered good security practice to serve user-generated content from a separate domain-name, in order to benefit from the additional XSS and CSRF protection.

Stored objects receive a full URL and need to be addressed through it. Relative file-system paths or constructed URLs based on an instance domain will not continue to work. Ideally the transport is encrypted. Signing of URLs allows for the client to generate links that can be opened, while a bucket can remain private.

E.g. with Minio:

Flysystem seems to be prepared for that:

In the documentation it says the public directory is depreciated. Are there concrete plans for phasing it out over the next releases?

In a perfect world, that of containers, the application does not store state in the file system and all side-effects are handed over to separate, decoupled (web) services. This would in turn allow for horizontal scaling of pkp-lib processes.

@asmecher
Copy link
Member Author

In the documentation it says the public directory is depreciated. Are there concrete plans for phasing it out over the next releases?

Not concrete. But we have a few related wishes/changes that are related:

  • Improve file management regarding orphaned files #10274: The current "public" files directory does not have database backing, so it's hard to have concrete knowledge of what got put where by whom. We'd like to change that eventually so that public files are properly tracked. This will be a comprehensive change so it makes sense to do it at the same time as the directory is moved.
  • Change web root from application root directory for security #8114: At some point we'd like to switch bootstrapping over to Laravel's, and probably at the same time, adopt Laravel-like deployment of the app as a whole. This will mean the public files directory's assets have to get treated more like a typical Laravel application would do.

Those are both very aggressive changes, though, and not yet scheduled. So whatever happens with public should be done with those later changes in mind, in order to avoid coupling them together into a big-bang rewrite.

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

No branches or pull requests

2 participants