mziech / nextcloud-majordomo

Nextcloud app to sync Majordomo mailing list members with Nextcloud users and groups
GNU Affero General Public License v3.0
3 stars 1 forks source link

Make the app usable for non admin users #27

Closed kolewu closed 2 years ago

kolewu commented 2 years ago
mziech commented 2 years ago

I gave this project some thought, for the sake of simplicity, the following settings should maintain exclusive for admins:

However, there could be a new list specific setting "Sub Admins" as a multi-select dropdown to select users and groups with should be allowed to modify the list policies. Consequently, the list policies are available to the selected users.

kolewu commented 2 years ago

Why do you think that some settings should be restricted to nextcloud admins? I have come to the conclusion that nothing regarding IMAP settings or list settings is anything that affects the administration or security of the nextcloud.

If you want to make it a full application, than you need the distinction between a list administrator (knows the admin password of the list and provides the IMAP mailbox) and what you called it "sub admins", that are able to modify the list policies. But list administrator and sub admins are all normal nextcloud users.

Or am I missing something fundamental about why list admins must also be nextcloud admins?

mziech commented 2 years ago

Well, lifting the restrictions by adding @NoAdminRequired is the easier part of the story, however each time you do that you'll have to think about a concept to restore security of the application. Having a bigger Nextcloud instance, I don't want all users to be able to create mailing lists and their own IMAP configurations. Even if I restrict the usage of the App in the global settings, I still don't see the need for a casual sub-admin user to mess with the IMAP settings. Allowing access to (random) IMAP mailboxes and sending mails on behalf of the Nextcloud instance may also require some threat modelling first.

In fact making this app user-specific would require careful reconsideration of a lot of concepts, e.g. are the IMAP settings global or a user-specific setting. While a user-specific setting would sound nice at the first glance, it would also mean that my co-admins would no longer be able to fix the settings without me.

Surely, all those concepts can be thought through, but I should be done in increments. Introducing sub-admins is just the smallest increment I could think of right now ...

kolewu commented 2 years ago

Thumbs up for small increments! From my perspective the first step is to just make the current app usable for non-admin, not at least because I never work with my cloud as an admin. With the current changes in this pull request, an admin could install the app and restrict it to one or more group ("admin" for you, "majordomo" for me) and all works as before. I can't find anything that speaks against using this app with a non admins -- otherwise apps like mail would not exist. This app only utilizes an imap mailbox and sends mails on behalf of this imap user (and not of the nextcloud!) to the mailinglist manager, receives their answer and processes it. So this is exact the same thing a normal user would do with the mail app: sending mails and receiving answers.

With only one imap user you could manage many different lists with different mailinglist managers, if all the users are trusted to handle the mails and mailinglists. Just like now, but not restricted to nextcloud admins, but to majordomo admins, so to speak.

For the future, it could be possible to share the imap mailbox and the mailinglists between different users and groups or use it for one user allone, but that is over the top of my current knowledge regarding nextcloud apps.

mziech commented 2 years ago

Sorry, I cannot accept this PR because it would be an unexpected change to the security concept for other users of this app (if any). IMAP and mailinglist passwords could be exposed.

I also don't think the comparison to the mail app is justified because it fulfilling an entirely different purpose.

kolewu commented 2 years ago

With reference to other users of the app (if there are any), I can totally understand and support this decision.

The comparison with the Mail app also only referred to the aspect that the extension sends mails and queries a mail account. But it is of course correct that mail accounts (at least as far as I know) cannot be shared.

That's why I think it's really better at the moment to release the app for admins only. But that should be clearly stated in the description: At the moment, the app can only be used by admins of a Nextcloud to configure mailing lists from users of that cloud. Nothing more, nothing less -- and that's exactly what you're using it for, if I understand correctly.

For a larger circle of applications, you need to limit the configuration to one group and expand this group to include pure users who are allowed to maintain the lists themselves. How to implement this in Nextcloud, however, is still beyond my level of knowledge and understanding of Nextcloud app development.

mziech commented 2 years ago

Maybe adding fine-grained access rules could be handy for my use case as well to replace an older PHP Majordomo PHP script I'm still running. So I may also look into this at some point ...

kolewu commented 2 years ago

Thank you -- I will, too. But have to learn a bit more about nextcloud apps before.