From 64964f740240e23844dc6eb8bebb44ae263d3eec Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Mon, 23 Apr 2018 17:01:28 +0100 Subject: [PATCH 1/8] unset http scheme on CLIRequestBuilder --- src/Control/CLIRequestBuilder.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Control/CLIRequestBuilder.php b/src/Control/CLIRequestBuilder.php index 660ffb7e0..9ad888355 100644 --- a/src/Control/CLIRequestBuilder.php +++ b/src/Control/CLIRequestBuilder.php @@ -2,6 +2,8 @@ namespace SilverStripe\Control; +use SilverStripe\Core\Environment; + /** * CLI specific request building logic */ @@ -66,4 +68,19 @@ class CLIRequestBuilder extends HTTPRequestBuilder // Parse rest of variables as standard return parent::cleanEnvironment($variables); } + + /** + * @param array $variables + * @param string $input + * @return HTTPRequest + */ + public static function createFromVariables(array $variables, $input) + { + $request = parent::createFromVariables($variables, $input); + // unset scheme so that SS_BASE_URL can provide `is_https` information if required + $scheme = parse_url(Environment::getEnv('SS_BASE_URL'), PHP_URL_SCHEME); + $request->setScheme($scheme); + + return $request; + } } From 873873dc303ce2041aa23e365464133a359e1561 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 15 Aug 2018 10:14:25 +1200 Subject: [PATCH 2/8] FIX Pass request to dummy controller before calling init --- src/Security/Security.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/Security.php b/src/Security/Security.php index e3952c056..2df20eada 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -577,8 +577,8 @@ class Security extends Controller implements TemplateGlobalProvider $holderPage->ID = -1 * random_int(1, 10000000); $controller = ModelAsController::controller_for($holderPage); - $controller->doInit(); $controller->setRequest($this->getRequest()); + $controller->doInit(); return $controller; } From 0db594b2d39c93dd2e911414bee5520c84048906 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 15 Aug 2018 17:57:40 +1200 Subject: [PATCH 3/8] FIX Remove double escaping of HTML values in print views Print view uses the SilverStripe templating to render values which means that values are safely escaped by default. This can be tested by chaing `$CellString` to `$CellString.RAW` in the GridField_print.ss template to see this escaping being disabled. This pull request removes double escaping of HTML in strings. --- src/Forms/GridField/GridFieldPrintButton.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Forms/GridField/GridFieldPrintButton.php b/src/Forms/GridField/GridFieldPrintButton.php index 758de42c6..ef144bb0e 100644 --- a/src/Forms/GridField/GridFieldPrintButton.php +++ b/src/Forms/GridField/GridFieldPrintButton.php @@ -227,10 +227,6 @@ class GridFieldPrintButton implements GridField_HTMLProvider, GridField_ActionPr foreach ($printColumns as $field => $label) { $value = $gridField->getDataFieldValue($item, $field); - if ($item->escapeTypeForField($field) != 'xml') { - $value = Convert::raw2xml($value); - } - $itemRow->push(new ArrayData(array( "CellString" => $value, ))); From 9f5b0086cb1a0259c5c87ea205390c5e69dcae90 Mon Sep 17 00:00:00 2001 From: Luke Edwards Date: Mon, 13 Aug 2018 08:40:52 +1200 Subject: [PATCH 4/8] FIX Paginating a gridfield causing a change event --- src/Forms/GridField/GridFieldPaginator.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Forms/GridField/GridFieldPaginator.php b/src/Forms/GridField/GridFieldPaginator.php index 0a860218a..f9aa43add 100755 --- a/src/Forms/GridField/GridFieldPaginator.php +++ b/src/Forms/GridField/GridFieldPaginator.php @@ -245,7 +245,7 @@ class GridFieldPaginator implements GridField_HTMLProvider, GridField_DataManipu } else { // First page button $firstPage = new GridField_FormAction($gridField, 'pagination_first', 'First', 'paginate', 1); - $firstPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-double-left ss-gridfield-firstpage'); + $firstPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-double-left ss-gridfield-pagination-action ss-gridfield-firstpage'); if ($state->currentPage == 1) { $firstPage = $firstPage->performDisabledTransformation(); } @@ -259,7 +259,7 @@ class GridFieldPaginator implements GridField_HTMLProvider, GridField_DataManipu 'paginate', $previousPageNum ); - $previousPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-left ss-gridfield-previouspage'); + $previousPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-left ss-gridfield-pagination-action ss-gridfield-previouspage'); if ($state->currentPage == 1) { $previousPage = $previousPage->performDisabledTransformation(); } @@ -273,14 +273,14 @@ class GridFieldPaginator implements GridField_HTMLProvider, GridField_DataManipu 'paginate', $nextPageNum ); - $nextPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-right ss-gridfield-nextpage'); + $nextPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-right ss-gridfield-pagination-action ss-gridfield-nextpage'); if ($state->currentPage == $totalPages) { $nextPage = $nextPage->performDisabledTransformation(); } // Last page button $lastPage = new GridField_FormAction($gridField, 'pagination_last', 'Last', 'paginate', $totalPages); - $lastPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-double-right ss-gridfield-lastpage'); + $lastPage->addExtraClass('btn btn-secondary btn--hide-text btn-sm font-icon-angle-double-right ss-gridfield-pagination-action ss-gridfield-lastpage'); if ($state->currentPage == $totalPages) { $lastPage = $lastPage->performDisabledTransformation(); } From dbab6966908f0a293ee6d469cec6b4650dc5a0f1 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Mon, 20 Aug 2018 22:30:12 +1200 Subject: [PATCH 5/8] FIX Message when changing password with invalid token now contains correct links to login The Security controller should be used to return these links rather than the ChangePasswordHandler --- .../ChangePasswordHandler.php | 6 +-- .../ChangePasswordHandlerTest.php | 49 +++++++++++++++++++ .../ChangePasswordHandlerTest.yml | 5 ++ 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.php create mode 100644 tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.yml diff --git a/src/Security/MemberAuthenticator/ChangePasswordHandler.php b/src/Security/MemberAuthenticator/ChangePasswordHandler.php index 74cc528fe..cc6bd7f5b 100644 --- a/src/Security/MemberAuthenticator/ChangePasswordHandler.php +++ b/src/Security/MemberAuthenticator/ChangePasswordHandler.php @@ -74,7 +74,7 @@ class ChangePasswordHandler extends RequestHandler } $token = $request->getVar('t'); - // Check whether we are merely changin password, or resetting. + // Check whether we are merely changing password, or resetting. if ($token !== null && $member && $member->validateAutoLoginToken($token)) { $this->setSessionToken($member, $token); @@ -124,8 +124,8 @@ class ChangePasswordHandler extends RequestHandler . '

You can request a new one here or change your password after' . ' you logged in.

', [ - 'link1' => $this->Link('lostpassword'), - 'link2' => $this->Link('login') + 'link1' => Security::lost_password_url(), + 'link2' => Security::login_url(), ] ) ); diff --git a/tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.php b/tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.php new file mode 100644 index 000000000..16fbcad2d --- /dev/null +++ b/tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.php @@ -0,0 +1,49 @@ +set(Security::class, 'login_url', 'Security/login') + ->set(Security::class, 'lost_password_url', 'Security/lostpassword'); + + $this->logOut(); + } + + public function testExpiredOrInvalidTokenProvidesLostPasswordAndLoginLink() + { + $request = new HTTPRequest('GET', '/Security/changepassword', [ + 'm' => $this->idFromFixture(Member::class, 'sarah'), + 't' => 'an-old-or-expired-hash', + ]); + $request->setSession(new Session([])); + + /** @var ChangePasswordHandler $handler */ + $handler = $this->getMockBuilder(ChangePasswordHandler::class) + ->disableOriginalConstructor() + ->setMethods(null) + ->getMock(); + + $result = $handler->setRequest($request)->changepassword(); + + $this->assertInternalType('array', $result, 'An array is returned'); + $this->assertContains('Security/lostpassword', $result['Content'], 'Lost password URL is included'); + $this->assertContains('Security/login', $result['Content'], 'Login URL is included'); + } +} diff --git a/tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.yml b/tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.yml new file mode 100644 index 000000000..8eb8967ca --- /dev/null +++ b/tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.yml @@ -0,0 +1,5 @@ +SilverStripe\Security\Member: + sarah: + FirstName: Sarah + Surname: Smith + AutoLoginToken: foobar From 18fff5c16cf6b18ceb330a39b18b277d4b292c4f Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Mon, 20 Aug 2018 22:31:23 +1200 Subject: [PATCH 6/8] Remove past tense for "log in" in expired token message --- lang/en.yml | 2 +- src/Security/MemberAuthenticator/ChangePasswordHandler.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lang/en.yml b/lang/en.yml index 6e3bdd7cb..18c2f4fe1 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -323,7 +323,7 @@ en: LOGOUT: 'Log out' LOSTPASSWORDHEADER: 'Lost Password' NOTEPAGESECURED: 'That page is secured. Enter your credentials below and we will send you right along.' - NOTERESETLINKINVALID: '

The password reset link is invalid or expired.

You can request a new one here or change your password after you logged in.

' + NOTERESETLINKINVALID: '

The password reset link is invalid or expired.

You can request a new one here or change your password after you log in.

' NOTERESETPASSWORD: 'Enter your e-mail address and we will send you a link with which you can reset your password' PASSWORDRESETSENTHEADER: 'Password reset link sent' PASSWORDRESETSENTTEXT: 'Thank you. A reset link has been sent, provided an account exists for this email address.' diff --git a/src/Security/MemberAuthenticator/ChangePasswordHandler.php b/src/Security/MemberAuthenticator/ChangePasswordHandler.php index cc6bd7f5b..483286162 100644 --- a/src/Security/MemberAuthenticator/ChangePasswordHandler.php +++ b/src/Security/MemberAuthenticator/ChangePasswordHandler.php @@ -122,7 +122,7 @@ class ChangePasswordHandler extends RequestHandler 'SilverStripe\\Security\\Security.NOTERESETLINKINVALID', '

The password reset link is invalid or expired.

' . '

You can request a new one here or change your password after' - . ' you logged in.

', + . ' you log in.

', [ 'link1' => Security::lost_password_url(), 'link2' => Security::login_url(), From 27ac001d5b27cce4f80ce4b3335c14708b116830 Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Tue, 14 Aug 2018 17:26:31 +0200 Subject: [PATCH 7/8] FIX email rendering should not include requirements If no body is defined, the email is rendered according to a template. Clearing requirements prevent unnecessary styles/scripts to be included in the html (and that needs to be processed/stripped down the line). --- src/Control/Email/Email.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Control/Email/Email.php b/src/Control/Email/Email.php index 230af870f..8d79db709 100644 --- a/src/Control/Email/Email.php +++ b/src/Control/Email/Email.php @@ -790,6 +790,9 @@ class Email extends ViewableData return $this; } + // Do not interfere with emails styles + Requirements::clear(); + // Render plain part if ($plainTemplate && !$plainPart) { $plainPart = $this->renderWith($plainTemplate, $this->getData()); @@ -806,6 +809,9 @@ class Email extends ViewableData $htmlPartObject = DBField::create_field('HTMLFragment', $htmlPart); $plainPart = $htmlPartObject->Plain(); } + + // Rendering is finished + Requirements::restore(); // Fail if no email to send if (!$plainPart && !$htmlPart) { From 4da5569232505ee574e0b5106ff2116611393aa4 Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Mon, 27 Aug 2018 15:34:45 +1200 Subject: [PATCH 8/8] FIX ensure createFromVariables takes correct params on CLIRequestBuilder --- src/Control/CLIRequestBuilder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Control/CLIRequestBuilder.php b/src/Control/CLIRequestBuilder.php index 9ad888355..445da1dbd 100644 --- a/src/Control/CLIRequestBuilder.php +++ b/src/Control/CLIRequestBuilder.php @@ -74,9 +74,9 @@ class CLIRequestBuilder extends HTTPRequestBuilder * @param string $input * @return HTTPRequest */ - public static function createFromVariables(array $variables, $input) + public static function createFromVariables(array $variables, $input, $url = null) { - $request = parent::createFromVariables($variables, $input); + $request = parent::createFromVariables($variables, $input, $url); // unset scheme so that SS_BASE_URL can provide `is_https` information if required $scheme = parse_url(Environment::getEnv('SS_BASE_URL'), PHP_URL_SCHEME); $request->setScheme($scheme);