Allow image assets download from email (#1211)

* Add hash validation to get asset controller

* Add coverage

* Add coverage

* Add coverage

* Update src/Security/Voter/AssetVoter.php

Co-authored-by: Copilot <[email protected]>

* Add coverage

---------

Co-authored-by: Copilot <[email protected]>

Author: frankdekker

.github/copilot-instructions.md

@@ -0,0 +1,87 @@
+# Agents.md
+
+## Project Overview
+
+123view is a Symfony-based code review and commit notification application built with PHP 8.3+ and Symfony 7.2. It provides features for creating code reviews, managing commit notifications, and integrating with version control systems.
+
+## Core Architecture
+
+**Backend (PHP/Symfony):**
+- **Namespace**: `DR\Review\` - all PHP classes use this base namespace
+- **Entity Layer**: Domain entities in `src/Entity/` organized by feature (Review, Repository, User, etc.)
+- **Controller Layer**: Split into `Api/` and `App/` controllers, with mail controllers in `Mail/`
+- **Service Layer**: Business logic in `src/Service/` with feature-based organization
+- **Repository Layer**: Doctrine repositories in `src/Repository/` following entity structure
+- **Message/Event System**: Async messaging in `src/Message/` with handlers in `src/MessageHandler/`
+- **Form Layer**: Symfony forms in `src/Form/` organized by domain
+
+**Frontend (TypeScript/Stimulus):**
+- **Assets**: TypeScript controllers in `assets/ts/controllers/`
+- **Styling**: SCSS files in `assets/styles/` with theme support (dark/light)
+- **Build System**: Webpack Encore configuration in `webpack.config.js`
+
+## Development Commands
+
+**Frontend Development:**
+```bash
+npm run dev         # Development build
+npm run watch       # Watch mode for development
+npm run build       # Production build
+npm run stylelint   # Lint SCSS files
+npm run eslint      # Lint TypeScript files
+```
+
+**PHP Development:**
+```bash
+composer check          # Run all checks (PHPStan, PHPMD, PHPCS)
+composer check:phpstan  # Static analysis
+composer check:phpmd    # Mess detection
+composer check:phpcs    # Code style check
+composer fix:phpcbf     # Auto-fix code style
+composer test           # Run all tests
+composer test:unit      # Unit tests only
+composer test:integration # Integration tests only
+composer test:functional  # Functional tests only
+```
+
+**Docker Environment:**
+```bash
+./bin/start.sh      # Start development environment
+./bin/install.sh    # Run installation wizard
+```
+
+## Key Domain Concepts
+
+**Code Reviews:**
+- Reviews are created from specific revisions (commits)
+- Support for attaching/detaching revisions
+- Comment system with threading and reactions
+- Reviewer workflow with accept/reject states
+
+**Repositories:**
+- Git repository integration with credential management
+- Branch and revision tracking
+- External tool integrations (GitLab, etc.)
+
+**Notifications:**
+- Rule-based commit notifications
+- Frequency controls (hourly, daily, weekly)
+- Filter system for including/excluding commits
+- Email delivery with theme support
+
+## Test Structure
+
+Tests are organized in three directories:
+- `tests/Unit/` - Isolated unit tests
+- `tests/Integration/` - Integration tests with database
+- `tests/Functional/` - Full application tests
+
+Use the specific test suites when working on particular areas to speed up feedback loops.
+
+## Configuration Files
+
+- `webpack.config.js` - Frontend build configuration
+- `tsconfig.json` - TypeScript compiler settings
+- `composer.json` - PHP dependencies and scripts
+- `phpstan.neon` - Static analysis configuration
+- `phpunit.xml.dist` - Test configuration

.gitignore

@@ -7,6 +7,7 @@
 /config.xml
 /docker/db
 /.eslintcache
+/.claude/settings.local.json
 
 ###> symfony/framework-bundle ###
 /.env.local

AGENTS.md

@@ -1,4 +1,4 @@
-# CLAUDE.md
+# Agents.md
 
 This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
 
@@ -83,7 +83,7 @@ Use the specific test suites when working on particular areas to speed up feedba
 ## Configuration Files
 
 - `webpack.config.js` - Frontend build configuration
-- `tsconfig.json` - TypeScript compiler settings  
+- `tsconfig.json` - TypeScript compiler settings
 - `composer.json` - PHP dependencies and scripts
 - `phpstan.neon` - Static analysis configuration
-- `phpunit.xml.dist` - Test configuration
+- `phpunit.xml.dist` - Test configuration

config/packages/security.php

@@ -10,6 +10,7 @@
 use DR\Review\Security\AzureAd\AzureAdAuthenticator;
 use DR\Review\Security\Role\Roles;
 use DR\Review\Security\UserChecker;
+use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
 use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
 use Symfony\Config\SecurityConfig;
 
@@ -56,6 +57,7 @@
         ->path(LogoutController::class)
         ->target(LoginController::class);
 
-    $security->accessControl()->path('^/app')->roles(['IS_AUTHENTICATED_FULLY']);
+    $security->accessControl()->path('^/app/assets/')->roles(AuthenticatedVoter::PUBLIC_ACCESS);
+    $security->accessControl()->path('^/app')->roles(AuthenticatedVoter::IS_AUTHENTICATED_FULLY);
     $security->accessControl()->path('^/log-viewer')->roles([Roles::ROLE_ADMIN]);
 };

src/Controller/App/Asset/AddAssetController.php

@@ -28,6 +28,6 @@ public function __invoke(AddAssetRequest $request): JsonResponse
         // save entity
         $this->assetRepository->save($asset, true);
 
-        return new JsonResponse(['url' => $this->generateUrl(GetAssetController::class, ['id' => $asset->getId()])]);
+        return new JsonResponse(['url' => $this->generateUrl(GetAssetController::class, ['id' => $asset->getId(), 'hash' => $asset->getHash()])]);
     }
 }

src/Controller/App/Asset/GetAssetController.php

@@ -4,16 +4,19 @@
 namespace DR\Review\Controller\App\Asset;
 
 use DR\Review\Entity\Asset\Asset;
-use DR\Review\Security\Role\Roles;
 use Symfony\Bridge\Doctrine\Attribute\MapEntity;
+use Symfony\Component\ExpressionLanguage\Expression;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Annotation\Route;
 use Symfony\Component\Security\Http\Attribute\IsGranted;
 
 class GetAssetController
 {
     #[Route('app/assets/{id<\d+>}', name: self::class, methods: 'GET')]
-    #[IsGranted(Roles::ROLE_USER)]
+    #[IsGranted(
+        new Expression('is_granted("ROLE_USER") or is_granted("VIEW", subject)'),
+        new Expression('args["asset"]')
+    )]
     public function __invoke(#[MapEntity] Asset $asset): Response
     {
         return (new Response($asset->getData(), 200, ['Content-Type' => $asset->getMimeType()]))->setPublic();

src/Entity/Asset/Asset.php

@@ -71,6 +71,11 @@ public function setData(string $data): Asset
         return $this;
     }
 
+    public function getHash(): string
+    {
+        return substr(hash('sha256', $this->data), 0, 8);
+    }
+
     public function getCreateTimestamp(): int
     {
         return $this->createTimestamp;

src/Security/Voter/AssetVoter.php

@@ -0,0 +1,43 @@
+<?php
+declare(strict_types=1);
+
+namespace DR\Review\Security\Voter;
+
+use DR\Review\Entity\Asset\Asset;
+use DR\Utils\Assert;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Vote;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+
+/**
+ * @extends Voter<string, Asset>
+ */
+class AssetVoter extends Voter
+{
+    public const VIEW = 'VIEW';
+
+    public function __construct(private readonly RequestStack $requestStack)
+    {
+    }
+
+    /**
+     * @inheritDoc
+     */
+    protected function supports(string $attribute, mixed $subject): bool
+    {
+        // only support assets, and correct attributes
+        return $this->requestStack->getCurrentRequest() !== null && $subject instanceof Asset && $attribute === self::VIEW;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool
+    {
+        $hash  = Assert::notNull($this->requestStack->getCurrentRequest())->query->getString('hash');
+        $asset = Assert::isInstanceOf($subject, Asset::class);
+
+        return $hash !== '' && hash_equals($asset->getHash(), $hash);
+    }
+}

tests/Unit/Controller/App/Asset/AddAssetControllerTest.php

@@ -35,6 +35,7 @@ public function testInvoke(): void
     {
         $asset = new Asset();
         $asset->setId(123);
+        $asset->setData('foobar');
         $user = new User();
 
         $request = $this->createMock(AddAssetRequest::class);
@@ -44,7 +45,7 @@ public function testInvoke(): void
         $this->expectGetUser($user);
         $this->assetFactory->expects($this->once())->method('create')->with($user, 'mime-type', 'data')->willReturn($asset);
         $this->assetRepository->expects($this->once())->method('save')->with($asset, true);
-        $this->expectGenerateUrl(GetAssetController::class, ['id' => 123]);
+        $this->expectGenerateUrl(GetAssetController::class, ['id' => 123, 'hash' => 'c3ab8ff1']);
 
         ($this->controller)($request);
     }

tests/Unit/Entity/Asset/AssetTest.php

@@ -17,4 +17,10 @@ public function testAccessorPairs(): void
     {
         static::assertAccessorPairs(Asset::class);
     }
+
+    public function testGetHash(): void
+    {
+        $asset = (new Asset())->setData('foobar');
+        static::assertSame('c3ab8ff1', $asset->getHash());
+    }
 }