From 81c52f667cc550d4d3fe8617079bf1a1e8ba3ea0 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 e7580437..5735cce0 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 (!str_starts_with($authHeader, '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 71581a52..fc56616b 100644 --- a/tests/Unit/Service/LoginClassifierTest.php +++ b/tests/Unit/Service/LoginClassifierTest.php @@ -168,4 +168,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'); + } }