Login with e-mail address should be moved to the DB user backend

brunt82 commented 7 years ago

Steps to reproduce

Open Nextcloud web portal with the desired browser.
Enter as user login the e-mail address of a LDAP user and the corresponding password.
Login is successfully.

Expected behaviour

Login should not work, because when I check the user login within the LDAP settings of Nextcloud administration, I get the response that the user was not found.

Actual behaviour

When I check the user login within the Nextcloud administration (http://nextcloud-test.test.de/index.php/settings/admin/ldap), I get following response:

"User not found. Please check your login attributes and username. Effective filter (to copy-and-paste for command line validation): (&(&(objectclass=inetOrgPerson)(mail=)(!(mail=@domain1.de))(!(mail=*@domain2.de)))(uid=groupware-test1@test.de))"

The userid of this user is "gtest1".

The following LDAP filter will be used to find the users: (&(&(objectclass=inetOrgPerson)(mail=)(!(mail=@domain1.de))(!(mail=*@domain2.de)))(uid=%uid))

So there is an inconsistence between this check and the login mechanism.

Additional information

I already wrote an issue because of a problem with capital letters and Outlook (#5154), therefore I checked this here again: As soon as the e-mail address contains a capital letter, the login also fails. A login with capitals letters within the userid works fine.

Server configuration

Operating system: Ubuntu 16.04.2 LTS

Web server: Apache/2.4.18

Database: sqlite3, Version: 3.11.0

PHP version: 7.0.15

Nextcloud version: 12.0

Updated from an older Nextcloud/ownCloud or fresh install: First install was a 12 beta, which was upgraded to the current version last week.

Where did you install Nextcloud from:

Signing status:

Signing status

No errors have been found.

List of activated apps:

App list

Enabled: - activity: 2.5.2 - bruteforcesettings: 1.0.2 - calendar: 1.5.3 - comments: 1.2.0 - contacts: 1.5.3 - dav: 1.3.0 - federatedfilesharing: 1.2.0 - files: 1.7.2 - files_pdfviewer: 1.1.1 - files_sharing: 1.4.0 - files_texteditor: 2.4.1 - files_trashbin: 1.2.0 - files_versions: 1.5.0 - files_videoplayer: 1.1.0 - firstrunwizard: 2.1 - gallery: 17.0.0 - logreader: 2.0.0 - lookup_server_connector: 1.0.0 - nextcloud_announcements: 1.1 - notifications: 2.0.0 - oauth2: 1.0.5 - password_policy: 1.2.2 - provisioning_api: 1.2.0 - serverinfo: 1.2.0 - sharebymail: 1.2.0 - survey_client: 1.0.0 - systemtags: 1.2.0 - theming: 1.3.0 - twofactor_backupcodes: 1.1.1 - updatenotification: 1.2.0 - user_ldap: 1.2.1 - workflowengine: 1.2.0 Disabled: - admin_audit - encryption - federation - files_external - spreed - user_external

Nextcloud configuration:

Nextcloud configuration

{ "system": { "passwordsalt": "***REMOVED SENSITIVE VALUE***", "secret": "***REMOVED SENSITIVE VALUE***", "trusted_domains": [ "nextcloud-test.test.de" ], "datadirectory": "\/nextcloud-data", "overwrite.cli.url": "http:\/\/nextcloud-test.test.de", "dbtype": "sqlite3", "version": "12.0.0.29", "dbname": "nextcloud", "dbhost": "127.0.0.1", "dbport": "", "dbtableprefix": "oc_", "instanceid": "och55ujpywqp", "logtimezone": "UTC", "installed": true, "ldapIgnoreNamingRules": false, "ldapProviderFactory": "\\OCA\\User_LDAP\\LDAPProviderFactory", "loglevel": 2, "maintenance": false, "updater.release.channel": "stable", "mail_from_address": "no-reply", "mail_smtpmode": "sendmail", "mail_smtpauthtype": "LOGIN", "mail_domain": "test.de", "theme": "" } }

Are you using external storage, if yes which one: no

Are you using encryption: no

Are you using an external user-backend, if yes which one: LDAP

LDAP configuration

LDAP config

Client configuration

Browser: FF, Chrome

Operating system: Ubuntu 16

blizzz commented 7 years ago

Wonder whether it's some side-effect of core user manager doing smart things.

brunt82 commented 7 years ago

Is there an update to this issue? Is the milestone "Nextcloud 13" still correct?

In our environment the users have two accounts (LDAP and Mail) and therefore two passwords. Mostly it is the same, but this is not mandatory. Because Nextcloud is connected to LDAP, user should not be able to login with their email address.

blizzz commented 7 years ago

I cannot reproduce this. And the login filter as shown is also that one that is used. Do you have local users with the same email address? Or other LDAP configurations in parallel?

brunt82 commented 7 years ago

I'm sorry, but I still can reproduce it. There are no local users and also no additional LDAP configurations. I'm able to login with my email address although there is no uid with these email address within the LDAP.

brunt82 commented 7 years ago

Maybe there is any kind of fallback implemented? To use the mail attritbute if login fails? Because the mail attribute of the LDAP contains my mail address. Anyhow the LDAP filter is "(uid=%uid)".

blizzz commented 7 years ago

To figure out what's being returned from the LDAP server, let's get some debug output. Please apply this patch:

diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php
index dcfc5cc13b..9b36eb201f 100644
--- a/apps/user_ldap/lib/User_LDAP.php
+++ b/apps/user_ldap/lib/User_LDAP.php
@@ -168,7 +168,22 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
    public function checkPassword($uid, $password) {
        try {
            $ldapRecord = $this->getLDAPUserByLoginName($uid);
+           \OC::$server->getLogger()->error(
+               'Found record {record} for loginname {uid} ',
+               [
+                   'app' => 'user_ldap',
+                   'record' => $ldapRecord['dn'][0],
+                   'uid' => $uid
+               ]
+           );
        } catch(NotOnLDAP $e) {
+           \OC::$server->getLogger()->error(
+               'No record found for loginname {uid} ',
+               [
+                   'app' => 'user_ldap',
+                   'uid' => $uid
+               ]
+           );
            if($this->ocConfig->getSystemValue('loglevel', Util::WARN) === Util::DEBUG) {
                \OC::$server->getLogger()->logException($e, ['app' => 'user_ldap']);
            }

And check the log. This is recorded on error level, so you won't have too many unrelated messages by switching to debug level.

When no record is found, something like this would show up in the log:

{"reqId":"FI62gyGlSaeSRb0rAKD9","level":3,"time":"2017-11-19T13:09:14+00:00","remoteAddr":"127.0.0.1","user":"--","app":"user_ldap","method":"POST","url":"\/stable12\/index.php\/login","message":"No record found for loginname person_444@example.org ","userAgent":"Mozilla\/5.0 (X11; Linux x86_64; rv:57.0) Gecko\/20100101 Firefox\/57.0","version":"12.0.3.3"}

Otherwise, you'd see something like this:

{"reqId":"94hJeYjjVvOjhQsMtU3L","level":3,"time":"2017-11-19T13:17:58+00:00","remoteAddr":"127.0.0.1","user":"3b3e14d7-f505-4c76-97ef-62b9c861a3a2","app":"user_ldap","method":"GET","url":"\/stable12\/index.php\/avatar\/3b3e14d7-f505-4c76-97ef-62b9c861a3a2\/37?v=2","message":"Found record uid=person_444,ou=users,ou=large,dc=nc,dc=zara for loginname person_444 ","userAgent":"Mozilla\/5.0 (X11; Linux x86_64; rv:57.0) Gecko\/20100101 Firefox\/57.0","version":"12.0.3.3"}

brunt82 commented 7 years ago

I added the lines into the User_LDAP.php. See below the result with log level=0. Keep in mind that I try to login with "groupware-test1@example.de" onetime only. Anyhow login was successful.

{"reqId":"0K2niHWV4CugcRNxGrS1","level":3,"time":"2017-11-20T09:13:42+01:00","remoteAddr":"10.10.229.150","user":"--","app":"user_ldap","method":"POST","url":"\/login","message":"No record found for loginname groupware-test1@example.de ","userAgent":"Mozilla\/5.0 (X11; Linux x86_64; rv:57.0) Gecko\/20100101 Firefox\/57.0","version":"12.0.3.3"}
{"reqId":"0K2niHWV4CugcRNxGrS1","level":3,"time":"2017-11-20T09:13:42+01:00","remoteAddr":"10.10.229.150","user":"--","app":"user_ldap","method":"POST","url":"\/login","message":"Exception: {\"Exception\":\"OCA\\\\User_LDAP\\\\Exceptions\\\\NotOnLDAP\",\"Message\":\"No user available for the given login name on ldaps:\\\/\\\/idm-auth.example.de:636\",\"Code\":0,\"Trace\":\"#0 \\\/var\\\/www\\\/nextcloud\\\/apps\\\/user_ldap\\\/lib\\\/User_LDAP.php(153): OCA\\\\User_LDAP\\\\User_LDAP->getLDAPUserByLoginName('groupware-test1...')\\n#1 [internal function]: OCA\\\\User_LDAP\\\\User_LDAP->checkPassword(*** sensitive parameters replaced ***)\\n#2 \\\/var\\\/www\\\/nextcloud\\\/apps\\\/user_ldap\\\/lib\\\/User_Proxy.php(71): call_user_func_array(Array, Array)\\n#3 \\\/var\\\/www\\\/nextcloud\\\/apps\\\/user_ldap\\\/lib\\\/Proxy.php(150): OCA\\\\User_LDAP\\\\User_Proxy->walkBackends('groupware-test1...', 'checkPassword', Array)\\n#4 \\\/var\\\/www\\\/nextcloud\\\/apps\\\/user_ldap\\\/lib\\\/User_Proxy.php(186): OCA\\\\User_LDAP\\\\Proxy->handleRequest('groupware-test1...', 'checkPassword', Array)\\n#5 \\\/var\\\/www\\\/nextcloud\\\/lib\\\/private\\\/User\\\/Manager.php(216): OCA\\\\User_LDAP\\\\User_Proxy->checkPassword(*** sensitive parameters replaced ***)\\n#6 \\\/var\\\/www\\\/nextcloud\\\/core\\\/Controller\\\/LoginController.php(231): OC\\\\User\\\\Manager->checkPasswordNoLogging(*** sensitive parameters replaced ***)\\n#7 [internal function]: OC\\\\Core\\\\Controller\\\\LoginController->tryLogin(*** sensitive parameters replaced ***)\\n#8 \\\/var\\\/www\\\/nextcloud\\\/lib\\\/private\\\/AppFramework\\\/Http\\\/Dispatcher.php(160): call_user_func_array(Array, Array)\\n#9 \\\/var\\\/www\\\/nextcloud\\\/lib\\\/private\\\/AppFramework\\\/Http\\\/Dispatcher.php(90): OC\\\\AppFramework\\\\Http\\\\Dispatcher->executeController(Object(OC\\\\Core\\\\Controller\\\\LoginController), 'tryLogin')\\n#10 \\\/var\\\/www\\\/nextcloud\\\/lib\\\/private\\\/AppFramework\\\/App.php(114): OC\\\\AppFramework\\\\Http\\\\Dispatcher->dispatch(Object(OC\\\\Core\\\\Controller\\\\LoginController), 'tryLogin')\\n#11 \\\/var\\\/www\\\/nextcloud\\\/lib\\\/private\\\/AppFramework\\\/Routing\\\/RouteActionHandler.php(47): OC\\\\AppFramework\\\\App::main('OC\\\\\\\\Core\\\\\\\\Control...', 'tryLogin', Object(OC\\\\AppFramework\\\\DependencyInjection\\\\DIContainer), Array)\\n#12 [internal function]: OC\\\\AppFramework\\\\Routing\\\\RouteActionHandler->__invoke(Array)\\n#13 \\\/var\\\/www\\\/nextcloud\\\/lib\\\/private\\\/Route\\\/Router.php(299): call_user_func(Object(OC\\\\AppFramework\\\\Routing\\\\RouteActionHandler), Array)\\n#14 \\\/var\\\/www\\\/nextcloud\\\/lib\\\/base.php(1004): OC\\\\Route\\\\Router->match('\\\/login')\\n#15 \\\/var\\\/www\\\/nextcloud\\\/index.php(48): OC::handleRequest()\\n#16 {main}\",\"File\":\"\\\/var\\\/www\\\/nextcloud\\\/apps\\\/user_ldap\\\/lib\\\/User_LDAP.php\",\"Line\":138}","userAgent":"Mozilla\/5.0 (X11; Linux x86_64; rv:57.0) Gecko\/20100101 Firefox\/57.0","version":"12.0.3.3"}
{"reqId":"0K2niHWV4CugcRNxGrS1","level":3,"time":"2017-11-20T09:13:42+01:00","remoteAddr":"10.10.229.150","user":"--","app":"user_ldap","method":"POST","url":"\/login","message":"Found record uid=gtest1,ou=users,dc=example,dc=de for loginname gtest1 ","userAgent":"Mozilla\/5.0 (X11; Linux x86_64; rv:57.0) Gecko\/20100101 Firefox\/57.0","version":"12.0.3.3"}
{"reqId":"bEOoAYGLsAc3saJ4W1e8","level":3,"time":"2017-11-20T09:13:42+01:00","remoteAddr":"10.10.229.150","user":"gtest1","app":"user_ldap","method":"GET","url":"\/apps\/apporder\/","message":"Found record uid=gtest1,ou=users,dc=example,dc=de for loginname gtest1 ","userAgent":"Mozilla\/5.0 (X11; Linux x86_64; rv:57.0) Gecko\/20100101 Firefox\/57.0","version":"12.0.3.3"}
{"reqId":"yxvoFWDgpcdjDY4nANGk","level":3,"time":"2017-11-20T09:13:43+01:00","remoteAddr":"10.10.229.150","user":"gtest1","app":"user_ldap","method":"GET","url":"\/apps\/mail\/","message":"Found record uid=gtest1,ou=users,dc=example,dc=de for loginname gtest1 ","userAgent":"Mozilla\/5.0 (X11; Linux x86_64; rv:57.0) Gecko\/20100101 Firefox\/57.0","version":"12.0.3.3"}

blizzz commented 7 years ago

@brunt82 login was successful against "gtest1", not the email address. It is from the same request however.

Okay, the LoginController tries to be smart and looks up the email address for the provided login name: https://github.com/nextcloud/server/blob/stable12/core/Controller/LoginController.php#L233

I am not sure why we have this in first place. If the local backend wants to fall back on email login, it should do it itself and not force other backends into it. @ChristophWurst

ChristophWurst commented 7 years ago

I am not sure why we have this in first place. If the local backend wants to fall back on email login, it should do it itself and not force other backends into it. @ChristophWurst

I was never a big fan of this feature nor the way it was implemented. But I guess the intention was that user back-ends don't need any change to support email login.

blizzz commented 7 years ago

I am not sure why we have this in first place. If the local backend wants to fall back on email login, it should do it itself and not force other backends into it. @ChristophWurst

I was never a big fan of this feature nor the way it was implemented. But I guess the intention was that user back-ends don't need any change to support email login.

Thanks… well, I totally agree the implementation is not ideal. OTOH it serves the purpose.

@brunt82 apparently, it is a feature, not a bug 🙊 🙈 🙉

blizzz commented 7 years ago

Anyway, in most cases other than the local backend where username and login name are similar, it won't work anyhow. @ChristophWurst let's move the logic to the local backend, what do you think? But rather for 14 only, since it is a behavioural change.

ChristophWurst commented 7 years ago

@ChristophWurst let's move the logic to the local backend, what do you think? But rather for 14 only, since it is a behavioural change.

Sure, totally fine by me. Let's clean this up for nc14 👍

brunt82 commented 7 years ago

Maybe I already wrote it: We are using 2 backends: mail and LDAP. Because NC is connected to the LDAP backend, users are unable to login if the passwords are not the same in both backends (although the password seems to be correct...).

However: I recommend to make it configurable, maybe this 'feature' is used by some users/customers without any knowing about it.

blizzz commented 7 years ago

However: I recommend to make it configurable, maybe this 'feature' is used by some users/customers without any knowing about it.

Well, it still sorta configurable when it is delegated to where belongs – the user backends. If there are other backends out there, they may need adjustments, therefore this change is ideally done in a major release. Maintaining another config option leaves a another bunch of cruft that would need to be carried along.

szaimen commented 3 years ago

Is this Issue still valid in NC21.0.2? If not, please close this issue. Thanks! :)

ghost commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

nextcloud / server