From b661c28046d87d56ee254d76c9997609dca76738 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Tue, 20 Oct 2026 19:18:27 -0400 Subject: [PATCH] feat(login-flow): warn when a login flow was started from another network Login flow v2 now remembers the address that started a flow (new nullable column login_flow_v2.initiator_ip). When the flow is granted from another network of the same IP family, the grant page warns the user that someone may be trying to get access to their account, which is what a phishing link to someone else's login flow looks like. A switch between IPv4 and IPv6 does not count, as that is common on dual-stack devices. After a flow is granted, LoginFlowV2GrantedEvent is dispatched with the new app password, the starting address and the start time, so apps can react, e.g. by notifying the user. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- .../ClientFlowLoginV2Controller.php | 25 +++++++- core/Db/LoginFlowV2.php | 5 ++ .../Version36000Date20261020200000.php | 49 ++++++++++++++ core/Service/LoginFlowV2Service.php | 5 +- core/src/views/LoginFlowGrant.vue | 6 ++ lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 2 + .../Events/LoginFlowV2GrantedEvent.php | 64 +++++++++++++++++++ .../ClientFlowLoginV2ControllerTest.php | 45 +++++++++++++ .../Service/LoginFlowV2ServiceUnitTest.php | 23 +++++-- version.php | 2 +- 11 files changed, 220 insertions(+), 8 deletions(-) create mode 100644 core/Migrations/Version36000Date20261020200000.php create mode 100644 lib/public/Authentication/Events/LoginFlowV2GrantedEvent.php diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index 30fbc96898b3e..0efd0ad7e158e 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -36,6 +36,7 @@ use OCP\ISession; use OCP\IURLGenerator; use OCP\IUserSession; +use OCP\Security\Ip\IFactory; use OCP\Security\ISecureRandom; use OCP\Server; use OCP\Util; @@ -60,6 +61,7 @@ public function __construct( private ?string $userId, private IL10N $l10n, private IInitialState $initialState, + private IFactory $ipFactory, ) { parent::__construct($appName, $request); } @@ -176,6 +178,7 @@ public function grantPage(?string $stateToken, int $direct = 0): StandaloneTempl 'instanceName' => $this->defaults->getName(), 'stateToken' => $stateToken, 'direct' => $direct === 1, + 'initiatorAddress' => $this->isStartedFromOtherNetwork($flow) ? $flow->getInitiatorIp() : null, ]); Util::addScript('core', 'login_flow'); @@ -265,6 +268,26 @@ public function generateAppPassword(?string $stateToken): Response { return $this->handleFlowDone($result); } + /** + * Whether the flow was started from another network than the one granting it. + * A switch between IPv4 and IPv6 is common on dual-stack devices and does not count. + */ + private function isStartedFromOtherNetwork(LoginFlowV2 $flow): bool { + $initiatorIp = $flow->getInitiatorIp(); + $currentIp = $this->request->getRemoteAddress(); + if ($initiatorIp === null || $initiatorIp === '' || str_contains($initiatorIp, ':') !== str_contains($currentIp, ':')) { + return false; + } + + $prefix = str_contains($initiatorIp, ':') ? 64 : 24; + try { + $network = $this->ipFactory->rangeFromString($initiatorIp . '/' . $prefix); + return !$network->contains($this->ipFactory->addressFromString($currentIp)); + } catch (\InvalidArgumentException) { + return false; + } + } + private function handleFlowDone(bool $result): StandaloneTemplateResponse { if ($result) { Util::addScript('core', 'login_flow'); @@ -304,7 +327,7 @@ public function init(): JSONResponse { // Get client user agent $userAgent = $this->request->getHeader('user-agent'); - $tokens = $this->loginFlowV2Service->createTokens($userAgent); + $tokens = $this->loginFlowV2Service->createTokens($userAgent, $this->request->getRemoteAddress()); $data = [ 'poll' => [ diff --git a/core/Db/LoginFlowV2.php b/core/Db/LoginFlowV2.php index 1a56ae3237d00..d2ee4887c2440 100644 --- a/core/Db/LoginFlowV2.php +++ b/core/Db/LoginFlowV2.php @@ -33,6 +33,8 @@ * @method void setServer(string $server) * @method string getAppPassword() * @method void setAppPassword(string $appPassword) + * @method string|null getInitiatorIp() + * @method void setInitiatorIp(?string $initiatorIp) */ class LoginFlowV2 extends Entity { /** @var int */ @@ -55,6 +57,8 @@ class LoginFlowV2 extends Entity { protected $server; /** @var string */ protected $appPassword; + /** @var string|null */ + protected $initiatorIp; public function __construct() { $this->addType('timestamp', Types::INTEGER); @@ -67,5 +71,6 @@ public function __construct() { $this->addType('loginName', 'string'); $this->addType('server', 'string'); $this->addType('appPassword', 'string'); + $this->addType('initiatorIp', 'string'); } } diff --git a/core/Migrations/Version36000Date20261020200000.php b/core/Migrations/Version36000Date20261020200000.php new file mode 100644 index 0000000000000..98a5bddfd3f1c --- /dev/null +++ b/core/Migrations/Version36000Date20261020200000.php @@ -0,0 +1,49 @@ +hasTable('login_flow_v2')) { + $table = $schema->getTable('login_flow_v2'); + if (!$table->hasColumn('initiator_ip')) { + $table->addColumn('initiator_ip', Types::STRING, [ + 'notnull' => false, + 'length' => 45, + ]); + return $schema; + } + } + + return null; + } +} diff --git a/core/Service/LoginFlowV2Service.php b/core/Service/LoginFlowV2Service.php index 47707b909ecf6..9ae817190759a 100644 --- a/core/Service/LoginFlowV2Service.php +++ b/core/Service/LoginFlowV2Service.php @@ -20,6 +20,7 @@ use OC\Core\Exception\LoginFlowV2NotFoundException; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\Authentication\Events\LoginFlowV2GrantedEvent; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; @@ -179,6 +180,7 @@ public function flowDone(string $loginToken, string $sessionId, string $server, $data->setAppPassword($this->encryptPassword($appPassword, $data->getPublicKey())); $this->mapper->update($data); + $this->eventDispatcher->dispatchTyped(new LoginFlowV2GrantedEvent($generatedToken, $data->getInitiatorIp(), $data->getTimestamp())); return true; } @@ -199,7 +201,7 @@ public function flowDoneWithAppPassword(string $loginToken, string $server, stri return true; } - public function createTokens(string $userAgent): LoginFlowV2Tokens { + public function createTokens(string $userAgent, string $initiatorIp): LoginFlowV2Tokens { $flow = new LoginFlowV2(); $pollToken = $this->random->generate(128, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER); $loginToken = $this->random->generate(128, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER); @@ -208,6 +210,7 @@ public function createTokens(string $userAgent): LoginFlowV2Tokens { $flow->setStarted(0); $flow->setTimestamp($this->time->getTime()); $flow->setClientName($userAgent); + $flow->setInitiatorIp($initiatorIp); [$publicKey, $privateKey] = $this->getKeyPair(); $privateKey = $this->crypto->encrypt($privateKey, $pollToken); diff --git a/core/src/views/LoginFlowGrant.vue b/core/src/views/LoginFlowGrant.vue index b2230bf9da69c..58e7df8b2a1dd 100644 --- a/core/src/views/LoginFlowGrant.vue +++ b/core/src/views/LoginFlowGrant.vue @@ -20,6 +20,7 @@ const { actionUrl, client, direct, + initiatorAddress, instanceName, stateToken, userDisplayName, @@ -32,6 +33,7 @@ const { actionUrl: string client: string direct: boolean + initiatorAddress?: string | null instanceName: string stateToken: string userId: string @@ -65,6 +67,10 @@ async function onSubmit(event: SubmitEvent) { {{ t('core', 'You are about to grant "{client}" access to your {instanceName} account.', { client, instanceName }) }} + + {{ t('core', 'This request was started from {address}, which is not the network you are using now. Only grant access if you started it on your own device just now. Otherwise someone may be trying to get access to your account.', { address: initiatorAddress }) }} + +
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 4467e4e6179c2..3557e2b3fd1f9 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -216,6 +216,7 @@ 'OCP\\App\\ManagerEvent' => $baseDir . '/lib/public/App/ManagerEvent.php', 'OCP\\Authentication\\Events\\AnyLoginFailedEvent' => $baseDir . '/lib/public/Authentication/Events/AnyLoginFailedEvent.php', 'OCP\\Authentication\\Events\\LoginFailedEvent' => $baseDir . '/lib/public/Authentication/Events/LoginFailedEvent.php', + 'OCP\\Authentication\\Events\\LoginFlowV2GrantedEvent' => $baseDir . '/lib/public/Authentication/Events/LoginFlowV2GrantedEvent.php', 'OCP\\Authentication\\Events\\TokenInvalidatedEvent' => $baseDir . '/lib/public/Authentication/Events/TokenInvalidatedEvent.php', 'OCP\\Authentication\\Exceptions\\CredentialsUnavailableException' => $baseDir . '/lib/public/Authentication/Exceptions/CredentialsUnavailableException.php', 'OCP\\Authentication\\Exceptions\\ExpiredTokenException' => $baseDir . '/lib/public/Authentication/Exceptions/ExpiredTokenException.php', @@ -1758,6 +1759,7 @@ 'OC\\Core\\Migrations\\Version34000Date20260518163022' => $baseDir . '/core/Migrations/Version34000Date20260518163022.php', 'OC\\Core\\Migrations\\Version34000Date20260521110333' => $baseDir . '/core/Migrations/Version34000Date20260521110333.php', 'OC\\Core\\Migrations\\Version35000Date20260527162338' => $baseDir . '/core/Migrations/Version35000Date20260527162338.php', + 'OC\\Core\\Migrations\\Version36000Date20261020200000' => $baseDir . '/core/Migrations/Version36000Date20261020200000.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php', 'OC\\Core\\Service\\CronService' => $baseDir . '/core/Service/CronService.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index dac59af04087a..54f59cab3eedc 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -257,6 +257,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\App\\ManagerEvent' => __DIR__ . '/../../..' . '/lib/public/App/ManagerEvent.php', 'OCP\\Authentication\\Events\\AnyLoginFailedEvent' => __DIR__ . '/../../..' . '/lib/public/Authentication/Events/AnyLoginFailedEvent.php', 'OCP\\Authentication\\Events\\LoginFailedEvent' => __DIR__ . '/../../..' . '/lib/public/Authentication/Events/LoginFailedEvent.php', + 'OCP\\Authentication\\Events\\LoginFlowV2GrantedEvent' => __DIR__ . '/../../..' . '/lib/public/Authentication/Events/LoginFlowV2GrantedEvent.php', 'OCP\\Authentication\\Events\\TokenInvalidatedEvent' => __DIR__ . '/../../..' . '/lib/public/Authentication/Events/TokenInvalidatedEvent.php', 'OCP\\Authentication\\Exceptions\\CredentialsUnavailableException' => __DIR__ . '/../../..' . '/lib/public/Authentication/Exceptions/CredentialsUnavailableException.php', 'OCP\\Authentication\\Exceptions\\ExpiredTokenException' => __DIR__ . '/../../..' . '/lib/public/Authentication/Exceptions/ExpiredTokenException.php', @@ -1799,6 +1800,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version34000Date20260518163022' => __DIR__ . '/../../..' . '/core/Migrations/Version34000Date20260518163022.php', 'OC\\Core\\Migrations\\Version34000Date20260521110333' => __DIR__ . '/../../..' . '/core/Migrations/Version34000Date20260521110333.php', 'OC\\Core\\Migrations\\Version35000Date20260527162338' => __DIR__ . '/../../..' . '/core/Migrations/Version35000Date20260527162338.php', + 'OC\\Core\\Migrations\\Version36000Date20261020200000' => __DIR__ . '/../../..' . '/core/Migrations/Version36000Date20261020200000.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php', 'OC\\Core\\Service\\CronService' => __DIR__ . '/../../..' . '/core/Service/CronService.php', diff --git a/lib/public/Authentication/Events/LoginFlowV2GrantedEvent.php b/lib/public/Authentication/Events/LoginFlowV2GrantedEvent.php new file mode 100644 index 0000000000000..b3b57382dbbc6 --- /dev/null +++ b/lib/public/Authentication/Events/LoginFlowV2GrantedEvent.php @@ -0,0 +1,64 @@ +token; + } + + /** + * The address the flow was started from, or null for flows started + * before it was recorded + * + * @since 36.0.0 + */ + public function getInitiatorIp(): ?string { + return $this->initiatorIp; + } + + /** + * When the flow was started, as a Unix timestamp + * + * @since 36.0.0 + */ + public function getStartedAt(): int { + return $this->startedAt; + } +} diff --git a/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php b/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php index 6a2a27b7cee13..071139d2403e8 100644 --- a/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php @@ -14,6 +14,7 @@ use OC\Core\Exception\LoginFlowV2ClientForbiddenException; use OC\Core\Exception\LoginFlowV2NotFoundException; use OC\Core\Service\LoginFlowV2Service; +use OC\Security\Ip\Factory as IpFactory; use OCP\AppFramework\Http; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\StandaloneTemplateResponse; @@ -73,6 +74,7 @@ protected function setUp(): void { 'user', $this->l, $this->initialState, + new IpFactory(), ); } @@ -280,6 +282,49 @@ public function testGrantPageValid(): void { $this->assertSame(Http::STATUS_OK, $result->getStatus()); } + public static function dataGrantPageInitiatorAddress(): array { + return [ + 'not recorded' => [null, '203.0.113.5', null], + 'same IPv4 network' => ['203.0.113.77', '203.0.113.5', null], + 'other IPv4 network' => ['198.51.100.7', '203.0.113.5', '198.51.100.7'], + 'same IPv6 network' => ['2001:db8:1:2::abcd', '2001:db8:1:2::1', null], + 'other IPv6 network' => ['2001:db8:9:9::1', '2001:db8:1:2::1', '2001:db8:9:9::1'], + 'dual stack' => ['198.51.100.7', '2001:db8:1:2::1', null], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataGrantPageInitiatorAddress')] + public function testGrantPageInitiatorAddress(?string $initiatorIp, string $currentIp, ?string $expected): void { + $this->session->method('get') + ->willReturnCallback(static fn (string $name): ?string => match ($name) { + 'client.flow.v2.state.token' => 'stateToken', + 'client.flow.v2.login.token' => 'loginToken', + default => null, + }); + $this->userSession->method('getUser') + ->willReturn($this->createMock(IUser::class)); + $this->request->method('getRemoteAddress') + ->willReturn($currentIp); + + $flow = new LoginFlowV2(); + $flow->setInitiatorIp($initiatorIp); + $this->loginFlowV2Service->method('getByLoginToken') + ->with('loginToken') + ->willReturn($flow); + + $grantState = null; + $this->initialState->method('provideInitialState') + ->willReturnCallback(static function (string $key, mixed $data) use (&$grantState): void { + if ($key === 'loginFlowGrant') { + $grantState = $data; + } + }); + + $this->controller->grantPage('stateToken'); + + $this->assertSame($expected, $grantState['initiatorAddress']); + } + public function testGenerateAppPasswordInvalidStateToken(): void { $this->session->method('get') ->willReturnCallback(function ($name) { diff --git a/tests/Core/Service/LoginFlowV2ServiceUnitTest.php b/tests/Core/Service/LoginFlowV2ServiceUnitTest.php index 300626b9bc5d4..12407e7b0c496 100644 --- a/tests/Core/Service/LoginFlowV2ServiceUnitTest.php +++ b/tests/Core/Service/LoginFlowV2ServiceUnitTest.php @@ -20,6 +20,7 @@ use OC\Core\Service\LoginFlowV2Service; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\Authentication\Events\LoginFlowV2GrantedEvent; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\Log\Audit\CriticalActionPerformedEvent; @@ -348,6 +349,8 @@ public function testFlowDone(): void { $loginFlowV2 = new LoginFlowV2(); $loginFlowV2->setPublicKey($publicKey); $loginFlowV2->setClientName('client_name'); + $loginFlowV2->setInitiatorIp('198.51.100.7'); + $loginFlowV2->setTimestamp(1700000000); $this->mapper->expects($this->once()) ->method('getByLoginToken') @@ -392,10 +395,12 @@ public function testFlowDone(): void { ) ->willReturn($appToken); - $this->eventDispatcher->expects($this->once()) + $events = []; + $this->eventDispatcher->expects($this->exactly(2)) ->method('dispatchTyped') - ->with($this->callback(fn (CriticalActionPerformedEvent $event): bool => $event->getOperation() === 'auth.app_password.created' - && $event->getParameters() === ['tokenId' => 7, 'tokenName' => 'client_name'])); + ->willReturnCallback(static function (object $event) use (&$events): void { + $events[] = $event; + }); $result = $this->subjectUnderTest->flowDone( 'login_token', @@ -405,6 +410,13 @@ public function testFlowDone(): void { ); $this->assertTrue($result); + $this->assertInstanceOf(CriticalActionPerformedEvent::class, $events[0]); + $this->assertSame(['tokenId' => 7, 'tokenName' => 'client_name'], $events[0]->getParameters()); + $this->assertInstanceOf(LoginFlowV2GrantedEvent::class, $events[1]); + $this->assertSame($appToken, $events[1]->getToken()); + $this->assertSame('198.51.100.7', $events[1]->getInitiatorIp()); + $this->assertSame(1700000000, $events[1]->getStartedAt()); + // app password is encrypted and must look like: // ZACZOOzxTpKz4+KXL5kZ/gCK0xvkaVi/8yzupAn6Ui6+5qCSKvfPKGgeDRKs0sivvSLzk/XSp811SZCZmH0Y3g== $this->assertMatchesRegularExpression('/[a-zA-Z\/0-9+=]+/', $loginFlowV2->getAppPassword()); @@ -453,12 +465,13 @@ public function testCreateTokens(): void { }); $this->mapper->expects($this->once()) - ->method('insert'); + ->method('insert') + ->with($this->callback(static fn (LoginFlowV2 $flow): bool => $flow->getInitiatorIp() === '198.51.100.7')); $this->secureRandom->expects($this->exactly(2)) ->method('generate'); - $token = $this->subjectUnderTest->createTokens('user_agent'); + $token = $this->subjectUnderTest->createTokens('user_agent', '198.51.100.7'); $this->assertTrue($token instanceof LoginFlowV2Tokens); } } diff --git a/version.php b/version.php index 31e5cb2bcda4c..ca54c28709bc6 100644 --- a/version.php +++ b/version.php @@ -11,7 +11,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level // when updating major/minor version number. -$OC_Version = [36, 0, 0, 0]; +$OC_Version = [36, 0, 0, 1]; // The human-readable string $OC_VersionString = '36.0.0 dev';