Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update roundcube carddav plugin to version 5.1.0 #2305

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kiekerjan
Copy link
Contributor

This updates the carddav plugin for Roundcube to the latest version.
There is a change in the configuration (usage of 'extra_addressbooks', see https://github.com/mstilkerich/rcmcarddav#upgrading-from-4x) There's also a note that perhaps a manual cleanup of the address books is needed, but that wasn't an issue for me.

@downtownallday
Copy link
Contributor

I'm having a problem where I can't create a new ownCloud contact from Roundcube after a password change (using roundcube's interface but I doubt it matters how the change is made).

I get "Login failed" in nextcloud.log and "Exception/NotAuthenticated" in roundcube/carddav.log. If I set the password back, it works. It's almost as if carddav is saving the password in the roundcube database when it shouldn't be (according to the docs)

@downtownallday
Copy link
Contributor

This is a bug in carddav. The "password" field in the carddav database is "%p" encrypted with the user's password. When the password can't be decrypted because the user changed their password, the carddav code (Utils.php) returns empty string as the password instead of "%p" and then uses that verbatim as the password in rest calls to nextcloud, which of course fail.

@downtownallday
Copy link
Contributor

Adding this to carddav/config.inc.php from setup/webmail.sh fixes the issue:

$prefs['_GLOBAL']['pwstore_scheme'] = 'plain';

No passwords are stored in the database, only "%p" appears in the password field.

@kiekerjan
Copy link
Contributor Author

kiekerjan commented Oct 18, 2023

Confirmed. Nice catch 👍
Do you think this is introduced with the 5.1.0 version of carddav?
If this is a bug in carddav, should we report it there? Or are we (MiaB) using it wrong?

@downtownallday
Copy link
Contributor

Yes. Also, here are some additional changes suggested by the carddav maintainer Michael Stilkerich, including using address book discovery.

mstilkerich/rcmcarddav#454

@downtownallday
Copy link
Contributor

I'm getting another error in the logs when migrating existing data from carddav v4:

[18-Oct-2023 18:06:05 +0000]: [5 ERR] Database::insert (INSERT INTO carddav_accounts("accountname","username","password","rediscover_time","presetname","flags","user_id") VALUES (?,?,?,?,?,?,?)) ERROR: [19] UNIQUE constraint failed: carddav_accounts.user_id, carddav_accounts.presetname 
[18-Oct-2023 18:06:05 +0000]: [5 ERR] Error adding/updating preset ownCloud for user 1 [19] UNIQUE constraint failed: carddav_accounts.user_id, carddav_accounts.presetname 

Still trying to work it out.

@kiekerjan
Copy link
Contributor Author

That is a most excellent reply by mstilkerich 💯
I'll update this pull request with his suggestions for suppress_version_warning, preemptive_auth and hide.
I would like to use the address book discovery, so I'm going to test that locally. If there's more interest, we can include that here (or in a separate pull request) as well.

@downtownallday
Copy link
Contributor

In addition to your changes, Mr. Stilkerich said 'accountname' should be set, and I suggest 'name' be set as well using the new template format so the address book title in roundcube looks like it used to:

	 'accountname'  =>  'ownCloud',
	 'name'         =>  'ownCloud (%N)',

@downtownallday
Copy link
Contributor

After a migration to carddav v5, I get duplicate address books in user accounts that had existing contacts. I see "Contacts" listed twice in Roundcube in the Contacts tab, and both address books have the same contacts. The database has duplicate address book entries and duplicate contacts data.

Because we have a setup where user's aren't creating their own carddav connections to other servers ('hide_preferences' is set to true), I'm thinking the best thing to do is nuke the carddav database's contacts and address books, which are all just cached entries anyway (copies of the data on nextcloud). When the user logs in, everything gets re-populated by the plugin.

So, right after untaring the new carddav code in setup/webmail.php

	# nuke carddav contacts cache when upgrading from carddav v4 -> v5
	if [ -e $STORAGE_ROOT/mail/roundcube/roundcube.sqlite -a "$(sqlite3 $STORAGE_ROOT/mail/roundcube/roundcube.sqlite 'select count(*) from carddav_migrations where filename="0017-accountentities"')" = "0" ]; then
		# we're upgrading from carddav v4 to v5 - start with a fresh cache
		say_verbose "Delete carddav contacts cache"
		cp $STORAGE_ROOT/mail/roundcube/roundcube.sqlite /var/backups/roundcube.sqlite.v4
		sqlite3 $STORAGE_ROOT/mail/roundcube/roundcube.sqlite 'delete from carddav_addressbooks'
	fi

@kiekerjan
Copy link
Contributor Author

kiekerjan commented Oct 28, 2023

@downtownallday I added your suggestions. I did not yet have time for proper testing, so this will need some attention before I consider it merge worthy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants