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

fixed publishing of user properties to lookup-server #25300

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

abangtor
Copy link

@abangtor abangtor commented Jan 24, 2021

Fix #25290

Fix of the bug in the lookup_server_connector module to publish public user information to the lookup-server.

There is a bug in the getUserAccountData method in the RetryJob.php of the lookup-server-connector module.
The properties are written into an one dimensional array $publicData:

$publicData[$property->getName()] = $property->getValue();

Later on the array is read as it is a two dimensional array:
$data['name'] = $publicData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] ?? '';
$data['email'] = $publicData[IAccountManager::PROPERTY_EMAIL]['value'] ?? '';
$data['address'] = $publicData[IAccountManager::PROPERTY_ADDRESS]['value'] ?? '';
$data['website'] = $publicData[IAccountManager::PROPERTY_WEBSITE]['value'] ?? '';
$data['twitter'] = $publicData[IAccountManager::PROPERTY_TWITTER]['value'] ?? '';
$data['phone'] = $publicData[IAccountManager::PROPERTY_PHONE]['value'] ?? '';
$data['twitter_signature'] = $publicData[IAccountManager::PROPERTY_TWITTER]['signature'] ?? '';
$data['website_signature'] = $publicData[IAccountManager::PROPERTY_WEBSITE]['signature'] ?? '';
$data['verificationStatus'] = [
IAccountManager::PROPERTY_WEBSITE => $publicData[IAccountManager::PROPERTY_WEBSITE]['verified'] ?? '',
IAccountManager::PROPERTY_TWITTER => $publicData[IAccountManager::PROPERTY_TWITTER]['verified'] ?? '',
];

This causes the values to be empty and no properties are send to the loopup-server.

Fix of the bug in the lookup_server_connector module to publish public user information to the lookup-server.
As described in issue [nextcloud#25290](nextcloud#25290)

Signed-off-by: AbangTor <[email protected]>
Signed-off-by: AbangTor <[email protected]>
@kesselb kesselb added 3. to review Waiting for reviews bug labels Jan 26, 2021
Signed-off-by: AbangTor <[email protected]>
@kesselb
Copy link
Contributor

kesselb commented Jan 26, 2021

Thanks 👍 Please squash the commits together and we are good.

$publicData[$property->getName()] = $property->getValue();
$publicData[$property->getName()] = [
'value' => $property->getValue(),
'verified' => $property->getVerified()
Copy link
Member

Choose a reason for hiding this comment

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

The lookup server also seems to check a signature at least for Twitter and website?

Copy link
Contributor

Choose a reason for hiding this comment

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

Mind to add how to get the signature? We talked briefly about the signature at #25290 and were lost ;)

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

@renaudlemaire renaudlemaire left a comment

Choose a reason for hiding this comment

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

Seems good to me.

@renaudlemaire
Copy link

Still not fixed in 2023.

Copy link

@Trolli07 Trolli07 left a comment

Choose a reason for hiding this comment

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

Seems good to me too.

Copy link

@Fleoxi Fleoxi left a comment

Choose a reason for hiding this comment

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

Seems good to me too.

@szaimen szaimen requested review from ArtificialOwl, a team, icewind1991 and nfebe and removed request for a team May 24, 2023 13:26
@szaimen szaimen added this to the Nextcloud 28 milestone May 24, 2023
@kesselb kesselb removed their request for review May 24, 2023 14:28
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv
Copy link
Member

@ArtificialOwl you're our expert in lookup server, could you have a look when time allows? :)

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 10, 2024
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
@sorbaugh
Copy link
Contributor

sorbaugh commented Aug 16, 2024

Reopening to review. Anything here that still needs to be done @abangtor ? We would like to help you get this one in :)

@sorbaugh sorbaugh reopened this Aug 16, 2024
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.

Server is not publishing user properties to lookup-server