craftcms / nitro

Speedy local dev environment for @craftcms.
https://getnitro.sh
MIT License
178 stars 24 forks source link

add permissions command #317

Closed jasonmccallister closed 3 years ago

jasonmccallister commented 3 years ago

Description

This command is to automatically set permissions on a Nitro Craft site. This applies to Linux users as Docker Desktop on macOS handles permissions.

To Do

jasonmccallister commented 3 years ago

@mattstein pushed this as a wip, would love your opinion on the output, it definitely needs work :)

Setting permissions for starter-blog.nitro
  … modifying /app/.env ✓
  … modifying /app/composer.json ✓
  … modifying /app/composer.lock ✓
  … modifying /app/config ✓
  … modifying /app/config/project ✓
  … modifying /app/storage ✓
  … modifying /app/vendor ✓
  … modifying /app/web/cpresources ✓
mattstein commented 3 years ago

@jasonmccallister I’m still on the fence about this being one more command in a growing list to learn—particularly because it’s for a specific situation and subset of users instead of a command like nitro init that everyone uses.

Is it possible to have Nitro quietly check permissions (being OS-specific as needed) as it’s doing other things like running apply, and prompting you to change any (Y/n) that are unworkable?

Nitro would like to fix your starter-blog.nitro permissions for Linux:
   /app/.env             600 → 777
   /app/composer.json    600 → 777
   /app/composer.lock    600 → 777
   /app/config           600 → 777
   /app/config/project   600 → 777
   /app/storage          600 → 777
   /app/vendor           600 → 777
   /app/web/cpresources  600 → 777
Apply permissions? [Y/n]
Permissions updated.

...or...

Failed to update permissions on
   /app/vendor
Try manually running `sudo chmod 777 /app/vendor`.

This way as a user I don’t have to:

Nitro lets me avoid thinking about the issue altogether, offering to help when I might need it, and otherwise gets out of the way so I can focus on building sites.

If that behavior is annoying for some users, there could be a NITRO_AUTOSUGGEST_PERMISSIONS constant to switch off.

Nitro would like to fix your starter-blog.nitro permissions for Linux:

This line’s “for Linux” could be questionable, but I was trying to:

  1. Make the default output no output at all; no problem, no message.
  2. Lean on your previous suggestion of identifying the relevant site and files inside the container.
  3. Indicate that the suggestions are specific to your OS.
emarthinsen commented 3 years ago

I'm in favor of something like what this command does. I'm not a fan of it just 777-ing everything though. I'd like permissions less wide open. That said, I'm not sure how to do it. I like having the owner of the files be the developer's account. I had tried to create a user called nitro on my host machine with a UID that matched the www-data user in the container and then added myself and that user to a common group. I changed ownership of all the files to be my user and the shared group. Then, I gave that group rwx permissions on all of the files and folders. Unfortunately, that didn't work. I still got permissions issues. I do wonder if there isn't something in this approach though. My shallow understanding of Linux permissions might be showing here. So, I'm not sure what the right solution is.

I am in favor of having permissions be a separate command. I do think it makes sense to have it automatically run when a subset of other command run (init and create come to mind). However, it's super easy to accidentally change some permissions, so having a command to reset them to a good state is important.

emarthinsen commented 3 years ago

Did this make it into the codebase? I'm not seeing it in the 2.0.8 release.