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

NC12 Beta1 displays ALL Users of the NC instance #4656

Closed
PietsHost opened this issue May 2, 2017 · 21 comments
Closed

NC12 Beta1 displays ALL Users of the NC instance #4656

PietsHost opened this issue May 2, 2017 · 21 comments
Labels
3. to review Waiting for reviews security

Comments

@PietsHost
Copy link

PietsHost commented May 2, 2017

Steps to reproduce

  1. Install NC12 Beta 1
  2. Create Users test1, test2, test3
  3. Click on the user-icon in the upper right corner (find contacts)

Expected behaviour

It should only display contacts, set up via the contacts-app

Actual behaviour

It shows the Full Name of 20 users within the actual installation! Thats a big security issue as everyone can see all other usernames active in that actual installation!

The following image is showing the users "admin", "test1" and "test3". I'm currently logged in as "test2". All users are in different groups and should be handled like different customers.
Image of Yaktocat

Server configuration

Operating system:
CentOS Linux release 7.3.1611 (Core)

Web server:
Apache/2.4.6 (CentOS)

Database:
5.5.52-MariaDB

PHP version:
PHP 5.6.30 (cli)

Nextcloud version: (see Nextcloud admin page)
Nextcloud 12.0 beta 1

Updated from an older Nextcloud/ownCloud or fresh install:
Fresh install

Where did you install Nextcloud from:
Nextcloud Homepage

@juliushaertl
Copy link
Member

@LukasReschke

@juliushaertl
Copy link
Member

and @ChristophWurst as well 😉

@ChristophWurst
Copy link
Member

ChristophWurst commented May 2, 2017

This is the intended behavior, IMO.

Note that apps that use the contacts manager API, like the Mail app for example, gets the same data when it queries contact information.
Also note that it will show a maximum of 20 contacts. Hence you don't see all other users.

@ChristophWurst ChristophWurst added 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed high labels May 2, 2017
@PietsHost
Copy link
Author

o.O intended behavior?

Well, I can't deploy NC 12 for my customers if they have the ability to see 20 other customers and their usernames from a simple drop-down menu...

@PietsHost
Copy link
Author

PietsHost commented May 2, 2017

I just noticed it displays the Full Name instead of the username.. That's a way more critical.. I'm just thinking about the privacy policy..

No user should see other user's Full Name without their approval.. that's like making my customers list publicly accessible

@jancborchardt
Copy link
Member

cc @LukasReschke @karlitschek what do you think? Seems we need an admin setting for this, just like the share API?

»Show internal users in contacts menu« – enabled by default

Or what do you think?

@PietsHost
Copy link
Author

cc @LukasReschke @karlitschek what do you think? Seems we need an admin setting for this, just like the share API?
»Show internal users in contacts menu« – enabled by default

SGTM. For now my workaround is to edit the core\templates\layout.user.php and comment out the <div id="contactsmenu"> section. I really hope, I don't have to do that in the final version ;-)

@juliushaertl
Copy link
Member

what do you think? Seems we need an admin setting for this, just like the share API?

Why doesn't it just use the sharing setting we have that can limits sharing to users within the own groups. That was what I would expect to apply there too.

@jancborchardt
Copy link
Member

@juliushaertl makes sense. @jospoortvliet @karlitschek @LukasReschke any further input?

@jospoortvliet
Copy link
Member

I think it makes sense. When you don't share you don't collaborate so not being able to communicate (unless you have that person in your own address book of course) makes sense. If there's a use case where it needs to be separated out later on, we can do that... But this is probably good enough for 99%.

@karlitschek
Copy link
Member

I think reusing the sharing setting makes sense. No need for another switch

@MariusBluem
Copy link
Member

MariusBluem commented May 5, 2017

If username-autocompletion is disabled, the contacts-menu should never ever show local users:
bildschirmfoto 2017-05-05 um 10 04 08

Groups, which are excluded from sharing should not see local users at all:
bildschirmfoto 2017-05-05 um 10 04 16

If sharing is restricted to users own groups, he should only see contacts from his groups:
bildschirmfoto 2017-05-05 um 10 04 38

We may also want to overthink the federation lookup-settings, since they could be also used for contacts-menu on user-side: 🤔 @schiessle
bildschirmfoto 2017-05-05 um 10 08 29

@MariusBluem
Copy link
Member

Just remarked, that setting your full name to "Local" makes you invisible from the contacts menu 🤔 Intended? @ChristophWurst

@joergmschulz
Copy link

So, the only way to separate personal data from different tenants would be to build separate NC instances, true?

I'd prefer groups to be the differentiator: People who don't share any access control group shouldn't be visible to each other. Remains the question: how does NC know a group is an access control group?
For LDAP, this could be a property like BusinessCategory.

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels May 9, 2017
@ChristophWurst
Copy link
Member

Fix is in #4757.

@MorrisJobke MorrisJobke added this to the Nextcloud 12.0 milestone May 9, 2017
@MariusBluem
Copy link
Member

MariusBluem commented May 24, 2017

Please visit my comment again: #4656 (comment) - I think this issue is only fixed in parts, since we still gut reports like:

https://help.nextcloud.com/t/hide-users-from-contacts-menu/12956/
#5096 (showing contacts menu with LDAP backend)

@ChristophWurst
Copy link
Member

Please file a new issue for this enhancement request.

@ajdeswart
Copy link

ajdeswart commented Nov 16, 2017

This hasn't been fixed. I don't understand how this isn't a priority security issue, especially if an entity like me has multiple platforms centralizing together. Where is the new enhancement request or do we really need to re-create the same issue?

Referring to issue from OP:

All users are in different groups and should be handled like different customers.

@karlitschek answer was the best. Give us that option with permissions:

I think reusing the sharing setting makes sense. No need for another switch

Also, give the contacts-app permissions to access the internal users and match the group permissions as well please (this might be a separate issue for contacts-app) <3

This seems to be the only solution until this is resolved (Not tested):
https://help.nextcloud.com/t/why-any-user-can-view-all-the-contacts-belonging-to-other-groups/22642/4

@trev-dev
Copy link

trev-dev commented Nov 8, 2019

I'm using NextCloud Pi and I can see right off the bat after installing XMPP that the 'ncp' user is exposed in the contacts list. It would be super cool if I could disable specific users from even showing up/being available for XMPP or limit the available contacts to a specific group.

@jospoortvliet
Copy link
Member

This is likely an issue in the XMPP app - which either doesn't respect the settings or handles this by itself, or would need that feature. It isn't really relevant here.

@ErikUden
Copy link

cc @LukasReschke @karlitschek what do you think? Seems we need an admin setting for this, just like the share API?
»Show internal users in contacts menu« – enabled by default

SGTM. For now my workaround is to edit the core\templates\layout.user.php and comment out the <div id="contactsmenu"> section. I really hope, I don't have to do that in the final version ;-)

Is there a better way to fix this nowadays?

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

No branches or pull requests