Merge pull request #4462 from danxuliu/fix-sharing-password-protected-link

Fix sharing a password protected link

.drone.yml

@@ -485,6 +485,13 @@ pipeline:
       when:
         matrix:
           TESTS-ACCEPTANCE: access-levels
+  acceptance-app-files:
+      image: nextcloudci/php7.0:php7.0-7
+      commands:
+        - tests/acceptance/run-local.sh allow-git-repository-modifications features/app-files.feature
+      when:
+        matrix:
+          TESTS-ACCEPTANCE: app-files
   acceptance-login:
       image: nextcloudci/php7.0:php7.0-7
       commands:
@@ -567,6 +574,8 @@ matrix:
     - TESTS: integration-trashbin
     - TESTS: acceptance
       TESTS-ACCEPTANCE: access-levels
+    - TESTS: acceptance
+      TESTS-ACCEPTANCE: app-files
     - TESTS: acceptance
       TESTS-ACCEPTANCE: login
     - TESTS: jsunit

core/js/sharedialoglinkshareview.js

@@ -307,10 +307,12 @@
 			this.model.saveLinkShare({
 				password: password
 			}, {
+				complete: function(model) {
+					$loading.removeClass('inlineblock').addClass('hidden');
+				},
 				error: function(model, msg) {
 					// destroy old tooltips
 					$input.tooltip('destroy');
-					$loading.removeClass('inlineblock').addClass('hidden');
 					$input.addClass('error');
 					$input.attr('title', msg);
 					$input.tooltip({placement: 'bottom', trigger: 'manual'});

core/js/shareitemmodel.js

@@ -104,7 +104,14 @@
 		/**
 		 * Saves the current link share information.
 		 *
-		 * This will trigger an ajax call and refetch the model afterwards.
+		 * This will trigger an ajax call and, if successful, refetch the model
+		 * afterwards. Callbacks "success", "error" and "complete" can be given
+		 * in the options object; "success" is called after a successful save
+		 * once the model is refetch, "error" is called after a failed save, and
+		 * "complete" is called both after a successful save and after a failed
+		 * save. Note that "complete" is called before "success" and "error" are
+		 * called (unlike in jQuery, in which it is called after them); this
+		 * ensures that "complete" is called even if refetching the model fails.
 		 *
 		 * TODO: this should be a separate model
 		 */
@@ -149,7 +156,6 @@
 
 		addShare: function(attributes, options) {
 			var shareType = attributes.shareType;
-			options = options || {};
 			attributes = _.extend({}, attributes);
 
 			// Default permissions are Edit (CRUD) and Share
@@ -173,53 +179,43 @@
 				attributes.path = this.fileInfoModel.getFullPath();
 			}
 
-			var self = this;
-			return $.ajax({
+			return this._addOrUpdateShare({
 				type: 'POST',
 				url: this._getUrl('shares'),
 				data: attributes,
 				dataType: 'json'
-			}).done(function() {
-				self.fetch().done(function() {
-					if (_.isFunction(options.success)) {
-						options.success(self);
-					}
-				});
-			}).fail(function(xhr) {
-				var msg = t('core', 'Error');
-				var result = xhr.responseJSON;
-				if (result && result.ocs && result.ocs.meta) {
-					msg = result.ocs.meta.message;
-				}
-
-				if (_.isFunction(options.error)) {
-					options.error(self, msg);
-				} else {
-					OC.dialogs.alert(msg, t('core', 'Error while sharing'));
-				}
-			});
+			}, options);
 		},
 
 		updateShare: function(shareId, attrs, options) {
-			var self = this;
-			options = options || {};
-			return $.ajax({
+			return this._addOrUpdateShare({
 				type: 'PUT',
 				url: this._getUrl('shares/' + encodeURIComponent(shareId)),
 				data: attrs,
 				dataType: 'json'
+			}, options);
+		},
+
+		_addOrUpdateShare: function(ajaxSettings, options) {
+			var self = this;
+			options = options || {};
+
+			return $.ajax(
+				ajaxSettings
+			).always(function() {
+				if (_.isFunction(options.complete)) {
+					options.complete(self);
+				}
 			}).done(function() {
-				self.fetch({
-					success: function() {
-						if (_.isFunction(options.success)) {
-							options.success(self);
-						}
+				self.fetch().done(function() {
+					if (_.isFunction(options.success)) {
+						options.success(self);
 					}
 				});
 			}).fail(function(xhr) {
 				var msg = t('core', 'Error');
 				var result = xhr.responseJSON;
-				if (result.ocs && result.ocs.meta) {
+				if (result && result.ocs && result.ocs.meta) {
 					msg = result.ocs.meta.message;
 				}
 

core/js/tests/specs/sharedialoglinkshareview.js

@@ -0,0 +1,143 @@
+/**
+ *
+ * @copyright Copyright (c) 2015, Tom Needham (tom@owncloud.com)
+ * @copyright Copyright (c) 2017, Daniel Calviño Sánchez (danxuliu@gmail.com)
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+describe('OC.Share.ShareDialogLinkShareView', function () {
+
+	var configModel;
+	var shareModel;
+	var view;
+
+	beforeEach(function () {
+
+		var fileInfoModel = new OCA.Files.FileInfoModel({
+			id: 123,
+			name: 'shared_file_name.txt',
+			path: '/subdir',
+			size: 100,
+			mimetype: 'text/plain',
+			permissions: OC.PERMISSION_ALL,
+			sharePermissions: OC.PERMISSION_ALL
+		});
+
+		var attributes = {
+			itemType: fileInfoModel.isDirectory() ? 'folder' : 'file',
+			itemSource: fileInfoModel.get('id'),
+			possiblePermissions: OC.PERMISSION_ALL,
+			permissions: OC.PERMISSION_ALL
+		};
+
+		configModel = new OC.Share.ShareConfigModel({
+			enforcePasswordForPublicLink: false,
+			isResharingAllowed: true,
+			enforcePasswordForPublicLink: false,
+			isDefaultExpireDateEnabled: false,
+			isDefaultExpireDateEnforced: false,
+			defaultExpireDate: 7
+		});
+
+		sinon.stub(configModel, 'isShareWithLinkAllowed');
+
+		shareModel = new OC.Share.ShareItemModel(attributes, {
+			configModel: configModel,
+			fileInfoModel: fileInfoModel
+		});
+
+		view = new OC.Share.ShareDialogLinkShareView({
+			configModel: configModel,
+			model: shareModel
+		});
+
+	});
+
+	afterEach(function () {
+		view.remove();
+		configModel.isShareWithLinkAllowed.restore();
+	});
+
+	describe('onPasswordEntered', function () {
+
+		var $passwordText;
+		var $workingIcon;
+
+		beforeEach(function () {
+
+			// Needed to render the view
+			configModel.isShareWithLinkAllowed.returns(true);
+
+			// Setting the share also triggers the rendering
+			shareModel.set({
+				linkShare: {
+					isLinkShare: true,
+					password: 'password'
+				}
+			});
+
+			var $passwordDiv = view.$el.find('#linkPass');
+			$passwordText = view.$el.find('.linkPassText');
+			$workingIcon = view.$el.find('.linkPass .icon-loading-small');
+
+			sinon.stub(shareModel, 'saveLinkShare');
+
+			expect($passwordDiv.hasClass('hidden')).toBeFalsy();
+			expect($passwordText.hasClass('hidden')).toBeFalsy();
+			expect($workingIcon.hasClass('hidden')).toBeTruthy();
+
+			$passwordText.val('myPassword');
+		});
+
+		afterEach(function () {
+			shareModel.saveLinkShare.restore();
+		});
+
+		it('shows the working icon when called', function () {
+			view.onPasswordEntered();
+
+			expect($workingIcon.hasClass('hidden')).toBeFalsy();
+			expect(shareModel.saveLinkShare.withArgs({ password: 'myPassword' }).calledOnce).toBeTruthy();
+		});
+
+		it('hides the working icon when saving the password succeeds', function () {
+			view.onPasswordEntered();
+
+			expect($workingIcon.hasClass('hidden')).toBeFalsy();
+			expect(shareModel.saveLinkShare.withArgs({ password: 'myPassword' }).calledOnce).toBeTruthy();
+
+			shareModel.saveLinkShare.yieldTo("complete", [shareModel]);
+
+			expect($workingIcon.hasClass('hidden')).toBeTruthy();
+		});
+
+		it('hides the working icon when saving the password fails', function () {
+			view.onPasswordEntered();
+
+			expect($workingIcon.hasClass('hidden')).toBeFalsy();
+			expect(shareModel.saveLinkShare.withArgs({ password: 'myPassword' }).calledOnce).toBeTruthy();
+
+			shareModel.saveLinkShare.yieldTo("complete", [shareModel]);
+			shareModel.saveLinkShare.yieldTo("error", [shareModel, "The error message"]);
+
+			expect($workingIcon.hasClass('hidden')).toBeTruthy();
+		});
+
+	});
+
+});

core/js/tests/specs/shareitemmodelSpec.js

@@ -670,6 +670,83 @@ describe('OC.Share.ShareItemModel', function() {
 				shareWith: 'group1'
 			});
 		});
+		it('calls complete handler before refreshing the model', function() {
+			var completeStub = sinon.stub();
+			model.addShare({
+				shareType: OC.Share.SHARE_TYPE_GROUP,
+				shareWith: 'group1'
+			}, {
+				complete: completeStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(completeStub.calledOnce).toEqual(true);
+		});
+		it('calls success handler after refreshing the model', function() {
+			var successStub = sinon.stub();
+			model.addShare({
+				shareType: OC.Share.SHARE_TYPE_GROUP,
+				shareWith: 'group1'
+			}, {
+				success: successStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(successStub.called).toEqual(false);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(successStub.calledOnce).toEqual(true);
+			expect(successStub.lastCall.args[0]).toEqual(model);
+		});
+		it('calls complete handler before error handler', function() {
+			var completeStub = sinon.stub();
+			var errorStub = sinon.stub();
+			model.addShare({
+				shareType: OC.Share.SHARE_TYPE_GROUP,
+				shareWith: 'group1'
+			}, {
+				complete: completeStub,
+				error: errorStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				400,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({
+					ocs: {
+						meta: {
+							message: 'Some error message'
+						}
+					}
+				})
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+			expect(errorStub.calledOnce).toEqual(true);
+			expect(completeStub.calledBefore(errorStub)).toEqual(true);
+		});
 		it('calls error handler with error message', function() {
 			var errorStub = sinon.stub();
 			model.addShare({
@@ -693,6 +770,7 @@ describe('OC.Share.ShareItemModel', function() {
 			);
 
 			expect(errorStub.calledOnce).toEqual(true);
+			expect(errorStub.lastCall.args[0]).toEqual(model);
 			expect(errorStub.lastCall.args[1]).toEqual('Some error message');
 		});
 	});
@@ -712,6 +790,80 @@ describe('OC.Share.ShareItemModel', function() {
 				permissions: '' + (OC.PERMISSION_READ | OC.PERMISSION_SHARE)
 			});
 		});
+		it('calls complete handler before refreshing the model', function() {
+			var completeStub = sinon.stub();
+			model.updateShare(123, {
+				permissions: OC.PERMISSION_READ | OC.PERMISSION_SHARE
+			}, {
+				complete: completeStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(completeStub.calledOnce).toEqual(true);
+		});
+		it('calls success handler after refreshing the model', function() {
+			var successStub = sinon.stub();
+			model.updateShare(123, {
+				permissions: OC.PERMISSION_READ | OC.PERMISSION_SHARE
+			}, {
+				success: successStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(successStub.called).toEqual(false);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(successStub.calledOnce).toEqual(true);
+			expect(successStub.lastCall.args[0]).toEqual(model);
+		});
+		it('calls complete handler before error handler', function() {
+			var completeStub = sinon.stub();
+			var errorStub = sinon.stub();
+			model.updateShare(123, {
+				permissions: OC.PERMISSION_READ | OC.PERMISSION_SHARE
+			}, {
+				complete: completeStub,
+				error: errorStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				400,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({
+					ocs: {
+						meta: {
+							message: 'Some error message'
+						}
+					}
+				})
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+			expect(errorStub.calledOnce).toEqual(true);
+			expect(completeStub.calledBefore(errorStub)).toEqual(true);
+		});
 		it('calls error handler with error message', function() {
 			var errorStub = sinon.stub();
 			model.updateShare(123, {
@@ -734,6 +886,7 @@ describe('OC.Share.ShareItemModel', function() {
 			);
 
 			expect(errorStub.calledOnce).toEqual(true);
+			expect(errorStub.lastCall.args[0]).toEqual(model);
 			expect(errorStub.lastCall.args[1]).toEqual('Some error message');
 		});
 	});

lib/private/Share20/Manager.php

@@ -713,36 +713,17 @@ class Manager implements IManager {
 		} else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
 			$this->linkCreateChecks($share);
 
-			// Password updated.
-			if ($share->getPassword() !== $originalShare->getPassword()) {
-				//Verify the password
-				$this->verifyPassword($share->getPassword());
-
-				// If a password is set. Hash it!
-				if ($share->getPassword() !== null) {
-					$share->setPassword($this->hasher->hash($share->getPassword()));
-				}
-			}
+			$this->updateSharePasswordIfNeeded($share, $originalShare);
 
 			if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
 				//Verify the expiration date
 				$this->validateExpirationDate($share);
 				$expirationDateUpdated = true;
 			}
-		}
-
-		$plainTextPassword = null;
-		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK || $share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) {
-			// Password updated.
-			if ($share->getPassword() !== $originalShare->getPassword()) {
-				//Verify the password
-				$this->verifyPassword($share->getPassword());
-
-				// If a password is set. Hash it!
-				if ($share->getPassword() !== null) {
-					$plainTextPassword = $share->getPassword();
-					$share->setPassword($this->hasher->hash($plainTextPassword));
-				}
+		} else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) {
+			$plainTextPassword = $share->getPassword();
+			if (!$this->updateSharePasswordIfNeeded($share, $originalShare)) {
+				$plainTextPassword = null;
 			}
 		}
 
@@ -795,6 +776,32 @@ class Manager implements IManager {
 		return $share;
 	}
 
+	/**
+	 * Updates the password of the given share if it is not the same as the
+	 * password of the original share.
+	 *
+	 * @param \OCP\Share\IShare $share the share to update its password.
+	 * @param \OCP\Share\IShare $originalShare the original share to compare its
+	 *        password with.
+	 * @return boolean whether the password was updated or not.
+	 */
+	private function updateSharePasswordIfNeeded(\OCP\Share\IShare $share, \OCP\Share\IShare $originalShare) {
+		// Password updated.
+		if ($share->getPassword() !== $originalShare->getPassword()) {
+			//Verify the password
+			$this->verifyPassword($share->getPassword());
+
+			// If a password is set. Hash it!
+			if ($share->getPassword() !== null) {
+				$share->setPassword($this->hasher->hash($share->getPassword()));
+
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 	/**
 	 * Delete all the children of this share
 	 * FIXME: remove once https://github.com/owncloud/core/pull/21660 is in

tests/acceptance/config/behat.yml

@@ -11,6 +11,7 @@ default:
 
         - FeatureContext
         - FilesAppContext
+        - FilesSharingAppContext
         - LoginPageContext
         - NotificationContext
         - SettingsMenuContext

tests/acceptance/features/app-files.feature

@@ -0,0 +1,31 @@
+Feature: app-files
+
+  Scenario: set a password to a shared link
+    Given I am logged in
+    And I share the link for "welcome.txt"
+    When I protect the shared link with the password "abcdef"
+    Then I see that the working icon for password protect is shown
+    And I see that the working icon for password protect is eventually not shown
+
+  Scenario: access a shared link protected by password with a valid password
+    Given I act as John
+    And I am logged in
+    And I share the link for "welcome.txt" protected by the password "abcdef"
+    And I write down the shared link
+    When I act as Jane
+    And I visit the shared link I wrote down
+    And I see that the current page is the Authenticate page for the shared link I wrote down
+    And I authenticate with password "abcdef"
+    Then I see that the current page is the shared link I wrote down
+    And I see that the shared file preview shows the text "Welcome to your Nextcloud account!"
+
+  Scenario: access a shared link protected by password with an invalid password
+    Given I act as John
+    And I am logged in
+    And I share the link for "welcome.txt" protected by the password "abcdef"
+    And I write down the shared link
+    When I act as Jane
+    And I visit the shared link I wrote down
+    And I authenticate with password "fedcba"
+    Then I see that the current page is the Authenticate page for the shared link I wrote down
+    And I see that a wrong password for the shared file message is shown

tests/acceptance/features/bootstrap/FilesAppContext.php

@@ -27,6 +27,105 @@ class FilesAppContext implements Context, ActorAwareInterface {
 
 	use ActorAware;
 
+	/**
+	 * @return Locator
+	 */
+	public static function currentSectionMainView() {
+		return Locator::forThe()->xpath("//*[starts-with(@id, 'app-content-')  and not(contains(concat(' ', normalize-space(@class), ' '), ' hidden '))]")->
+				describedAs("Current section main view in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function currentSectionDetailsView() {
+		return Locator::forThe()->xpath("/preceding-sibling::*[position() = 1 and @id = 'app-sidebar']")->
+				descendantOf(self::currentSectionMainView())->
+				describedAs("Current section details view in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function shareLinkCheckbox() {
+		return Locator::forThe()->content("Share link")->descendantOf(self::currentSectionDetailsView())->
+				describedAs("Share link checkbox in the details view in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function shareLinkField() {
+		return Locator::forThe()->css(".linkText")->descendantOf(self::currentSectionDetailsView())->
+				describedAs("Share link field in the details view in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function passwordProtectCheckbox() {
+		return Locator::forThe()->content("Password protect")->descendantOf(self::currentSectionDetailsView())->
+				describedAs("Password protect checkbox in the details view in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function passwordProtectField() {
+		return Locator::forThe()->css(".linkPassText")->descendantOf(self::currentSectionDetailsView())->
+				describedAs("Password protect field in the details view in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function passwordProtectWorkingIcon() {
+		return Locator::forThe()->css(".linkPass .icon-loading-small")->descendantOf(self::currentSectionDetailsView())->
+				describedAs("Password protect working icon in the details view in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function rowForFile($fileName) {
+		return Locator::forThe()->xpath("//*[@id = 'fileList']//span[contains(concat(' ', normalize-space(@class), ' '), ' nametext ') and normalize-space() = '$fileName']/ancestor::tr")->
+				descendantOf(self::currentSectionMainView())->
+				describedAs("Row for file $fileName in Files app");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function shareActionForFile($fileName) {
+		return Locator::forThe()->css(".action-share")->descendantOf(self::rowForFile($fileName))->
+				describedAs("Share action for file $fileName in Files app");
+	}
+
+	/**
+	 * @Given I share the link for :fileName
+	 */
+	public function iShareTheLinkFor($fileName) {
+		$this->actor->find(self::shareActionForFile($fileName), 10)->click();
+
+		$this->actor->find(self::shareLinkCheckbox(), 5)->click();
+	}
+
+	/**
+	 * @Given I write down the shared link
+	 */
+	public function iWriteDownTheSharedLink() {
+		$this->actor->getSharedNotebook()["shared link"] = $this->actor->find(self::shareLinkField(), 10)->getValue();
+	}
+
+	/**
+	 * @When I protect the shared link with the password :password
+	 */
+	public function iProtectTheSharedLinkWithThePassword($password) {
+		$this->actor->find(self::passwordProtectCheckbox(), 10)->click();
+
+		$this->actor->find(self::passwordProtectField(), 2)->setValue($password . "\r");
+	}
+
 	/**
 	 * @Then I see that the current page is the Files app
 	 */
@@ -36,4 +135,43 @@ class FilesAppContext implements Context, ActorAwareInterface {
 				$this->actor->getSession()->getCurrentUrl());
 	}
 
+	/**
+	 * @Then I see that the working icon for password protect is shown
+	 */
+	public function iSeeThatTheWorkingIconForPasswordProtectIsShown() {
+		PHPUnit_Framework_Assert::assertNotNull($this->actor->find(self::passwordProtectWorkingIcon(), 10));
+	}
+
+	/**
+	 * @Then I see that the working icon for password protect is eventually not shown
+	 */
+	public function iSeeThatTheWorkingIconForPasswordProtectIsEventuallyNotShown() {
+		$timeout = 10;
+		$timeoutStep = 1;
+
+		$actor = $this->actor;
+		$passwordProtectWorkingIcon = self::passwordProtectWorkingIcon();
+
+		$workingIconNotFoundCallback = function() use ($actor, $passwordProtectWorkingIcon) {
+			try {
+				return !$actor->find($passwordProtectWorkingIcon)->isVisible();
+			} catch (NoSuchElementException $exception) {
+				return true;
+			}
+		};
+		if (!Utils::waitFor($workingIconNotFoundCallback, $timeout, $timeoutStep)) {
+			PHPUnit_Framework_Assert::fail("The working icon for password protect is still shown after $timeout seconds");
+		}
+	}
+
+	/**
+	 * @Given I share the link for :fileName protected by the password :password
+	 */
+	public function iShareTheLinkForProtectedByThePassword($fileName, $password) {
+		$this->iShareTheLinkFor($fileName);
+		$this->iProtectTheSharedLinkWithThePassword($password);
+		$this->iSeeThatTheWorkingIconForPasswordProtectIsShown();
+		$this->iSeeThatTheWorkingIconForPasswordProtectIsEventuallyNotShown();
+	}
+
 }

tests/acceptance/features/bootstrap/FilesSharingAppContext.php

@@ -0,0 +1,110 @@
+<?php
+
+/**
+ *
+ * @copyright Copyright (c) 2017, Daniel Calviño Sánchez (danxuliu@gmail.com)
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+use Behat\Behat\Context\Context;
+
+class FilesSharingAppContext implements Context, ActorAwareInterface {
+
+	use ActorAware;
+
+	/**
+	 * @return Locator
+	 */
+	public static function passwordField() {
+		return Locator::forThe()->field("password")->
+				describedAs("Password field in Authenticate page");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function authenticateButton() {
+		return Locator::forThe()->id("password-submit")->
+				describedAs("Authenticate button in Authenticate page");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function wrongPasswordMessage() {
+		return Locator::forThe()->content("The password is wrong. Try again.")->
+				describedAs("Wrong password message in Authenticate page");
+	}
+
+	/**
+	 * @return Locator
+	 */
+	public static function textPreview() {
+		return Locator::forThe()->css(".text-preview")->
+				describedAs("Text preview in Shared file page");
+	}
+
+	/**
+	 * @When I visit the shared link I wrote down
+	 */
+	public function iVisitTheSharedLinkIWroteDown() {
+		$this->actor->getSession()->visit($this->actor->getSharedNotebook()["shared link"]);
+	}
+
+	/**
+	 * @When I authenticate with password :password
+	 */
+	public function iAuthenticateWithPassword($password) {
+		$this->actor->find(self::passwordField(), 10)->setValue($password);
+		$this->actor->find(self::authenticateButton())->click();
+	}
+
+	/**
+	 * @Then I see that the current page is the Authenticate page for the shared link I wrote down
+	 */
+	public function iSeeThatTheCurrentPageIsTheAuthenticatePageForTheSharedLinkIWroteDown() {
+		PHPUnit_Framework_Assert::assertEquals(
+				$this->actor->getSharedNotebook()["shared link"] . "/authenticate",
+				$this->actor->getSession()->getCurrentUrl());
+	}
+
+	/**
+	 * @Then I see that the current page is the shared link I wrote down
+	 */
+	public function iSeeThatTheCurrentPageIsTheSharedLinkIWroteDown() {
+		PHPUnit_Framework_Assert::assertEquals(
+				$this->actor->getSharedNotebook()["shared link"],
+				$this->actor->getSession()->getCurrentUrl());
+	}
+
+	/**
+	 * @Then I see that a wrong password for the shared file message is shown
+	 */
+	public function iSeeThatAWrongPasswordForTheSharedFileMessageIsShown() {
+		PHPUnit_Framework_Assert::assertTrue(
+				$this->actor->find(self::wrongPasswordMessage(), 10)->isVisible());
+	}
+
+	/**
+	 * @Then I see that the shared file preview shows the text :text
+	 */
+	public function iSeeThatTheSharedFilePreviewShowsTheText($text) {
+		PHPUnit_Framework_Assert::assertContains($text, $this->actor->find(self::textPreview(), 10)->getText());
+	}
+
+}

tests/acceptance/features/core/Actor.php

@@ -48,6 +48,10 @@
  * before giving up without modifying the tests themselves. Note that the
  * multiplier affects the timeout, but not the timeout step; the rate at which
  * find() will try again to find the element does not change.
+ *
+ * All actors share a notebook in which data can be annotated. This makes
+ * possible to share data between different test steps, no matter which Actor
+ * performs them.
  */
 class Actor {
 
@@ -66,16 +70,23 @@ class Actor {
 	 */
 	private $findTimeoutMultiplier;
 
+	/**
+	 * @var array
+	 */
+	private $sharedNotebook;
+
 	/**
 	 * Creates a new Actor.
 	 *
 	 * @param \Behat\Mink\Session $session the Mink Session used to control its
 	 *        web browser.
 	 * @param string $baseUrl the base URL used when solving relative URLs.
+	 * @param array $sharedNotebook the notebook shared between all actors.
 	 */
-	public function __construct(\Behat\Mink\Session $session, $baseUrl) {
+	public function __construct(\Behat\Mink\Session $session, $baseUrl, &$sharedNotebook) {
 		$this->session = $session;
 		$this->baseUrl = $baseUrl;
+		$this->sharedNotebook = &$sharedNotebook;
 		$this->findTimeoutMultiplier = 1;
 	}
 
@@ -221,4 +232,13 @@ class Actor {
 		return $ancestorElement;
 	}
 
+	/**
+	 * Returns the shared notebook of the Actors.
+	 *
+	 * @return array the shared notebook of the Actors.
+	 */
+	public function &getSharedNotebook() {
+		return $this->sharedNotebook;
+	}
+
 }

tests/acceptance/features/core/ActorContext.php

@@ -53,6 +53,11 @@ class ActorContext extends RawMinkContext {
 	 */
 	private $actors;
 
+	/**
+	 * @var array
+	 */
+	private $sharedNotebook;
+
 	/**
 	 * @var Actor
 	 */
@@ -102,8 +107,9 @@ class ActorContext extends RawMinkContext {
 	 */
 	public function initializeActors() {
 		$this->actors = array();
+		$this->sharedNotebook = array();
 
-		$this->actors["default"] = new Actor($this->getSession(), $this->getMinkParameter("base_url"));
+		$this->actors["default"] = new Actor($this->getSession(), $this->getMinkParameter("base_url"), $this->sharedNotebook);
 		$this->actors["default"]->setFindTimeoutMultiplier($this->actorFindTimeoutMultiplier);
 
 		$this->currentActor = $this->actors["default"];
@@ -127,7 +133,7 @@ class ActorContext extends RawMinkContext {
 	 */
 	public function iActAs($actorName) {
 		if (!array_key_exists($actorName, $this->actors)) {
-			$this->actors[$actorName] = new Actor($this->getSession($actorName), $this->getMinkParameter("base_url"));
+			$this->actors[$actorName] = new Actor($this->getSession($actorName), $this->getMinkParameter("base_url"), $this->sharedNotebook);
 			$this->actors[$actorName]->setFindTimeoutMultiplier($this->actorFindTimeoutMultiplier);
 		}
 

tests/lib/Share20/ManagerTest.php

@@ -2510,6 +2510,7 @@ class ManagerTest extends \Test\TestCase {
 		$share->setProviderId('foo')
 			->setId('42')
 			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setToken('token')
 			->setSharedBy('owner')
 			->setShareOwner('owner')
 			->setPassword('password')
@@ -2520,6 +2521,12 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->once())->method('canShare')->willReturn(true);
 		$manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
 		$manager->expects($this->once())->method('validateExpirationDate')->with($share);
+		$manager->expects($this->once())->method('verifyPassword')->with('password');
+
+		$this->hasher->expects($this->once())
+			->method('hash')
+			->with('password')
+			->willReturn('hashed');
 
 		$this->defaultProvider->expects($this->once())
 			->method('update')
@@ -2536,9 +2543,94 @@ class ManagerTest extends \Test\TestCase {
 		]);
 
 		$hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
-		\OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner2, 'post');
-		$hookListner2->expects($this->never())->method('post');
+		\OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post');
+		$hookListner2->expects($this->once())->method('post')->with([
+			'itemType' => 'file',
+			'itemSource' => 100,
+			'uidOwner' => 'owner',
+			'token' => 'token',
+			'disabled' => false,
+		]);
+
+		$hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post');
+		$hookListner3->expects($this->never())->method('post');
+
+
+		$manager->updateShare($share);
+	}
+
+	public function testUpdateShareMail() {
+		$manager = $this->createManagerMock()
+			->setMethods([
+				'canShare',
+				'getShareById',
+				'generalCreateChecks',
+				'verifyPassword',
+				'pathCreateChecks',
+				'linkCreateChecks',
+				'validateExpirationDate',
+			])
+			->getMock();
+
+		$originalShare = $this->manager->newShare();
+		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+
+		$tomorrow = new \DateTime();
+		$tomorrow->setTime(0,0,0);
+		$tomorrow->add(new \DateInterval('P1D'));
+
+		$file = $this->createMock(File::class);
+		$file->method('getId')->willReturn(100);
+
+		$share = $this->manager->newShare();
+		$share->setProviderId('foo')
+			->setId('42')
+			->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
+			->setToken('token')
+			->setSharedBy('owner')
+			->setShareOwner('owner')
+			->setPassword('password')
+			->setExpirationDate($tomorrow)
+			->setNode($file)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+
+		$manager->expects($this->once())->method('canShare')->willReturn(true);
+		$manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
+		$manager->expects($this->once())->method('generalCreateChecks')->with($share);
+		$manager->expects($this->once())->method('verifyPassword')->with('password');
+		$manager->expects($this->once())->method('pathCreateChecks')->with($file);
+		$manager->expects($this->never())->method('linkCreateChecks');
+		$manager->expects($this->never())->method('validateExpirationDate');
+
+		$this->hasher->expects($this->once())
+			->method('hash')
+			->with('password')
+			->willReturn('hashed');
+
+		$this->defaultProvider->expects($this->once())
+			->method('update')
+			->with($share, 'password')
+			->willReturn($share);
+
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post');
+		$hookListner->expects($this->never())->method('post');
+
+		$hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post');
+		$hookListner2->expects($this->once())->method('post')->with([
+			'itemType' => 'file',
+			'itemSource' => 100,
+			'uidOwner' => 'owner',
+			'token' => 'token',
+			'disabled' => false,
+		]);
 
+		$hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post');
+		$hookListner3->expects($this->never())->method('post');
 
 		$manager->updateShare($share);
 	}