From 1bc049f4ef73df5f2604ebd615151f2309359509 Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Fri, 14 Aug 2026 13:08:49 +0200 Subject: [PATCH] fix: avoid double app password check The server already tests the password to be an app password and passes that info through the event. The login listener has an early exit. Yet this app also tested the shape of the password, which can lead to false negatives. Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/Service/LoginClassifier.php | 35 ---------------------- tests/Unit/Service/LoginClassifierTest.php | 19 ++++++++++++ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/lib/Service/LoginClassifier.php b/lib/Service/LoginClassifier.php index 467e444e..15a11dca 100644 --- a/lib/Service/LoginClassifier.php +++ b/lib/Service/LoginClassifier.php @@ -20,11 +20,6 @@ use OCP\IRequest; use Psr\Log\LoggerInterface; use Throwable; -use function base64_decode; -use function explode; -use function preg_match; -use function strlen; -use function substr; class LoginClassifier { @@ -38,37 +33,7 @@ public function __construct( ) { } - /** - * @todo find a more reliable way of checking this - */ - private function isAuthenticatedWithAppPassword(IRequest $request): bool { - $authHeader = $request->getHeader('Authorization'); - if (empty($authHeader)) { - return false; - } - if (substr($authHeader, 0, strlen('Basic ')) !== 'Basic ') { - return false; - } - $pwd = explode( - ':', - base64_decode(substr($authHeader, strlen('Basic '))) - ); - if (!isset($pwd[1])) { - return false; - } - - return preg_match( - '/^([0-9A-Za-z]{5})-([0-9A-Za-z]{5})-([0-9A-Za-z]{5})-([0-9A-Za-z]{5})-([0-9A-Za-z]{5})$/', - $pwd[1] - ) === 1; - } - public function process(string $uid, string $ip) { - if ($this->isAuthenticatedWithAppPassword($this->request)) { - // We don't care about those logins - $this->logger->debug('App password detected. No address classification is performed'); - return; - } try { $strategy = AddressClassifier::isIpV4($ip) ? new Ipv4Strategy() : new IpV6Strategy(); if ($this->estimator->predict($uid, $ip, $strategy)) { diff --git a/tests/Unit/Service/LoginClassifierTest.php b/tests/Unit/Service/LoginClassifierTest.php index 43f64dd2..5b2b7810 100644 --- a/tests/Unit/Service/LoginClassifierTest.php +++ b/tests/Unit/Service/LoginClassifierTest.php @@ -166,4 +166,23 @@ public function testProcessNoPeakReached(): void { $this->classifier->process('user', '1.2.3.4'); } + + public function testProcessClassifiesRealPasswordShapedLikeAppPassword(): void { + // A genuine account password that happens to match the app-password display format. + $realPassword = 'abcde-fghij-klmno-pqrst-uvwxy'; + $this->request->method('getHeader') + ->with('Authorization') + ->willReturn('Basic ' . base64_encode('user:' . $realPassword)); + $this->timeFactory->method('getTime')->willReturn(1000000); + $this->estimatorService->expects(self::once()) + ->method('predict') + ->with('user', '1.2.3.4', self::equalTo(new Ipv4Strategy())) + ->willReturn(false); + $this->mapper->method('findRelated')->willReturn([]); + $this->mapper->method('findRecentByUid')->willReturn([]); + $this->dispatcher->expects(self::once()) + ->method('dispatchTyped'); + + $this->classifier->process('user', '1.2.3.4'); + } }