Skip to content

Conversation

@danxuliu
Copy link
Member

Backport of #21143

@danxuliu danxuliu force-pushed the backport/21143/stable17-fix-password-changes-in-link-and-mail-shares branch from e24970f to c10af91 Compare May 29, 2020 17:20
danxuliu added 8 commits June 1, 2020 10:43
When a mail share was created with a password the given password was
ignored. Now the given password is properly set and, if passwords are
enforced, it is not overriden with the autogenerated password.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
The step names were adjusted accordingly.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
@danxuliu danxuliu force-pushed the backport/21143/stable17-fix-password-changes-in-link-and-mail-shares branch from c10af91 to 5f53688 Compare June 1, 2020 08:54
@danxuliu danxuliu added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 1, 2020
@danxuliu
Copy link
Member Author

danxuliu commented Jun 1, 2020

Test failures are unrelated.

However, in a previous Drone run something strange happened in the sharing-v1 integration test. Creating a new mail share failed because the mailer tried to connect to port 25 instead of to port 2525 as configured. I guess that the OCC call failed, but I have no idea why; also the rest of integration tests worked fine, locally it works and running again in Drone worked too 🤷

Before it was possible to disable "send password by Talk" without
setting a new password, so the UI allowed that. Now that the server
prevents it the UI had to be adjusted to not update the share until a
new password is set when "send password by Talk" is disabled.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu
Copy link
Member Author

danxuliu commented Jun 1, 2020

As now it is no longer allowed to disable password protect by Talk without setting a new password the UI had to be adjusted. The change is specific to this backport, as #20938 will be backported only to 19 and 18.

@rullzer rullzer added this to the Nextcloud 17.0.8 milestone Jun 3, 2020
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

🐘

Those tests 🤯

$this->fd = $fd;
}

public function receive() {
Copy link
Member

Choose a reason for hiding this comment

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

👀 what am I reading here 😮

@nickvergessen nickvergessen merged commit 0c408a8 into stable17 Jun 8, 2020
@nickvergessen nickvergessen deleted the backport/21143/stable17-fix-password-changes-in-link-and-mail-shares branch June 8, 2020 17:23
@rullzer rullzer mentioned this pull request Jul 6, 2020
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants