Contacts: Non-sharable address book shows share dialog

[ChristophWurst]

Describe the bug

While working on https://github.com/nextcloud/server/pull/19075 I saw that for each address book the sharing icon is displayed and when you click it, you get to chose a recipient. However, some address books are not sharable. This might not apply to Nextcloud ABs, but as soon as we have ABs from apps, that might change.

To Reproduce
Steps to reproduce the behavior:

1. Check out https://github.com/nextcloud/server/pull/19075
2. Open Contacts
3. Open settings menu

Expected behavior
No sharing icon or dialog for non-sharable ABs.

Actual behavior
Sharing for all ABs.

Screenshots

[skjnldsv]

Can you dump the network request of the addressbook properties?
Because for me this doesn't have the sharing

[ChristophWurst]

Please pull again. I think there were some local changes pending when I tried.

[skjnldsv]

Please add a dump the network request of the addressbook properties anyway :grin:
How do you know the addressbook is not shareable ?

[ChristophWurst]

<d:response>
    <d:href>/remote.php/dav/addressbooks/users/admin/app-generated--contactsinteraction--recent/</d:href>
    <d:propstat>
      <d:prop>
        <d:resourcetype>
          <d:collection/>
          <card:addressbook/>
        </d:resourcetype>
        <d:displayname>Recently contacted</d:displayname>
        <d:owner>
          <d:href>/remote.php/dav/principals/users/admin/</d:href>
        </d:owner>
        <d:current-user-privilege-set>
          <d:privilege>
            <d:read/>
          </d:privilege>
          <d:privilege>
            <d:read-acl/>
          </d:privilege>
          <d:privilege>
            <d:read-current-user-privilege-set/>
          </d:privilege>
        </d:current-user-privilege-set>
        <oc:invite/>
        <card:supported-address-data>
          <card:address-data-type content-type="text/vcard" version="3.0"/>
          <card:address-data-type content-type="text/vcard" version="4.0"/>
          <card:address-data-type content-type="application/vcard+json" version="4.0"/>
        </card:supported-address-data>
        <card:max-resource-size>10000000</card:max-resource-size>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
    <d:propstat>
      <d:prop>
        <d:getcontenttype/>
        <d:getetag/>
        <d:sync-token/>
        <x1:allowed-sharing-modes xmlns:x1="http://calendarserver.org/ns/"/>
        <card:addressbook-description/>
        <x1:getctag xmlns:x1="http://calendarserver.org/ns/"/>
        <oc:enabled/>
        <oc:read-only/>
      </d:prop>
      <d:status>HTTP/1.1 404 Not Found</d:status>
    </d:propstat>
  </d:response>

How do you know the addressbook is not shareable ?

I make an educated guess. But really I have no idea what I'm doing here with DAV :man_shrugging:

[skjnldsv]

We're not checking current-user-privilege-set but read only ;)

[ChristophWurst]

Are you confirming the bug? :)

[ChristophWurst]

I can open a separate ticket but same applies to individual contacts of an immutable AB: the contacts app still shows the props editable and the entry can be deleted

[skjnldsv]

Well, we never used the current-user-privilege-set no? Why do we have the oc:read-only then?

[ChristophWurst]

Nobody told me about that prop :)

[skjnldsv]

We're not checking current-user-privilege-set but read only ;)

:see_no_evil:

[ChristophWurst]

Should I reopen? I added the custom oc:read-only and it works for my use case

[skjnldsv]

I mean, I can use the other data, but I don't know really!
Pinging Dr @georgehrke :man_health_worker: :rotating_light:

[georgehrke]

current-user-privilege-set should be preferred. It's calculated by Sabre/DAV based on the ACL.
The oc:read-only thingy is coming somewhere from CardDAVBackend.php

[georgehrke]

The addressbook object you get from c-dav has two helper functions:
https://github.com/nextcloud/cdav-library/blob/master/src/models/davCollection.js#L214L228

[skjnldsv]

Can I safely transition to current-user-privilege-set for 16-19 ?

[georgehrke]

I would say so. It's a native DAV property, so it's always been there.

[ChristophWurst]

Another thing I noticed: all props switch into disabled state, except groups. So you can move things around, while it throws errors afterwards.

[skjnldsv]

@georgehrke can you help a bit with how to transition to current-user-privilege-set ?

[georgehrke]

cdav is already using current-user-privilege-set:
https://github.com/nextcloud/cdav-library/blob/a9642c7c83db37120f8c74306c7e761f3e78524d/src/models/davCollection.js#L219

[skjnldsv]

So I don't have anything to do?

[georgehrke]

Ah, the issue is that you display any addressbook that is writable as sharable.

As far as i can tell, this needs a small change in the server. Let me look into it.
(We need to provide <x1:allowed-sharing-modes xmlns:x1="http://calendarserver.org/ns/"/>for addressbooks as well.)