Skip to content

Conversation

@schiessle
Copy link
Member

@schiessle schiessle commented Oct 5, 2020

This was motivated by how we manage our address book at c.nc.com with all the shortcomings. As you know it happens regularly that users delete/edit contacts by accident or add contacts to the address book which doesn't belong there. Also from a user perspective it is highly irritating that you have to keep your contact information up-to-date twice, in your personal settings and in the company address book. All information are available through the personal profile and we store it already as a address book because we use it for example to sync the user directories between trusted servers for federated sharing.

This PR makes this address book available in the contacts app and for other carddav clients, read-only. Following constraints apply to the address book and its visibility.

  1. It is a opt-in admin settings. By default the system address book will not be visible. I implemented the setting and the default to tackle privacy issues, especially for long term users who doesn't expect a accessible system address book after the next update. I also hope that this addresses some concerns, mentioned here Show system address book as read-only address book in contacts #19575. The setting is in the admin "Groupware" section:

image

  1. The system address book only contains information set to "public" or "trusted" by the user in their personal settings

  2. All users are already searchable through the "people menu" which exposes additional information as well, like their email address, so there shouldn't be any privacy issue. Especially as the feature is opt-in and even than each user can tweak their privacy settings on the personal page.

fix #19575

PS: Please be nice to me, it is my first PR since almost two years 😉

@nickvergessen
Copy link
Member

2. Is wrong, the privacy setting is only about federation and should not impact this. 🤷‍♂️

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is e.g. with our company addressbook we store data that is not available from the ldap, e.g. birthday, private email, phone number, etc.

<label for="carddavExposeSystemAddressBook"><?php p($l->t('Expose system address book')); ?></label>
<br>
<em>
<?php print_unescaped($l->t('Only information set to "Public" or "Trusted" in the user\'s personal settings will be exposed.')); ?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true? Because it shouldn't as per the privacy description on the fields.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the system address book was introduced explicitly for the trusted servers implementation and doesn't contain information set to "private". Just double checked it, to make sure it is still this way

@schiessle
Copy link
Member Author

schiessle commented Oct 5, 2020

The thing is e.g. with our company addressbook we store data that is not available from the ldap, e.g. birthday, private email, phone number, etc.

Maybe we will lose some information in the first step, true. But stuff which can be useful in general could and should be added to the profile at some point anyway. For example you might want to show your birthday on your profile page in the social app, so it should be part of your personal setting's profile. At the end I think the advantages surpass the (temporarily) drawbacks.

Also nobody is forced to use it, if someone still prefer a full featured address book they can keep it as it is and don't enable the system address book.

Regarding phone numbers and other stuff not provided by LDAP, this can be added to the profile, even if other parameters come from LDAP. I tested it on c.nc.com

@schiessle schiessle requested a review from skjnldsv October 5, 2020 18:07
…fault the address book will not be exposed.

Signed-off-by: Bjoern Schiessle <[email protected]>
Signed-off-by: Bjoern Schiessle <[email protected]>
@schiessle schiessle force-pushed the expose-system-address-book branch from 6ee1187 to 0ee6634 Compare October 5, 2020 18:17
@schiessle schiessle requested a review from skjnldsv October 5, 2020 18:39
Co-authored-by: John Molakvoæ <[email protected]>

Signed-off-by: Bjoern Schiessle <[email protected]>
@skjnldsv
Copy link
Member

skjnldsv commented Oct 12, 2020

Capture d’écran_2020-10-12_17-07-04
One issue is how many entries it adds to the sharing suggestion :)

[
  {
    "shareWith": "Test03",
    "shareType": 0,
    "user": "Test03",
    "isNoUser": false,
    "displayName": "Test03",
    "icon": ""
  },
  {
    "shareWith": "Test03",
    "shareType": 0,
    "user": "Test03",
    "isNoUser": false,
    "displayName": "Test03",
    "icon": ""
  },
  {
    "shareWith": "[email protected]",
    "shareType": 6,
    "user": "Test01",
    "isNoUser": true,
    "displayName": "Test01",
    "desc": "on dev.skjnldsv.com",
    "icon": ""
  },
  {
    "shareWith": "[email protected]",
    "shareType": 6,
    "user": "Test02",
    "isNoUser": true,
    "displayName": "Test02",
    "desc": "on dev.skjnldsv.com",
    "icon": ""
  },
  {
    "shareWith": "[email protected]",
    "shareType": 6,
    "user": "Test03",
    "isNoUser": true,
    "displayName": "Test03",
    "desc": "on dev.skjnldsv.com",
    "icon": ""
  },
  {
    "isNoUser": true,
    "displayName": "Search globally",
    "lookup": true
  }
]

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 12, 2020
@nickvergessen
Copy link
Member

Well we should ignore it in that case I guess

@jancborchardt
Copy link
Member

Yep, the general rule is that a "person" should only be shown once. It doesn’t matter if it is via contacts, users, email etc → people care about sharing with another person no matter the means.

(This has been unified to some extent, and should be the same here. :)

@wiswedel
Copy link
Contributor

wiswedel commented Nov 5, 2020

Hey everyone. I have another inquiry for this kind of user profile exposition.
Is this PR moving forward?

@wiswedel
Copy link
Contributor

wiswedel commented Jan 7, 2021

I guess there's no interest in getting this into 21? 22 maybe? Otherwise please reject it but stop leaving us hanging out to dry.

@skjnldsv
Copy link
Member

skjnldsv commented Jan 8, 2021

Is this PR moving forward?

Ask @schiessle?

@MorrisJobke MorrisJobke removed their request for review July 4, 2021 11:35
@adamshand
Copy link

Has there been any progress on this? It would be very helpful if system users could show up in contacts!

@cybergitti
Copy link

I started NC with v22 and use the Registration app.
As an admin I am admitting the registrants one by one, but I cannot turn the the growing user list into an automatically growing "global address book". Very sad for many (at least admin ) use cases.

@ChristophWurst ChristophWurst added 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 2. developing Work in progress labels Mar 17, 2022
@Eweol
Copy link

Eweol commented Apr 14, 2023

Is this still in progress or will it not been implemented?

@ChristophWurst ChristophWurst marked this pull request as draft April 18, 2023 08:50
@ChristophWurst
Copy link
Member

See #19575 and related work packages.

@ChristophWurst ChristophWurst deleted the expose-system-address-book branch May 15, 2023 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: dav

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show system address book as read-only address book in contacts

10 participants