nextcloud / user_external

👥 External user authentication methods like IMAP, SMB and FTP
https://apps.nextcloud.com/apps/user_external
107 stars 64 forks source link

Inquiry: Should user_external be split into a separate plugin for each authenticator? #183

Closed mmccarn closed 2 years ago

mmccarn commented 2 years ago

Is there a case to be made for splitting user_external into separate apps for each authentication mechanisme (IMAP, WebDAV, FTP, Samba, BasicAuth, SSH, and XMPP)?

Splitting the app might make support and updates easier.

For example, I use IMAP, and could provide support for that mechanism, but I do not have any way to test or maintain most of the other mechanisms.

I suppose the optimum solution would be a basic user_external framework with modular add-ins for each auth type -- but I don't know how this would fit into the Nextcloud app ecosystem.

violoncelloCH commented 2 years ago

thanks for the idea @mmccarn however I don't think this would reduce the workload for the maintainers (currently only me, doing this in my spare time) having multiple apps would mean creating new releases for each one of these separately, so effectively multiplying that time

however If you could help testing (also just for one of the authentication methods is worth it), you'd be more than welcome. I'm personally far from being able to test all possible authentication methods and configuration possibilities. Assuming that existing code doesn't break out of nowhere, testing is only really needed for the code that actually gets changed (that's not much currently). E.g. we don't need to test smb or xmpp if IMAP code gets changed... If you want to be part of the nextcloud/user_external team to get notified about things to test and review: let me know and I'll send you an invite! (currently e.g. for #184 in regards to #179)

I'll close this issue, but discussion can of course follow here and we can reopen it if we actually have something actionable in here :)

mmccarn commented 2 years ago

I'd be happy to help verify changes, but I already customize lib/imap.php on my server to require username@domain, and to set the user's email to match their login name -- changes which work for me but would not work for everyone...

--- imap.php    2021-09-06 08:21:43.158023934 -0400
+++ /var/www/nextcloud/apps/user_external/lib/imap.php  2021-04-11 10:01:27.012630375 -0400
@@ -63,7 +63,7 @@
        $pieces = explode('@', $uid);
        if ($this->domain !== '') {
            if (count($pieces) === 1) {
-               $username = $uid . '@' . $this->domain;
+               return false;
            } else if(count($pieces) === 2 && $pieces[1] === $this->domain) {
                $username = $uid;
                if ($this->stripeDomain) {
@@ -103,6 +103,13 @@
            curl_close($ch);
            $uid = mb_strtolower($uid);
            $this->storeUser($uid, $groups);
+                        if ( ! $this->stripeDomain) {
+                          /**
+                          * Set the IMAP user's email to their login
+                          */
+                           $config = \OC::$server->getConfig();
+                           $config->setUserValue( $uid, 'settings', 'email', $uid);
+                        }
            return $uid;
        } else {
            OC::$server->getLogger()->error(