From 641fa451595e611332504aa91f24edf6a9c449d2 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 18 Oct 2016 18:28:46 +0200 Subject: [PATCH 1/3] make ChangePasswordController and UI Code respect Message and Hint from HintException Signed-off-by: Arthur Schiwon --- .../Controller/ChangePasswordController.php | 42 +++++++++++-------- settings/js/personal.js | 3 ++ settings/js/users/users.js | 6 ++- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/settings/Controller/ChangePasswordController.php b/settings/Controller/ChangePasswordController.php index 832cdbefdbe93..8e671f4a321b6 100644 --- a/settings/Controller/ChangePasswordController.php +++ b/settings/Controller/ChangePasswordController.php @@ -82,6 +82,27 @@ public function __construct($appName, $this->l = $l; } + /** + * formats HintException into a JSONResponse for output + * + * @param HintException $e + * @return JSONResponse + */ + private function caughtHintException(HintException $e) { + $message = $e->getMessage(); + $hint = $e->getHint(); + if($message === $hint) { + $hint = ''; + } + return new JSONResponse([ + 'status' => 'error', + 'data' => [ + 'message' => $message, + 'hint' => $hint + ], + ]); + } + /** * @NoAdminRequired * @NoSubadminRequired @@ -111,12 +132,7 @@ public function changePersonalPassword($oldpassword = '', $newpassword = null) { } // password policy app throws exception } catch(HintException $e) { - return new JSONResponse([ - 'status' => 'error', - 'data' => [ - 'message' => $e->getHint(), - ], - ]); + return $this->caughtHintException($e); } $this->userSession->updateSessionTokenPassword($newpassword); @@ -232,12 +248,7 @@ public function changeUserPassword($username = null, $password = null, $recovery $result = $targetUser->setPassword($password, $recoveryPassword); // password policy app throws exception } catch(HintException $e) { - return new JSONResponse([ - 'status' => 'error', - 'data' => [ - 'message' => $e->getHint(), - ], - ]); + return $this->caughtHintException($e); } if (!$result && $recoveryEnabledForUser) { return new JSONResponse([ @@ -267,12 +278,7 @@ public function changeUserPassword($username = null, $password = null, $recovery } // password policy app throws exception } catch(HintException $e) { - return new JSONResponse([ - 'status' => 'error', - 'data' => [ - 'message' => $e->getHint(), - ], - ]); + return $this->caughtHintException($e); } } diff --git a/settings/js/personal.js b/settings/js/personal.js index 9045851ba0c12..01282d67df062 100644 --- a/settings/js/personal.js +++ b/settings/js/personal.js @@ -242,6 +242,9 @@ $(document).ready(function () { $('#pass2').val('').change(); } if (typeof(data.data) !== "undefined") { + if(!_.isEmpty(data.data.hint)) { + data.data.message += "\n" + data.data.hint; + } OC.msg.finishedSaving('#password-error-msg', data); } else { OC.msg.finishedSaving('#password-error-msg', diff --git a/settings/js/users/users.js b/settings/js/users/users.js index 71defeaa18a02..ee813b20e12ea 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -681,7 +681,11 @@ $(document).ready(function () { if (result.status === 'success') { OC.Notification.showTemporary(t('admin', 'Password successfully changed')); } else { - OC.Notification.showTemporary(t('admin', result.data.message)); + var message = _.escape(result.data.message); + if(!_.isEmpty(result.data.hint)) { + message += "
" + _.escape(result.data.hint); + } + OC.Notification.showTemporary(t('admin', message), {isHTML: true}); } } ).fail(blurFunction); From 5d4e0644bfce7719da9d33619a24d12920e17ba9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 19 Oct 2016 01:05:11 +0200 Subject: [PATCH 2/3] adjust test Signed-off-by: Arthur Schiwon --- tests/Core/Controller/ChangePasswordControllerTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Core/Controller/ChangePasswordControllerTest.php b/tests/Core/Controller/ChangePasswordControllerTest.php index 869ef98b514d6..0e2cf0900ead8 100644 --- a/tests/Core/Controller/ChangePasswordControllerTest.php +++ b/tests/Core/Controller/ChangePasswordControllerTest.php @@ -105,12 +105,13 @@ public function testChangePersonalPasswordCommonPassword() { $user->expects($this->once()) ->method('setPassword') ->with('new') - ->will($this->throwException(new HintException('Common password'))); + ->will($this->throwException(new HintException('Common password', 'Pick a less common one'))); $expects = [ 'status' => 'error', 'data' => [ 'message' => 'Common password', + 'hint' => 'Pick a less common one', ], ]; From 376d34b21030cc9d36641a9976a7a68e4e6b394a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 19 Oct 2016 16:01:58 +0200 Subject: [PATCH 3/3] Don't attempt to translate variables * stings should be translated (if applicable) where they happen * ChangePasswordController returns all its fixed strings already translated * local user backend does not throw HintException at all Signed-off-by: Arthur Schiwon --- settings/js/users/users.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings/js/users/users.js b/settings/js/users/users.js index ee813b20e12ea..80556e359559b 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -685,7 +685,7 @@ $(document).ready(function () { if(!_.isEmpty(result.data.hint)) { message += "
" + _.escape(result.data.hint); } - OC.Notification.showTemporary(t('admin', message), {isHTML: true}); + OC.Notification.showTemporary(message, {isHTML: true}); } } ).fail(blurFunction);