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

[stable19] Fix account data visibility after disabling public addressbook upload #25974

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Mar 6, 2021

Backport of #25417

This was backported from the original version of #25417; #25417 was rebased and adjusted as it conflicted with changes introduced in #26243 and #26548, but those pull requests were not backported to stable19.

Kept as draft until #25417 is merged (just in case something else that needs to be adjusted in stable19 is found too).

@danxuliu danxuliu added this to the Nextcloud 19.0.10 milestone Mar 6, 2021
@danxuliu danxuliu force-pushed the backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload branch from c45dd4c to bfbffed Compare March 6, 2021 08:22
@rullzer rullzer mentioned this pull request Mar 26, 2021
@danxuliu danxuliu force-pushed the backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload branch from bfbffed to 5691573 Compare April 14, 2021 16:06
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Right now it makes no difference, but this should make future tests
clearer, specially in case of failure.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"AccountManager::updateUser()" wipes previous user data with whichever
user data is given (except for some adjustments, like resetting the
verified status when needed). As the controller overrode the properties
those properties would lose some of their attributes even if they are
not affected by the changes made by the controller. Now the controller
only modifies the attributes set ("value" and "scope") to prevent that.

Note that with this change the controller no longer removes the
"verified" status, but this is not a problem because, as mentioned,
"AccountManager::updateUser()" resets them when needed (for example,
when the value of the website property changes).

This change is a previous step to fix overwritting properties with null
values, and it will prevent the controller from making unexpected
changes if more attributes are added in the future.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The controller can receive an optional subset of the properties of the
user settings; values not given are set to "null" by default. However,
those null values overwrote the previously existing values, so in
practice any value not given was deleted from the user settings. Now
only non null values overwrite the previous values.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"sendingAToWithRequesttoken" needs to be used to test some non OCS
endpoints which require the request token to be sent in the request. Now
it is possible to specify the body (or, rather, additional contents
beside the cookies and the request token) for those requests, as it will
be needed for example to update the user profile.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When upload to the lookup server is disabled the scope menu was hidden
in the personal information settings. However, even if the lookup server
upload is disabled the personal information is still accesible from the
local server as well as trusted servers. Users should be able to still
set if their information is available to other users or if it is
private, so now the scope menu is always show (although the "Public"
option is hidden if the lookup server upload is disabled).

If the user set the information as public before the upload to the
lookup server was disabled the menu will also show the "Public" option
as active, although disabled. Setting the visibility to any of the other
options will hide the "Public" option from the menu (until the lookup
server upload is enabled again).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Due to a bug (fixed some commits ago) in the UsersController of the
settings app the scope of the properties can be null (for example, if
lookup server upload was disabled and the user then changed the display
name in the profile information). In that case now the scope menu icon
shows an error to inform the user.

The scope value will not change when other properties are modified until
the user chooses an explicit value from the menu. Note that until a
scope is explicitly set the property will behave as if it is private.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload branch from 5691573 to 184a7ca Compare April 23, 2021 15:20
@danxuliu danxuliu marked this pull request as ready for review April 23, 2021 15:27
@danxuliu
Copy link
Member Author

Unit test failures are unrelated.

@rullzer rullzer mentioned this pull request Apr 29, 2021
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

🐘

@rullzer rullzer merged commit 1092ba8 into stable19 Apr 29, 2021
@rullzer rullzer deleted the backport/25417/stable19-fix-account-data-visibility-after-disabling-public-addressbook-upload branch April 29, 2021 18:50
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.

3 participants