diff --git a/settings/Activity/Provider.php b/settings/Activity/Provider.php index e71a33e0d4..7113157a8e 100644 --- a/settings/Activity/Provider.php +++ b/settings/Activity/Provider.php @@ -1,4 +1,5 @@ * @@ -34,12 +35,15 @@ use OCP\L10N\IFactory; class Provider implements IProvider { - const PASSWORD_CHANGED_BY = 'password_changed_by'; - const PASSWORD_CHANGED_SELF = 'password_changed_self'; - const PASSWORD_RESET = 'password_changed'; - const EMAIL_CHANGED_BY = 'email_changed_by'; - const EMAIL_CHANGED_SELF = 'email_changed_self'; - const EMAIL_CHANGED = 'email_changed'; + public const PASSWORD_CHANGED_BY = 'password_changed_by'; + public const PASSWORD_CHANGED_SELF = 'password_changed_self'; + public const PASSWORD_RESET = 'password_changed'; + public const EMAIL_CHANGED_BY = 'email_changed_by'; + public const EMAIL_CHANGED_SELF = 'email_changed_self'; + public const EMAIL_CHANGED = 'email_changed'; + public const APP_TOKEN_CREATED = 'app_token_created'; + public const APP_TOKEN_UPDATED = 'app_token_updated'; + public const APP_TOKEN_DELETED = 'app_token_deleted'; /** @var IFactory */ protected $languageFactory; @@ -59,13 +63,10 @@ class Provider implements IProvider { /** @var string[] cached displayNames - key is the UID and value the displayname */ protected $displayNames = []; - /** - * @param IFactory $languageFactory - * @param IURLGenerator $url - * @param IUserManager $userManager - * @param IManager $activityManager - */ - public function __construct(IFactory $languageFactory, IURLGenerator $url, IUserManager $userManager, IManager $activityManager) { + public function __construct(IFactory $languageFactory, + IURLGenerator $url, + IUserManager $userManager, + IManager $activityManager) { $this->languageFactory = $languageFactory; $this->url = $url; $this->userManager = $userManager; @@ -80,9 +81,9 @@ class Provider implements IProvider { * @throws \InvalidArgumentException * @since 11.0.0 */ - public function parse($language, IEvent $event, IEvent $previousEvent = null) { + public function parse($language, IEvent $event, IEvent $previousEvent = null): IEvent { if ($event->getApp() !== 'settings') { - throw new \InvalidArgumentException(); + throw new \InvalidArgumentException('Unknown app'); } $this->l = $this->languageFactory->get('settings', $language); @@ -107,8 +108,15 @@ class Provider implements IProvider { } else if ($event->getSubject() === self::EMAIL_CHANGED) { $subject = $this->l->t('Your email address was changed by an administrator'); + } else if ($event->getSubject() === self::APP_TOKEN_CREATED) { + $subject = $this->l->t('You created app password "{token}"'); + } else if ($event->getSubject() === self::APP_TOKEN_UPDATED) { + $subject = $this->l->t('You updated app password "{token}"'); + } else if ($event->getSubject() === self::APP_TOKEN_DELETED) { + $subject = $this->l->t('You deleted app password "{token}"'); + } else { - throw new \InvalidArgumentException(); + throw new \InvalidArgumentException('Unknown subject'); } $parsedParameters = $this->getParameters($event); @@ -122,7 +130,7 @@ class Provider implements IProvider { * @return array * @throws \InvalidArgumentException */ - protected function getParameters(IEvent $event) { + protected function getParameters(IEvent $event): array { $subject = $event->getSubject(); $parameters = $event->getSubjectParameters(); @@ -137,9 +145,19 @@ class Provider implements IProvider { return [ 'actor' => $this->generateUserParameter($parameters[0]), ]; + case self::APP_TOKEN_CREATED: + case self::APP_TOKEN_UPDATED: + case self::APP_TOKEN_DELETED: + return [ + 'token' => [ + 'type' => 'highlight', + 'id' => $event->getObjectId(), + 'name' => $parameters[0], + ] + ]; } - throw new \InvalidArgumentException(); + throw new \InvalidArgumentException('Unknown subject'); } /** @@ -148,7 +166,7 @@ class Provider implements IProvider { * @param array $parameters * @throws \InvalidArgumentException */ - protected function setSubjects(IEvent $event, $subject, array $parameters) { + protected function setSubjects(IEvent $event, string $subject, array $parameters): void { $placeholders = $replacements = []; foreach ($parameters as $placeholder => $parameter) { $placeholders[] = '{' . $placeholder . '}'; @@ -159,11 +177,7 @@ class Provider implements IProvider { ->setRichSubject($subject, $parameters); } - /** - * @param string $uid - * @return array - */ - protected function generateUserParameter($uid) { + protected function generateUserParameter(string $uid): array { if (!isset($this->displayNames[$uid])) { $this->displayNames[$uid] = $this->getDisplayName($uid); } @@ -175,11 +189,7 @@ class Provider implements IProvider { ]; } - /** - * @param string $uid - * @return string - */ - protected function getDisplayName($uid) { + protected function getDisplayName(string $uid): string { $user = $this->userManager->get($uid); if ($user instanceof IUser) { return $user->getDisplayName(); diff --git a/settings/Controller/AuthSettingsController.php b/settings/Controller/AuthSettingsController.php index 06cabd00b0..5f834b3013 100644 --- a/settings/Controller/AuthSettingsController.php +++ b/settings/Controller/AuthSettingsController.php @@ -27,16 +27,19 @@ namespace OC\Settings\Controller; +use BadMethodCallException; use OC\AppFramework\Http; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; +use OC\Settings\Activity\Provider; +use OCP\Activity\IManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; +use OCP\ILogger; use OCP\IRequest; use OCP\ISession; -use OCP\IUserManager; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; @@ -45,9 +48,6 @@ class AuthSettingsController extends Controller { /** @var IProvider */ private $tokenProvider; - /** @var IUserManager */ - private $userManager; - /** @var ISession */ private $session; @@ -57,23 +57,37 @@ class AuthSettingsController extends Controller { /** @var ISecureRandom */ private $random; + /** @var IManager */ + private $activityManager; + + /** @var ILogger */ + private $logger; + /** * @param string $appName * @param IRequest $request * @param IProvider $tokenProvider - * @param IUserManager $userManager * @param ISession $session * @param ISecureRandom $random - * @param string $userId + * @param string|null $userId + * @param IManager $activityManager + * @param ILogger $logger */ - public function __construct($appName, IRequest $request, IProvider $tokenProvider, IUserManager $userManager, - ISession $session, ISecureRandom $random, $userId) { + public function __construct(string $appName, + IRequest $request, + IProvider $tokenProvider, + ISession $session, + ISecureRandom $random, + ?string $userId, + IManager $activityManager, + ILogger $logger) { parent::__construct($appName, $request); $this->tokenProvider = $tokenProvider; - $this->userManager = $userManager; $this->uid = $userId; $this->session = $session; $this->random = $random; + $this->activityManager = $activityManager; + $this->logger = $logger; } /** @@ -84,7 +98,7 @@ class AuthSettingsController extends Controller { */ public function index() { $tokens = $this->tokenProvider->getTokenByUser($this->uid); - + try { $sessionId = $this->session->getId(); } catch (SessionNotAvailableException $ex) { @@ -96,7 +110,7 @@ class AuthSettingsController extends Controller { return $this->getServiceNotAvailableResponse(); } - return array_map(function(IToken $token) use ($sessionToken) { + return array_map(function (IToken $token) use ($sessionToken) { $data = $token->jsonSerialize(); if ($sessionToken->getId() === $token->getId()) { $data['canDelete'] = false; @@ -140,6 +154,8 @@ class AuthSettingsController extends Controller { $tokenData = $deviceToken->jsonSerialize(); $tokenData['canDelete'] = true; + $this->publishActivity(Provider::APP_TOKEN_CREATED, $deviceToken->getId(), $deviceToken->getName()); + return new JSONResponse([ 'token' => $token, 'loginName' => $loginName, @@ -175,10 +191,18 @@ class AuthSettingsController extends Controller { * @NoAdminRequired * @NoSubadminRequired * - * @return array + * @param int $id + * @return array|JSONResponse */ public function destroy($id) { - $this->tokenProvider->invalidateTokenById($this->uid, $id); + try { + $token = $this->findTokenByIdAndUser($id); + } catch (InvalidTokenException $e) { + return new JSONResponse([], Http::STATUS_NOT_FOUND); + } + + $this->tokenProvider->invalidateTokenById($this->uid, $token->getId()); + $this->publishActivity(Provider::APP_TOKEN_DELETED, $token->getId(), $token->getName()); return []; } @@ -192,10 +216,7 @@ class AuthSettingsController extends Controller { */ public function update($id, array $scope) { try { - $token = $this->tokenProvider->getTokenById((string)$id); - if ($token->getUID() !== $this->uid) { - throw new InvalidTokenException('User mismatch'); - } + $token = $this->findTokenByIdAndUser($id); } catch (InvalidTokenException $e) { return new JSONResponse([], Http::STATUS_NOT_FOUND); } @@ -203,7 +224,47 @@ class AuthSettingsController extends Controller { $token->setScope([ 'filesystem' => $scope['filesystem'] ]); + $this->tokenProvider->updateToken($token); + $this->publishActivity(Provider::APP_TOKEN_UPDATED, $token->getId(), $token->getName()); return []; } + + /** + * @param string $subject + * @param int $id + * @param string|null $tokenName + */ + private function publishActivity(string $subject, int $id, ?string $tokenName = null): void { + $event = $this->activityManager->generateEvent(); + $event->setApp('settings') + ->setType('security') + ->setAffectedUser($this->uid) + ->setAuthor($this->uid) + ->setSubject($subject, [$tokenName]) + ->setObject('app_token', $id, 'App Password'); + + try { + $this->activityManager->publish($event); + } catch (BadMethodCallException $e) { + $this->logger->warning('could not publish activity'); + $this->logger->logException($e); + } + } + + /** + * Find a token by given id and check if uid for current session belongs to this token + * + * @param int $id + * @return IToken + * @throws InvalidTokenException + * @throws \OC\Authentication\Exceptions\ExpiredTokenException + */ + private function findTokenByIdAndUser(int $id): IToken { + $token = $this->tokenProvider->getTokenById($id); + if ($token->getUID() !== $this->uid) { + throw new InvalidTokenException('This token does not belong to you!'); + } + return $token; + } } diff --git a/tests/Settings/Controller/AuthSettingsControllerTest.php b/tests/Settings/Controller/AuthSettingsControllerTest.php index 1c957299e3..73741bea57 100644 --- a/tests/Settings/Controller/AuthSettingsControllerTest.php +++ b/tests/Settings/Controller/AuthSettingsControllerTest.php @@ -28,11 +28,12 @@ use OC\Authentication\Token\DefaultToken; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; use OC\Settings\Controller\AuthSettingsController; +use OCP\Activity\IEvent; +use OCP\Activity\IManager; use OCP\AppFramework\Http\JSONResponse; +use OCP\ILogger; use OCP\IRequest; use OCP\ISession; -use OCP\IUser; -use OCP\IUserManager; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; use Test\TestCase; @@ -41,26 +42,39 @@ class AuthSettingsControllerTest extends TestCase { /** @var AuthSettingsController */ private $controller; + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ private $request; /** @var IProvider|\PHPUnit_Framework_MockObject_MockObject */ private $tokenProvider; - private $userManager; + /** @var ISession|\PHPUnit_Framework_MockObject_MockObject */ private $session; + /** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */ private $secureRandom; - private $uid; + /** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ + private $activityManager; + private $uid = 'jane'; protected function setUp() { parent::setUp(); $this->request = $this->createMock(IRequest::class); $this->tokenProvider = $this->createMock(IProvider::class); - $this->userManager = $this->createMock(IUserManager::class); $this->session = $this->createMock(ISession::class); $this->secureRandom = $this->createMock(ISecureRandom::class); - $this->uid = 'jane'; - $this->user = $this->createMock(IUser::class); + $this->activityManager = $this->createMock(IManager::class); + /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject $logger */ + $logger = $this->createMock(ILogger::class); - $this->controller = new AuthSettingsController('core', $this->request, $this->tokenProvider, $this->userManager, $this->session, $this->secureRandom, $this->uid); + $this->controller = new AuthSettingsController( + 'core', + $this->request, + $this->tokenProvider, + $this->session, + $this->secureRandom, + $this->uid, + $this->activityManager, + $logger + ); } public function testIndex() { @@ -78,14 +92,14 @@ class AuthSettingsControllerTest extends TestCase { $this->tokenProvider->expects($this->once()) ->method('getTokenByUser') ->with($this->uid) - ->will($this->returnValue($tokens)); + ->willReturn($tokens); $this->session->expects($this->once()) ->method('getId') - ->will($this->returnValue('session123')); + ->willReturn('session123'); $this->tokenProvider->expects($this->once()) ->method('getToken') ->with('session123') - ->will($this->returnValue($sessionToken)); + ->willReturn($sessionToken); $this->assertEquals([ [ @@ -116,39 +130,42 @@ class AuthSettingsControllerTest extends TestCase { $this->session->expects($this->once()) ->method('getId') - ->will($this->returnValue('sessionid')); + ->willReturn('sessionid'); $this->tokenProvider->expects($this->once()) ->method('getToken') ->with('sessionid') - ->will($this->returnValue($sessionToken)); + ->willReturn($sessionToken); $this->tokenProvider->expects($this->once()) ->method('getPassword') ->with($sessionToken, 'sessionid') - ->will($this->returnValue($password)); + ->willReturn($password); $sessionToken->expects($this->once()) ->method('getLoginName') - ->will($this->returnValue('User13')); + ->willReturn('User13'); $this->secureRandom->expects($this->exactly(5)) ->method('generate') ->with(5, ISecureRandom::CHAR_HUMAN_READABLE) - ->will($this->returnValue('XXXXX')); + ->willReturn('XXXXX'); $newToken = 'XXXXX-XXXXX-XXXXX-XXXXX-XXXXX'; $this->tokenProvider->expects($this->once()) ->method('generateToken') ->with($newToken, $this->uid, 'User13', $password, $name, IToken::PERMANENT_TOKEN) - ->will($this->returnValue($deviceToken)); + ->willReturn($deviceToken); $deviceToken->expects($this->once()) ->method('jsonSerialize') - ->will($this->returnValue(['dummy' => 'dummy', 'canDelete' => true])); + ->willReturn(['dummy' => 'dummy', 'canDelete' => true]); + + $this->mockActivityManager(); $expected = [ 'token' => $newToken, 'deviceToken' => ['dummy' => 'dummy', 'canDelete' => true], 'loginName' => 'User13', ]; + $response = $this->controller->create($name); $this->assertInstanceOf(JSONResponse::class, $response); $this->assertEquals($expected, $response->getData()); @@ -172,7 +189,7 @@ class AuthSettingsControllerTest extends TestCase { $this->session->expects($this->once()) ->method('getId') - ->will($this->returnValue('sessionid')); + ->willReturn('sessionid'); $this->tokenProvider->expects($this->once()) ->method('getToken') ->with('sessionid') @@ -185,23 +202,49 @@ class AuthSettingsControllerTest extends TestCase { } public function testDestroy() { - $id = 123; - $user = $this->createMock(IUser::class); + $tokenId = 124; + $token = $this->createMock(DefaultToken::class); + + $this->mockGetTokenById($tokenId, $token); + $this->mockActivityManager(); + + $token->expects($this->exactly(2)) + ->method('getId') + ->willReturn($tokenId); + + $token->expects($this->once()) + ->method('getUID') + ->willReturn('jane'); $this->tokenProvider->expects($this->once()) ->method('invalidateTokenById') - ->with($this->uid, $id); + ->with($this->uid, $tokenId); - $this->assertEquals([], $this->controller->destroy($id)); + $this->assertEquals([], $this->controller->destroy($tokenId)); } - public function testUpdateToken() { + public function testDestroyWrongUser() { + $tokenId = 124; $token = $this->createMock(DefaultToken::class); - $this->tokenProvider->expects($this->once()) - ->method('getTokenById') - ->with($this->equalTo(42)) - ->willReturn($token); + $this->mockGetTokenById($tokenId, $token); + + $token->expects($this->once()) + ->method('getUID') + ->willReturn('foobar'); + + $response = $this->controller->destroy($tokenId); + $this->assertSame([], $response->getData()); + $this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus()); + } + + + public function testUpdateToken() { + $tokenId = 42; + $token = $this->createMock(DefaultToken::class); + + $this->mockGetTokenById($tokenId, $token); + $this->mockActivityManager(); $token->expects($this->once()) ->method('getUID') @@ -217,16 +260,14 @@ class AuthSettingsControllerTest extends TestCase { ->method('updateToken') ->with($this->equalTo($token)); - $this->assertSame([], $this->controller->update(42, ['filesystem' => true])); + $this->assertSame([], $this->controller->update($tokenId, ['filesystem' => true])); } public function testUpdateTokenWrongUser() { + $tokenId = 42; $token = $this->createMock(DefaultToken::class); - $this->tokenProvider->expects($this->once()) - ->method('getTokenById') - ->with($this->equalTo(42)) - ->willReturn($token); + $this->mockGetTokenById($tokenId, $token); $token->expects($this->once()) ->method('getUID') @@ -237,7 +278,7 @@ class AuthSettingsControllerTest extends TestCase { $this->tokenProvider->expects($this->never()) ->method('updateToken'); - $response = $this->controller->update(42, ['filesystem' => true]); + $response = $this->controller->update($tokenId, ['filesystem' => true]); $this->assertSame([], $response->getData()); $this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus()); } @@ -256,4 +297,22 @@ class AuthSettingsControllerTest extends TestCase { $this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus()); } + private function mockActivityManager(): void { + $this->activityManager->expects($this->once()) + ->method('generateEvent') + ->willReturn($this->createMock(IEvent::class)); + $this->activityManager->expects($this->once()) + ->method('publish'); + } + + /** + * @param int $tokenId + * @param $token + */ + private function mockGetTokenById(int $tokenId, $token): void { + $this->tokenProvider->expects($this->once()) + ->method('getTokenById') + ->with($this->equalTo($tokenId)) + ->willReturn($token); + } }