Move from app.php to Application.php

Use event `OCP\User\Events\BeforeUserLoggedInEvent`
nextcloud · Aug 7, 2023 · 65b3eac · 65b3eac
1 parent acd1670
commit 65b3eac
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 47 deletions.
diff --git a/appinfo/app.php → lib/AppInfo/Application.php b/appinfo/app.php → lib/AppInfo/Application.php
@@ -1,7 +1,12 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
  *
+ * @author Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
+ *
  * @license GNU AGPL version 3 or any later version
  *
  * This program is free software: you can redistribute it and/or modify
@@ -16,33 +21,31 @@
  *
  * You should have received a copy of the GNU Affero General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
  */
 
+namespace OCA\LimitLoginToIp\AppInfo;
 
-$l10n = \OC::$server->getL10N('limit_login_to_ip');
-$config = \OC::$server->getConfig();
-$request = \OC::$server->getRequest();
-$urlGenerator = \OC::$server->getURLGenerator();
-$isLoginPage = parse_url($request->getRequestUri(), PHP_URL_PATH) === $urlGenerator->linkToRoute('core.login.showLoginForm');
+use OCA\LimitLoginToIp\LoginHookListener;
+use OCP\AppFramework\App;
+use OCP\AppFramework\Bootstrap\IBootContext;
+use OCP\AppFramework\Bootstrap\IBootstrap;
+use OCP\AppFramework\Bootstrap\IRegistrationContext;
+use OCP\User\Events\BeforeUserLoggedInEvent;
 
-$loginHookListener = new \OCA\LimitLoginToIp\LoginHookListener(
-	$config,
-	$request,
-	$urlGenerator,
-	$isLoginPage
-);
+class Application extends App implements IBootstrap {
+	public const APP_ID = 'limit_login_to_ip';
 
-if(!$loginHookListener->isLoginAllowed()) {
-	if($isLoginPage) {
-		header('Location: ' . \OC::$WEBROOT . '/index.php/apps/limit_login_to_ip/denied');
-		exit();
+	public function __construct() {
+		parent::__construct(self::APP_ID);
 	}
 
-	\OCP\Util::connectHook(
-		'OC_User',
-		'pre_login',
-		$loginHookListener,
-		'handleLoginRequest'
-	);
+	public function register(IRegistrationContext $context): void {
+		$context->registerEventListener(
+			BeforeUserLoggedInEvent::class,
+			LoginHookListener::class,
+		);
+	}
+
+	public function boot(IBootContext $context): void {
+	}
 }
diff --git a/lib/LoginHookListener.php b/lib/LoginHookListener.php
@@ -33,11 +33,32 @@ public function __construct(
 		private IConfig $config,
 		private IRequest $request,
 		private IURLGenerator $urlGenerator,
-		private bool $isLoginPage
 	) {
 	}
 
-	public function isLoginAllowed(): bool {
+	public function handle(): void {
+		if ($this->isLoginAllowed()) {
+			return;
+		}
+
+		// Web UI
+		if ($this->isLoginPage()) {
+			$url = $this->urlGenerator->linkToRouteAbsolute('limit_login_to_ip.LoginDenied.showErrorPage');
+			header('Location: ' . $url);
+			exit();
+		}
+
+		// All other clients
+		http_response_code(403);
+		exit();
+	}
+
+	private function isLoginPage(): bool {
+		return parse_url($this->request->getRequestUri(), PHP_URL_PATH) 
+			=== $this->urlGenerator->linkToRoute('core.login.showLoginForm');
+	}
+
+	private function isLoginAllowed(): bool {
 		/** @psalm-suppress RedundantCastGivenDocblockType */
 		$allowedRanges = (string) $this->config->getAppValue('limit_login_to_ip', 'whitelisted.ranges', '');
 		if ('' === $allowedRanges) {
@@ -62,17 +83,4 @@ public function isLoginAllowed(): bool {
 
 		return false;
 	}
-
-	public function handleLoginRequest(): void {
-		// Web UI
-		if($this->isLoginPage) {
-			$url = $this->urlGenerator->linkToRouteAbsolute('limit_login_to_ip.LoginDenied.showErrorPage');
-			header('Location: ' . $url);
-			exit();
-		}
-
-		// All other clients
-		http_response_code(403);
-		exit();
-	}
 }
diff --git a/tests/integration/features/Login.feature b/tests/integration/features/Login.feature
@@ -6,12 +6,11 @@ Feature: Login
     Then the response status code should be "403"
     And the response URL should be "http://localhost:8080/index.php/apps/limit_login_to_ip/denied"
 
-  # FIXME Broken feature
-  # Scenario: Authenticating with blocked IP via API
-  #   Given The range "192.168.0.0/24" is permitted
-  #   When I try to login via "api"
-  #   Then the response status code should be "403"
-  #   And the response URL should be "http://localhost:8080/remote.php/webdav/"
+  Scenario: Authenticating with blocked IP via API
+    Given The range "192.168.0.0/24" is permitted
+    When I try to login via "api"
+    Then the response status code should be "403"
+    And the response URL should be "http://localhost:8080/remote.php/webdav/"
 
   Scenario: Authenticating with whitelisted IP via Web
     Given The range "127.0.0.0/24" is permitted

diff --git a/tests/unit/LoginHookListenerTest.php b/tests/unit/LoginHookListenerTest.php
@@ -28,6 +28,7 @@
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use OCP\User\Events\BeforeUserLoggedInEvent;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
@@ -38,6 +39,8 @@ class LoginHookListenerTest extends TestCase {
 	private $request;
 	/** @var IURLGenerator|MockObject */
 	private $urlGenerator;
+	/** @var BeforeUserLoggedInEvent|MockObject */
+	private $event;
 	private LoginHookListener $listener;
 
 	protected function setUp(): void {
@@ -46,19 +49,28 @@ protected function setUp(): void {
 		$this->config = $this->createMock(IConfig::class);
 		$this->request = $this->createMock(IRequest::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->event = $this->createMock(BeforeUserLoggedInEvent::class);
 
 		$this->listener = new LoginHookListener(
 			$this->config,
 			$this->request,
 			$this->urlGenerator,
-			true,
 		);
 	}
 
+	protected function setUpOnce(): void {
+			function http_response_code(int $code) {
+				$this->lastHttpCode = $code;
+			}
+			function header() {
+			}
+	}
+
 	/**
 	 * @dataProvider ipSubnetProvider
 	 */
 	public function testMatchRange(string $ipAddress, ?string $subnet, bool $shouldBeTrue): void {
+		$this->lastHttpCode = 200;
 		if (!empty($subnet)) {
 			$this->config->expects($this->once())
 				->method('getAppValue')
@@ -69,10 +81,15 @@ public function testMatchRange(string $ipAddress, ?string $subnet, bool $shouldB
 				->method('getRemoteAddress')
 				->willReturn($ipAddress);
 		}
-		$shouldBeTrue
-			? $this->assertTrue($this->listener->isLoginAllowed())
-			: $this->assertFalse($this->listener->isLoginAllowed());
 
+		$result = $this->listener->handle($this->event);
+
+
+		if ($shouldBeTrue) {
+			$this->assertEquals(200, $this->lastHttpCode);	
+		} else {
+			$this->assertEquals(403, $this->lastHttpCode);	
+		}
 	}
 
 	public function ipSubnetProvider(): array {