From 956e913baf0bdf08923b735a2f2077a6e143779c Mon Sep 17 00:00:00 2001 From: Arthur M <4rthem@users.noreply.github.com> Date: Wed, 13 Nov 2024 16:48:14 +0100 Subject: [PATCH] PS-711 databox - add security to workspace list (#482) * PS-711 databox - add security to workspace list * PS-714 fix asset rendition query * PS-713 fix collection inherited privacy * nofollow for robots * PS-713 databox privacy widget: fix collection inheritance constraint --- .../src/Api/Extension/WorkspaceExtension.php | 58 +++++++++++++++++ .../src/Api/Provider/CollectionProvider.php | 9 +-- .../Admin/WorkspaceCrudController.php | 3 +- .../src/Elasticsearch/CollectionSearch.php | 52 --------------- databox/api/src/Storage/RenditionManager.php | 4 ++ databox/client/docker/nginx/conf.d/app.conf | 2 +- databox/client/src/api/collection.ts | 1 - .../client/src/components/Ui/PrivacyField.tsx | 64 +++++++++---------- databox/client/src/types.ts | 2 +- expose/api/docker/nginx/tpl/default.conf | 2 +- expose/client/docker/nginx/conf.d/app.conf | 2 +- infra/docker/matomo-nginx/nginx.conf | 2 +- infra/docker/nginx-fpm-base/tpl/default.conf | 2 +- .../Controller/Admin/TargetCrudController.php | 2 +- uploader/client/docker/nginx/conf.d/app.conf | 2 +- 15 files changed, 103 insertions(+), 104 deletions(-) create mode 100644 databox/api/src/Api/Extension/WorkspaceExtension.php diff --git a/databox/api/src/Api/Extension/WorkspaceExtension.php b/databox/api/src/Api/Extension/WorkspaceExtension.php new file mode 100644 index 000000000..fc100d65a --- /dev/null +++ b/databox/api/src/Api/Extension/WorkspaceExtension.php @@ -0,0 +1,58 @@ +addWhere($queryBuilder, $resourceClass, $context); + } + + private function addWhere(QueryBuilder $queryBuilder, string $resourceClass, array $context): void + { + if (Workspace::class !== $resourceClass) { + return; + } + + $rootAlias = $queryBuilder->getRootAliases()[0]; + + $user = $this->security->getUser(); + if ($user instanceof JwtUser) { + AccessControlEntryRepository::joinAcl( + $queryBuilder, + $user->getId(), + $user->getGroups(), + $this->objectMapping->getObjectKey(Workspace::class), + $rootAlias, + PermissionInterface::VIEW, + false + ); + $queryBuilder->andWhere(sprintf('ace.id IS NOT NULL OR %1$s.ownerId = :uid OR %1$s.public = true', $rootAlias)); + } else { + $queryBuilder->andWhere(sprintf('%1$s.public = true', $rootAlias)); + } + } +} diff --git a/databox/api/src/Api/Provider/CollectionProvider.php b/databox/api/src/Api/Provider/CollectionProvider.php index a2590d55f..8a76cd314 100644 --- a/databox/api/src/Api/Provider/CollectionProvider.php +++ b/databox/api/src/Api/Provider/CollectionProvider.php @@ -14,7 +14,6 @@ class CollectionProvider extends AbstractCollectionProvider public function __construct( private readonly CollectionSearch $search, - private readonly CollectionSearch $collectionSearch, ) { } @@ -23,13 +22,7 @@ protected function provideCollection( array $uriVariables = [], array $context = [], ): array|object { - $filters = $context['filters'] ?? []; - - if ($filters['groupByWorkspace'] ?? false) { - return $this->search->searchAggregationsByWorkspace($context['userId'], $context['groupIds'], $filters); - } - - $result = $this->search->search($context['userId'], $context['groupIds'], $filters); + $result = $this->search->search($context['userId'], $context['groupIds'], $context['filters'] ?? []); return new PagerFantaApiPlatformPaginator($result); } diff --git a/databox/api/src/Controller/Admin/WorkspaceCrudController.php b/databox/api/src/Controller/Admin/WorkspaceCrudController.php index f12ea4d19..5c234c4cd 100644 --- a/databox/api/src/Controller/Admin/WorkspaceCrudController.php +++ b/databox/api/src/Controller/Admin/WorkspaceCrudController.php @@ -52,8 +52,7 @@ public function configureFields(string $pageName): iterable yield TextField::new('slug'); yield TextField::new('ownerId') ->onlyOndetail(); - yield $this->userChoiceField->create('ownerId', 'Owner') - ->onlyOnForms(); + yield $this->userChoiceField->create('ownerId', 'Owner'); yield ArrayField::new('enabledLocales'); yield ArrayField::new('localeFallbacks'); yield BooleanField::new('public') diff --git a/databox/api/src/Elasticsearch/CollectionSearch.php b/databox/api/src/Elasticsearch/CollectionSearch.php index a1a45ba26..5e6f0efc9 100644 --- a/databox/api/src/Elasticsearch/CollectionSearch.php +++ b/databox/api/src/Elasticsearch/CollectionSearch.php @@ -6,7 +6,6 @@ use App\Entity\Core\Collection; use App\Entity\Core\WorkspaceItemPrivacyInterface; -use Elastica\Aggregation; use Elastica\Query; use FOS\ElasticaBundle\Finder\PaginatedFinderInterface; use Pagerfanta\Pagerfanta; @@ -48,57 +47,6 @@ public function search( return $data; } - public function searchAggregationsByWorkspace( - ?string $userId, - array $groupIds, - array $options = [], - ): array { - $query = new Query(); - $query->setSize(0); - - $boolQuery = new Query\BoolQuery(); - - $this->applyFilters($boolQuery, $userId, $groupIds, $options); - - $aggregation = new Aggregation\Filter('ws'); - $query->addAggregation($aggregation); - $aggregation->setFilter($boolQuery); - - $termAgg = new Aggregation\Terms('workspaceId'); - $termAgg->setField('workspaceId'); - $termAgg->setSize(300); - - $aggregation->addAggregation($termAgg); - - $maxLimit = 50; - $limit = (int) ($options['limit'] ?? $maxLimit); - if ($limit > $maxLimit) { - $limit = $maxLimit; - } - $top = new Aggregation\TopHits('top'); - $top->setSize($limit); - $top->setSort([ - 'sortName' => ['order' => 'asc'], - ]); - $termAgg->addAggregation($top); - - $result = $this->finder->findPaginated($query); - $aggregations = $result->getAdapter()->getAggregations(); - - $data = []; - foreach ($aggregations['ws']['workspaceId']['buckets'] as $bucket) { - foreach ($bucket['top']['hits']['hits'] as $hit) { - $object = $this->em->find(Collection::class, $hit['_id']); - - if ($object instanceof Collection) { - $data[] = $object; - } - } - } - - return $data; - } - private function applyFilters( Query\BoolQuery $boolQuery, ?string $userId, diff --git a/databox/api/src/Storage/RenditionManager.php b/databox/api/src/Storage/RenditionManager.php index d4771db0f..bd08e97f9 100644 --- a/databox/api/src/Storage/RenditionManager.php +++ b/databox/api/src/Storage/RenditionManager.php @@ -173,12 +173,16 @@ public function getAssetRenditionUsedAs(string $as, string $assetId): ?AssetRend ->select('r') ->from(AssetRendition::class, 'r') ->innerJoin('r.definition', 'd') + ->innerJoin('d.class', 'c') ->andWhere('r.asset = :asset') + ->andWhere('c.public = true') ->andWhere(sprintf('d.useAs%s = :as', ucfirst($as))) ->setParameters([ 'asset' => $assetId, 'as' => true, ]) + ->addOrderBy('d.priority', 'DESC') + ->setMaxResults(1) ->getQuery() ->getOneOrNullResult(); } diff --git a/databox/client/docker/nginx/conf.d/app.conf b/databox/client/docker/nginx/conf.d/app.conf index 33b0a1828..5278bf68d 100644 --- a/databox/client/docker/nginx/conf.d/app.conf +++ b/databox/client/docker/nginx/conf.d/app.conf @@ -4,7 +4,7 @@ server { server_name _; server_tokens off; - add_header X-Robots-Tag "noindex"; + add_header X-Robots-Tag "noindex, nofollow"; add_header X-Content-Type-Options "nosniff"; add_header X-Frame-Options "deny"; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains"; diff --git a/databox/client/src/api/collection.ts b/databox/client/src/api/collection.ts index abdd39edc..6518bd6d9 100644 --- a/databox/client/src/api/collection.ts +++ b/databox/client/src/api/collection.ts @@ -14,7 +14,6 @@ export type CollectionOptions = { query?: string; parent?: string; workspaces?: string[]; - groupByWorkspace?: boolean; }; export async function getCollections( diff --git a/databox/client/src/components/Ui/PrivacyField.tsx b/databox/client/src/components/Ui/PrivacyField.tsx index f70837fb6..14694e9fd 100644 --- a/databox/client/src/components/Ui/PrivacyField.tsx +++ b/databox/client/src/components/Ui/PrivacyField.tsx @@ -1,6 +1,7 @@ import React, {useState} from 'react'; import MenuItem from '@mui/material/MenuItem'; import { + Alert, Checkbox, FormControl, FormControlLabel, @@ -33,6 +34,10 @@ function getValue(value: string, workspace: boolean, auth: boolean): number { } } +function getAllowedValue(value: number, inheritedPrivacy?: number): number { + return Math.max(inheritedPrivacy ?? 0, value); +} + function getKeyValue(value: string): number { switch (value) { default: @@ -83,7 +88,6 @@ export default function PrivacyField({ defaultValue: 0 as any, }); - const firstValue = React.useMemo(() => value, []); const [p, w, a] = getFields(value); const [privacy, setPrivacy] = useState(p); const [workspaceOnly, setWorkspaceOnly] = useState(w); @@ -91,59 +95,51 @@ export default function PrivacyField({ const ip = inheritedPrivacy ?? 0; const inheritedKeyPrivacy = getKeyValue(getFields(ip)[0]); - const resolvedPrivacy = - inheritedKeyPrivacy > 0 && getKeyValue(privacy) <= inheritedKeyPrivacy - ? getFields(inheritedKeyPrivacy)[0] - : privacy; - const workspaceOnlyLocked = getValue(resolvedPrivacy, false, true) === ip; - const resolvedWorkspaceOnly = workspaceOnlyLocked ? false : workspaceOnly; - const authLocked = - getValue(resolvedPrivacy, resolvedWorkspaceOnly, false) === ip; - const resolveAuth = authLocked ? false : auth; React.useEffect(() => { - const [p, w, a] = getFields(value); + const [p, w, a] = getFields(getAllowedValue(value, inheritedPrivacy)); setPrivacy(p); - setWorkspaceOnly( - w || - (firstValue === value && - getKeyValue(privacy) < inheritedKeyPrivacy && - getValue(resolvedPrivacy, true, resolveAuth) === ip) - ); - setAuth( - a || - (firstValue === value && - getValue(resolvedPrivacy, resolvedWorkspaceOnly, true) === - ip) - ); + setWorkspaceOnly(w); + setAuth(a); }, [value]); const handlePChange = (e: SelectChangeEvent): void => { const v = e.target.value; setPrivacy(v); - onChange(getValue(v, resolvedWorkspaceOnly, resolveAuth)); + onChange(getAllowedValue(getValue(v, workspaceOnly, auth), inheritedPrivacy)); }; const handleWSOnlyChange = ( e: React.ChangeEvent ): void => { setWorkspaceOnly(e.target.checked); - onChange(getValue(resolvedPrivacy, e.target.checked, resolveAuth)); + onChange(getAllowedValue(getValue(privacy, e.target.checked, auth), inheritedPrivacy)); }; const handleAuthChange = (e: React.ChangeEvent): void => { setAuth(e.target.checked); onChange( - getValue(resolvedPrivacy, resolvedWorkspaceOnly, e.target.checked) + getAllowedValue(getValue(privacy, workspaceOnly, e.target.checked), inheritedPrivacy) ); }; + const workspaceOnlyLocked = !!inheritedPrivacy && getValue(privacy, true, auth) < inheritedPrivacy; + const authLocked = !!inheritedPrivacy && getValue(privacy, workspaceOnly, false) < inheritedPrivacy; + const label = t('form.privacy.label', 'Privacy'); return ( + <> + {!!inheritedPrivacy ? <> + + {t('form.privacy.inherited', 'This collection cannot be more restricted than its parent collection.')} + + : null} - {label} + + {label} + label={label} - value={resolvedPrivacy} + value={privacy} onChange={handlePChange} > {Object.keys(choices).map(k => { @@ -164,12 +160,13 @@ export default function PrivacyField({ ); })} - {['private', 'public'].includes(resolvedPrivacy) && ( + + {['private', 'public'].includes(privacy) && ( } @@ -180,12 +177,12 @@ export default function PrivacyField({ labelPlacement="end" /> )} - {resolvedPrivacy === 'public' && ( + {privacy === 'public' && ( } @@ -197,5 +194,6 @@ export default function PrivacyField({ /> )} + ); } diff --git a/databox/client/src/types.ts b/databox/client/src/types.ts index 652f7c6dc..21a01a812 100644 --- a/databox/client/src/types.ts +++ b/databox/client/src/types.ts @@ -246,7 +246,7 @@ export interface Collection extends IPermissions { public: boolean; shared: boolean; privacy: number; - inheritedPrivacy: number; + inheritedPrivacy?: number; createdAt: string; updatedAt: string; owner?: User; diff --git a/expose/api/docker/nginx/tpl/default.conf b/expose/api/docker/nginx/tpl/default.conf index 7e622a7ad..0c67304f7 100644 --- a/expose/api/docker/nginx/tpl/default.conf +++ b/expose/api/docker/nginx/tpl/default.conf @@ -7,7 +7,7 @@ server { server_tokens off; - add_header X-Robots-Tag "noindex"; + add_header X-Robots-Tag "noindex, nofollow"; add_header X-Content-Type-Options "nosniff"; add_header X-Frame-Options "deny"; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains"; diff --git a/expose/client/docker/nginx/conf.d/app.conf b/expose/client/docker/nginx/conf.d/app.conf index 1333174d1..6e5621090 100644 --- a/expose/client/docker/nginx/conf.d/app.conf +++ b/expose/client/docker/nginx/conf.d/app.conf @@ -4,7 +4,7 @@ server { server_name _; server_tokens off; - add_header X-Robots-Tag "noindex"; + add_header X-Robots-Tag "noindex, nofollow"; add_header X-Content-Type-Options "nosniff"; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains"; diff --git a/infra/docker/matomo-nginx/nginx.conf b/infra/docker/matomo-nginx/nginx.conf index bf93994e7..2f0c8c4fe 100644 --- a/infra/docker/matomo-nginx/nginx.conf +++ b/infra/docker/matomo-nginx/nginx.conf @@ -5,7 +5,7 @@ upstream php-handler { server { listen 80; - add_header X-Robots-Tag "noindex"; + add_header X-Robots-Tag "noindex, nofollow"; add_header Referrer-Policy origin; # make sure outgoing links don't show the URL to the Matomo instance root /var/www/html; # replace with path to your matomo instance index index.php; diff --git a/infra/docker/nginx-fpm-base/tpl/default.conf b/infra/docker/nginx-fpm-base/tpl/default.conf index 6c78656d4..508d724ba 100644 --- a/infra/docker/nginx-fpm-base/tpl/default.conf +++ b/infra/docker/nginx-fpm-base/tpl/default.conf @@ -5,7 +5,7 @@ server { client_max_body_size $UPLOAD_MAX_FILE_SIZE; - add_header X-Robots-Tag "noindex"; + add_header X-Robots-Tag "noindex, nofollow"; add_header X-Content-Type-Options "nosniff"; add_header X-Frame-Options "deny"; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains"; diff --git a/uploader/api/src/Controller/Admin/TargetCrudController.php b/uploader/api/src/Controller/Admin/TargetCrudController.php index 8f811e6b0..ffd7298c3 100644 --- a/uploader/api/src/Controller/Admin/TargetCrudController.php +++ b/uploader/api/src/Controller/Admin/TargetCrudController.php @@ -52,7 +52,7 @@ public function configureFields(string $pageName): iterable yield CodeField::new('pullModeUrl', 'Pull mode URL') ->onlyOnIndex(); yield TextField::new('targetUrl') - ->setHelp('Leave empty for pull mode. i.e: "https://phraseanet.phrasea.local/api/v1/upload/enqueue/" for Phraseanet, "http://databox-api/incoming-uploads" for Databox upload'); + ->setHelp('Leave empty for pull mode. i.e: "https://phraseanet.phrasea.local/api/v1/upload/enqueue/" for Phraseanet, "http://api-databox.phrasea.local/incoming-uploads" for Databox upload'); yield TextField::new('targetTokenType') ->setHelp('Use "OAuth" for Phraseanet') ->setFormTypeOptions(['attr' => ['placeholder' => 'Defaults to "Bearer"']]) diff --git a/uploader/client/docker/nginx/conf.d/app.conf b/uploader/client/docker/nginx/conf.d/app.conf index 33b0a1828..5278bf68d 100644 --- a/uploader/client/docker/nginx/conf.d/app.conf +++ b/uploader/client/docker/nginx/conf.d/app.conf @@ -4,7 +4,7 @@ server { server_name _; server_tokens off; - add_header X-Robots-Tag "noindex"; + add_header X-Robots-Tag "noindex, nofollow"; add_header X-Content-Type-Options "nosniff"; add_header X-Frame-Options "deny"; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains";