feat(dav): store scopes for properties and filter locally scoped properties for federated address book sync

[miaulalala]

• Resolves: https://github.com/nextcloud/server/issues/38021

Summary

Adds a users property scopes to the system address book. Private properties are not written at all. When syncing with a federated share, the locally scoped properties should not be shared with the remote host.

Todo

• [x] Store scopes for properties
• [x] Write local properties to system address book too
• [x] Filter locally scoped properties for federated address book sync
• [x] Repair step to add scopes to existing card data
• [ ] CardDav sync token adjustment for cards inaccessible to federated instances

How to test

Needs a federated instance setup

1. Change a property on a local user to be scope local only
2. trigger address book sync from remote host
3. check the db for the changed contact by looking into the second system address book. Before

BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.4.2//EN
UID:admin
FN;X-NC-SCOPE=v2-federated:admin
N;X-NC-SCOPE=v2-federated:admin;;;;
ADR;TYPE=OTHER;X-NC-SCOPE=v2-local:Testing test test test;;;;;;
EMAIL;TYPE=OTHER;X-NC-SCOPE=v2-federated:[email protected]
TEL;TYPE=OTHER;X-NC-SCOPE=v2-local:+435454454544
CLOUD:admin@http://localhost
END:VCARD

After

BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.4.2//EN
UID:admin
FN;X-NC-SCOPE=v2-federated:admin
N;X-NC-SCOPE=v2-federated:admin;;;;
EMAIL;TYPE=OTHER;X-NC-SCOPE=v2-federated:[email protected]
CLOUD:admin@http://localhost
END:VCARD

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• [ ] Tests (unit, integration, api and/or acceptance) are included
• [ ] Screenshots before/after for front-end changes
• [ ] Documentation (manuals or wiki) has been updated or is not required
• [ ] Backports requested where applicable (ex: critical bugfixes)

[ChristophWurst]

Only one remark. Existing users with local properties don't get a trigger to update their vcards, right? Only when a user property changes we regenerate the system card

[miaulalala]

Only one remark. Existing users with local properties don't get a trigger to update their vcards, right? Only when a user property changes we regenerate the system card

Yes that is indeed an issue, everything that already exists and doesn't update is gonna stay the same. Maybe a repair step could help?

[ChristophWurst]

\OCA\DAV\CardDAV\SyncService::syncInstance can do the full instance update. Repair step would run for every future migration. This is a one-time thing. A migration without db schema feels wrong too. So yeah, repair step, but you have to add a flag to oc_preferences whether users were updated or not.

[ChristophWurst]

Is this generally ready to review?

[tcitworld]

At this point there's hopefully a way to factorize things in SystemAddressbook.php.

[ChristophWurst]

To be sure the filtered sync changes work as expected I'd like to throw DAVxc5 and Thunderbird against the federated address book using system:sharedsecret. That should work, right?

[miaulalala]

To be sure the filtered sync changes work as expected I'd like to throw DAVxc5 and Thunderbird against the federated address book using system:sharedsecret. That should work, right?

If you know the shared secret that should work, yes.

[ChristophWurst]

I'm struggling to connect to the collection with the system user. ~~I have a debugger attached at \OCA\Federation\DAV\FedAuth::validateUserPass but it doesn't go there apparently.~~ It does but I can't list the system user address book home nor the SAB

Could not access /remote.php/dav/addressbooks/users/system/system/ (not WebDAV-enabled?):
404 Not Found

Edit: had the wrong URL

Authentication required for Nextcloud on server `localhost':
Username: system
Password: 
dav:/remote.php/dav/addressbooks/system/system/system/> ls
Listing collection `/remote.php/dav/addressbooks/system/system/system/': collection is empty.

[miaulalala]

drone fails because of merge conflict

[ChristophWurst]

Listing collection `/remote.php/dav/addressbooks/system/system/system/': collection is empty.

Unintentionally tested the sharing enumeration restriction. Works :+1:

Without it, I finally get my system contacts:

dav:/remote.php/dav/addressbooks/system/system/system/> ls
Listing collection `/remote.php/dav/addressbooks/system/system/system/': succeeded.
        Database:admin.vcf                   515  May  4 20:01
        Database:asdfasdfdasfsdfdsfasd.vcf        248  May  4 19:46
        Database:asdfasfasfasdf.vcf          259  May  4 20:26

My user's location is only shown in the downloaded .vcf if set to federated or published. If set to local it's not readable for the remote instance :+1:

[ChristophWurst]

davx5 can connect but doesn't fetch any contacts. Same issue on master. \OCA\DAV\CardDAV\SystemAddressbook::getChildren is not called.

[miaulalala]

davx5 can connect but doesn't fetch any contacts. Same issue on master. \OCA\DAV\CardDAV\SystemAddressbook::getChildren is not called.

I think the array_map function in https://github.com/nextcloud/server/blob/0f0c67c88f4773923e1995f639481e4848c594c9/apps/dav/lib/CardDAV/UserAddressBooks.php#L79 might be the cause.

Can you add a debugger and check if the systemaddressbook is actually instantiated?

[ChristophWurst]

Can you add a debugger and check if the systemaddressbook is actually instantiated?

it is

[ChristophWurst]

Fixed the static analysis issue with nullable changes. Even tried to fix upstream but the usual suspect was faster: https://github.com/sabre-io/dav/commit/9b10ec69d8fa56a1920c9dc4ad6ef131aa8cf0eb

[ChristophWurst]

contacts menu integration test fails reliably -> merge