service: added ability to configure dovecot master accounts#2535
service: added ability to configure dovecot master accounts#2535polarathene merged 12 commits intodocker-mailserver:masterfrom
Conversation
|
I will review this PR ASAP, but it's going into v11.1.0 and not in the upcoming v11.0.0 release as there already are plenty of changes for v11. Hope that this is okay, you will be able to immediate get with the |
|
Not a strict requirement, but it would be nice, if you could also add a test for this, to ensure it's working. This can also prevent accidentally regressions done in future PRs. You might take a look at the current IMAP tests. |
…dy exist in dovecot-masters.cf
|
I've fixed master accounts not being created on container start-up; previously they were only created while container was running when I've also added a test to check a master user can log in. I will investigate more tests around |
|
I think I have worked out why my new test is failing - I added a new function "generate-masters" to the makefile, and added it to "all". When you follow the documentation for running the tests, this function gets run and the test passes. I think the command used to run the tests on github only runs "generate-accounts" and "tests", which means my "generate-masters" does not get run - so there isn't a master account...so the test fails. @georglauterbach, would you prefer I move the contents of "generate-masters" into "generate-accounts" or do you want to change the command used to run the tests in github (to include "generate-masters")? |
Definitely. I'd go as far as to say that this is a requirement :) It should definitely go into the |
Co-authored-by: Casper <[email protected]>
polarathene
left a comment
There was a problem hiding this comment.
LGTM 👍
Just a few changes:
DATABASEvariable also checks for an ENV that may conflict with the original*mailuserfunctions if that fallback is ever used, I'd suggest adjusting this like you have with other variables.DOVECOT_DB_MASTER,DATABASE_MASTER,CONFIG_MASTERor similar should be fine.- The command
masteris too generic/vague, at the very least a bit more context such asdovecot-masterwould be better. - I would like justification for
*as the choice of separator configured. - A query about LDAP check not being relevant for
_create_masters. _create_masterscould also be_create_dovecot_master_accounts, no one is going to manually invoke that, but it would provide a bit more context that would be appreciated when maintaining.- Minor change suggestions.
* setup.sh "master" command changed to "dovecot-master" * Usage information corrections * Removed comments and indented comments from dovecot masters test user DB
|
Sorry for the slow response - I have been away on holiday. I think I have addressed all your comments above, with one still waiting on input from you. Assuming you're happy with my changes, would we now be ready to merge? |
The feature freeze for |
There was a problem hiding this comment.
LGTM 👍
Just depends on minor additions for LDAP user support discussed in #2535 (comment)
If #2565 is merged before this PR, you might want to update your commands output to use the colour vars instead too.
|
LGTM but tests are still failing. If they are resolved, we can merge this :) |
|
To be honest I'm struggling to work out where to start with the failed tests. I'll keep trying but if anyone has any pointers I'd be very grateful! |
I'd start by going through the commits that did not show success first, i.e. 177220c, then 50e45b0, etc. It seems as if the test users are not created ( |
OutdatedI can assist for the first failure, but for LDAP we may have to choose to revert that support and document that it's only for non-LDAP ( Probably best to wait until another maintainer agrees on dropping the LDAP support for this feature, but I don't think any want to debug it so they'd probably agree. If an LDAP user wants the feature, the required changes here is small but they'll have to investigate how to get the tests passing themselves to contribute it. LetsEncrypt failure (
|
Head branch was pushed to by a user without write access
|
Thank you very much for the help - it's helped me to work out what is actually going on a bit more. I've committed those changes (verbose) and the the tests that failed last time now pass on my machine - waiting for the github ones run to be sure. On the note of dropping LDAP support, that is a shame. If I have time I will try and look into it properly so we can add support back in - if I get anywhere I'll raise another PR. |
|
Documentation preview for this PR is ready! 🎉 Built with commit: 2af0c11 |
|
Hey, @groupmsl! Have you had time to check this functionality with LDAP? |
|
The concern discussed earlier related to this feature and LDAP was incorrect IIRC. Just neither of us understood LDAP at the time well enough, should be doable. I need to find time to allocate to wrapping up the open LDAP rework PR of mine first though. |
|
@polarathene just maybe a helpful link, here there is a guy implementing this in Nextcloud, maybe his work, especially the configs he mentioned/showed can be helpful. Should I open an issue to track this? |
I don't think it's particularly wise to use a master account (meant for admin) to grant Nextcloud the ability to login as any user in Dovecot, but I guess that's up to the sysadmin deploying DMS 🤷♂️ There is the OIDC WIP PR, which would contribute a Dovecot passdb config to authenticate/login with SSO. AFAIK that would need to rely on one of the existing provisioners for accounts though, so supporting it may require some changes to how DMS currently manages passdb/userdb config setup. |
|
If there is another possible approach, I would also recommend it. I am not sure how Nextcloud will redirect the token to DMS in this case, I mean even if the OIDC PR gets merged, I hope soon, Nextcloud should also be forwarding the token somehow. I am not sure if it supports this at the moment. |
|
I have been thinking about using Master Password for Dovecot again. Actually if the password is being rotated frequently, and DMS is configured to accept connections to use Master Password only from a specific IP CIDR (I am not sure if this is possible), then this should be no problem. What do you think? Do you have any any clue where the issue with LDAP could be located? UPDATE: OIDC seems to be supported by Nextcloud particularly. This is mentioned here to be supported by Mailcow. Can this be helpful for the PR with OIDC? Is this the PR you meant? Because it seems kind of inactive to me. |
Dovecot can be restricted to a subnet/IP I think. DMS doesn't have any direct configuration for that at present AFAIK. Postfix submission ports can use a CIDR table config, something
If you trust that Nextcloud won't be compromised with some vulnerability that an attacker could exploit, go for it? It's just another attack vector. It's just that letting users connect to their account through a master account intended for admins seems like something you should be cautious about. I'm not sure if the feature was intended to be used that way. Dovecot / Postfix LDAP support does similar I think depending on config for LDAP queries but IIRC can be restricted to read-only or via auth that is restricted in scope like a subtree.
Yes, and correct it's not that active. As I mentioned, supporting OIDC will likely require some restructuring with how DMS currently approaches Dovecot auth + accounts management. I don't anticipate much movement on that front unless someone has time to dig into that and also convince maintainers to be comfortable with that. I may have input on such at a later date, but my available time is scarce for new features currently.
Are you asking about the Lua + Nextcloud + LDAP? It's a PR: #3579 |
|
Hello, |
|
LDAP support is mostly dependent upon community. There is an open PR of mine that seeks to refactor it as an improvement but it's reliant upon me finding time to push it over the finish line. As for this feature, without sinking some time into refreshing on it and the concerns with LDAP, I can't say. Chances are it's probably possible, someone just needs to look at what we're doing with the feature here and the concerns I raised/documented about LDAP support. Once you're able to grok that, you should be able to put DMS aside and configure Dovecot directly (we also have For the time being though I'm unable to take on more commitments, my backlog is too large. |
|
Thanks a lot @polarathene for your reply ❤️ |
Added ability to configure dovecot master accounts
Dovecot master accounts can now be configured directly in dms
setup.sh. A master account is useful for administration purposes, or (my use case) to perform mailbox backups of very user over IMAP.Fixes #2529
Type of change
Checklist:
docs/)