Skip to content

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Jun 15, 2017

Backport of #5384 to stable12

Works also with LDAP users btw. Just cherry-picked, did not need to touch code β†’ πŸ‘ from me

Allow to find local users by their email address

Signed-off-by: Joas Schilling <coding@schilljs.com>

Make sure to only add system users once

Signed-off-by: Joas Schilling <coding@schilljs.com>

Add unit test

Signed-off-by: Joas Schilling <coding@schilljs.com>
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works πŸ‘

@MorrisJobke MorrisJobke merged commit 3040eae into stable12 Jun 15, 2017
@MorrisJobke MorrisJobke deleted the 5384-stable12 branch June 15, 2017 17:53

if (isset($contact['isLocalSystemBook'])) {
if ($exactEmailMatch) {
$cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0]);

Choose a reason for hiding this comment

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

This addition led to a crash on my NC instance: users cannot share anymore with users with an email address set in NC.

For user "pab", with an email address, $contact['CLOUD'][0] resolves to "pab@" and resolveCloudId fails saying: Invalid cloud id.

See here for full error stack: https://help.nextcloud.com/t/cant-share-to-a-local-user/12339/4?u=pab

Choose a reason for hiding this comment

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

Can someone tell me where the ['CLOUD'][0] value is supposed to be found? Is there a doc of the contactsManager to see to which fields of the database CLOUD shall be linked to?

Copy link
Member

Choose a reason for hiding this comment

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

cc @nickvergessen this is where this weird share name could come from

Copy link
Member

Choose a reason for hiding this comment

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

Fix is in #6099

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants