CanastaWiki / Canasta-CLI

The Canasta command line interface, written in Go
MIT License
5 stars 14 forks source link

Canasta config strategy #64

Closed bawolff closed 9 months ago

bawolff commented 1 year ago

Filing as a follow up to #48

Canasta needs some persistent state to keep track of wikis it manages. Right now it stores that state in /etc/canasta if you are root, and ~/.config/canasta if you are not root. If these directories/files do not exist, then the directory is created 0777 and the file is created 0644.

I think first of all, that it makes more sense to create the directory as 0755 not 0777 since it is a per-user directory. Normally I think of /etc as a place for global configuration files that are not generally written to by the application and apply to all users. I think it might make more sense to have the root state file live in /root/.config like the other users.

I think there is the larger question of when castana is used by different users, should they share information on what wikis exist, or should every user get a different view. I'm not sure what the correct answer to that is. If it is expected that different users all have the same view of which wikis are managed by castana, then i would expect the file to live in somewhere like /var/lib or /var/local.

[Edit: As i write this out more fully, i realize that maybe all this doesn't really matter all that much] [Edit: fix typo]

yaronkoren commented 1 year ago

@bawolff - can you clarify what you mean by it not mattering that much? Not having a set location for the Canasta config file seems like a pretty big deal. What if a user iinstalls Canasta as themselves, then later on runs it as root - Canasta will keep not finding its own config file, no? Or am I misunderstanding this?

bawolff commented 1 year ago

I mean for the part, if we decide to stick with different config file per user, I suppose even if it feels strange to me that the root file lives in /etc/castana where the other files are under ~/config, that part of it ultimately doesn't matter much, since its just a convention.

yaronkoren commented 1 year ago

Alright.

@hexmode - any thoughts on this?

hexmode commented 1 year ago

The locations are meant to make sense for a multi-user system. This could be, for example, a wiki hoster that allows wiki owners to log in and control their own wikis as well as site administrators who would (also) have access to the global wikis.

The root user is rarely an individual, but is a role that is shared between users. The reason to put the root config in /etc was that this is the global config that people with root (e.g. sudo) access would use.

On a multi-user system, users who have been delegated the privilege to spin up wiki containers might not have access or control over the global configuration. In this sense, @bawolff is definitely correct about 0777 permissions not being appropriate. A site may decide that permissions of 0700 are appropriate for the global wiki config, but a default of 0755 is acceptable.

bawolff commented 1 year ago

I think the confusing part here is that if you accidentally run a canasta command as the wrong user, its as if the wiki doesn't exist instead of giving an error message. Perhaps it might make more sense for the config to be per-wiki instead of per user. e.g. you could have a json file for each wiki id. e.g. if you had wiki1 and wiki2, you might have /var/lib/canasta/wiki1.json and /var/lib/canasta/wiki2.json. That would allow the config to be per user, but also allow detection that a wiki exists with the chosen name but the current user cannot access it. However perhaps this doesn't make sense to worry about now.

I'll make a patch for the permission change.

bawolff commented 1 year ago

Oh, i forgot that umask is a thing, and that by default its probably 0022, so in practice the directories usually come out 0755 anyways, but probably still good to specify them as 0755.

bawolff commented 1 year ago

https://github.com/CanastaWiki/Canasta-CLI/pull/71 (I'm a bit of github noob, hopefully i did that right)

hexmode commented 1 year ago

71 (I'm a bit of github noob, hopefully i did that right)

If you're saying this patch fixes that bug, then you should really update the message to say "Fixes #71"

hexmode commented 1 year ago

and, I hope it is clear, but, in case it isn't, that I mean the commit message in the pull request.