Merge pull request #10053 from nextcloud/feature/7378/password-security-move

Move password change to security settings

lib/private/Settings/Manager.php

@@ -308,7 +308,7 @@ class Manager implements IManager {
 		}
 		if($section === 'security') {
 			/** @var ISettings $form */
-			$form = new Personal\Security();
+			$form = new Personal\Security($this->userManager);
 			$forms[$form->getPriority()] = [$form];
 		}
 		if ($section === 'additional') {

lib/private/Settings/Personal/PersonalInfo.php

@@ -135,7 +135,6 @@ class PersonalInfo implements ISettings {
 			'twitterScope' => $userData[AccountManager::PROPERTY_TWITTER]['scope'],
 			'twitterVerification' => $userData[AccountManager::PROPERTY_TWITTER]['verified'],
 			'groups' => $this->getGroups($user),
-			'passwordChangeSupported' => $user->canChangePassword(),
 		] + $messageParameters + $languageParameters + $localeParameters;
 
 

lib/private/Settings/Personal/Security.php

@@ -25,16 +25,33 @@ namespace OC\Settings\Personal;
 
 
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\IUserManager;
 use OCP\Settings\ISettings;
 
 class Security implements ISettings {
 
+	private $userManager;
+
+	public function __construct(
+		IUserManager $userManager
+	) {
+		$this->userManager = $userManager;
+	}
+
 	/**
 	 * @return TemplateResponse returns the instance with all parameters set, ready to be rendered
 	 * @since 9.1
 	 */
 	public function getForm() {
-		return new TemplateResponse('settings', 'settings/personal/security');
+		$user = $this->userManager->get(\OC_User::getUser());
+		$passwordChangeSupported = false;
+		if ($user !== null) {
+			$passwordChangeSupported = $user->canChangePassword();
+		}
+
+		return new TemplateResponse('settings', 'settings/personal/security', [
+			'passwordChangeSupported' => $passwordChangeSupported
+		]);
 	}
 
 	/**

settings/css/settings.scss

@@ -124,14 +124,6 @@ select {
 	}
 }
 
-input {
-	&#pass1,
-	&#pass2,
-	&#passwordbutton {
-		width: 100%;
-	}
-}
-
 #personal-settings {
 	display: grid;
 	padding: 20px;
@@ -380,9 +372,7 @@ input#identity {
 	}
 }
 
-.password-state {
-	display: inline-block;
-}
+
 
 table.nostyle {
 	label {
@@ -393,6 +383,33 @@ table.nostyle {
 	}
 }
 
+#security-password {
+	#passwordform {
+		display: flex;
+		flex-wrap: wrap;
+		#pass1, .personal-show-container, #passwordbutton {
+			flex-shrink: 1;
+			width: 200px;
+			min-width: 150px;
+		}
+		#pass2 {
+			width: 100%;
+		}
+		.password-state {
+			display: inline-block;
+		}
+		.strengthify-wrapper {
+			position: absolute;
+			left: 0;
+			width: 100%;
+			border-radius: 0 0 2px 2px;
+			margin-top: -6px;
+			overflow: hidden;
+			height: 3px;
+		}
+	}
+}
+
 #security {
 	table {
 		width: 100%;
@@ -1246,16 +1263,7 @@ span {
 	}
 }
 
-/* PASSWORD */
-#passwordform .strengthify-wrapper {
-	position: absolute;
-	left: 0;
-	width: 100%;
-	border-radius: 0 0 2px 2px;
-	margin-top: -6px;
-	overflow: hidden;
-	height: 3px;
-}
+
 
 /* OPERA hack for strengthify*/
 doesnotexist:-o-prefocus, .strengthify-wrapper {

settings/js/security_password.js

@@ -0,0 +1,85 @@
+/* global OC */
+
+/**
+ * Copyright (c) 2011, Robin Appelman <icewind1991@gmail.com>
+ *               2013, Morris Jobke <morris.jobke@gmail.com>
+ *               2016, Christoph Wurst <christoph@owncloud.com>
+ *               2017, Arthur Schiwon <blizzz@arthur-schiwon.de>
+ *               2017, Thomas Citharel <tcit@tcit.fr>
+ * This file is licensed under the Affero General Public License version 3 or later.
+ * See the COPYING-README file.
+ */
+
+$(document).ready(function () {
+	if($('#pass2').length) {
+		$('#pass2').showPassword().keyup();
+	}
+
+	var removeloader = function () {
+		setTimeout(function(){
+			if ($('.password-state').length > 0) {
+				$('.password-state').remove();
+			}
+		}, 5000)
+	};
+
+	$("#passwordbutton").click(function () {
+		if ($('#pass1').val() !== '' && $('#pass2').val() !== '') {
+			// Serialize the data
+			var post = $("#passwordform").serialize();
+			$('#passwordchanged').hide();
+			$('#passworderror').hide();
+			$("#passwordbutton").attr('disabled', 'disabled');
+			$("#passwordbutton").after("<span class='password-loading icon icon-loading-small-dark password-state'></span>");
+			$(".personal-show-label").hide();
+			// Ajax foo
+			$.post(OC.generateUrl('/settings/personal/changepassword'), post, function (data) {
+				if (data.status === "success") {
+					$("#passwordbutton").after("<span class='checkmark icon icon-checkmark password-state'></span>");
+					removeloader();
+					$('#pass1').val('');
+					$('#pass2').val('').change();
+				}
+				if (typeof(data.data) !== "undefined") {
+					OC.msg.finishedSaving('#password-error-msg', data);
+				} else {
+					OC.msg.finishedSaving('#password-error-msg',
+						{
+							'status' : 'error',
+							'data' : {
+								'message' : t('settings', 'Unable to change password')
+							}
+						}
+					);
+				}
+				$(".personal-show-label").show();
+				$(".password-loading").remove();
+				$("#passwordbutton").removeAttr('disabled');
+			});
+			return false;
+		} else {
+			OC.msg.finishedSaving('#password-error-msg',
+				{
+					'status' : 'error',
+					'data' : {
+						'message' : t('settings', 'Unable to change password')
+					}
+				}
+			);
+			return false;
+		}
+	});
+
+	$('#pass2').strengthify({
+		zxcvbn: OC.linkTo('core','vendor/zxcvbn/dist/zxcvbn.js'),
+		titles: [
+			t('settings', 'Very weak password'),
+			t('settings', 'Weak password'),
+			t('settings', 'So-so password'),
+			t('settings', 'Good password'),
+			t('settings', 'Strong password')
+		],
+		drawTitles: true,
+		$addAfter: $('input[name="newpassword-clone"]'),
+	});
+});

settings/js/settings/personalInfo.js

@@ -145,67 +145,6 @@ $(document).ready(function () {
 		$('#pass2').showPassword().keyup();
 	}
 
-	var removeloader = function () {
-		setTimeout(function(){
-			if ($('.password-state').length > 0) {
-				$('.password-state').remove();
-			}
-		}, 5000)
-	};
-
-	$("#passwordbutton").click(function () {
-		var isIE8or9 = $('html').hasClass('lte9');
-		// FIXME - TODO - once support for IE8 and IE9 is dropped
-		// for IE8 and IE9 this will check additionally if the typed in password
-		// is different from the placeholder, because in IE8/9 the placeholder
-		// is simply set as the value to look like a placeholder
-		if ($('#pass1').val() !== '' && $('#pass2').val() !== ''
-			&& !(isIE8or9 && $('#pass2').val() === $('#pass2').attr('placeholder'))) {
-			// Serialize the data
-			var post = $("#passwordform").serialize();
-			$('#passwordchanged').hide();
-			$('#passworderror').hide();
-			$("#passwordbutton").attr('disabled', 'disabled');
-			$("#passwordbutton").after("<span class='password-loading icon icon-loading-small-dark password-state'></span>");
-			$(".personal-show-label").hide();
-			// Ajax foo
-			$.post(OC.generateUrl('/settings/personal/changepassword'), post, function (data) {
-				if (data.status === "success") {
-					$("#passwordbutton").after("<span class='checkmark icon icon-checkmark password-state'></span>");
-					removeloader();
-					$('#pass1').val('');
-					$('#pass2').val('').change();
-				}
-				if (typeof(data.data) !== "undefined") {
-					OC.msg.finishedSaving('#password-error-msg', data);
-				} else {
-					OC.msg.finishedSaving('#password-error-msg',
-						{
-							'status' : 'error',
-							'data' : {
-								'message' : t('settings', 'Unable to change password')
-							}
-						}
-					);
-				}
-				$(".personal-show-label").show();
-				$(".password-loading").remove();
-				$("#passwordbutton").removeAttr('disabled');
-			});
-			return false;
-		} else {
-			OC.msg.finishedSaving('#password-error-msg',
-				{
-					'status' : 'error',
-					'data' : {
-						'message' : t('settings', 'Unable to change password')
-					}
-				}
-			);
-			return false;
-		}
-	});
-
 	var showVerifyDialog = function(dialog, howToVerify, verificationCode) {
 		var dialogContent = dialog.children('.verification-dialog-content');
 		dialogContent.children(".explainVerification").text(howToVerify);
@@ -417,19 +356,6 @@ $(document).ready(function () {
 		sendCropData();
 	});
 
-	$('#pass2').strengthify({
-		zxcvbn: OC.linkTo('core','vendor/zxcvbn/dist/zxcvbn.js'),
-		titles: [
-			t('settings', 'Very weak password'),
-			t('settings', 'Weak password'),
-			t('settings', 'So-so password'),
-			t('settings', 'Good password'),
-			t('settings', 'Strong password')
-		],
-		drawTitles: true,
-		$addAfter: $('input[name="newpassword-clone"]'),
-	});
-
 	// Load the big avatar
 	var user = OC.getCurrentUser();
 	$('#avatarform .avatardiv').avatar(user.uid, 145, true, null, function() {

settings/templates/settings/personal/personal.info.php

@@ -31,8 +31,6 @@ script('settings', [
 	'federationscopemenu',
 	'settings/personalInfo',
 ]);
-vendor_script('strengthify/jquery.strengthify');
-vendor_style('strengthify/strengthify');
 vendor_script('jcrop/js/jquery.Jcrop');
 vendor_style('jcrop/css/jquery.Jcrop');
 
@@ -376,36 +374,6 @@ vendor_style('jcrop/css/jquery.Jcrop');
 				</form>
 			<?php } ?>
 		</div>
-		<div class="personal-settings-setting-box personal-settings-password-box">
-			<?php
-			if($_['passwordChangeSupported']) {
-				script('jquery-showpassword');
-				?>
-				<form id="passwordform" class="section">
-					<h2 class="inlineblock"><?php p($l->t('Password'));?></h2>
-					<div id="password-error-msg" class="msg success inlineblock" style="display: none;">Saved</div>
-
-					<label for="pass1" class="hidden-visually"><?php p($l->t('Current password')); ?>: </label>
-					<input type="password" id="pass1" name="oldpassword"
-						   placeholder="<?php p($l->t('Current password'));?>"
-						   autocomplete="off" autocapitalize="none" autocorrect="off" />
-
-					<div class="personal-show-container">
-						<label for="pass2" class="hidden-visually"><?php p($l->t('New password'));?>: </label>
-						<input type="password" id="pass2" name="newpassword"
-							   placeholder="<?php p($l->t('New password')); ?>"
-							   data-typetoggle="#personal-show"
-							   autocomplete="off" autocapitalize="none" autocorrect="off" />
-						<input type="checkbox" id="personal-show" name="show" /><label for="personal-show" class="personal-show-label"></label>
-					</div>
-
-					<input id="passwordbutton" type="submit" value="<?php p($l->t('Change password')); ?>" />
-
-				</form>
-				<?php
-			}
-			?>
-		</div>
 		<span class="msg"></span>
 	</div>
 

settings/templates/settings/personal/security.php

@@ -22,14 +22,50 @@
  */
 
 script('settings', [
+	'security_password',
 	'authtoken',
 	'authtoken_collection',
 	'authtoken_view',
 	'settings/authtoken-init'
 ]);
 
+if($_['passwordChangeSupported']) {
+	script('jquery-showpassword');
+	vendor_script('strengthify/jquery.strengthify');
+	vendor_style('strengthify/strengthify');
+}
+
 ?>
 
+<div id="security-password" class="section">
+	<h2 class="inlineblock"><?php p($l->t('Password'));?></h2>
+	<span id="password-error-msg" class="msg success hidden">Saved</span>
+	<div class="personal-settings-setting-box personal-settings-password-box">
+		<?php if($_['passwordChangeSupported']) { ?>
+			<form id="passwordform">
+				<label for="pass1" class="hidden-visually"><?php p($l->t('Current password')); ?>: </label>
+				<input type="password" id="pass1" name="oldpassword"
+					   placeholder="<?php p($l->t('Current password'));?>"
+					   autocomplete="off" autocapitalize="none" autocorrect="off" />
+
+				<div class="personal-show-container">
+					<label for="pass2" class="hidden-visually"><?php p($l->t('New password'));?>: </label>
+					<input type="password" id="pass2" name="newpassword"
+						   placeholder="<?php p($l->t('New password')); ?>"
+						   data-typetoggle="#personal-show"
+						   autocomplete="off" autocapitalize="none" autocorrect="off" />
+					<input type="checkbox" id="personal-show" name="show" /><label for="personal-show" class="personal-show-label"></label>
+				</div>
+
+				<input id="passwordbutton" type="submit" value="<?php p($l->t('Change password')); ?>" />
+
+			</form>
+			<?php
+		}
+		?>
+	</div>
+	<span class="msg"></span>
+</div>
 
 <div id="security" class="section">
 	<h2><?php p($l->t('Devices & sessions'));?></h2>

tests/lib/Settings/ManagerTest.php

@@ -219,7 +219,7 @@ class ManagerTest extends TestCase {
 
 	public function testGetPersonalSettings() {
 		$this->assertEquals([
-			10 => [new Security()],
+			10 => [new Security($this->userManager)],
 		], $this->manager->getPersonalSettings('security'));
 	}
 }