diff --git a/README.md b/README.md index 5c8abfb..9a51be4 100644 --- a/README.md +++ b/README.md @@ -401,17 +401,24 @@ tabla es derivada, así que reconstruirla siempre es seguro. ### Subir por la API `POST /api/v1/media` es el campo de subida de una ficha, sin la ficha: guarda la -imagen donde el panel pone las de las páginas y contesta con el `path` que se -manda como `featured_image`. Existe para el [MCP](#servidor-mcp), que sin él +imagen donde el panel pone las de ese tipo de ficha y contesta con el `path` que +se manda como `featured_image` de una página o `logo` de una tecnología. `for` +dice para qué es (`pages`, por defecto, o `technologies`), y con ello la carpeta +y el permiso que hace falta. Existe para el [MCP](#servidor-mcp), que sin él podía crear una página pero nunca ponerle una foto. Pasa por `Media::for()`, así que se procesa, se sirve y se poda igual que una subida desde el panel. Dos formas de entrada, una por petición: **multipart** con `file`, o **JSON** -con una `url` que la API descarga ella misma. Solo JPEG, PNG, WebP, GIF y AVIF, -decidido por los bytes y no por el nombre ni el tipo declarado. **Sin SVG**: el -panel lo acepta de quien lo administra, pero esto también descarga de URLs -arbitrarias, y un SVG es un documento que puede llevar script servido desde -este dominio. 20 MB como mucho (`MEDIA_UPLOAD_MAX_KB`). +con una `url` que la API descarga ella misma. JPEG, PNG, WebP, GIF, AVIF y SVG, +decidido por los bytes y no por el nombre ni el tipo declarado. 20 MB como mucho +(`MEDIA_UPLOAD_MAX_KB`). + +**El SVG entra, pero nunca tal cual**: es lo que es un logo, y también un +documento que puede llevar script servido desde este dominio. Pasa antes por +[`enshrined/svg-sanitize`](https://github.com/darylldoyle/svg-sanitizer), que +quita scripts, manejadores de eventos y referencias a cualquier cosa fuera del +fichero; y Caddy sirve los `/storage/*.svg` con una `Content-Security-Policy` +que pararía un script igualmente si se colara. **La URL es la parte delicada**, porque la petición la hace este servidor desde dentro de la red del stack: sin control, sería una forma de pedirle @@ -424,8 +431,8 @@ cambie entre la comprobación y la petición no cuele otra, **no sigue redirecciones** y deja de leer al pasar del límite, diga lo que diga `Content-Length`. -El permiso es escribir páginas (`pages.create` o `pages.update`), no ninguno de -`media.*`: esos siempre describieron cuidar ficheros que ya existen, y +El permiso es escribir el tipo de ficha de `for` (`pages.create` o +`pages.update`, `technologies.create` o…), no ninguno de `media.*`: esos siempre describieron cuidar ficheros que ya existen, y `MediaPolicy::create` sigue siendo `false` para que el panel no gane un botón de «nuevo fichero» suelto. @@ -1256,6 +1263,8 @@ POST /api/v1/pages/{page}/restore GET /api/v1/users PATCH para roles, is_active, locale GET /api/v1/roles solo lectura GET /api/v1/roles/catalogue catálogo de permisos agrupado + /api/v1/technologies CRUD completo; filtros: category, + radar_ring (none = fuera del radar), search POST /api/v1/media subir una imagen (fichero o URL) ``` diff --git a/app/Http/Controllers/Api/V1/MediaController.php b/app/Http/Controllers/Api/V1/MediaController.php index 5cf5999..305ff8b 100644 --- a/app/Http/Controllers/Api/V1/MediaController.php +++ b/app/Http/Controllers/Api/V1/MediaController.php @@ -9,6 +9,7 @@ use App\Http\Requests\Api\V1\MediaRequest; use App\Http\Resources\MediaResource; use App\Models\Media; use App\Services\Media\RemoteImage; +use enshrined\svgSanitize\Sanitizer; use finfo; use Illuminate\Http\JsonResponse; use Illuminate\Support\Facades\Storage; @@ -40,11 +41,14 @@ class MediaController extends Controller 'image/webp' => 'webp', 'image/gif' => 'gif', 'image/avif' => 'avif', + 'image/svg+xml' => 'svg', ]; public function store(MediaRequest $request, RemoteImage $remote): JsonResponse { - $this->authorize('upload', Media::class); + $for = (string) $request->input('for', 'pages'); + + $this->authorize('upload', [Media::class, $for]); $field = $request->hasFile('file') ? 'file' : 'url'; @@ -53,16 +57,22 @@ class MediaController extends Controller : $remote->fetch((string) $request->string('url')); $mime = (new finfo(FILEINFO_MIME_TYPE))->buffer($contents) ?: null; - $size = @getimagesizefromstring($contents); - // Both, because each catches what the other misses: the type is read - // from the magic bytes, and a file that has them but does not decode - // is not an image anybody can render. - if ($size === false || ! in_array($mime, (array) config('media.upload.mimes'), true)) { - throw ValidationException::withMessages([$field => [__('rules.media.not_an_image')]]); + if (! in_array($mime, (array) config('media.upload.mimes'), true)) { + $this->refuse($field); } - $path = config('media.upload.directory').'/'.Str::random(40).'.'.self::EXTENSIONS[$mime]; + if ($mime === 'image/svg+xml') { + $contents = $this->sanitize($contents) ?? $this->refuse($field); + // A vector has no pixel size; the processor copies it through. + $size = [null, null]; + } else { + // The type is read from the magic bytes; this catches a file that + // has them but does not decode, which nobody could render either. + $size = @getimagesizefromstring($contents) ?: $this->refuse($field); + } + + $path = config("media.upload.for.{$for}").'/'.Str::random(40).'.'.self::EXTENSIONS[$mime]; Storage::disk((string) config('media.disk'))->put($path, $contents); @@ -82,4 +92,24 @@ class MediaController extends Controller return MediaResource::make($media->fresh())->response()->setStatusCode(201); } + + /** + * An SVG with what could run taken out: scripts, event handlers, and + * references to anything outside the file. Null when what is left is not + * an SVG at all. + */ + protected function sanitize(string $svg): ?string + { + $sanitizer = new Sanitizer; + $sanitizer->removeRemoteReferences(true); + + $clean = $sanitizer->sanitize($svg); + + return is_string($clean) && str_contains($clean, ' [__('rules.media.not_an_image')]]); + } } diff --git a/app/Http/Controllers/Api/V1/TechnologyController.php b/app/Http/Controllers/Api/V1/TechnologyController.php new file mode 100644 index 0000000..50d1b6c --- /dev/null +++ b/app/Http/Controllers/Api/V1/TechnologyController.php @@ -0,0 +1,93 @@ +authorize('viewAny', Technology::class); + + $technologies = Technology::query() + ->with('logoMedia') + ->withCount('projects') + ->when($request->filled('category'), fn ($q) => $q->where('category', $request->string('category'))) + ->when($request->filled('radar_ring'), fn ($q) => $request->string('radar_ring')->toString() === 'none' + ? $q->whereNull('radar_ring') + : $q->where('radar_ring', $request->string('radar_ring'))) + ->when($request->filled('search'), fn ($q) => $q->where('name', 'like', '%'.$request->string('search').'%')) + ->orderBy('position') + ->orderBy('name') + ->paginate(perPage: min($request->integer('per_page', 25), 100)) + ->withQueryString(); + + return TechnologyResource::collection($technologies); + } + + public function store(TechnologyRequest $request): JsonResponse + { + $this->authorize('create', Technology::class); + + $technology = new Technology; + $technology->fill(TranslatablePayload::merge($technology, $request->validated(), $technology->translatable)); + // The column's default, and the panel's, stated here so the model + // knows it too: otherwise the answer to a create without a category + // would be missing the one the row was given. + $technology->category ??= TechnologyCategory::Tool; + $technology->save(); + + return TechnologyResource::make($this->loaded($technology))->response()->setStatusCode(201); + } + + public function show(Technology $technology): TechnologyResource + { + $this->authorize('view', $technology); + + return TechnologyResource::make($this->loaded($technology)); + } + + public function update(TechnologyRequest $request, Technology $technology): TechnologyResource + { + $this->authorize('update', $technology); + + $technology->fill(TranslatablePayload::merge($technology, $request->validated(), $technology->translatable)); + $technology->save(); + + return TechnologyResource::make($this->loaded($technology)); + } + + public function destroy(Technology $technology): JsonResponse + { + $this->authorize('delete', $technology); + + $technology->delete(); + + return response()->json(status: 204); + } + + protected function loaded(Technology $technology): Technology + { + return $technology->load('logoMedia')->loadCount('projects'); + } +} diff --git a/app/Http/Requests/Api/V1/MediaRequest.php b/app/Http/Requests/Api/V1/MediaRequest.php index 324e23e..4af0536 100644 --- a/app/Http/Requests/Api/V1/MediaRequest.php +++ b/app/Http/Requests/Api/V1/MediaRequest.php @@ -40,6 +40,7 @@ class MediaRequest extends FormRequest 'mimetypes:'.implode(',', (array) config('media.upload.mimes')), ], 'url' => ['required_without:file', 'string', 'url:https', 'max:2048'], + 'for' => ['sometimes', 'string', 'in:'.implode(',', array_keys((array) config('media.upload.for')))], ]; } } diff --git a/app/Http/Requests/Api/V1/TechnologyRequest.php b/app/Http/Requests/Api/V1/TechnologyRequest.php new file mode 100644 index 0000000..5f3529d --- /dev/null +++ b/app/Http/Requests/Api/V1/TechnologyRequest.php @@ -0,0 +1,59 @@ + + */ + public function rules(): array + { + $technology = $this->route('technology'); + $creating = ! $technology instanceof Technology; + $sometimes = $creating ? 'required' : 'sometimes'; + + return [ + 'name' => [$sometimes, 'string', 'max:120'], + 'slug' => [ + $sometimes, + 'string', + 'max:120', + 'alpha_dash:ascii', + Rule::unique('technologies', 'slug')->ignore($technology?->id), + ], + 'description' => ['sometimes', 'nullable', new Translatable(max: 1000)], + 'radar_note' => ['sometimes', 'nullable', new Translatable(max: 1000)], + + 'category' => ['sometimes', Rule::enum(TechnologyCategory::class)], + // Null keeps it off the radar while projects can still be tagged with it. + 'radar_ring' => ['sometimes', 'nullable', Rule::enum(RadarRing::class)], + 'color' => ['sometimes', 'nullable', 'string', 'regex:/^#[0-9a-fA-F]{6}$/'], + 'logo' => ['sometimes', 'nullable', 'string', 'max:255'], + // 255, not the panel's 2048: that is the column's width. + 'website' => ['sometimes', 'nullable', 'url', 'max:255'], + 'position' => ['sometimes', 'integer', 'min:0'], + ]; + } +} diff --git a/app/Http/Resources/TechnologyResource.php b/app/Http/Resources/TechnologyResource.php new file mode 100644 index 0000000..9a9d53f --- /dev/null +++ b/app/Http/Resources/TechnologyResource.php @@ -0,0 +1,56 @@ + + */ + public function toArray(Request $request): array + { + $locale = $request->query(config('locales.query_parameter')) ?? app()->getLocale(); + + return [ + 'id' => $this->id, + 'locale' => $locale, + 'name' => $this->name, + 'slug' => $this->slug, + + ...TranslatablePayload::resolve($this->resource, $this->translatable, $locale), + + 'category' => $this->category->value, + 'radar_ring' => $this->radar_ring?->value, + 'color' => $this->color, + // The stored path, which is what a client PATCHes back; what was + // derived from it travels alongside, as on a page. + 'logo' => $this->logo, + 'logo_media' => ImagePayload::make( + $this->logo, + $this->resource->relationLoaded('logoMedia') ? $this->resource->getRelation('logoMedia') : null, + $this->name, + ), + 'website' => $this->website, + 'position' => $this->position, + 'projects_count' => $this->whenCounted('projects'), + 'created_at' => $this->created_at?->toIso8601String(), + 'updated_at' => $this->updated_at?->toIso8601String(), + + 'translations' => $this->when( + in_array('translations', explode(',', (string) $request->query('with')), true), + fn () => TranslatablePayload::all($this->resource, $this->translatable), + ), + ]; + } +} diff --git a/app/Policies/MediaPolicy.php b/app/Policies/MediaPolicy.php index 8b22962..2be1ce1 100644 --- a/app/Policies/MediaPolicy.php +++ b/app/Policies/MediaPolicy.php @@ -24,14 +24,15 @@ class MediaPolicy extends ResourcePolicy /** * Uploading through the API, which has no form to attach a file to. * - * Still true to the rule above in spirit: through the API the one record - * that holds an image is a page, so whoever may write pages may upload the - * image for one — and nobody else, whatever their `media.*` permissions, - * which only ever described looking after files that already exist. + * Still true to the rule above in spirit: an upload says which kind of + * record it is for — a page, a technology — and whoever may write that kind + * of record may upload its image. Nobody else, whatever their `media.*` + * permissions, which only ever described looking after files that already + * exist. */ - public function upload(User $user): bool + public function upload(User $user, string $for = 'pages'): bool { - return $user->can('pages.create') || $user->can('pages.update'); + return $user->can("{$for}.create") || $user->can("{$for}.update"); } /** diff --git a/composer.json b/composer.json index 28de4de..85d1a92 100644 --- a/composer.json +++ b/composer.json @@ -7,6 +7,7 @@ "license": "MIT", "require": { "php": "^8.4", + "enshrined/svg-sanitize": "^1.0", "filament/filament": "^5.7", "laravel/framework": "^13.17", "laravel/tinker": "^3.0", diff --git a/composer.lock b/composer.lock index e854e19..ffe7219 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "e9bbf5ad6f9f32f9f506caf56db81710", + "content-hash": "6f1347758fcd5a7f4c56b93c83b60453", "packages": [ { "name": "anourvalar/eloquent-serialize", @@ -989,6 +989,51 @@ ], "time": "2025-03-06T22:45:56+00:00" }, + { + "name": "enshrined/svg-sanitize", + "version": "1.0.0", + "source": { + "type": "git", + "url": "https://github.com/darylldoyle/svg-sanitizer.git", + "reference": "f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff", + "reference": "f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff", + "shasum": "" + }, + "require": { + "ext-dom": "*", + "ext-libxml": "*", + "php": "^7.1 || ^8.0" + }, + "require-dev": { + "phpunit/phpunit": "^6.5 || ^8.5" + }, + "type": "library", + "autoload": { + "psr-4": { + "enshrined\\svgSanitize\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "GPL-2.0-or-later" + ], + "authors": [ + { + "name": "Daryll Doyle", + "email": "daryll@enshrined.co.uk" + } + ], + "description": "An SVG sanitizer for PHP", + "support": { + "issues": "https://github.com/darylldoyle/svg-sanitizer/issues", + "source": "https://github.com/darylldoyle/svg-sanitizer/tree/1.0.0" + }, + "time": "2026-09-01T09:35:47+00:00" + }, { "name": "filament/actions", "version": "v5.7.6", diff --git a/config/media.php b/config/media.php index 9e2458f..b88239b 100644 --- a/config/media.php +++ b/config/media.php @@ -84,9 +84,12 @@ return [ | | POST /api/v1/media, which the MCP server uses to put an image on a page. | - | Raster formats only. The panel's own upload accepts SVG from the people - | who run it; this endpoint also fetches from arbitrary URLs, and an SVG is - | a document that can carry script, served from this application's domain. + | SVG is accepted because it is what a logo is, but never as sent: an SVG + | is a document that can carry script, served from this application's + | domain, and this endpoint also fetches from arbitrary URLs. It goes + | through enshrined/svg-sanitize first — scripts, event handlers and + | remote references removed — and Caddy serves stored SVGs under a + | Content-Security-Policy that would stop a script anyway. | | The size ceiling sits under php.ini's `upload_max_filesize` (24M) so the | answer is a readable 422 rather than PHP silently dropping the file. @@ -94,9 +97,15 @@ return [ */ 'upload' => [ - 'mimes' => ['image/jpeg', 'image/png', 'image/webp', 'image/gif', 'image/avif'], + 'mimes' => ['image/jpeg', 'image/png', 'image/webp', 'image/gif', 'image/avif', 'image/svg+xml'], 'max_kb' => (int) env('MEDIA_UPLOAD_MAX_KB', 20 * 1024), - 'directory' => 'pages', + + // What the image is for, and so where it lands — the same directories + // the panel's own upload fields use — and whose permission it takes. + 'for' => [ + 'pages' => 'pages', + 'technologies' => 'technologies', + ], // Fetching from a URL: how long to wait, in seconds. The request that // triggered it is waiting too. diff --git a/docker/Caddyfile b/docker/Caddyfile index b3f2672..829e657 100644 --- a/docker/Caddyfile +++ b/docker/Caddyfile @@ -21,6 +21,13 @@ @media path /storage/* header @media Cache-Control "public, max-age=31536000, immutable" + # Uploaded SVGs are sanitized on the way in (MediaController). This is the + # second line: opened directly rather than through an , an SVG is a + # document, and nothing stored here has any business running script or + # loading anything from elsewhere. + @svg path /storage/*.svg + header @svg Content-Security-Policy "default-src 'none'; style-src 'unsafe-inline'; img-src data:" + # The frontend is a separate application on another origin. @api path /api/* header @api { diff --git a/lang/ca/rules.php b/lang/ca/rules.php index 869bdfe..a4a2686 100644 --- a/lang/ca/rules.php +++ b/lang/ca/rules.php @@ -30,7 +30,7 @@ return [ ], 'media' => [ - 'not_an_image' => 'El fitxer no és una imatge JPEG, PNG, WebP, GIF o AVIF.', + 'not_an_image' => 'El fitxer no és una imatge JPEG, PNG, WebP, GIF, AVIF o SVG.', 'remote' => [ 'https_only' => 'La URL de la imatge ha de començar per https://.', 'unresolvable' => 'No s\'ha pogut resoldre el domini de la URL.', diff --git a/lang/en/rules.php b/lang/en/rules.php index 62ae1fe..cef56e1 100644 --- a/lang/en/rules.php +++ b/lang/en/rules.php @@ -30,7 +30,7 @@ return [ ], 'media' => [ - 'not_an_image' => 'The file is not a JPEG, PNG, WebP, GIF or AVIF image.', + 'not_an_image' => 'The file is not a JPEG, PNG, WebP, GIF, AVIF or SVG image.', 'remote' => [ 'https_only' => 'The image URL must start with https://.', 'unresolvable' => 'The URL\'s domain could not be resolved.', diff --git a/lang/es/rules.php b/lang/es/rules.php index 87cbe32..1d9f03e 100644 --- a/lang/es/rules.php +++ b/lang/es/rules.php @@ -30,7 +30,7 @@ return [ ], 'media' => [ - 'not_an_image' => 'El fichero no es una imagen JPEG, PNG, WebP, GIF o AVIF.', + 'not_an_image' => 'El fichero no es una imagen JPEG, PNG, WebP, GIF, AVIF o SVG.', 'remote' => [ 'https_only' => 'La URL de la imagen tiene que empezar por https://.', 'unresolvable' => 'No se ha podido resolver el dominio de la URL.', diff --git a/mcp/src/tools.mjs b/mcp/src/tools.mjs index b13b358..1196e68 100644 --- a/mcp/src/tools.mjs +++ b/mcp/src/tools.mjs @@ -145,7 +145,17 @@ function buildTool(spec, paths, path, method, item, op, localFiles) { // it has no reason to touch. if (method === 'patch') delete body.required; - properties.body = { ...body, description: 'JSON request body.' }; + // Beside a file, the body carries the upload's other fields (`for`, say) + // rather than the JSON alternative, whose own requirements — a `url` — + // would then be wrong. + if (files.length > 0) delete body.required; + + properties.body = { + ...body, + description: files.length > 0 + ? 'JSON request body; with a file, its other fields go here too.' + : 'JSON request body.', + }; // With a file to send instead, the JSON body is the other way in rather // than a requirement; the API says which one it missed if neither comes. diff --git a/mcp/test/tools.test.mjs b/mcp/test/tools.test.mjs index c8eb8d0..c2b2e7d 100644 --- a/mcp/test/tools.test.mjs +++ b/mcp/test/tools.test.mjs @@ -23,6 +23,10 @@ test('names follow the Laravel routes', () => { admin_pages_destroy: 'DELETE /v1/pages/{page}', admin_pages_restore: 'POST /v1/pages/{page}/restore', admin_roles_catalogue: 'GET /v1/roles/catalogue', + admin_technologies_index: 'GET /v1/technologies', + admin_technologies_store: 'POST /v1/technologies', + admin_technologies_update: 'PATCH /v1/technologies/{technology}', + admin_media_store: 'POST /v1/media', }; for (const [name, route] of Object.entries(expected)) { @@ -86,7 +90,10 @@ test('over stdio an upload takes a local path, and the JSON body becomes optiona assert.deepEqual(local.operation.files, ['file']); assert.equal(local.inputSchema.properties.file_path.type, 'string'); assert.ok(local.inputSchema.properties.body.properties.url); + assert.ok(local.inputSchema.properties.body.properties.for); assert.equal(local.inputSchema.required, undefined); + // `url` is the JSON alternative; beside a file it must not be demanded. + assert.equal(local.inputSchema.properties.body.required, undefined); }); test('over HTTP there is no local path to offer: only the URL', () => { diff --git a/public/openapi.yaml b/public/openapi.yaml index 1810313..fc2bb97 100644 --- a/public/openapi.yaml +++ b/public/openapi.yaml @@ -106,6 +106,8 @@ tags: description: Translatable page content - name: Categories description: Translatable page categories + - name: Technologies + description: What the collective uses, and its place on the tech radar - name: Users description: Local user administration - name: Media @@ -1174,6 +1176,109 @@ paths: "204": { description: Deleted } "403": { $ref: "#/components/responses/Forbidden" } + /v1/technologies: + get: + tags: [Technologies] + summary: List technologies + description: Ordered by `position`, then name. + parameters: + - $ref: "#/components/parameters/Lang" + - $ref: "#/components/parameters/With" + - $ref: "#/components/parameters/PerPage" + - name: category + in: query + schema: { type: string, enum: [language, framework, datastore, infrastructure, tool, practice] } + - name: radar_ring + in: query + description: '`none` for the technologies kept off the radar.' + schema: { type: string, enum: [adopt, trial, assess, hold, none] } + - name: search + in: query + description: Matches the name. + schema: { type: string } + responses: + "200": + description: Paginated technologies + content: + application/json: + schema: { $ref: "#/components/schemas/TechnologyCollection" } + "403": { $ref: "#/components/responses/Forbidden" } + post: + tags: [Technologies] + summary: Create a technology + description: | + For a logo, upload it first with `POST /v1/media` and `for: + technologies`, and send the `path` it answers with as `logo`. + requestBody: + required: true + content: + application/json: + schema: { $ref: "#/components/schemas/TechnologyWrite" } + responses: + "201": + description: Technology created + content: + application/json: + schema: + type: object + properties: + data: { $ref: "#/components/schemas/Technology" } + "403": { $ref: "#/components/responses/Forbidden" } + "422": { $ref: "#/components/responses/ValidationError" } + + /v1/technologies/{technology}: + parameters: + - name: technology + in: path + required: true + schema: { type: integer } + get: + tags: [Technologies] + summary: Show a technology + parameters: + - $ref: "#/components/parameters/Lang" + - $ref: "#/components/parameters/With" + responses: + "200": + description: The technology + content: + application/json: + schema: + type: object + properties: + data: { $ref: "#/components/schemas/Technology" } + "403": { $ref: "#/components/responses/Forbidden" } + "404": { description: Not found } + patch: + tags: [Technologies] + summary: Update a technology + description: | + Partial update. `description` and `radar_note` merge into the existing + translations; send `radar_ring: null` to take it off the radar. + requestBody: + required: true + content: + application/json: + schema: { $ref: "#/components/schemas/TechnologyWrite" } + responses: + "200": + description: Technology updated + content: + application/json: + schema: + type: object + properties: + data: { $ref: "#/components/schemas/Technology" } + "403": { $ref: "#/components/responses/Forbidden" } + "422": { $ref: "#/components/responses/ValidationError" } + delete: + tags: [Technologies] + summary: Delete a technology + description: Refused with 403 while a project, member or call still uses it. + responses: + "204": { description: Deleted } + "403": { $ref: "#/components/responses/Forbidden" } + /v1/media: post: tags: [Media] @@ -1188,12 +1293,19 @@ paths: reserved or loopback address is refused — and redirects are not followed: send the image's final address. - JPEG, PNG, WebP, GIF or AVIF, decided from the file's bytes rather than - its name or declared type; SVG is not accepted here. 20 MB at most. + JPEG, PNG, WebP, GIF, AVIF or SVG, decided from the file's bytes rather + than its name or declared type. An SVG is stored sanitized: scripts, + event handlers and references to anything outside the file are + removed. 20 MB at most. + + `for` says which kind of record the image is for — `pages` (the + default) or `technologies` — which decides where it is stored and whose + permission it takes. The smaller versions and the placeholder are derived afterwards, on the queue: `processed` says whether that has happened, and the image serves - either way. Requires permission to create or edit pages. + either way. Requires permission to create or edit the records named in + `for`. requestBody: required: true content: @@ -1206,6 +1318,7 @@ paths: type: string format: binary description: The image. + for: { $ref: "#/components/schemas/MediaFor" } application/json: schema: type: object @@ -1216,6 +1329,7 @@ paths: format: uri description: An `https://` address the API downloads the image from. example: https://images.example.org/hero.jpg + for: { $ref: "#/components/schemas/MediaFor" } responses: "201": description: Image stored @@ -2181,6 +2295,76 @@ components: nullable: true description: Must be after `published_at`. When it arrives the record leaves the site on its own. + MediaFor: + type: string + enum: [pages, technologies] + default: pages + description: The kind of record the image is for. + + Technology: + type: object + properties: + id: { type: integer } + locale: { type: string, description: The language the prose below was resolved into. } + name: { type: string, description: A proper noun — the same in every language. } + slug: { type: string } + description: { type: string, nullable: true } + radar_note: { type: string, nullable: true, description: Why it sits in its ring. } + category: { type: string, enum: [language, framework, datastore, infrastructure, tool, practice] } + radar_ring: + type: string + enum: [adopt, trial, assess, hold] + nullable: true + description: Null keeps it off the radar; projects can still use it. + color: { type: string, nullable: true, example: "#DD0031" } + logo: + type: string + nullable: true + description: The stored path — what `POST /v1/media` answered — and what a client PATCHes back. + logo_media: + allOf: [{ $ref: "#/components/schemas/Image" }] + nullable: true + readOnly: true + website: { type: string, nullable: true, format: uri } + position: { type: integer } + projects_count: { type: integer } + created_at: { type: string, format: date-time } + updated_at: { type: string, format: date-time } + translations: + type: object + nullable: true + description: Every language of `description` and `radar_note`, with `?with=translations`. + + TechnologyWrite: + type: object + required: [name, slug] + properties: + name: { type: string, maxLength: 120 } + slug: { type: string, maxLength: 120, pattern: "^[A-Za-z0-9_-]+$" } + description: { $ref: "#/components/schemas/TranslatedField" } + radar_note: { $ref: "#/components/schemas/TranslatedField" } + category: + type: string + enum: [language, framework, datastore, infrastructure, tool, practice] + default: tool + radar_ring: { type: string, enum: [adopt, trial, assess, hold], nullable: true } + color: { type: string, nullable: true, pattern: "^#[0-9a-fA-F]{6}$" } + logo: + type: string + nullable: true + description: A path from `POST /v1/media`, uploaded with `for=technologies`. + website: { type: string, format: uri, nullable: true, maxLength: 255 } + position: { type: integer, minimum: 0 } + + TechnologyCollection: + type: object + properties: + data: + type: array + items: { $ref: "#/components/schemas/Technology" } + links: { $ref: "#/components/schemas/PaginationLinks" } + meta: { $ref: "#/components/schemas/PaginationMeta" } + Media: type: object properties: diff --git a/routes/api.php b/routes/api.php index 2ab798d..01e9e78 100644 --- a/routes/api.php +++ b/routes/api.php @@ -15,6 +15,7 @@ use App\Http\Controllers\Api\V1\MeController; use App\Http\Controllers\Api\V1\MediaController; use App\Http\Controllers\Api\V1\PageController; use App\Http\Controllers\Api\V1\RoleController; +use App\Http\Controllers\Api\V1\TechnologyController; use App\Http\Controllers\Api\V1\TokenController; use App\Http\Controllers\Api\V1\UserController; use Illuminate\Support\Facades\Route; @@ -166,6 +167,8 @@ Route::prefix('v1')->name('api.v1.')->group(function () { Route::apiResource('categories', CategoryController::class); + Route::apiResource('technologies', TechnologyController::class); + // Upload only: the file is then referenced from a record by its path. Route::post('media', [MediaController::class, 'store'])->name('media.store'); diff --git a/tests/Feature/Api/AdminApiContractTest.php b/tests/Feature/Api/AdminApiContractTest.php index fae19b6..b23fa06 100644 --- a/tests/Feature/Api/AdminApiContractTest.php +++ b/tests/Feature/Api/AdminApiContractTest.php @@ -6,6 +6,7 @@ namespace Tests\Feature\Api; use App\Models\Category; use App\Models\Page; +use App\Models\Technology; use App\Models\User; use Database\Seeders\RolePermissionSeeder; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -64,6 +65,7 @@ class AdminApiContractTest extends TestCase 'pages' => ['path' => '/v1/pages'], 'categories' => ['path' => '/v1/categories'], 'users' => ['path' => '/v1/users'], + 'technologies' => ['path' => '/v1/technologies'], ]; } @@ -72,6 +74,7 @@ class AdminApiContractTest extends TestCase { $page = Page::factory()->create(); $category = Category::factory()->create(); + $technology = Technology::factory()->create(); // One request, one documented path — the URL a real caller hits and // the path key that describes it, side by side rather than derived @@ -82,6 +85,8 @@ class AdminApiContractTest extends TestCase 'page' => ["/v1/pages/{$page->id}", '/v1/pages/{page}'], 'categories' => ['/v1/categories', '/v1/categories'], 'category' => ["/v1/categories/{$category->id}", '/v1/categories/{category}'], + 'technologies' => ['/v1/technologies', '/v1/technologies'], + 'technology' => ["/v1/technologies/{$technology->id}", '/v1/technologies/{technology}'], 'users' => ['/v1/users', '/v1/users'], 'user' => ["/v1/users/{$this->caller->id}", '/v1/users/{user}'], 'me' => ['/v1/me', '/v1/me'], diff --git a/tests/Feature/Api/MediaUploadTest.php b/tests/Feature/Api/MediaUploadTest.php index 0950ceb..0427d4e 100644 --- a/tests/Feature/Api/MediaUploadTest.php +++ b/tests/Feature/Api/MediaUploadTest.php @@ -240,8 +240,7 @@ class MediaUploadTest extends TestCase { return [ 'text renamed to .jpg' => ['photo.jpg', 'not an image at all'], - // Served from this domain, an SVG is a document that can run script. - 'an SVG' => ['logo.svg', ''], + 'HTML renamed to .svg' => ['logo.svg', 'not a vector'], ]; } @@ -260,6 +259,73 @@ class MediaUploadTest extends TestCase $this->assertDatabaseCount('media', 0); } + #[Test] + public function an_svg_is_stored_with_whatever_could_run_taken_out(): void + { + $this->actingWithRole('editor'); + + $svg = '' + .'' + .'' + .'' + .''; + + $response = $this->post('/api/v1/media', [ + 'file' => UploadedFile::fake()->createWithContent('logo.svg', $svg), + ], ['Accept' => 'application/json']) + ->assertCreated() + ->assertJsonPath('data.mime', 'image/svg+xml') + ->assertJsonPath('data.width', null); + + $stored = Storage::disk((string) config('media.disk'))->get($response->json('data.path')); + + $this->assertStringEndsWith('.svg', $response->json('data.path')); + $this->assertStringContainsString('#21759B', $stored); + + foreach (['assertStringNotContainsString($removed, $stored); + } + } + + #[Test] + public function an_upload_for_a_technology_lands_where_the_panel_puts_their_logos(): void + { + $this->actingWithRole('editor'); + + $this->post('/api/v1/media', [ + 'file' => UploadedFile::fake()->image('logo.png'), + 'for' => 'technologies', + ], ['Accept' => 'application/json']) + ->assertCreated() + ->assertJsonPath('data.path', fn (string $path) => str_starts_with($path, 'technologies/')); + } + + #[Test] + public function it_takes_the_permission_of_the_record_it_is_for(): void + { + // A writer authors pages but only reads technologies. + $this->actingWithRole('writer'); + + $this->post('/api/v1/media', [ + 'file' => UploadedFile::fake()->image('logo.png'), + 'for' => 'technologies', + ], ['Accept' => 'application/json']) + ->assertForbidden(); + } + + #[Test] + public function it_only_knows_the_records_it_is_told_about(): void + { + $this->actingWithRole('super_admin'); + + $this->post('/api/v1/media', [ + 'file' => UploadedFile::fake()->image('logo.png'), + 'for' => '../../etc', + ], ['Accept' => 'application/json']) + ->assertUnprocessable() + ->assertJsonValidationErrors('for'); + } + #[Test] public function it_wants_exactly_one_source(): void { diff --git a/tests/Feature/Api/TechnologyApiTest.php b/tests/Feature/Api/TechnologyApiTest.php new file mode 100644 index 0000000..13596d4 --- /dev/null +++ b/tests/Feature/Api/TechnologyApiTest.php @@ -0,0 +1,165 @@ +seed(RolePermissionSeeder::class); + } + + protected function actingWithRole(string $role): void + { + $user = User::factory()->create(); + $user->syncRoles([$role]); + + $this->withHeaders($this->apiHeadersFor($user)); + } + + #[Test] + public function it_creates_a_technology_with_its_prose_in_several_languages(): void + { + $this->actingWithRole('editor'); + + $this->postJson('/api/v1/technologies?lang=es&with=translations', [ + 'name' => 'Kotlin Multiplatform', + 'slug' => 'kotlin-multiplatform', + 'category' => 'framework', + 'radar_ring' => 'trial', + 'color' => '#7F52FF', + 'website' => 'https://kotlinlang.org/docs/multiplatform.html', + 'description' => ['es' => 'Código Kotlin compartido.', 'en' => 'Shared Kotlin code.'], + ]) + ->assertCreated() + ->assertJsonPath('data.name', 'Kotlin Multiplatform') + ->assertJsonPath('data.radar_ring', 'trial') + ->assertJsonPath('data.description', 'Código Kotlin compartido.') + ->assertJsonPath('data.translations.description.en', 'Shared Kotlin code.'); + + $this->assertSame(RadarRing::Trial, Technology::query()->sole()->radar_ring); + } + + #[Test] + public function the_logo_is_a_path_from_the_upload_endpoint(): void + { + Storage::fake((string) config('media.disk')); + $this->actingWithRole('editor'); + + $path = $this->post('/api/v1/media', [ + 'file' => UploadedFile::fake()->createWithContent( + 'angular.svg', + '', + ), + 'for' => 'technologies', + ], ['Accept' => 'application/json'])->json('data.path'); + + $this->postJson('/api/v1/technologies', [ + 'name' => 'Angular', + 'slug' => 'angular', + 'logo' => $path, + ]) + ->assertCreated() + ->assertJsonPath('data.logo', $path) + ->assertJsonPath('data.logo_media.url', fn (string $url) => str_ends_with($url, $path)) + ->assertJsonPath('data.logo_media.alt', 'Angular'); + } + + #[Test] + public function a_partial_update_leaves_the_rest_alone(): void + { + $this->actingWithRole('editor'); + + $technology = Technology::factory()->create([ + 'name' => 'Laravel', + 'radar_ring' => RadarRing::Assess, + 'description' => ['es' => 'Framework PHP.', 'en' => 'PHP framework.'], + ]); + + $this->patchJson("/api/v1/technologies/{$technology->id}", [ + 'radar_ring' => 'adopt', + 'description' => ['ca' => 'Framework PHP.'], + ]) + ->assertOk() + ->assertJsonPath('data.name', 'Laravel') + ->assertJsonPath('data.radar_ring', 'adopt'); + + $this->assertSame('PHP framework.', $technology->fresh()->getTranslation('description', 'en')); + } + + #[Test] + public function a_ring_can_be_cleared_to_take_it_off_the_radar(): void + { + $this->actingWithRole('editor'); + + $technology = Technology::factory()->create(['radar_ring' => RadarRing::Hold]); + + $this->patchJson("/api/v1/technologies/{$technology->id}", ['radar_ring' => null]) + ->assertOk() + ->assertJsonPath('data.radar_ring', null); + + $this->getJson('/api/v1/technologies?radar_ring=none') + ->assertOk() + ->assertJsonCount(1, 'data'); + } + + #[Test] + public function slugs_are_unique_and_url_safe(): void + { + $this->actingWithRole('editor'); + + Technology::factory()->create(['slug' => 'react']); + + $this->postJson('/api/v1/technologies', ['name' => 'React', 'slug' => 'react']) + ->assertUnprocessable() + ->assertJsonValidationErrors('slug'); + + $this->postJson('/api/v1/technologies', ['name' => 'Spring Boot', 'slug' => 'spring boot']) + ->assertUnprocessable() + ->assertJsonValidationErrors('slug'); + } + + #[Test] + public function one_still_in_use_cannot_be_deleted(): void + { + $this->actingWithRole('editor'); + + $technology = Technology::factory()->create(); + Project::factory()->create()->technologies()->attach($technology); + + $this->deleteJson("/api/v1/technologies/{$technology->id}")->assertForbidden(); + + $this->assertModelExists($technology); + } + + #[Test] + public function a_reader_can_list_but_not_write(): void + { + $this->actingWithRole('viewer'); + + Technology::factory()->create(); + + $this->getJson('/api/v1/technologies')->assertOk()->assertJsonCount(1, 'data'); + + $this->postJson('/api/v1/technologies', ['name' => 'Flutter', 'slug' => 'flutter']) + ->assertForbidden(); + } +}