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

Enumeration of email adresses is possible #8726

Closed
tbsbdr opened this issue Mar 25, 2024 · 18 comments
Closed

Enumeration of email adresses is possible #8726

tbsbdr opened this issue Mar 25, 2024 · 18 comments
Assignees
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug

Comments

@tbsbdr
Copy link
Contributor

tbsbdr commented Mar 25, 2024

Describe the bug

A clear and concise description of what the bug is.

Steps to reproduce

  1. open share panel and type "alb"
  2. email adress of albert is exposed
  3. Enumeration of email adresses is possible

screenshot_002503

Expected behavior

email adresses of users can not be enumerated

  • remove email adress so that it complies with GDPR (remove from get user and search using graphapi)
  • show username (eg. alb.einstein) next to display name so that users can be identified uniquely

screenshot_002504

Actual behavior

  • Enumeration of email adresses is possible

relates to
https://github.com/owncloud/enterprise/issues/6516

Implementation detail

  • fix for OCS and graph
@tbsbdr tbsbdr added Type:Bug Priority:p2-high Escalation, on top of current planning, release blocker labels Mar 25, 2024
@kulmann
Copy link
Member

kulmann commented Mar 25, 2024

Needs backend enhancement. Ocis should send a configurable attribute in the "additionalAttributes" sharee field. Actually no code change in web needed.

@JammingBen
Copy link
Contributor

Needs backend enhancement. Ocis should send a configurable attribute in the "additionalAttributes" sharee field. Actually no code change in web needed.

This is true for the OCS API, with sharing NG we need to adjust some things though. There is no such thing as additionalAttributes anymore, we go for a user's email directly instead (which we need to change now).

@tbsbdr Any objections moving this issue to the oCIS repo? This is just a tiny change for Web, but a bit more work on the server side. IMO it fits there.

@kulmann
Copy link
Member

kulmann commented Mar 25, 2024

Ah cool, thanks for clarifying!

@tbsbdr tbsbdr transferred this issue from owncloud/web Mar 25, 2024
@micbar
Copy link
Contributor

micbar commented Mar 25, 2024

Needs fix on ocs and backport to 5.0

@dragonchaser dragonchaser self-assigned this Mar 26, 2024
@dragonchaser
Copy link
Member

dragonchaser commented Mar 26, 2024

@tbsbdr should this be a configuration switch to choose whether the mail should be shown/searchable or not? I could imagine that being able to search people by e-mail could be desirable in some enterprise environments.

@tbsbdr
Copy link
Contributor Author

tbsbdr commented Mar 26, 2024

Yes, agreed. Making ist configurable would ne Cherry in top. Default should be the "Safe path" - No email

@dragonchaser
Copy link
Member

fixed in master, backport incoming

@dragonchaser
Copy link
Member

@JammingBen is it working for you to use the additionalAttributes again?

@JammingBen
Copy link
Contributor

@JammingBen is it working for you to use the additionalAttributes again?

I don't quite understand, what do you mean by "use it again"? 😅

In the OCS API, the shareWithAdditionalInfo prop just needs to replace the email with the username for Web to display it correctly. In graph I think you just mask the mail property in the response? Web then goes for the onPremisesSamAccountName instead I suppose.

@dragonchaser
Copy link
Member

@JammingBen from your comment above I assumed you have replaced using the shareWithAdditionalInfo by an explicit call that retrieves the user emailadresse.

@JammingBen
Copy link
Contributor

Ah no, but the endpoint for retrieving users to share a file with has been changed with sharing NG. Web is using graph/v1.0/users for that. And since it doesn't return shareWithAdditionalInfo, Web currently uses the mail prop (onPremisesSamAccountName in the future then).

@dragonchaser
Copy link
Member

Ok then this needs to be adapted aswell. Thx for clarifying, will look into that on tuesday.

@dragonchaser
Copy link
Member

@JammingBen should we do seperate backports to 5.0 or do a all-in-one including the changes to web?

@JammingBen
Copy link
Contributor

Web actually doesn't need a backport since the old version only relies on the shareWithAdditionalInfo property. Meaning if this now contains the username instead of the email, Web is totally fine with it.

@dragonchaser
Copy link
Member

dragonchaser commented Apr 4, 2024

Merged in Master, backport in progress, currently blocked until cs3org/reva#4609 is merged

@dragonchaser
Copy link
Member

All backports done, just needs a new ocis release for the backports, can we close this?

@micbar
Copy link
Contributor

micbar commented Apr 12, 2024

Released with ocis 5.0.1.

@micbar micbar closed this as completed Apr 12, 2024
@ScharfViktor
Copy link
Contributor

tested on stable-5.0 branch

start ocis with OCIS_SHOW_USER_EMAIL_IN_RESULTS=false:

  • no email in the sharees response
    GET https://localhost:9200/ocs/v2.php/apps/files_sharing/api/v1/sharees?search=mar
  • but recipient email is exist in the shares response
    POST https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares
Screenshot 2024-04-15 at 09 43 03 - recipient email is exist in the Access details: ❌ Screenshot 2024-04-15 at 09 45 07 Screenshot 2024-04-15 at 09 44 24 - recipient's email is available using the graph endpoints ❌ Screenshot 2024-04-15 at 09 49 01 Screenshot 2024-04-15 at 09 52 01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Projects
Archived in project
Development

No branches or pull requests

6 participants