Document recent database changes for IMAP-Backend

mtippmann commented 5 years ago

If you upgrade user_external to 0.6.x+ you need to adopt to new IMAP configuration settings that's more or less documented in #52 - however due to the changes the contents of the users_external database tables also need to be modified if you are having users that predate the update - the contents of the backend table are different - where before the full connection string was used now only the hostname is used - this creates some unintended effects:

users with the old entry style are not recognized from nextcloud anymore - that creates subtle errors with shares / user listings etc.pp.
if users login a new entry with the hostname of the imap server is created on the backend table that creates duplicate uid entries and possible duplicate users. You can fix that directly if you edit the users_external database table and replace/rename the old naming in the backendcolumn to the new naming and remove any duplicate uid entries - however some kind of warning or tutorial for migration would be nice!

Steps to reproduce

Use Nextcloud with user_external plugin
Upgrade to new Nextcloud Version / user_external 0.6.x version
Things will break in various subtle and not so subtle ways:

users can't login due to different imap connect string
users aren't in the database due to different backend-string in the database column backend in the users_external table.

Expected behaviour

Tell us what should happen

ideal world: everything should be migrated on it's own with a notification on the migration
at least: notification for admins that manual intervention is required with a detailled tutorial on how to fix things.

Actual behaviour

Tell us what happens instead

users can't login and if you fix the imap string existing users from that backend are missing until you manually fix the database tables.

Affected Authentication backend

Eg. FTP or IMAP or is it a general problem?

I can confirm it for the IMAP backend, maybe other backends are also affected

Server configuration

User External App version: (see Nextcloud apps page)

0.6.1

Operating system:

Linux

Web server:

nginx

Database:

MySQL 8.0

PHP version:

7.3

Nextcloud version: (see Nextcloud admin page)

15.0.5

Updated from an older Nextcloud/ownCloud or fresh install:

Constantly updated for a about 2 years, so lot's of versions.

lsbbs commented 5 years ago

This are not subtile errors. They are causing a hell of a mess.

All group assignments are gone. "user@domain" was assigned to a group. "user" is not. Shared folders, files, contacts, calendars with a group: Gone. Shares with single users: Also gone. But all this shares still appearing in your account. But you can not access them any more because they are shared with "user@domain" and not with "user". Collateral damage: Desktop synchronisation stops working because of this access violations.

And another really nice feature is also gone: Login to one instance with different domain names.

Sorry but this update broke a lot of things. Authentication over IMAP was a very convenient way. No need to replicate things, no need to grant access to LDAP or SMB if nextcloud is installed in a different network (what is causing a number of security problems and headache, complicating the setup unneeded).

violoncelloCH commented 5 years ago

thank you for reporting @mtippmann I understand that the best solution would have been to do a magic automatic migration... However we can't simply do this, but would have to check if the user somehow changed the configuration and do it only after that... As you said, you can of course manually change the database: instead of the old {imap.example.com:143/imap/readonly} you now need to just have the imap domain there like you have it in the config.php... (and any new "false" created entry would need to be removed before obviously) how to alter your database type can be found on the internet... I can't really give a clear advice here, because there exist a bunch of different database systems with some differences in commands... I also don't want to just give commands people just run in their terminal without knowing what they are doing, because they could destroy quite a lot of things... If you're smart enough you will still find out how to do it...

violoncelloCH commented 5 years ago

@lsbbs what you say about the username is something completely different from this issue here... take a look at #68 please...

mtippmann commented 5 years ago

@violoncelloCH sure, it's a difficult problem and there is no good and simple solution that works for everyone. But I think it would be good if users of the plugin are made aware that they need to take action and modify also the database tables in addition to changing the IMAP-Settings - that caught me by surprise, maybe for more experienced users it's clear but I doubt everyone is aware what todo - I'm just asking for a migration section in the docs or maybe a notice-popup for admins if you upgrade from pre 0.6.0 - I wasted an evening trying to figure out what went wrong, so maybe others don't have to do that. Maybe most have migrated by now, I'm not sure.

DJCrashdummy commented 5 years ago

no, e.g. i haven't migrated yet!

...and i am now a little bit worried and confused about it:

do i have to change something additional than the config.php (described in the readme)?
is it enough to prevent users login until upgrade and updating the settings in the config.php is completed?
if not, what causes the problems or does this happen anyways with the upgrade?
what exactly must be changed in the database? resp. how must the database look like to "fit a newly created one"?

and i'm not talking about a copy&paste tutorial, but a clear statement about information what to expect and actions to take resp. may be necessary to take! ...because i don't like surprises, especially not when upgrading an essential database to access everything. (which should be done as quickly as possible, without researching & fixing half an eternity!)

mtippmann commented 5 years ago

@DJCrashdummy as far as I'm aware the chanages are only for the IMAP login because the underlying library was changed from php-imap to roundcube. So you need to change the settings in the config.php to fit the new roundcube library.

is it enough to prevent users login until upgrade and updating the settings in the config.php is completed?

I think maintenance mode should be enough to make the changes if that prevents users from logging in. If you have existing users in the users_external table in the database you have to manually change the database tables. Especially backend is now in a different format: {imap.example.com:143/imap/readonly} should now be imap.example.com

now, if I understood @lsbbs correctly, there might be a problem with your usernames if they have the format user@example.com they might appear only as user - I don't use the @ in the username. Maybe @violoncelloCH knows more about this.

Basically for shares and nextcloud to work after the upgrade the user-mapping must be correct - that means no duplicate users i.e. 2 rows with identical uid and {imap.example.com:143/imap/readonly} and imap.example.com in backend column.

Usernames need to be identical to before or you have to also edit the username in other database tables.

I'd definitly test this before in a dev-setup. Check if shares and shared-folders work, as of NC 15.0.7. occ files:scan should not complain but I'm not sure if it's sufficient to validate.

However I'm not a dev and this is how I fixed my installation, but from readin the source this should be all that needs to be done, however no warranty whatsoever.

violoncelloCH commented 5 years ago

@mtippmann Hmm, a UI popup would have been a lot of work and I simply don't have enough spare time atm, sorry :/ It's documented at the top of the Readme that changes of the config.php require updating the database manually... But you're right, that I should have pointed this out. Also, concrete suggestions for enhancing this documentation are more than welcome (best in the form of a PR :) )...

@DJCrashdummy if you consider the points @mtippmann explained, you should be safe! what @lsbbs said is not related to this update, it simply depends on if you have the restriction domain specified or not... take a look at #68 for an explanation

Aquariu commented 5 years ago

Dear all,

I can confirm that

Manually updating all the backend fields for all external users works. The command for me on my mysql instance: update ownclouddb.oc_users_external SET backend='mail.exemple.org'; (yes, this is an old Owncloud upgraded since 2015)

Other related points:

The 6.0 update itself was deployed as any other app update through the appstore but required a change in configuration. Is there a way to warn the admins that (like me) may just blindly apply app updates and break their instance ?
I have never seen such a breaking change in NO/OC that was due to an app update before so I fully understand the pain caused by this to several admins (although I don't agree with the way this is communicated sometimes around here). Thanks for the devs for the communication skills and keeping your head cool in the storm.
I have left the last argument in the configuration file to '' (empty string) to keep my user@domain.com usernames intact. I would welcome the option to validate the domain AND keep my users unchanged. I someone can please explain the rationale behind removing this, I'd be grateful.

Regards,

Olivier

violoncelloCH commented 5 years ago

@Aquariu thanks for sharing your experience!

Not really... if you think about a NC notification: this could have been done, however it would have taken a lot of effort and a preparing release to deploy the necessary message inside of an update (which itself could be skipped if updated in bigger steps)...
This is probably mostly my fault, because I'm just student with not that much experience yet; also I just took maintainership of this app to safe it out of it's unhappy position it has been before, without knowing it in detail myself... so this will definitely improve in the future as I'll get more experienced as well...
This is coming - take a look at PR #65 (and issue #68)... for the reasons explained above I don't know either why exactly restricting the domain and striping it from the uid was coupled...

DJCrashdummy commented 4 years ago

sorry for my late "reply", but i had no time for dealing with this issue (reading, researching, testing and still probably break and fix my nextcloud instances), but after EOL of NC15 it had to be done... :wink:

beforehand to @violoncelloCH and all contributors: thank you very much for all your work! this app has sightly improved... awesome work! :clap:

@mtippmann thank you for sharing your findings, preventing me of surprises & a lot of headache and additionally clearly summing up the fix in your https://github.com/nextcloud/user_external/issues/64#issuecomment-483224020.

as you can see i have submitted a PR #125... hopefully it is something you all are ok with?

nextcloud / user_external