Merge pull request #4491 from LibreSign/fix/redirect-to-login-when-va…

…lidation-page-is-not-public fix: redirect to login when validation page is not public
LibreSign · Jan 28, 2025 · 364c8e6 · 364c8e6
2 parents 9a094b0 + 8b9e69f
commit 364c8e6
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 15 deletions.
diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php
@@ -8,7 +8,6 @@
 
 namespace OCA\Libresign\Controller;
 
-use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException;
 use OCA\Libresign\AppInfo\Application;
 use OCA\Libresign\Exception\LibresignException;
 use OCA\Libresign\Helper\JSActions;
@@ -34,7 +33,6 @@
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
-use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -61,7 +59,7 @@ public function __construct(
 		private FileService $fileService,
 		private ValidateHelper $validateHelper,
 		private IEventDispatcher $eventDispatcher,
-		private IURLGenerator $url,
+		private IURLGenerator $urlGenerator,
 	) {
 		parent::__construct(
 			request: $request,
@@ -496,7 +494,7 @@ public function validation(): TemplateResponse {
 	 * The path is used only by frontend
 	 *
 	 * @param string $uuid Sign request uuid
-	 * @return RedirectResponse<Http::STATUS_SEE_OTHER, array{}>
+	 * @return TemplateResponse<Http::STATUS_OK, array{}>
 	 *
 	 * 303: Redirected to validation page
 	 * 401: Validation page not accessible if unauthenticated
@@ -507,9 +505,9 @@ public function validation(): TemplateResponse {
 	#[PublicPage]
 	#[AnonRateLimit(limit: 30, period: 60)]
 	#[FrontpageRoute(verb: 'GET', url: '/validation/{uuid}')]
-	public function validationFileWithShortUrl(): RedirectResponse {
+	public function validationFileWithShortUrl(): TemplateResponse {
 		$this->throwIfValidationPageNotAccessible();
-		return new RedirectResponse($this->url->linkToRoute('libresign.page.validationFilePublic', ['uuid' => $this->request->getParam('uuid')]));
+		return $this->validationFilePublic($this->request->getParam('uuid'));
 	}
 
 	/**
@@ -607,7 +605,24 @@ private function throwIfValidationPageNotAccessible(): void {
 			return;
 		}
 		if ($isValidationUrlPrivate) {
-			throw new NotLoggedInException();
+			if ($uuid = $this->request->getParam('uuid')) {
+				$redirectUrl = $this->urlGenerator->linkToRoute(
+					'libresign.page.validationFilePublic',
+					['uuid' => $uuid]
+				);
+			} else {
+				$redirectUrl = $this->urlGenerator->linkToRoute(
+					'libresign.page.validation',
+				);
+			}
+
+			throw new LibresignException(json_encode([
+				'action' => JSActions::ACTION_REDIRECT,
+				'errors' => [$this->l10n->t('You are not logged in. Please log in.')],
+				'redirect' => $this->urlGenerator->linkToRoute('core.login.showLoginForm', [
+					'redirect_url' => $redirectUrl,
+				]),
+			]), Http::STATUS_UNAUTHORIZED);
 		}
 	}
 }
diff --git a/lib/Middleware/InjectionMiddleware.php b/lib/Middleware/InjectionMiddleware.php
@@ -30,12 +30,14 @@
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\IL10N;
 use OCP\IRequest;
+use OCP\ISession;
 use OCP\IUser;
 use OCP\IUserSession;
 use OCP\Util;
@@ -45,6 +47,7 @@ class InjectionMiddleware extends Middleware {
 
 	public function __construct(
 		private IRequest $request,
+		private ISession $session,
 		private IUserSession $userSession,
 		private ValidateHelper $validateHelper,
 		private SignRequestMapper $signRequestMapper,
@@ -173,7 +176,16 @@ public function afterException($controller, $methodName, \Exception $exception):
 		if (str_contains($this->request->getHeader('Accept'), 'html')) {
 			$template = 'external';
 			if ($this->isJson($exception->getMessage())) {
-				foreach (json_decode($exception->getMessage(), true) as $key => $value) {
+				$settings = json_decode($exception->getMessage(), true);
+				if (isset($settings['action']) && $settings['action'] === JSActions::ACTION_REDIRECT && isset($settings['redirect'])) {
+					if (isset($settings['errors'])) {
+						$this->session->set('loginMessages', [
+							[], $settings['errors'],
+						]);
+					}
+					return new RedirectResponse($settings['redirect']);
+				}
+				foreach ($settings as $key => $value) {
 					if ($key === 'template') {
 						$template = $value;
 						continue;

diff --git a/tests/Unit/Middleware/InjectionMiddlewareTest.php b/tests/Unit/Middleware/InjectionMiddlewareTest.php
@@ -15,11 +15,13 @@
 use OCA\Libresign\Service\SignFileService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IServerContainer;
+use OCP\ISession;
 use OCP\IUserSession;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
@@ -31,6 +33,7 @@
 
 final class InjectionMiddlewareTest extends \OCA\Libresign\Tests\Unit\TestCase {
 	private IRequest&MockObject $request;
+	private ISession&MockObject $session;
 	private IUserSession&MockObject $userSession;
 	private ValidateHelper&MockObject $validateHelper;
 	private SignRequestMapper&MockObject $signRequestMapper;
@@ -45,6 +48,7 @@ final class InjectionMiddlewareTest extends \OCA\Libresign\Tests\Unit\TestCase {
 
 	public function setUp(): void {
 		$this->request = $this->createMock(IRequest::class);
+		$this->session = $this->createMock(ISession::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->validateHelper = $this->createMock(ValidateHelper::class);
 		$this->signRequestMapper = $this->createMock(SignRequestMapper::class);
@@ -65,6 +69,7 @@ public function setUp(): void {
 	public function getInjectionMiddleware(): InjectionMiddleware {
 		return new InjectionMiddleware(
 			$this->request,
+			$this->session,
 			$this->userSession,
 			$this->validateHelper,
 			$this->signRequestMapper,
@@ -189,6 +194,27 @@ function (self $self, $message, int $code, $actual):void {
 					);
 				},
 			],
+			[
+				json_encode(['action' => 1000, 'redirect' => 'http://fake.url']), 1, PageException::class,
+				function (self $self, $message, int $code, $actual):void {
+					/** @var RedirectResponse $actual */
+					$self->assertInstanceOf(
+						RedirectResponse::class,
+						$actual,
+						'The response need to be RedirectResponse'
+					);
+					$self->assertEquals(
+						'http://fake.url',
+						$actual->getRedirectURL(),
+						'Invalid redirect URL'
+					);
+					$self->assertEquals(
+						303,
+						$actual->getStatus(),
+						'Invalid response status code'
+					);
+				},
+			],
 		];
 	}
 }
diff --git a/tests/integration/features/page/validate.feature b/tests/integration/features/page/validate.feature
@@ -1,6 +1,7 @@
 Feature: page/validate
   Background: Make setup ok
     Given run the command "config:app:set libresign authkey --value=dummy" with result code 0
+    And run the command "libresign:configure:openssl --cn test" with result code 0
 
   Scenario: Unauthenticated user can see sign page
     Given as user "admin"
@@ -16,5 +17,7 @@ Feature: page/validate
     And as user ""
     When sending "get" to "/apps/libresign/p/validation"
     And the response should be a JSON array with the following mandatory values
-      | key     | value                         |
-      | message | Current user is not logged in |
+      | key      | value                                                                |
+      | errors   | ["You are not logged in. Please log in."]                            |
+      | action   | 1000                                                                 |
+      | redirect | /index.php/login?redirect_url=/index.php/apps/libresign/p/validation |
diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml
@@ -53,11 +53,6 @@
       <code><![CDATA[$storage]]></code>
     </UndefinedDocblockClass>
   </file>
-  <file src="lib/Controller/PageController.php">
-    <InvalidThrow>
-      <code><![CDATA[throw new NotLoggedInException();]]></code>
-    </InvalidThrow>
-  </file>
   <file src="lib/Db/PagerFantaQueryAdapter.php">
     <MissingTemplateParam>
       <code><![CDATA[AdapterInterface]]></code>