diff --git a/src/Components/Security/AdminAccountService.php b/src/Components/Security/AdminAccountService.php new file mode 100644 index 00000000..f6ecbf5b --- /dev/null +++ b/src/Components/Security/AdminAccountService.php @@ -0,0 +1,235 @@ +> + */ + public function getUsers(): array + { + $rows = $this->connection->fetchAllAssociative( + 'SELECT LOWER(HEX(`u`.`id`)) AS `id`, + `u`.`email`, + `u`.`active`, + `u`.`admin`, + `u`.`created_at`, + `u`.`updated_at`, + `u`.`last_updated_password_at`, + COUNT(DISTINCT `k`.`id`) AS `access_key_count`, + MAX(`k`.`last_usage_at`) AS `access_key_last_usage_at`, + `r`.`user_id` IS NOT NULL AS `password_recovery_pending` + FROM `user` AS `u` + LEFT JOIN `user_access_key` AS `k` ON `k`.`user_id` = `u`.`id` + LEFT JOIN `user_recovery` AS `r` ON `r`.`user_id` = `u`.`id` + GROUP BY `u`.`id`, `r`.`user_id` + ORDER BY `u`.`created_at`', + ); + + $roles = $this->fetchRoleAssignments( + 'SELECT LOWER(HEX(`user_id`)) AS `owner_id`, LOWER(HEX(`acl_role_id`)) AS `role_id` FROM `acl_user_role`', + ); + + $users = []; + foreach ($rows as $row) { + $id = (string) $row['id']; + + $users[] = [ + 'id' => $id, + 'maskedEmail' => $this->maskEmail($row['email']), + 'active' => (bool) $row['active'], + 'admin' => (bool) $row['admin'], + 'aclRoleIds' => $roles[$id] ?? [], + 'accessKeyCount' => (int) $row['access_key_count'], + 'accessKeyLastUsageAt' => $this->formatDate($row['access_key_last_usage_at']), + 'passwordRecoveryPending' => (bool) $row['password_recovery_pending'], + 'lastUpdatedPasswordAt' => $this->formatDate($row['last_updated_password_at']), + 'createdAt' => $this->formatDate($row['created_at']), + 'updatedAt' => $this->formatDate($row['updated_at']), + 'adminPath' => self::ADMIN_USER_PATH . $id, + ]; + } + + return $users; + } + + /** + * @return list> + */ + public function getIntegrations(): array + { + $rows = $this->connection->fetchAllAssociative( + 'SELECT LOWER(HEX(`i`.`id`)) AS `id`, + `i`.`label`, + `i`.`admin`, + `i`.`last_usage_at`, + `i`.`deleted_at`, + `i`.`created_at`, + `i`.`updated_at`, + `a`.`name` AS `app_name`, + LOWER(HEX(`a`.`acl_role_id`)) AS `app_acl_role_id` + FROM `integration` AS `i` + LEFT JOIN `app` AS `a` ON `a`.`integration_id` = `i`.`id` + ORDER BY `i`.`created_at`', + ); + + $roles = $this->fetchRoleAssignments( + 'SELECT LOWER(HEX(`integration_id`)) AS `owner_id`, LOWER(HEX(`acl_role_id`)) AS `role_id` FROM `integration_role`', + ); + + $integrations = []; + foreach ($rows as $row) { + $id = (string) $row['id']; + $appName = $row['app_name'] !== null ? (string) $row['app_name'] : null; + + // An app grants its integration permissions through `app.acl_role_id`, + // not through `integration_role`, and that path even takes precedence. + $roleIds = $roles[$id] ?? []; + if ($row['app_acl_role_id'] !== null) { + $roleIds[] = (string) $row['app_acl_role_id']; + $roleIds = array_values(array_unique($roleIds)); + sort($roleIds); + } + + $integrations[] = [ + 'id' => $id, + 'label' => (string) $row['label'], + 'admin' => (bool) $row['admin'], + 'aclRoleIds' => $roleIds, + // App-owned integrations come and go with the app, no manual credentials. + 'managedByApp' => $appName !== null, + 'appName' => $appName, + 'lastUsageAt' => $this->formatDate($row['last_usage_at']), + 'deletedAt' => $this->formatDate($row['deleted_at']), + 'createdAt' => $this->formatDate($row['created_at']), + 'updatedAt' => $this->formatDate($row['updated_at']), + 'adminPath' => self::ADMIN_INTEGRATION_PATH, + ]; + } + + return $integrations; + } + + /** + * Hashed so a role gaining privileges stays detectable without shipping the + * full list on every poll. + * + * @return list> + */ + public function getAclRoles(): array + { + $rows = $this->connection->fetchAllAssociative( + 'SELECT LOWER(HEX(`id`)) AS `id`, `name`, `privileges`, `deleted_at`, `created_at`, `updated_at` + FROM `acl_role` + ORDER BY `created_at`', + ); + + $roles = []; + foreach ($rows as $row) { + $privileges = $this->decodePrivileges($row['privileges']); + sort($privileges); + + $roles[] = [ + 'id' => (string) $row['id'], + 'name' => (string) $row['name'], + 'privilegeCount' => \count($privileges), + 'privilegesHash' => hash('sha256', implode("\n", $privileges)), + 'deletedAt' => $this->formatDate($row['deleted_at']), + 'createdAt' => $this->formatDate($row['created_at']), + 'updatedAt' => $this->formatDate($row['updated_at']), + ]; + } + + return $roles; + } + + /** + * @return array> + */ + private function fetchRoleAssignments(string $sql): array + { + $assignments = []; + foreach ($this->connection->fetchAllAssociative($sql) as $row) { + $assignments[(string) $row['owner_id']][] = (string) $row['role_id']; + } + + foreach ($assignments as &$roleIds) { + sort($roleIds); + } + + return $assignments; + } + + /** + * @return list + */ + private function decodePrivileges(mixed $value): array + { + if (!\is_string($value) || $value === '') { + return []; + } + + try { + $decoded = json_decode($value, true, 512, \JSON_THROW_ON_ERROR); + } catch (\JsonException) { + return []; + } + + if (!\is_array($decoded)) { + return []; + } + + return array_values(array_map(strval(...), $decoded)); + } + + private function maskEmail(mixed $value): ?string + { + if (!\is_string($value) || $value === '') { + return null; + } + + $at = strrpos($value, '@'); + if ($at === false || $at === 0 || $at === \strlen($value) - 1) { + return null; + } + + $domain = substr($value, $at + 1); + $dot = strrpos($domain, '.'); + + return $value[0] . '***@' . $domain[0] . '***' . ($dot !== false ? substr($domain, $dot) : ''); + } + + private function formatDate(mixed $value): ?string + { + if (!\is_string($value) || $value === '') { + return null; + } + + try { + return (new \DateTimeImmutable($value, new \DateTimeZone('UTC')))->format(\DATE_ATOM); + } catch (\Exception) { + return null; + } + } +} diff --git a/src/Controller/SecurityController.php b/src/Controller/SecurityController.php index ee492c69..6d879288 100644 --- a/src/Controller/SecurityController.php +++ b/src/Controller/SecurityController.php @@ -6,8 +6,10 @@ use Frosh\Tools\Acl\FroshToolsPrivileges; use Frosh\Tools\Components\Sbom\CycloneDxSbomGenerator; +use Frosh\Tools\Components\Security\AdminAccountService; use Frosh\Tools\Components\Security\Checker\SecurityCheckerInterface; use Frosh\Tools\Components\Security\SecurityCollection; +use Psr\Clock\ClockInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\DependencyInjection\Attribute\Autowire; use Symfony\Component\DependencyInjection\Attribute\AutowireIterator; @@ -28,6 +30,8 @@ public function __construct( #[Autowire(param: 'frosh_tools.checker.disabled_checks')] private readonly array $ignoredChecks, private readonly CycloneDxSbomGenerator $sbomGenerator, + private readonly AdminAccountService $adminAccountService, + private readonly ClockInterface $clock, ) { } @@ -48,6 +52,28 @@ public function status(): JsonResponse ]); } + /** + * Inventory of everything that can authenticate against the Admin API, for + * external monitoring to diff against the previous snapshot and alert on new + * accounts or privilege escalation. + * + * Contains no personal data by design - see {@see AdminAccountService}. + * + * Deliberately no aggregated counts: consumers diff the full lists against + * the previous snapshot anyway, so counters would only duplicate what they + * can derive and risk drifting from the lists. + */ + #[Route(path: '/security/accounts', name: 'api.frosh.tools.security.accounts', methods: ['GET'])] + public function accounts(): JsonResponse + { + return new JsonResponse([ + 'generatedAt' => $this->clock->now()->format(\DATE_ATOM), + 'users' => $this->adminAccountService->getUsers(), + 'integrations' => $this->adminAccountService->getIntegrations(), + 'aclRoles' => $this->adminAccountService->getAclRoles(), + ]); + } + #[Route(path: '/security/sbom', name: 'api.frosh.tools.security.sbom', methods: ['GET'])] public function sbom(Request $request): Response { diff --git a/tests/Components/Security/AdminAccountServiceTest.php b/tests/Components/Security/AdminAccountServiceTest.php new file mode 100644 index 00000000..5ebf3087 --- /dev/null +++ b/tests/Components/Security/AdminAccountServiceTest.php @@ -0,0 +1,288 @@ +accounts = static::getContainer()->get(AdminAccountService::class); + } + + public function testUsersExposeOnlyNonPersonalFields(): void + { + $users = $this->accounts->getUsers(); + + static::assertNotEmpty($users, 'The test fixture ships at least one admin user'); + + foreach ($users as $user) { + foreach (self::FORBIDDEN_KEYS as $key) { + static::assertArrayNotHasKey($key, $user); + } + + static::assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $user['id']); + static::assertMatchesRegularExpression('/^.\\*{3}@.\\*{3}(\\..+)?$/', (string) $user['maskedEmail']); + static::assertIsBool($user['active']); + static::assertIsBool($user['admin']); + static::assertIsBool($user['passwordRecoveryPending']); + static::assertIsArray($user['aclRoleIds']); + static::assertIsInt($user['accessKeyCount']); + static::assertSame('/admin#/sw/users/permissions/user/detail/' . $user['id'], $user['adminPath']); + static::assertNotNull($user['createdAt']); + } + } + + /** + * @return iterable + */ + public static function emails(): iterable + { + yield 'company address' => ['admin@example.com', 'a***@e***.com']; + yield 'long local part' => ['verylongfirstname.lastname@example.com', 'v***@e***.com']; + yield 'single char local part' => ['a@example.com', 'a***@e***.com']; + yield 'subdomain' => ['admin@mail.example.com', 'a***@m***.com']; + yield 'multi part tld' => ['admin@example.co.uk', 'a***@e***.uk']; + yield 'no tld' => ['admin@localhost', 'a***@l***']; + yield 'plus addressing' => ['admin+shop@example.com', 'a***@e***.com']; + yield 'empty' => ['', null]; + yield 'no at sign' => ['notanemail', null]; + yield 'nothing before at' => ['@example.com', null]; + yield 'nothing after at' => ['admin@', null]; + } + + #[DataProvider('emails')] + public function testEmailMaskingHidesLengthAndContent(string $email, ?string $expected): void + { + $mask = new \ReflectionMethod(AdminAccountService::class, 'maskEmail'); + + static::assertSame($expected, $mask->invoke($this->accounts, $email)); + } + + public function testMaskedEmailsOfDifferentLengthAreIndistinguishable(): void + { + $mask = new \ReflectionMethod(AdminAccountService::class, 'maskEmail'); + + static::assertSame( + $mask->invoke($this->accounts, 'a@example.com'), + $mask->invoke($this->accounts, 'averylongadminaddress@example.com'), + 'The star count must not reveal the real length', + ); + } + + public function testIntegrationsExposeNoCredentials(): void + { + $context = Context::createDefaultContext(); + $ids = new IdsCollection(); + + $this->createIntegration($ids->get('integration'), 'frosh_tools_credentials_test', $context); + + $integrations = $this->accounts->getIntegrations(); + static::assertNotEmpty($integrations, 'The fixture integration must be returned'); + + foreach ($integrations as $integration) { + foreach (self::FORBIDDEN_KEYS as $key) { + static::assertArrayNotHasKey($key, $integration); + } + + static::assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $integration['id']); + static::assertIsString($integration['label']); + static::assertIsBool($integration['admin']); + static::assertIsBool($integration['managedByApp']); + static::assertSame($integration['appName'] !== null, $integration['managedByApp']); + static::assertIsArray($integration['aclRoleIds']); + } + } + + public function testAppIntegrationReportsItsAppAclRole(): void + { + $context = Context::createDefaultContext(); + $ids = new IdsCollection(); + + $this->createIntegration($ids->get('integration'), 'frosh_tools_app_test', $context); + $this->createAclRoles([ + 'appRole' => 'frosh_tools_app_role', + 'otherRole' => 'frosh_tools_other_role', + ], $context, $ids); + + // Attached the way Shopware wires an app: through integration_role, which the + // service must not mistake for the app's own role. + $this->getRepository(IntegrationDefinition::ENTITY_NAME, IntegrationCollection::class)->update([[ + 'id' => $ids->get('integration'), + 'aclRoles' => [['id' => $ids->get('otherRole')]], + ]], $context); + + $this->getRepository(AppDefinition::ENTITY_NAME, AppCollection::class)->create([[ + 'id' => $ids->get('app'), + 'name' => 'FroshToolsAppTest', + 'label' => 'FroshToolsAppTest', + 'path' => 'custom/apps/FroshToolsAppTest', + 'version' => '1.0.0', + 'active' => true, + 'integrationId' => $ids->get('integration'), + 'aclRoleId' => $ids->get('appRole'), + ]], $context); + + $integration = $this->findIntegration('frosh_tools_app_test'); + + static::assertTrue($integration['managedByApp']); + static::assertSame('FroshToolsAppTest', $integration['appName']); + + // Apps grant permissions via app.acl_role_id, which Shopware resolves with + // precedence over integration_role - that role must not go missing here. + static::assertContains($ids->get('appRole'), $integration['aclRoleIds']); + static::assertContains($ids->get('otherRole'), $integration['aclRoleIds']); + } + + public function testAclRolesAreHashedInsteadOfListed(): void + { + $context = Context::createDefaultContext(); + $ids = new IdsCollection(); + + $this->createAclRoles(['role' => 'frosh_tools_hash_test'], $context, $ids, ['order:read']); + + $roles = $this->accounts->getAclRoles(); + static::assertNotEmpty($roles, 'The fixture role must be returned'); + + foreach ($roles as $role) { + static::assertArrayNotHasKey('privileges', $role); + static::assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $role['id']); + static::assertIsString($role['name']); + static::assertIsInt($role['privilegeCount']); + static::assertMatchesRegularExpression('/^[0-9a-f]{64}$/', $role['privilegesHash']); + } + } + + public function testPrivilegeHashIsStableAndOrderIndependent(): void + { + $context = Context::createDefaultContext(); + $ids = new IdsCollection(); + + $this->createAclRoles(['role' => 'frosh_tools_account_test'], $context, $ids, ['b:read', 'a:read']); + + $before = $this->findRole('frosh_tools_account_test'); + + $aclRoles = $this->getRepository(AclRoleDefinition::ENTITY_NAME, AclRoleCollection::class); + $aclRoles->update([[ + 'id' => $ids->get('role'), + 'privileges' => ['a:read', 'b:read'], + ]], $context); + + $reordered = $this->findRole('frosh_tools_account_test'); + static::assertSame($before['privilegesHash'], $reordered['privilegesHash'], 'Reordering privileges must not look like a change'); + + $aclRoles->update([[ + 'id' => $ids->get('role'), + 'privileges' => ['a:read', 'b:read', 'c:write'], + ]], $context); + + $escalated = $this->findRole('frosh_tools_account_test'); + static::assertNotSame($before['privilegesHash'], $escalated['privilegesHash'], 'An added privilege must be detectable'); + static::assertSame(3, $escalated['privilegeCount']); + } + + private function createIntegration(string $id, string $label, Context $context): void + { + $this->getRepository(IntegrationDefinition::ENTITY_NAME, IntegrationCollection::class)->create([[ + 'id' => $id, + 'label' => $label, + 'accessKey' => $id, + 'secretAccessKey' => 'frosh_tools_test_secret', + ]], $context); + } + + /** + * @param array $names keyed by IdsCollection key + * @param list $privileges + */ + private function createAclRoles(array $names, Context $context, IdsCollection $ids, array $privileges = ['app:read']): void + { + $payload = []; + foreach ($names as $key => $name) { + $payload[] = [ + 'id' => $ids->get($key), + 'name' => $name, + 'privileges' => $privileges, + ]; + } + + $this->getRepository(AclRoleDefinition::ENTITY_NAME, AclRoleCollection::class)->create($payload, $context); + } + + /** + * @param class-string> $collectionClass documents the repository's entity type + * + * @return EntityRepository> + */ + private function getRepository(string $entity, string $collectionClass): EntityRepository + { + $repository = static::getContainer()->get($entity . '.repository'); + static::assertInstanceOf(EntityRepository::class, $repository); + + return $repository; + } + + /** + * @return array + */ + private function findIntegration(string $label): array + { + foreach ($this->accounts->getIntegrations() as $integration) { + if ($integration['label'] === $label) { + return $integration; + } + } + + static::fail(\sprintf('Integration "%s" not found', $label)); + } + + /** + * @return array + */ + private function findRole(string $name): array + { + foreach ($this->accounts->getAclRoles() as $role) { + if ($role['name'] === $name) { + return $role; + } + } + + static::fail(\sprintf('Role "%s" not found', $name)); + } +} diff --git a/tests/Controller/AclRouteTest.php b/tests/Controller/AclRouteTest.php index 84988f37..042ceb0d 100644 --- a/tests/Controller/AclRouteTest.php +++ b/tests/Controller/AclRouteTest.php @@ -111,6 +111,7 @@ public static function routes(): iterable yield 'logs.file' => [LogController::class, 'getLog', FroshToolsPrivileges::LOGS_READ]; yield 'security.status' => [SecurityController::class, 'status', FroshToolsPrivileges::SECURITY_READ]; + yield 'security.accounts' => [SecurityController::class, 'accounts', FroshToolsPrivileges::SECURITY_READ]; yield 'security.sbom' => [SecurityController::class, 'sbom', FroshToolsPrivileges::SECURITY_READ]; yield 'security.audit' => [ComposerAuditController::class, 'audit', FroshToolsPrivileges::SECURITY_READ]; yield 'security.extensions' => [ExtensionFilesController::class, 'listExtensionFiles', FroshToolsPrivileges::SECURITY_READ]; diff --git a/tests/Controller/SecurityControllerTest.php b/tests/Controller/SecurityControllerTest.php index e4f7bb0b..1c6ccd69 100644 --- a/tests/Controller/SecurityControllerTest.php +++ b/tests/Controller/SecurityControllerTest.php @@ -20,6 +20,29 @@ protected function setUp(): void $this->controller = static::getContainer()->get(SecurityController::class); } + public function testAccountsReturnsInventoryWithoutPersonalData(): void + { + $response = $this->controller->accounts(); + + static::assertSame(200, $response->getStatusCode()); + + $data = json_decode((string) $response->getContent(), true, 512, JSON_THROW_ON_ERROR); + + static::assertIsArray($data); + static::assertArrayHasKey('generatedAt', $data); + static::assertArrayNotHasKey('summary', $data); + static::assertNotEmpty($data['users']); + + // Exact keys, not substrings: accessKeyCount legitimately contains "accessKey". + foreach (['users', 'integrations'] as $section) { + foreach ($data[$section] as $entry) { + foreach (['username', 'email', 'firstName', 'lastName', 'password', 'accessKey', 'secretAccessKey', 'storeToken'] as $forbidden) { + static::assertArrayNotHasKey($forbidden, $entry); + } + } + } + } + public function testStatusReturnsSummaryAndFindings(): void { $response = $this->controller->status();