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

Fix federated scope not shown when public addressbook upload is disabled #26725

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Apr 23, 2021

Follow up to #26243

Besides a small change in a test, when the public address book is disabled now it is possible to select the Federated scope for the profile attributes, as the address books can still be exchanged between trusted servers. Besides that the Federated option is now hidden when the Federation app is disabled

Please note that this might need to be adjusted in the mobile apps as well. Should the property in the capabilities be renamed too? The capability AccountPropertyScopesFederationEnabled was split in AccountPropertyScopesFederatedEnabled and AccountPropertyScopesPublishedEnabled.

How to test (scenario 1)

  • Log in as an admin
  • Open the settings
  • Open the Sharing section under Administration
  • Disable Allow users to publish their data to a global and public address book
  • Open the Personal info section
  • Show the visibility menu on any of the elements, like Phone number

Result with this pull request

The Federated option is still available, although the Published option is not

Result without this pull request

Neither the Federated nor the Published options are available

How to test (scenario 2)

  • Log in as an admin
  • Disable the Federation app
  • Open the settings
  • Open the Personal info section
  • Show the visibility menu on any of the elements, like Phone number

Result with this pull request

The Federated option is no longer available

Result without this pull request

The Federated option is still available

Additional context

It can be checked that even if uploads to the lookup server are disabled the address book is still exchanged between trusted servers in the following way:

  • Start server A
  • Create user A in server A
  • Start server B
  • Create user B in server B
  • In server A, open the (Administration) Sharing section
  • Disable Allow users to publish their data to a global and public address book
  • In server B, open the (Administration) Sharing section
  • Disable Allow users to publish their data to a global and public address book
  • In server A, add server B as a trusted server
  • In server B, add server A as a trusted server

Now both servers need to trust each other. This is done using background jobs, so either force somehow the jobs to run or wait patiently until they run :-) (or, if you are using the AJAX method, entertain yourself reloading pages on each server around 30 times, although you need to do it alternatively as the jobs need to run in both servers :party:). There are probably much better ways to do it, but in order to know when the servers have trusted each other you can print a log message in TrustedServers::addSharedSecret

Once the servers trust each other, synchronize their address books running occ federation:sync-addressbooks in both servers. If the sharing settings are then opened again the other server should be shown as trusted and synchronized.

Now, if you open the Contacts menu in any of the servers you will see the users from the other server.

@danxuliu
Copy link
Member Author

/backport to stable21

@nickvergessen
Copy link
Member

Please note that this might need to be adjusted in the mobile apps as well. Should the property in the capabilities be renamed too?

Not renamed, but we actually need 2 capabilities?

@danxuliu
Copy link
Member Author

Not renamed, but we actually need 2 capabilities?

I have checked and if the federation app is disabled in server B it is no longer possible to synchronize its users from server A. So I guess that it would make sense to hide the Federated scope if the federation app is disabled.

However, if I am not mistaken in the WebUI the Federated scope was never hidden before, so I do not know if there is a special reason for that or if it was just an oversight 🤷

@nickvergessen
Copy link
Member

However, if I am not mistaken in the WebUI the Federated scope was never hidden before, so I do not know if there is a special reason for that or if it was just an oversight shrug

Before @PVince81 recent work all scope settings were always hidden when the settings was off. Now it only hides the federation+published one as the others still make sense

This was referenced May 20, 2021
@blizzz blizzz mentioned this pull request Jun 2, 2021
57 tasks
@MorrisJobke MorrisJobke mentioned this pull request Jun 10, 2021
59 tasks
@blizzz blizzz mentioned this pull request Jun 16, 2021
45 tasks
@blizzz
Copy link
Member

blizzz commented Jun 16, 2021

So what's to do with this?

@blizzz blizzz mentioned this pull request Jun 23, 2021
39 tasks
@danxuliu danxuliu force-pushed the fix-federated-scope-not-shown-when-public-addressbook-upload-is-disabled branch 2 times, most recently from df9c510 to 7bb2507 Compare June 24, 2021 01:13
@danxuliu
Copy link
Member Author

Besides the initial fix the Federated option is now hidden when the Federation app is disabled, and the capability was split in two different capabilities.

@blizzz blizzz modified the milestones: Nextcloud 22, Nextcloud 23 Jun 24, 2021
@MorrisJobke MorrisJobke removed their request for review July 4, 2021 11:32
@skjnldsv
Copy link
Member

@danxuliu rebase? close?

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 18, 2021
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the lookup server is disabled the address books can still be
exchanged between trusted servers. Therefore the user should be able to
set the "Federated" scope in that case.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the fix-federated-scope-not-shown-when-public-addressbook-upload-is-disabled branch from 7bb2507 to b006c9c Compare August 18, 2021 17:27
If the Federation app is disabled it is not possible to synchronize the
users from a different server.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The "federated" and "published" scopes are independent one from each
other, so the capability that encompassed both needs to be split.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the fix-federated-scope-not-shown-when-public-addressbook-upload-is-disabled branch from b006c9c to 193cf6c Compare August 18, 2021 17:56
@danxuliu danxuliu added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 18, 2021
@danxuliu
Copy link
Member Author

Rebased and ready to review again.

@skjnldsv skjnldsv requested review from Pytal and kesselb and removed request for rullzer August 19, 2021 08:29
Copy link
Member

@Pytal Pytal left a comment

Choose a reason for hiding this comment

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

👍 will carry this over to Vue for 23 :)

Vuetification of the Personal info settings page has been postponed, should get this in :)

@skjnldsv skjnldsv mentioned this pull request Oct 13, 2021
@skjnldsv skjnldsv merged commit c98039c into master Oct 22, 2021
@skjnldsv skjnldsv deleted the fix-federated-scope-not-shown-when-public-addressbook-upload-is-disabled branch October 22, 2021 09:52
@nickvergessen
Copy link
Member

/backport to stable22

@nickvergessen
Copy link
Member

I guess we backport to 22 before to 21

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