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

CLI Flush & Asset Permissions #81

Closed
2 tasks done
nglasl opened this issue Oct 5, 2017 · 4 comments
Closed
2 tasks done

CLI Flush & Asset Permissions #81

nglasl opened this issue Oct 5, 2017 · 4 comments

Comments

@nglasl
Copy link

nglasl commented Oct 5, 2017

When running the following from the command line (in SS4)..

php vendor/silverstripe/framework/cli-script.php dev/build flush=1 disable_perms=1

The /assets/.htaccess and /assets/.protected/.htaccess files don't get created. However, running the same thing from the browser causes them to be created (only a flush is actually needed). At first I thought it may have been a permission thing (similar to silverstripe/silverstripe-framework#3092), however I've narrowed it down to the following..

When hitting this from the browser, the type comes through as apache (in my case). However doing this from the command line, the type comes through as php. So neither of the following are hit..

This is currently my work around, but it seems like we should be able to pass something through to the cli-script such as type=apache at the very least.

Thoughts?

Pull requests

@nglasl nglasl changed the title CLI Asset Permissions CLI Flush & Asset Permissions Oct 5, 2017
@chillu
Copy link
Member

chillu commented Oct 6, 2017

Hmm, yeah it's just a bit too much magic, isn't it? We can't mandate that you run dev/build through a browser/webserver. And this is an essential setting, so we have to be careful with adding explicit steps that might be missed and accidentally expose protected files, as well as break website operation (not redirecting correctly). @tractorcow Ideas?

@tractorcow
Copy link
Contributor

I suggest an env var for cli, and assume Apache if not configured.

@flamerohr
Copy link
Contributor

flamerohr commented Oct 17, 2017

one issue when implementing this is the file permissions on the generated .htaccess files:
if it's generated by the web-user (_www, www-data, etc) then the using CLI will not have access to those files, and vice versa for the web-user to access if generated the admin user logged in...
The workaround is that you log in specifically to the web-user to run the CLI - not ideal.

@tractorcow has suggested something similar to this to address a part of the issue:

--- a/src/Flysystem/AssetAdapter.php
+++ b/src/Flysystem/AssetAdapter.php
@@ -133,7 +133,7 @@ class AssetAdapter extends Local
 
         // Apply each configuration
         $config = new FlysystemConfig();
-        $config->set('visibility', 'private');
+        $config->set('visibility', 'public');
         foreach ($configurations as $file => $template) {
             // Ensure file contents
             if ($forceOverwrite || !$this->has($file)) {
@@ -145,7 +145,7 @@ class AssetAdapter extends Local
                 }
             }
             // Ensure correct permissions
-            $this->setVisibility($file, 'private');
+            $this->setVisibility($file, 'public');
         }
     }

@flamerohr
Copy link
Contributor

I've added #91 and silverstripe/silverstripe-framework#7488 to address this issue :) primarily the $default_server config option is now available on the AssetAdaptor class.

Other things tweaked were the forgotten but needed file_permissions leniency, and a check before setting file visibility (tiny performance boost).

@flamerohr flamerohr removed their assignment Oct 18, 2017
@tractorcow tractorcow self-assigned this Oct 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants