diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php
index 9e994b80eb9..7cd8e8655dc 100644
--- a/apps/oauth2/lib/Controller/SettingsController.php
+++ b/apps/oauth2/lib/Controller/SettingsController.php
@@ -8,6 +8,7 @@ declare(strict_types=1);
*/
namespace OCA\OAuth2\Controller;
+use OC\Authentication\Token\IProvider as IAuthTokenProvider;
use OCA\OAuth2\Db\AccessTokenMapper;
use OCA\OAuth2\Db\Client;
use OCA\OAuth2\Db\ClientMapper;
@@ -15,13 +16,15 @@ use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
use OCP\AppFramework\Http\JSONResponse;
-use OCP\Authentication\Token\IProvider as IAuthTokenProvider;
+use OCP\Authentication\Exceptions\InvalidTokenException;
+use OCP\Authentication\Exceptions\WipeTokenException;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Security\ICrypto;
use OCP\Security\ISecureRandom;
+use Psr\Log\LoggerInterface;
class SettingsController extends Controller {
@@ -37,6 +40,7 @@ class SettingsController extends Controller {
private IAuthTokenProvider $tokenProvider,
private IUserManager $userManager,
private ICrypto $crypto,
+ private LoggerInterface $logger,
) {
parent::__construct($appName, $request);
}
@@ -73,7 +77,26 @@ class SettingsController extends Controller {
$client = $this->clientMapper->getByUid($id);
$this->userManager->callForSeenUsers(function (IUser $user) use ($client): void {
- $this->tokenProvider->invalidateTokensOfUser($user->getUID(), $client->getName());
+ // Skip tokens that are marked for remote wipe so revoking the
+ // OAuth2 client does not silently cancel a pending wipe.
+ $tokens = $this->tokenProvider->getTokenByUser($user->getUID());
+ foreach ($tokens as $token) {
+ if ($token->getName() !== $client->getName()) {
+ continue;
+ }
+ try {
+ $this->tokenProvider->getTokenById($token->getId());
+ } catch (WipeTokenException $e) {
+ $this->logger->info('Preserving token {tokenId} of user {uid}: marked for remote wipe, OAuth2 client revoke would cancel the wipe.', [
+ 'tokenId' => $token->getId(),
+ 'uid' => $user->getUID(),
+ ]);
+ continue;
+ } catch (InvalidTokenException $e) {
+ // Token already invalid; let invalidateTokenById handle it.
+ }
+ $this->tokenProvider->invalidateTokenById($user->getUID(), $token->getId());
+ }
});
$this->accessTokenMapper->deleteByClientId($id);
diff --git a/apps/oauth2/tests/Controller/SettingsControllerTest.php b/apps/oauth2/tests/Controller/SettingsControllerTest.php
index f7b9fb39257..41db7fe595a 100644
--- a/apps/oauth2/tests/Controller/SettingsControllerTest.php
+++ b/apps/oauth2/tests/Controller/SettingsControllerTest.php
@@ -6,13 +6,15 @@
*/
namespace OCA\OAuth2\Tests\Controller;
+use OC\Authentication\Token\IProvider as IAuthTokenProvider;
use OCA\OAuth2\Controller\SettingsController;
use OCA\OAuth2\Db\AccessTokenMapper;
use OCA\OAuth2\Db\Client;
use OCA\OAuth2\Db\ClientMapper;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\JSONResponse;
-use OCP\Authentication\Token\IProvider as IAuthTokenProvider;
+use OCP\Authentication\Exceptions\WipeTokenException;
+use OCP\Authentication\Token\IToken;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUser;
@@ -20,6 +22,8 @@ use OCP\IUserManager;
use OCP\Security\ICrypto;
use OCP\Security\ISecureRandom;
use OCP\Server;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
use Test\TestCase;
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
@@ -42,6 +46,7 @@ class SettingsControllerTest extends TestCase {
private $l;
/** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */
private $crypto;
+ private LoggerInterface&MockObject $logger;
protected function setUp(): void {
parent::setUp();
@@ -53,6 +58,7 @@ class SettingsControllerTest extends TestCase {
$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->crypto = $this->createMock(ICrypto::class);
+ $this->logger = $this->createMock(LoggerInterface::class);
$this->l = $this->createMock(IL10N::class);
$this->l->method('t')
->willReturnArgument(0);
@@ -65,7 +71,8 @@ class SettingsControllerTest extends TestCase {
$this->l,
$this->authTokenProvider,
$this->userManager,
- $this->crypto
+ $this->crypto,
+ $this->logger,
);
}
@@ -132,11 +139,15 @@ class SettingsControllerTest extends TestCase {
$user1->updateLastLoginTimestamp();
$tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
- // expect one call per user and ensure the correct client name
+ // One getTokenByUser call per user; we return no matching tokens here
+ // so invalidateTokenById is never invoked.
$tokenProviderMock
->expects($this->exactly($count + 1))
- ->method('invalidateTokensOfUser')
- ->with($this->isType('string'), 'My Client Name');
+ ->method('getTokenByUser')
+ ->willReturn([]);
+ $tokenProviderMock
+ ->expects($this->never())
+ ->method('invalidateTokenById');
$client = new Client();
$client->setId(123);
@@ -167,7 +178,8 @@ class SettingsControllerTest extends TestCase {
$this->l,
$tokenProviderMock,
$userManager,
- $this->crypto
+ $this->crypto,
+ $this->logger,
);
$result = $settingsController->deleteClient(123);
@@ -177,6 +189,96 @@ class SettingsControllerTest extends TestCase {
$user1->delete();
}
+ public function testDeleteClientPreservesWipePendingToken(): void {
+ $userManager = Server::get(IUserManager::class);
+ $user = $userManager->createUser('test_wipe_preserve', 'test_wipe_preserve');
+ $user->updateLastLoginTimestamp();
+
+ $client = new Client();
+ $client->setId(456);
+ $client->setName('My Client Name');
+ $client->setRedirectUri('https://example.com/');
+ $client->setSecret(bin2hex('MyHashedSecret'));
+ $client->setClientIdentifier('MyClientIdentifier');
+
+ // Token marked for wipe with a matching client name: must NOT be invalidated.
+ $wipeToken = $this->createMock(IToken::class);
+ $wipeToken->method('getId')->willReturn(11);
+ $wipeToken->method('getName')->willReturn('My Client Name');
+
+ // Regular token with matching name: must be invalidated.
+ $regularToken = $this->createMock(IToken::class);
+ $regularToken->method('getId')->willReturn(12);
+ $regularToken->method('getName')->willReturn('My Client Name');
+
+ // Non-matching name: must be left alone.
+ $otherToken = $this->createMock(IToken::class);
+ $otherToken->method('getId')->willReturn(13);
+ $otherToken->method('getName')->willReturn('Some Other Client');
+
+ $tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
+ $tokenProviderMock
+ ->method('getTokenByUser')
+ ->willReturnCallback(function (string $uid) use ($wipeToken, $regularToken, $otherToken) {
+ return $uid === 'test_wipe_preserve'
+ ? [$wipeToken, $regularToken, $otherToken]
+ : [];
+ });
+ // Wipe state is signalled via WipeTokenException from getTokenById.
+ $tokenProviderMock
+ ->method('getTokenById')
+ ->willReturnCallback(function (int $id) use ($wipeToken, $regularToken) {
+ if ($id === 11) {
+ throw new WipeTokenException($wipeToken);
+ }
+ return $regularToken;
+ });
+ $tokenProviderMock
+ ->expects($this->once())
+ ->method('invalidateTokenById')
+ ->with('test_wipe_preserve', 12);
+
+ $this->clientMapper
+ ->method('getByUid')
+ ->with(456)
+ ->willReturn($client);
+ $this->accessTokenMapper
+ ->expects($this->once())
+ ->method('deleteByClientId')
+ ->with(456);
+ $this->clientMapper
+ ->expects($this->once())
+ ->method('delete')
+ ->with($client);
+
+ $logger = $this->createMock(LoggerInterface::class);
+ $logger->expects($this->atLeastOnce())
+ ->method('info')
+ ->with($this->stringContains('Preserving token'), $this->callback(function (array $context) {
+ return ($context['tokenId'] ?? null) === 11
+ && ($context['uid'] ?? null) === 'test_wipe_preserve';
+ }));
+
+ $settingsController = new SettingsController(
+ 'oauth2',
+ $this->request,
+ $this->clientMapper,
+ $this->secureRandom,
+ $this->accessTokenMapper,
+ $this->l,
+ $tokenProviderMock,
+ $userManager,
+ $this->crypto,
+ $logger,
+ );
+
+ $result = $settingsController->deleteClient(456);
+ $this->assertInstanceOf(JSONResponse::class, $result);
+ $this->assertEquals([], $result->getData());
+
+ $user->delete();
+ }
+
public function testInvalidRedirectUri(): void {
$result = $this->settingsController->addClient('test', 'invalidurl');
diff --git a/apps/settings/lib/Activity/Provider.php b/apps/settings/lib/Activity/Provider.php
index 52c9f90d449..f6489833de3 100644
--- a/apps/settings/lib/Activity/Provider.php
+++ b/apps/settings/lib/Activity/Provider.php
@@ -27,6 +27,7 @@ class Provider implements IProvider {
public const EMAIL_CHANGED = 'email_changed';
public const APP_TOKEN_CREATED = 'app_token_created';
public const APP_TOKEN_DELETED = 'app_token_deleted';
+ public const APP_TOKEN_DELETED_WIPE_CANCELLED = 'app_token_deleted_wipe_cancelled';
public const APP_TOKEN_RENAMED = 'app_token_renamed';
public const APP_TOKEN_FILESYSTEM_GRANTED = 'app_token_filesystem_granted';
public const APP_TOKEN_FILESYSTEM_REVOKED = 'app_token_filesystem_revoked';
@@ -86,6 +87,8 @@ class Provider implements IProvider {
}
} elseif ($event->getSubject() === self::APP_TOKEN_DELETED) {
$subject = $this->l->t('You deleted app password "{token}"');
+ } elseif ($event->getSubject() === self::APP_TOKEN_DELETED_WIPE_CANCELLED) {
+ $subject = $this->l->t('You deleted app password "{token}" and cancelled its pending remote wipe');
} elseif ($event->getSubject() === self::APP_TOKEN_RENAMED) {
$subject = $this->l->t('You renamed app password "{token}" to "{newToken}"');
} elseif ($event->getSubject() === self::APP_TOKEN_FILESYSTEM_GRANTED) {
@@ -125,6 +128,7 @@ class Provider implements IProvider {
];
case self::APP_TOKEN_CREATED:
case self::APP_TOKEN_DELETED:
+ case self::APP_TOKEN_DELETED_WIPE_CANCELLED:
case self::APP_TOKEN_FILESYSTEM_GRANTED:
case self::APP_TOKEN_FILESYSTEM_REVOKED:
return [
diff --git a/apps/settings/lib/Controller/AuthSettingsController.php b/apps/settings/lib/Controller/AuthSettingsController.php
index f39deeddd4e..dc7fa2ebd82 100644
--- a/apps/settings/lib/Controller/AuthSettingsController.php
+++ b/apps/settings/lib/Controller/AuthSettingsController.php
@@ -179,17 +179,21 @@ class AuthSettingsController extends Controller {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
}
+ $subject = Provider::APP_TOKEN_DELETED;
try {
$token = $this->findTokenByIdAndUser($id);
} catch (WipeTokenException $e) {
- //continue as we can destroy tokens in wipe
+ // Deleting a wipe-pending token cancels the pending wipe; the device
+ // may already be uninstalled so we allow it, but record it under a
+ // distinct subject so the audit trail captures the consequence.
$token = $e->getToken();
+ $subject = Provider::APP_TOKEN_DELETED_WIPE_CANCELLED;
} catch (InvalidTokenException $e) {
return new JSONResponse([], Http::STATUS_NOT_FOUND);
}
$this->tokenProvider->invalidateTokenById($this->userId, $token->getId());
- $this->publishActivity(Provider::APP_TOKEN_DELETED, $token->getId(), ['name' => $token->getName()]);
+ $this->publishActivity($subject, $token->getId(), ['name' => $token->getName()]);
return [];
}
diff --git a/apps/settings/src/components/AuthToken.spec.ts b/apps/settings/src/components/AuthToken.spec.ts
index fb3c0c06453..ca63bf77a76 100644
--- a/apps/settings/src/components/AuthToken.spec.ts
+++ b/apps/settings/src/components/AuthToken.spec.ts
@@ -119,7 +119,8 @@ describe('AuthToken revoke flow', () => {
dialog.vm.$emit('update:open', false)
await wrapper.vm.$nextTick()
- expect(dialog.props('open')).toBe(false)
+ // Dialog is v-if'd off the tree once closed
+ expect(wrapper.findComponent(AuthTokenDeleteDialog).exists()).toBe(false)
expect(store.deleteToken).not.toHaveBeenCalled()
})
diff --git a/apps/settings/src/components/AuthToken.vue b/apps/settings/src/components/AuthToken.vue
index 0d9662952a8..d94b9175930 100644
--- a/apps/settings/src/components/AuthToken.vue
+++ b/apps/settings/src/components/AuthToken.vue
@@ -83,11 +83,13 @@
- {{ bodyText }} + {{ copy.body }}
@@ -65,22 +65,19 @@ export default defineComponent({ return this.token.type === TokenType.WIPING_TOKEN }, - dialogTitle(): string { - return this.wiping - ? t('settings', 'Revoke and cancel pending wipe?') - : t('settings', 'Revoke app password?') - }, - - bodyText(): string { - return this.wiping - ? t('settings', 'Only continue if you no longer need the device to be wiped.') - : t('settings', 'The app or device will lose access on its next sync. This cannot be undone.') - }, - - destructiveLabel(): string { - return this.wiping - ? t('settings', 'Revoke and cancel wipe') - : t('settings', 'Revoke') + copy(): { title: string, body: string, action: string } { + if (this.wiping) { + return { + title: t('settings', 'Revoke and cancel pending wipe?'), + body: t('settings', 'Only continue if you no longer need the device to be wiped.'), + action: t('settings', 'Revoke and cancel wipe'), + } + } + return { + title: t('settings', 'Revoke app password?'), + body: t('settings', 'The app or device will lose access on its next sync. This cannot be undone.'), + action: t('settings', 'Revoke'), + } }, buttons(): IDialogButton[] { @@ -93,7 +90,7 @@ export default defineComponent({ }, }, { - label: this.destructiveLabel, + label: this.copy.action, variant: 'error', callback: () => { this.$emit('confirm') diff --git a/apps/settings/tests/Controller/AuthSettingsControllerTest.php b/apps/settings/tests/Controller/AuthSettingsControllerTest.php index 5d75a1aa09a..805e810144a 100644 --- a/apps/settings/tests/Controller/AuthSettingsControllerTest.php +++ b/apps/settings/tests/Controller/AuthSettingsControllerTest.php @@ -233,6 +233,43 @@ class AuthSettingsControllerTest extends TestCase { $this->assertSame([], $this->controller->destroy($tokenId)); } + public function testDestroyWipePendingEmitsCancelledSubject(): void { + $tokenId = 125; + $token = $this->createMock(PublicKeyToken::class); + + $token->method('getId')->willReturn($tokenId); + $token->method('getName')->willReturn('My phone'); + + $this->tokenProvider->expects($this->once()) + ->method('getTokenById') + ->with($tokenId) + ->willThrowException(new \OCP\Authentication\Exceptions\WipeTokenException($token)); + + // The token is still invalidated (the user opted into cancelling the wipe). + $this->tokenProvider->expects($this->once()) + ->method('invalidateTokenById') + ->with($this->uid, $tokenId); + + // Activity event must use the distinguishing subject. + $event = $this->createMock(IEvent::class); + $event->method('setApp')->willReturnSelf(); + $event->method('setType')->willReturnSelf(); + $event->method('setAffectedUser')->willReturnSelf(); + $event->method('setAuthor')->willReturnSelf(); + $event->method('setObject')->willReturnSelf(); + $event->expects($this->once()) + ->method('setSubject') + ->with(\OCA\Settings\Activity\Provider::APP_TOKEN_DELETED_WIPE_CANCELLED, ['name' => 'My phone']) + ->willReturnSelf(); + $this->activityManager->expects($this->once()) + ->method('generateEvent') + ->willReturn($event); + $this->activityManager->expects($this->once()) + ->method('publish'); + + $this->assertEquals([], $this->controller->destroy($tokenId)); + } + public function testDestroyWrongUser(): void { $tokenId = 124; $token = $this->createMock(PublicKeyToken::class); diff --git a/core/Command/User/AuthTokens/Delete.php b/core/Command/User/AuthTokens/Delete.php index 2c7a958d9a7..803bcf6a0b9 100644 --- a/core/Command/User/AuthTokens/Delete.php +++ b/core/Command/User/AuthTokens/Delete.php @@ -9,6 +9,8 @@ namespace OC\Core\Command\User\AuthTokens; use DateTimeImmutable; use OC\Authentication\Token\IProvider; use OC\Core\Command\Base; +use OCP\Authentication\Exceptions\InvalidTokenException; +use OCP\Authentication\Exceptions\WipeTokenException; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\RuntimeException; use Symfony\Component\Console\Input\InputArgument; @@ -43,6 +45,12 @@ class Delete extends Base { null, InputOption::VALUE_REQUIRED, 'Delete tokens last used before a given date.' + ) + ->addOption( + 'cancel-wipe', + null, + InputOption::VALUE_NONE, + 'Allow deleting a token that is marked for remote wipe. The pending wipe will not run.' ); } @@ -51,6 +59,7 @@ class Delete extends Base { $uid = $input->getArgument('uid'); $id = (int)$input->getArgument('id'); $before = $input->getOption('last-used-before'); + $cancelWipe = (bool)$input->getOption('cancel-wipe'); if ($before) { if ($id) { @@ -63,6 +72,19 @@ class Delete extends Base { if (!$id) { throw new RuntimeException('Not enough arguments. Specify the token