nextcloud / user_external

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

Replace imap_rcube library with curl call #122

Closed rollbrettler closed 4 years ago

rollbrettler commented 4 years ago

This commit removes the imap_rcube library with a curl call to the imap endpoint. Since curl is a dependency of nextcloud this should not cause any issues.

This fixes #116 fixes #59 fixes #97 and fixes #105

rollbrettler commented 4 years ago

@violoncelloCH I understand that this is a quite an intrusive PR let me now what you think.

violoncelloCH commented 4 years ago

hi @rollbrettler first of all: thank you very much for your PR! this seems to be a really interesting approach... as php-curl seems to be required for nextcloud anyway, this should indeed be working for everyone. However we need to be really sure with this to not break anything. I'm not 100% sure, if nextcloud doesn't run at all if the module isn't present or if it could still be the case that some use nextcloud e.g. on a shared hosting where php-curl isn't present... So if you could check this, that would be great! I really like the approach for its cleanliness... But I'm at the same time not sure if IMAP is not too complex with the different authentication methods etc. Sadly I'm not an IMAP expert :upside_down_face: . I'll try to find some time during the next days to do some testing and maybe we'll also find some volunteers (hopefully from #116 and also #59 #97) to test this...

violoncelloCH commented 4 years ago

looks very promising and worked flawlessly in a first test... it even seems to be quicker with this than before... :+1:

violoncelloCH commented 4 years ago

So to make this as easy as possible for potential testers, I've packaged a release with the changes from this pull-request: user_external-0.8.1-dev2.tar.gz ~user_external-0.8.1-dev.tar.gz~ To apply it, you first need to delete (or rename) the current folder user_external/ in the apps/ directory inside your NC installation and then extract this archive into apps/. (So you'll have a new user_external folder with this version from the PR here.) If you have any questions don't hesitate to ask! And last but not least: Please give feedback if this works! :)

ghost commented 4 years ago

After extracting you need to activate "External user Authentication 0.8.1-dev" under https://your.cloud/index.php/settings/apps/disabled

I can confirm, that I could just login via imap auth with that version on a local NC 18.0.0 (debian stretch, apache2, mariadb 15.1)

Mannshoch commented 4 years ago

[PHP] Error: Undefined variable: rcube at /nextcloud/apps/user_external/lib/imap.php#106

POST /login from XXX.XXX.XXX.XXX at 2020-02-05T08:06:17+01:00

[PHP] Error: Trying to get property 'error' of non-object at /nextcloud/apps/user_external/lib/imap.php#106

POST /login from XXX.XXX.XXX.XXX at 2020-02-05T08:06:17+01:00

[user_external] Error: ERROR: Could not connect via roundcube lib:

POST /login from XXX.XXX.XXX.XXX at 2020-02-05T08:06:17+01:00

kevo-gt commented 4 years ago

I have a similar error to https://github.com/nextcloud/user_external/pull/122#issuecomment-582271855

rollbrettler commented 4 years ago

@Mannshoch @kevo-gt thanks for reporting, I completely missed the error case. I assume it would still not work for you though. Since there is still an error with connecting to your imap server, but maybe the error is now more descriptive.

benbrummer commented 4 years ago

Resolves #59 for me, perfect!

Mannshoch commented 4 years ago

@rollbrettler Is the File updated? https://github.com/nextcloud/user_external/files/4155923/user_external-0.8.1-dev.tar.gz

violoncelloCH commented 4 years ago

here is a new archive with the updates: user_external-0.8.1-dev2.tar.gz thanks everyone for testing and reporting and thanks @rollbrettler for implementing those changes!

kevo-gt commented 4 years ago

here is a new archive with the updates: user_external-0.8.1-dev2.tar.gz thanks everyone for testing and reporting and thanks @rollbrettler for implementing those changes!

Thanks. I tried it and get the below error:

"message":"curl_error(): supplied resource is not a valid cURL handle resource at \/usr\/local\/www\/nextcloud\/apps\/user_external\/lib\/imap.php#106","userAgent":"","app":"user_external","

method":"POST","url":"\/index.php\/login","message":"ERROR: Could not connect to imap server via curl: "

I have curl and php73-curl installed and the OS is FreeBSD 12.0

rollbrettler commented 4 years ago

@kevo-gt I see. Thanks for reporting. I think I fixed it. Iam not a php expert though. @Mannshoch I dont know how @violoncelloCH creates the archives but I tested it myself by cloning the repo in the apps directory git clone https://github.com/rollbrettler/user_external.git. Also https://github.com/rollbrettler/user_external/archive/master.tar.gz should work in the meantime.

kevo-gt commented 4 years ago

@kevo-gt I see. Thanks for reporting. I think I fixed it. Iam not a php expert though. @Mannshoch I dont know how @violoncelloCH creates the archives but I tested it myself by cloning the repo in the apps directory git clone https://github.com/rollbrettler/user_external.git. Also https://github.com/rollbrettler/user_external/archive/master.tar.gz should work in the meantime.

This has now worked, thanks very much! It is quite fast logging in as well

Mannshoch commented 4 years ago

With https://github.com/rollbrettler/user_external/archive/master.tar.gz I get the Error:

[user_external] Error: ERROR: Could not connect to imap server via curl: 

POST /login
from XXX.XXX.XXX.XXX at 2020-02-06T10:34:26+01:00

Curl is working if I log into my webspace and run curl inside it: `curl imap://localhost:143 --user 'user@domainname.net:password'

Please check my config.php:

user_backends' => 
  array (
    0 => 
    array (
      'class' => 'OC_User_IMAP',
      'arguments' => 
      array (
        0 => 'localhost', //imap server
        1 => 143, // PORT
        2 => null,  // With SSL or null
        3 => 'domainname.net',  // allowed domain for mail addresses
        4 => false, // username without domain? (true, false)
        5 => false, // move new user to Group with name of the mail domainname (true, false)
      ),
    ),
  ),
rollbrettler commented 4 years ago

@Mannshoch according to your comment here https://github.com/nextcloud/user_external/issues/97#issue-472254876 your setting before was not validating the certificate novalidate-cert. I created a branch to disable that with my changes. Are you able to try if that works for you? Otherwise I guess its related to the imap server. Maybe you are able to share some more configuration specifics about that server?

https://github.com/rollbrettler/user_external/archive/disable-certificate-check.tar.gz

Mannshoch commented 4 years ago

@rollbrettler SSL do not work over ssh If I exchange imap://... with imaps://... I get this error. curl: (35) SSL received a record that exceeded the maximum permissible length. (setting in Thunderbird is STARTTLS and encrypted Password)

I'll test the update as fast as possible.

Mannshoch commented 4 years ago

Was able to do the test earlier than expected. I get the same Error:

[user_external] Error: ERROR: Could not connect to imap server via curl: 

POST /login

Could It may be a problem of the special characters in my E-Mail password? If I use Curl in bash I get a strange behavior if I use " instead of ' May it affect you with PHP also in a way?

rollbrettler commented 4 years ago

imaps:// should be a specific port (993 by default https://en.wikipedia.org/wiki/Internet_Message_Access_Protocol#Security).

Bash indeed treats " and ' differently. You can run curl also without the password curl imap://localhost:143 --user 'user@domainname.net' so you have to type it in as a hidden password. Still that does not explain why the error is empty

Error: ERROR: Could not connect to imap server via curl:

Any chance you can share some more information about the env? Are you able to use curl imaps://domainname.net --user 'user@domainname.net' or whatever imap server you set in thunderbird? Are you able to use the imap domain as well in config.php?

Mannshoch commented 4 years ago

Completet test:

On an other domain of the same hoster I use Nextcloud at moment with the user_external version 5.1 with this still working config. localhost:143/novalidate-cert/imap/readonly

Over SSH I tested following cases:

curl imap://localhost:143 --user 'user@domainname.net:password working

curl imaps://localhost:143 --user 'user@domainname.net:password curl: (35) SSL received a record that exceeded the maximum permissible length.

curl imaps://localhost:993 --user 'user@domainname.net:password have to be killed with CTRL+C

curl imaps://localhost:993 --user 'user@domainname.net:password

More details here: https://curl.haxx.se/docs/sslcerts.html

curl failed to verify the legitimacy of the server and therefore could not
establish a secure connection to it. To learn more about this situation and
how to fix it, please visit the web page mentioned above.

curl imap://domainname.net:143 --user 'user@domainname.net:password working

curl imaps://domainname.net:143 --user 'user@domainname.net:password curl: (35) SSL received a record that exceeded the maximum permissible length.

`curl imap://domainname.net:993 --user 'user@domainname.net:password' have to be killed with CTRL+C

`curl imaps://domainname.net:993 --user 'user@domainname.net:password' working

If I run curl from my desktop PC at home I get:

`curl imaps://localhost:143 --user 'user@domainname.net:password' curl: (35) error:1408F10B:SSL routines:ssl3_get_record:wrong version number

`curl imaps://localhost:993 --user 'user@domainname.net:password' working

Webtest1

I tested user_external-0.8.1-dev2.tar.gz and changed config.php to: array ( 0 => 'domainname.net', 1 => 993, 2 => 'ssl', 3 => 'domainname.net', 4 => false, 5 => false, ),

Result

[PHP] Error: curl_error(): supplied resource is not a valid cURL handle resource at /nextcloud/apps/user_external/lib/imap.php#106

POST /login

[user_external] Error: ERROR: Could not connect to imap server via curl: 

POST /login

Webtest2

I tested user_external-disable-certificate-check.tar.gz and changed config.php to: array ( 0 => 'domainname.net', 1 => 143, 2 => null, 3 => 'domainname.net', 4 => false, 5 => false, ),

Result

[user_external] Error: ERROR: Could not connect to imap server via curl: 

POST /login
rollbrettler commented 4 years ago

@Mannshoch thanks for all the testing, unfortunately I cannot reason why it does not work. Still the curl error is empty

[user_external] Error: ERROR: Could not connect to imap server via curl:

My only idea is that there might be something blocking the curl request when it is done from the php user!? You run on a shared hoster, right? Maybe they can shed some light on why the curl command works but not in the context of php!?

Mannshoch commented 4 years ago

The Support told me that there is no restriction in PHP for using curl all error message should be available.

I searched in Web about error curl and php, so I detected in examples on stackoverflow that beside .curl_error($ch) there is also a .curl_errno($ch) After I add this to imap.php I get this:

[user_external] Error: ERROR: Could not connect to imap server via curl: 67

POST /login

There is now a number added to the error. May this is helping?

I also created a new mail address with simpler password. No difference.

Mannshoch commented 4 years ago

My hoster found the error! Their curl was v7.29 compiled by Red Hat they replaced this version with curl 7.68 and now it works!

violoncelloCH commented 4 years ago

@Mannshoch nice to hear this finally worked... curl v7.29 is indeed quite old (released in 2013) vs v7.68 the current stable from January...

so we now have successfully tested this in at least 6 different environment - sounds like we can soon safely merge and release this :) I expect this to fix all the four referenced issues! thanks everyone for their work! (and of course further testing is still welcome)

rollbrettler commented 4 years ago

@violoncelloCH will you squash and merge or shall I squash the commits? Otherwise there will be quite some noise in the commit history.

Mannshoch commented 4 years ago

Thanks a lot @rollbrettler and @violoncelloCH for your work. I see the time comming to update it in Nextcloud 15 and be able then to update this nextcloud to Nextcloud 18.

violoncelloCH commented 4 years ago

@rollbrettler I would be glad if you could squash the commits :)

rollbrettler commented 4 years ago

Commits are squashed

DJCrashdummy commented 4 years ago

just a question about the "migration path" to include it into #126: is there an automatic migration to rewrite 'ssl' and 'tls' to true and null to false, or does it need a manual operation? or does the config.php (at least for now) stay like it is and are the "old values" are considered by the app itself as their needed resp. then "recommended" pendants?

violoncelloCH commented 4 years ago

@DJCrashdummy In fact there is no automatic migration, but any value is considered as true while an empty string/value is considered as false; so this should(TM) work out of the box without a need to adapt the config. Of course if anyone is up for writing such a migration step this would be highly appreciated :)

DJCrashdummy commented 4 years ago

so let's hope that nobody has accidentally used 'null' instead of null... or would this alredy with version <0.9 have thrown an error?

violoncelloCH commented 4 years ago

this should have never worked because you would have passed an invalid string into the rcube lib...

Mannshoch commented 4 years ago

When could the update be released?

violoncelloCH commented 4 years ago

as soon as possible :upside_down_face: ... this should have already been done, I see... will try to do it very soon :)

violoncelloCH commented 4 years ago

here it comes: #133

Mannshoch commented 4 years ago

Yea. Thanks a lot. Now I was able to upgrade from 15 -> 16 -> 17 and everything is working. 👍 Only Problem is the auto logout on the website I do not know how to deactivate.

violoncelloCH commented 4 years ago

Cool! Nice to hear... What do you mean by auto logout? (but that's off topic in here, so maybe better in a new issue)...

Mannshoch commented 4 years ago

I dont know how long it is. But I'm not able to stay loged in. I have three tabs wit three nextcloud pined in Firefox but every time I try to use the nextcloud with user_external I get sent to the login page.

Mannshoch commented 4 years ago

Seems this ticket is already existing #101

Mannshoch commented 4 years ago

Sorry, but that's an annoying bug. I don't know what is the different between 0.5.1 and 0.9 but could you please fix this logout problem?

violoncelloCH commented 4 years ago

I'm sorry, but this has nothing to do with those changes here, so please stick to #101 :)