Fix the Talk verification #20938
Fix the Talk verification #20938
Conversation
ChristophWurst
requested review from
danxuliu,
juliusknorr,
nickvergessen and
rullzer
|
/backport to stable19 |
|
/backport to stable18 |
danxuliu
left a comment
danxuliu
left a comment
There was a problem hiding this comment.
Preventing to enable Video verification if no new password is set should be applied only to mail shares. In public link shares it should be possible to enable and disable the video verification without changing the password.
Similarly, in mail shares it should be possible to disable Video verification without having to change the password. Only enabling Video verification should be prevented if a new password is not set.
ChristophWurst
force-pushed
the
fix/share-talk-verification
branch
from
8586545 to
32135f8
Compare
|
@danxuliu re-review, so we can backport it for 19.0.1 at least? |
When enabling or disabling Talk verification in mail shares the server expects also a new password to be set. As we always just update one property at a time this means the Talk verification was impossible to activate or deactivate. With this patch, we send the talk option AND the new password. If there is no new password, the Talk option is disabled (in mail shares; in link shares it is possible to enable or disable the video verification without changing the password). When we finally have descriptive text on ActionCheckbox'es we should definitely add some explanatory text for the user. Right now this is as good as it gets. We'll have to backport to 18. Signed-off-by: Christoph Wurst <[email protected]>
When video verification can not be enabled or disabled the previous state is set again in the JavaScript share object. This ensures that the UI will not reflect a misleading state. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
force-pushed
the
fix/share-talk-verification
branch
from
32135f8 to
fdac2ff
Compare
|
Although the password was sent along with sendPasswordByTalk it was always the password already set, so enabling video verification in mail shares still failed (well, it does after rebasing on #21143; before it did not, but if was a bug in the server, it should have failed :-P ). To fix this now the new password is sent (provided a new one has been set; otherwise the existing one is still sent, although this only happens for link shares). Also, in #21143 the backend was changed to require a new password also when video verification was disabled (because otherwise only the hashed version of the existing password was available, which made no sense when sent in the e-mail). This pull request was also updated to require a new password also when disabling video verification. |
Requires #21143
When enabling Talk verification the server expects also a new password
to be set. As we always just update one property at a time this means
the Talk verification was impossible to activate. With this patch, we
send the talk option AND the new password. If there is no new password,
the Talk option is disabled. When we finally have descriptive text on
ActionCheckbox'es we should definitely add some explanatory text for the
user. Right now this is as good as it gets. We'll have to backport to
18.