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 password changes in link and mail shares #21151
Conversation
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When a mail share was created with a password the given password was not hashed, so it was not possible to open the share with that password. Moreover, if passwords were enforced the given password was ignored and a new one was set (although in this case it was hashed so it worked as expected). Now the given password is properly hashed and not overriden. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When "send password by Talk" is enabled in a link share now a non empty password is enforced. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When "send password by Talk" is enabled in a mail share a new password must be also set. However, when the passwords of the original and the new share were compared it was not taken into account that the original password is now hashed, while the new one is not (unless no new password was sent, in which case the password of the original share was set in the new share by the controller, but that was already prevented due to both passwords being literally the same), so it was possible to set the same password again. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When "send password by Talk" was disabled in a mail share it was possible to keep the same password as before, as it does not pose any security issue (unlike keeping it when "send password by Talk" is enabled, as in that case the password was already disclosed by mail). However, if a mail share is updated but the password is not set again only the hashed password will be available. In that case it would not make sense to send the password by mail, so now the password must be changed when disabling "send password by Talk". Note that, even if explicitly setting the same password again along with the "send password by Talk" property would work, this was also prevented for simplicity. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The step names were adjusted accordingly. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Note that the "last link share can be downloaded" step was kept as it tests the "url" property specific of link shares. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
In most cases, when a mail share is created or updated an e-mail is sent to the sharee, which is done by connecting to the SMTP server set in the configuration. If the server can not be contacted then the creation or update of the mail share fails. To make possible to test mail shares without using a real SMTP server a fake one has been added. The original script, which is MIT licensed, was based on inetd, so it was slightly modified to run on its own. In order to use it from the integration tests the "Given dummy mail server is listening" step has to be called in the scenarios in which the mail server is needed. For now that is the only available step; things like checking the sent mails, while possible (as the script can log the mails to certain file), have not been added yet. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐘
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, do not merge!
06904dd
to
c9eac9a
Compare
Enabling the "send password by Talk" property of shares require that Talk is installed and enabled, so the Drone step that runs them has to first clone the Talk repository. When the integration tests are run on a local development instance, however, it is not guaranteed that Talk is installed. Due to this the "@Talk" tag was added, which ensures that any feature or scenario marked with it will first check if Talk is installed and, if not, skip the scenario (instead of failing). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
c9eac9a
to
fcafad2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The video verification integration tests failed because master branch of Talk was used instead of stable19. It should be fine now.
JSUNit failure due to coverage upload issue. |
backport of #21143