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 Permissions Error Messages for Initial Install #2327

Closed
askvortsov1 opened this issue Sep 29, 2020 · 12 comments · Fixed by #2435 or flarum/docs#147
Closed

Improve Permissions Error Messages for Initial Install #2327

askvortsov1 opened this issue Sep 29, 2020 · 12 comments · Fixed by #2435 or flarum/docs#147

Comments

@askvortsov1
Copy link
Member

Instead of just saying "Please chmod this directory and it's contents to 0775", we should also tell them to check that the folder / contents are owned by the proper user/group, usually www-data.

@askvortsov1 askvortsov1 changed the title Improve Permissions Error Messages for Install Improve Permissions Error Messages for Initial Install Sep 29, 2020
@Ralkage
Copy link
Contributor

Ralkage commented Sep 30, 2020

Maybe create a method that checks which platform is used and display a different message for each?

In Windows/IIS, you need to grant the "Full Control" permission to the IUSR_ account, unlike Linux, where you have to chmod those directories.

@askvortsov1
Copy link
Member Author

Not sure how I feel about including implementation details in the message. Also, the vast majority of servers run Linux, so if we listed something, I feel it should be that. But then again, if we have a brief overview in the code and a solid explanation in the docs, that would be ideal

@tankerkiller125
Copy link
Contributor

We do have some instructions regarding file ownership (notably for linux) in the docs https://docs.flarum.org/install.html#folder-ownership a link to that information might be helpful?

@askvortsov1
Copy link
Member Author

I like adding a link, and we should also mention to check file ownership. Between the two, that should make stuff a lot easier for people with less server experience

@franzliedke
Copy link
Contributor

The important thing is the files being writable, right? There are many ways to get there - chmod or ownership are just two of them.

@askvortsov1
Copy link
Member Author

Then maybe we should do a more generic message and include a link to docs?

@nina-py
Copy link
Contributor

nina-py commented Nov 3, 2020

How about "Please make sure your web server/PHP user has write access to this directory [and its contents]. Please read the installation documentation for a detailed explanation and steps to follow to resolve this error."? With a link to https://docs.flarum.org/install.html#folder-ownership. I think it would be a good idea to update the docs, too, with actual commands so that inexperienced users don't have to google for a solution. I'll give it a go and link the pull request to this issue.

@tankerkiller125
Copy link
Contributor

@nina-py We purposely did not include the command for this due to differences in operating systems.

@askvortsov1
Copy link
Member Author

Which command in particular? I think we already have chown in the docs, and chmod might be better there as well.

@nina-py
Copy link
Contributor

nina-py commented Nov 3, 2020

@tankerkiller125 , but at the moment the chmod command is included in the error message during the installation process:

return [
    'message' => 'The '.$this->getAbsolutePath($path).' directory is not writable.',
    'detail' => 'Please chmod this directory'.(in_array($index, $this->wildcards) ? ' and its contents' : '').' to 0775.'
];

Using "chmod" as a verb would be quite confusing to a Windows user, I agree. The changes proposed make the error message in the codebase more generic, so that it caters to all environments, and include a link to the docs. The docs spell out the way to fix these error messages for the most likely installation environment - a Linux distribution.

I think inexperienced users would appreciate having these commands right in the documentation - that would save them the trouble of googling for a solution at the very least and potentially prevent them from giving up entirely at this point. As @askvortsov1 says, the chown command is already in the docs as a follow-up step for when the chmod changes requested by the installation process didn't make the error messages go away due to file ownership issues.

@tankerkiller125
Copy link
Contributor

@nina-py to be honest I don't think we'll need to worry about windows users too much longer given that PHP8.0 will not be ported to windows. Meaning that all PHP stuff in the future on windows will be done via WSL (which in the end is linux)

@nina-py
Copy link
Contributor

nina-py commented Nov 3, 2020

@tankerkiller125, totally agree - it is unlikely that anyone would be setting up a PHP forum on a Windows hosting server anyway. Perhaps for local development, but that would be an edge case (and you would hope they already know how to fix any permissions issues themselves).

askvortsov1 pushed a commit that referenced this issue Nov 7, 2020
- Made the wording of the error more generic
- Added link to the relevant section in the installation guide

Resolves #2327.
@askvortsov1 askvortsov1 added this to the 0.1.0-beta.15 milestone Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants