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

Privacy - Auto-Complete with emails from the instance #848

Closed
pierreozoux opened this Issue Mar 26, 2018 · 11 comments

Comments

@pierreozoux
Copy link
Member

pierreozoux commented Mar 26, 2018

Steps to reproduce

  1. run a shared instance with many users
  2. create an email
  3. start to fill the to field

Expected behaviour

You should see auto-completion from your address book or your group.

Actual behaviour

You see auto-completion from the full instance, which is a privacy leak.

Mail app

Mail app version: 0.7.10

This issue is a regression from #171.

Screenshot from the sharing settings

@ChristophWurst ChristophWurst added this to SELECTED in Christoph's Tasks via automation Mar 26, 2018

@ChristophWurst ChristophWurst self-assigned this Mar 26, 2018

@ChristophWurst

This comment has been minimized.

Copy link
Member

ChristophWurst commented Mar 27, 2018

You see auto-completion from the full instance, which is a privacy leak.

Tried to reproduce on my dev instance and I can confirm that other system users are suggested for the autocompletion. Is that what you observed?

I also added a contact to another user's address book via the contacts app but that one was not listed.

@ChristophWurst

This comment has been minimized.

Copy link
Member

ChristophWurst commented Mar 27, 2018

Looks like the ContactsManager provides these entries from the system address book: https://github.com/nextcloud/server/blob/7b4e51dbc71c1112663b65c08b14eed44dcb50a3/lib/private/ContactsManager.php#L41-L54

It's the DAV app that registers both the user's address book and the system address book: https://github.com/nextcloud/server/blob/7b4e51dbc71c1112663b65c08b14eed44dcb50a3/apps/dav/lib/CardDAV/ContactsManager.php#L55-L59

@nextcloud/sharing @schiessle @rullzer should we remove the registration of the system address book if "Allow username autocompletion …" in the admin sharing settings is disabled?

@ChristophWurst ChristophWurst added this to the 0.7.11 milestone Mar 27, 2018

@rullzer

This comment has been minimized.

Copy link
Member

rullzer commented Mar 27, 2018

Yes seems like it

@journey123

This comment has been minimized.

Copy link

journey123 commented Mar 27, 2018

Can you actually link into anyone's phone?

@ChristophWurst

This comment has been minimized.

Copy link
Member

ChristophWurst commented Mar 28, 2018

Yes seems like it

It kinda feels wrong to hack this into the DAV app. This doesn't seem like the appropriate place. Also, I have the feeling change will cause other problems as well as these address books are not only used for Mail but also for example in the contacts menu … 🤔

ChristophWurst added a commit that referenced this issue Mar 28, 2018

Fix contacts integration with disabled sharing enumeration option
Contacts integration must not list system users when the sharing option
is set to no enumerate system users.

Fixes #848

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

@ChristophWurst ChristophWurst moved this from SELECTED to TO REVIEW (max 4 PRs) in Christoph's Tasks Mar 28, 2018

@ChristophWurst

This comment has been minimized.

Copy link
Member

ChristophWurst commented Mar 28, 2018

It kinda feels wrong to hack this into the DAV app. This doesn't seem like the appropriate place. Also, I have the feeling change will cause other problems as well as these address books are not only used for Mail but also for example in the contacts menu … thinking

Fixed (aka hacked) it in the mail app instead. See #856 😉

@rullzer

This comment has been minimized.

Copy link
Member

rullzer commented Mar 28, 2018

Still. mind opening a issue in the server repo? because if the address book is registered in theory anybody could access it all via dav (that is on the server).

@ChristophWurst ChristophWurst moved this from TO REVIEW (max 4 PRs) to SELECTED in Christoph's Tasks Mar 31, 2018

@ChristophWurst ChristophWurst moved this from SELECTED to TO REVIEW (max 4 PRs) in Christoph's Tasks Apr 3, 2018

Christoph's Tasks automation moved this from TO REVIEW (max 4 PRs) to DONE Apr 5, 2018

@pierreozoux

This comment has been minimized.

Copy link
Member Author

pierreozoux commented May 9, 2018

@ChristophWurst sorry to disappoint you, just upgraded to 0.8, did the migration, but I still can reproduce the issue.
These are my sharing settings:
image

I checked, I'm using 0.8 and Nc 13.0.2 should we reopen, or create a new issue?

@ChristophWurst

This comment has been minimized.

Copy link
Member

ChristophWurst commented May 9, 2018

@pierreozoux that shoudn't have happened but it's totally possible that there is another bug.

@ChristophWurst ChristophWurst reopened this May 9, 2018

Christoph's Tasks automation moved this from DONE to IN PROGRESS (max 3 PRs) May 9, 2018

Christoph's Tasks automation moved this from IN PROGRESS (max 3 PRs) to DONE May 9, 2018

@ChristophWurst

This comment has been minimized.

Copy link
Member

ChristophWurst commented May 9, 2018

I checked, I'm using 0.8 and Nc 13.0.2 should we reopen, or create a new issue?

Please open a new ticket. Thanks a lot for your feedback!

@lock

This comment has been minimized.

Copy link

lock bot commented Nov 20, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and questions.

@lock lock bot locked and limited conversation to collaborators Nov 20, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.