From 1635ad13ff3b83f976094ecf4fc7c0f1dfe4f682 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 1 Jul 2024 10:53:50 -0400 Subject: [PATCH 1/3] fix(Wipe): Better handle missing tokens Check for non-empty token string to avoid 500/internal server error due to `Argument #1 ($token) must be of type string, null given` - Refactor slightly to streamline code - Fix noted bug in both checkWipe() and wipeDone() (initially reported in the community forum then reproduced) Signed-off-by: Josh --- core/Controller/WipeController.php | 40 ++++++++++++++++-------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/core/Controller/WipeController.php b/core/Controller/WipeController.php index 44f80dc5510..6d3ff2aac1f 100644 --- a/core/Controller/WipeController.php +++ b/core/Controller/WipeController.php @@ -42,18 +42,20 @@ class WipeController extends Controller { * 404: Device should not be wiped */ #[FrontpageRoute(verb: 'POST', url: '/core/wipe/check')] - public function checkWipe(string $token): JSONResponse { - try { - if ($this->remoteWipe->start($token)) { - return new JSONResponse([ - 'wipe' => true - ]); + public function checkWipe(string $token = ''): JSONResponse { + if ($token !== '') { + try { + if ($this->remoteWipe->start($token)) { + return new JSONResponse([ + 'wipe' => true + ]); + } + } catch (InvalidTokenException $e) { + // do nothing special, handled below } - - return new JSONResponse([], Http::STATUS_NOT_FOUND); - } catch (InvalidTokenException $e) { - return new JSONResponse([], Http::STATUS_NOT_FOUND); } + + return new JSONResponse([], Http::STATUS_NOT_FOUND); } @@ -74,15 +76,17 @@ class WipeController extends Controller { * 404: Device should not be wiped */ #[FrontpageRoute(verb: 'POST', url: '/core/wipe/success')] - public function wipeDone(string $token): JSONResponse { - try { - if ($this->remoteWipe->finish($token)) { - return new JSONResponse([]); + public function wipeDone(string $token = ''): JSONResponse { + if ($token !== '') { + try { + if ($this->remoteWipe->finish($token)) { + return new JSONResponse([]); + } + } catch (InvalidTokenException $e) { + // do nothing special, handled below } - - return new JSONResponse([], Http::STATUS_NOT_FOUND); - } catch (InvalidTokenException $e) { - return new JSONResponse([], Http::STATUS_NOT_FOUND); } + + return new JSONResponse([], Http::STATUS_NOT_FOUND); } } From 168384579f16372cd384c9453821821b79e6059b Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 1 Jul 2024 14:59:18 -0400 Subject: [PATCH 2/3] test(Wipe): Expand test coverage for more token scenarios Signed-off-by: Josh --- tests/Core/Controller/WipeControllerTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Core/Controller/WipeControllerTest.php b/tests/Core/Controller/WipeControllerTest.php index f07fe4c9282..08acec3aa87 100644 --- a/tests/Core/Controller/WipeControllerTest.php +++ b/tests/Core/Controller/WipeControllerTest.php @@ -55,6 +55,12 @@ class WipeControllerTest extends TestCase { $this->remoteWipe->method('start') ->with('mytoken') ->willThrowException(new InvalidTokenException()); + $this->remoteWipe->method('start') + ->with('') + ->willThrowException(new InvalidTokenException()); + $this->remoteWipe->method('start') + ->with(NULL) + ->willThrowException(new InvalidTokenException()); } else { $this->remoteWipe->method('start') ->with('mytoken') From 3b353ede605260a31dc5a71832710298975ca1ad Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 1 Jul 2024 15:07:04 -0400 Subject: [PATCH 3/3] fixup: make nullable + switch to empty() for check Signed-off-by: Josh --- core/Controller/WipeController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/Controller/WipeController.php b/core/Controller/WipeController.php index 6d3ff2aac1f..303d9e682d2 100644 --- a/core/Controller/WipeController.php +++ b/core/Controller/WipeController.php @@ -42,8 +42,8 @@ class WipeController extends Controller { * 404: Device should not be wiped */ #[FrontpageRoute(verb: 'POST', url: '/core/wipe/check')] - public function checkWipe(string $token = ''): JSONResponse { - if ($token !== '') { + public function checkWipe(?string $token = ''): JSONResponse { + if (!empty($token)) { try { if ($this->remoteWipe->start($token)) { return new JSONResponse([ @@ -76,8 +76,8 @@ class WipeController extends Controller { * 404: Device should not be wiped */ #[FrontpageRoute(verb: 'POST', url: '/core/wipe/success')] - public function wipeDone(string $token = ''): JSONResponse { - if ($token !== '') { + public function wipeDone(?string $token = ''): JSONResponse { + if (!empty($token)) { try { if ($this->remoteWipe->finish($token)) { return new JSONResponse([]);