diff --git a/.env.example b/.env.example index 652be4e..084230e 100644 --- a/.env.example +++ b/.env.example @@ -1,10 +1,10 @@ # Copy to .env and fill in. `docker compose up` aborts if a required value is missing. # --- Public hostnames ------------------------------------------------------- -# Each needs an A record (and please an AAAA record) pointing at this host. +# Two names, each with an A record (and please an AAAA record) pointing at this +# host. The API has none of its own; it is served under /api on both. # FRONTEND_HOST is the domain your readers visit. It must additionally be # registered as a Website/Origin on the ticker itself, in the admin interface. -API_HOST=api.ticker.example.org ADMIN_HOST=admin.ticker.example.org FRONTEND_HOST=ticker.example.org diff --git a/compose.dev.yaml b/compose.dev.yaml index 78d1446..99793ab 100644 --- a/compose.dev.yaml +++ b/compose.dev.yaml @@ -7,11 +7,12 @@ # # Admin: http://admin.ticker.localhost # Frontend: http://ticker.localhost -# API: http://api.ticker.localhost # Traefik: http://localhost:8081 # +# The API has no hostname of its own; it is reached through /api on both. +# # Browsers resolve *.localhost to 127.0.0.1 themselves. If your setup does not, -# add the three names to /etc/hosts. +# add the two names to /etc/hosts. name: ticker-dev @@ -57,7 +58,6 @@ services: TICKER_DATABASE_TYPE: "postgres" TICKER_DATABASE_DSN: "host=postgres port=5432 user=ticker password=ticker dbname=ticker sslmode=disable TimeZone=UTC" TICKER_UPLOAD_PATH: "/data/uploads" - TICKER_UPLOAD_URL: "http://api.ticker.localhost" volumes: - ticker-data:/data healthcheck: @@ -71,9 +71,10 @@ services: - traefik.enable=true - traefik.http.services.ticker.loadbalancer.server.port=8080 - - traefik.http.routers.ticker-api.rule=Host(`api.ticker.localhost`) - - traefik.http.routers.ticker-api.entrypoints=web - - traefik.http.routers.ticker-api.service=ticker + - traefik.http.routers.ticker-health.rule=Host(`ticker.localhost`) && Path(`/healthz`) + - traefik.http.routers.ticker-health.priority=100 + - traefik.http.routers.ticker-health.entrypoints=web + - traefik.http.routers.ticker-health.service=ticker - traefik.http.middlewares.api-strip.stripprefix.prefixes=/api - traefik.http.middlewares.api-v1.addprefix.prefix=/v1 @@ -94,6 +95,11 @@ services: admin: image: systemli/ticker-admin:latest + depends_on: + ticker: + condition: service_healthy + environment: + TICKER_API_URL: "http://ticker:8080/v1" networks: - default labels: @@ -104,6 +110,11 @@ services: frontend: image: systemli/ticker-frontend:latest + depends_on: + ticker: + condition: service_healthy + environment: + TICKER_API_URL: "http://ticker:8080/v1" networks: - default labels: diff --git a/compose.yaml b/compose.yaml index 4e8d440..b80c78b 100644 --- a/compose.yaml +++ b/compose.yaml @@ -10,6 +10,9 @@ # /api/** to /v1/** on their own hostnames. It also injects the Origin header, # which the API needs in order to know which ticker a request belongs to -- # browsers omit Origin on same-origin GET requests. +# +# The API has no hostname of its own. Attachments are served below /v1, so they +# arrive through the same /api path as every other request. name: ticker @@ -90,10 +93,6 @@ services: TICKER_DATABASE_TYPE: "postgres" TICKER_DATABASE_DSN: "host=postgres port=5432 user=ticker password=${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in .env} dbname=ticker sslmode=disable TimeZone=UTC" TICKER_UPLOAD_PATH: "/data/uploads" - # Attachment URLs are absolute and built from this value. It must be the - # public API base -- no /v1, no trailing slash -- because /media is served - # by the API at its root. - TICKER_UPLOAD_URL: "https://${API_HOST:?set API_HOST in .env}" volumes: - ticker-data:/data networks: @@ -112,11 +111,15 @@ services: - traefik.http.services.ticker.loadbalancer.healthcheck.path=/healthz - traefik.http.services.ticker.loadbalancer.healthcheck.interval=30s - # --- Public API host: /v1/**, /media/**, /healthz, feeds --- - - traefik.http.routers.ticker-api.rule=Host(`${API_HOST}`) - - traefik.http.routers.ticker-api.entrypoints=websecure - - traefik.http.routers.ticker-api.tls.certresolver=le - - traefik.http.routers.ticker-api.service=ticker + # --- /healthz, for uptime monitoring --- + # It lives outside /v1, so the /api rewrite below cannot reach it. Without + # its own router the frontend's SPA fallback would answer with index.html + # and a monitor would report healthy no matter what. + - traefik.http.routers.ticker-health.rule=Host(`${FRONTEND_HOST}`) && Path(`/healthz`) + - traefik.http.routers.ticker-health.priority=100 + - traefik.http.routers.ticker-health.entrypoints=websecure + - traefik.http.routers.ticker-health.tls.certresolver=le + - traefik.http.routers.ticker-health.service=ticker # --- Shared /api/** -> /v1/** rewrite, applied in this order --- - traefik.http.middlewares.api-strip.stripprefix.prefixes=/api @@ -145,6 +148,13 @@ services: admin: image: systemli/ticker-admin:${ADMIN_TAG:-latest} restart: unless-stopped + depends_on: + - ticker + environment: + # The image renders its nginx config at start and proxies /api itself. In + # this stack Traefik gets there first, but nginx refuses to start without + # the value. + TICKER_API_URL: "http://ticker:8080/v1" networks: - proxy labels: @@ -158,6 +168,10 @@ services: frontend: image: systemli/ticker-frontend:${FRONTEND_TAG:-latest} restart: unless-stopped + depends_on: + - ticker + environment: + TICKER_API_URL: "http://ticker:8080/v1" networks: - proxy labels: diff --git a/config.yml.dist b/config.yml.dist index c30ba9c..e9f55c0 100644 --- a/config.yml.dist +++ b/config.yml.dist @@ -25,8 +25,6 @@ secret: "" # listen address for the prometheus metrics exporter metrics_listen: ":8181" upload: - # path where uploaded files are stored + # path where uploaded files are stored. Attachment links are host-relative, + # so there is nothing else to configure here. path: "uploads" - # public base URL of this API, used to build attachment links. - # No "/v1" and no trailing slash. - url: "http://localhost:8080" diff --git a/docs/api.md b/docs/api.md index 099bc82..9dcc340 100644 --- a/docs/api.md +++ b/docs/api.md @@ -7,12 +7,12 @@ [route definitions](https://github.com/systemli/ticker/blob/main/internal/api/api.go) are the authoritative reference for it. -All endpoints are served under the `/v1` prefix, with two exceptions that live at the root: +All endpoints are served under the `/v1` prefix, including `GET /v1/media/{file}` for uploaded +attachments. The only exception is `GET /healthz`, which lives at the root. -| Endpoint | Purpose | -| --- | --- | -| `GET /media/{file}` | uploaded attachments | -| `GET /healthz` | health check | +Attachment URLs in responses are **relative to the site that served them**, of the form +`/api/media/{file}` — that is the path the admin and frontend expose the API under. When you talk to +the API directly, replace `/api` with `/v1`. ## Identifying a ticker @@ -24,7 +24,7 @@ Clients that do not send an `Origin` header — RSS readers, scripts — can pas query parameter, which takes precedence: ```shell -curl 'https://api.ticker.example.org/v1/feed?origin=https://ticker.example.org' +curl 'https://ticker.example.org/api/feed?origin=https://ticker.example.org' ``` Requests that match no ticker return HTTP 200 with a `ticker not found` error body, or, for `/init`, diff --git a/docs/configuration.md b/docs/configuration.md index 4a16779..f765341 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -26,7 +26,6 @@ For container deployments, environment variables alone are usually enough. | `database.dsn` | `TICKER_DATABASE_DSN` | `ticker.db` | Connection string, see below. | | `metrics_listen` | `TICKER_METRICS_LISTEN` | `:8181` | Address for the Prometheus exporter, on a separate listener. | | `upload.path` | `TICKER_UPLOAD_PATH` | `uploads` | Directory for uploaded files. | -| `upload.url` | `TICKER_UPLOAD_URL` | `http://localhost:8080` | Public base URL used to build attachment links. | That is the complete list. There is no environment variable for any setting not named above. @@ -110,33 +109,31 @@ The schema is migrated automatically at startup; there is no separate migrate co ## Uploads -Two settings work together: - -- `TICKER_UPLOAD_PATH` is where files are written. It must be a **persistent, writable** directory, - otherwise attachments are lost when the container is replaced while the database still references - them. -- `TICKER_UPLOAD_URL` is the **public base URL of the API**, and is used to build absolute - attachment links of the form `/media/`. +There is one setting: `TICKER_UPLOAD_PATH`, the directory files are written to. It must be +**persistent and writable**, otherwise attachments are lost when the container is replaced while the +database still references them. ```shell TICKER_UPLOAD_PATH=/data/uploads -TICKER_UPLOAD_URL=https://api.ticker.example.org ``` -`TICKER_UPLOAD_URL` must be the API's own public hostname, with **no `/v1`** and **no trailing -slash**. Media is served by the API at its root, so: +Nothing else needs configuring. Attachments are served at `/v1/media/` and the URLs in API +responses are relative — `/api/media/`, resolved against whichever site served the response. +So the same response works for both interfaces, and the API needs no public address of its own. -- `https://api.ticker.example.org/v1` produces `…/v1/media/x`, which is a 404. -- Pointing it at the frontend produces links the frontend answers with its own HTML page rather - than an image. +!!! note "`TICKER_UPLOAD_URL` was removed" -These URLs are generated per response rather than stored, so correcting the value fixes existing -messages too, once the response cache expires. + Earlier versions built absolute attachment links from it. It is ignored now; the API logs a + warning when it is still set so you can drop it from your environment. Uploads accept `image/jpeg`, `image/gif` and `image/png` only, and the API rejects request bodies over 10 MB. If a reverse proxy in front of it imposes a smaller limit, uploads fail there first — nginx defaults to 1 MB, for instance. +The stored file extension is derived from the detected content type, not from the uploaded filename, +and media responses carry `Content-Type` from the database plus `X-Content-Type-Options: nosniff`. +That matters because attachments share an origin with the admin interface. + ## Metrics Prometheus metrics are served on a **separate** listener, `metrics_listen` (`:8181` by default), at diff --git a/docs/development.md b/docs/development.md index b25a344..de213ea 100644 --- a/docs/development.md +++ b/docs/development.md @@ -27,9 +27,10 @@ docker compose -f compose.dev.yaml up -d --build | --- | --- | | | public frontend | | | admin interface | -| | API | | | Traefik dashboard | +The API has no address of its own; it answers under `/api` on both hostnames. + Create a user, then set up a ticker: ```shell @@ -123,17 +124,20 @@ npm run dev | ticker-admin | | | ticker-frontend | | -Point them at an API with a `.env` file. The variable must include the `/v1` suffix: +Both dev servers proxy `/api` to `http://localhost:8080/v1`, so run the API alongside them with +`go run . run` and nothing needs configuring. -```shell title=".env" -TICKER_API_URL=http://localhost:8080/v1 -``` +!!! warning "Delete a leftover `.env`" + + `TICKER_API_URL` overrides the proxy with an absolute address. Requests then work but attachment + images do not, because their URLs are relative and resolve against the dev server instead. The + file is gitignored, so an old one may still be sitting in your checkout. !!! warning "Register the dev server's own origin" - With an absolute `TICKER_API_URL`, the browser sends the **dev server's** address as `Origin`. - For the public frontend that means the ticker needs `http://localhost:4000` registered under its - websites, or you will only ever see the inactive page. The admin interface is unaffected. + The proxy sends the **dev server's** address as `Origin`. For the public frontend that means the + ticker needs `http://localhost:4000` registered under its websites, or you will only ever see + the inactive page. The admin interface is unaffected. Other commands, in both repositories: diff --git a/docs/index.md b/docs/index.md index 69b1f01..b4e9aa0 100644 --- a/docs/index.md +++ b/docs/index.md @@ -16,7 +16,7 @@ A complete installation is three services, published as three Docker images: | Component | Image | Role | | --- | --- | --- | -| [ticker](https://github.com/systemli/ticker) | `systemli/ticker` | The API. Stores everything, serves the public endpoints and media, dispatches to integrations. | +| [ticker](https://github.com/systemli/ticker) | `systemli/ticker` | The API. Stores everything, serves the public endpoints and media, dispatches to integrations. It needs no public hostname of its own. | | [ticker-admin](https://github.com/systemli/ticker-admin) | `systemli/ticker-admin` | Admin interface. Editors log in here to manage tickers, messages and users. | | [ticker-frontend](https://github.com/systemli/ticker-frontend) | `systemli/ticker-frontend` | The public page your readers visit. | @@ -26,28 +26,26 @@ data of their own. ## How a request flows ``` - ┌─────────────────────────────┐ - readers ────────────▶│ ticker.example.org │ - │ ticker-frontend (SPA) │ - │ /api/** ──────────────────┼──┐ - └─────────────────────────────┘ │ - │ - ┌─────────────────────────────┐ │ ┌──────────────┐ - editors ────────────▶│ admin.ticker.example.org │ ├──▶│ ticker │ - │ ticker-admin (SPA) │ │ │ (API) │ - │ /api/** ──────────────────┼──┤ │ │ - └─────────────────────────────┘ │ └──────┬───────┘ - │ │ - ┌─────────────────────────────┐ │ ┌──────▼───────┐ - feeds, media ───────▶│ api.ticker.example.org │──┘ │ PostgreSQL │ - │ /v1/**, /media/** │ └──────────────┘ - └─────────────────────────────┘ + readers ┌─────────────────────────────┐ + feeds, ────────▶│ ticker.example.org │ + media │ ticker-frontend (SPA) │ + │ /api/** ──────────────────┼──┐ + └─────────────────────────────┘ │ ┌──────────────┐ + ├─────▶│ ticker │ + ┌─────────────────────────────┐ │ │ (API) │ + editors ───────▶│ admin.ticker.example.org │ │ │ │ + │ ticker-admin (SPA) │ │ └──────┬───────┘ + │ /api/** ──────────────────┼──┘ │ + └─────────────────────────────┘ ┌──────▼───────┐ + │ PostgreSQL │ + └──────────────┘ ``` Two details of this shape matter, and explain most of the configuration: -- **The API needs its own public hostname.** Attachment URLs are absolute and served by the API - at `/media/...`, outside the `/v1` prefix. RSS readers also fetch feeds directly. +- **The API has no public hostname of its own.** Everything it serves — the public endpoints, + attachments, RSS feeds — lives below `/v1`, and both interfaces expose that as `/api` on their own + address. Attachment URLs in API responses are relative for the same reason. - **The API works out which ticker a request is for from the browser's `Origin` header.** That is why the public frontend's address must be registered on the ticker itself, and why the reverse proxy has to pass a correct `Origin` along. See [Installation](installation.md). diff --git a/docs/installation.md b/docs/installation.md index 8efbdca..f93ce7e 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -9,20 +9,19 @@ automatically. - A host with Docker and the Compose plugin. - Ports **80** and **443** reachable from the internet. Port 80 is required for the Let's Encrypt HTTP challenge, even though all traffic is redirected to HTTPS. -- **Three hostnames** pointing at the host, each with an `A` record and — please — an `AAAA` +- **Two hostnames** pointing at the host, each with an `A` record and — please — an `AAAA` record: | Example | Purpose | | --- | --- | | `ticker.example.org` | the public page readers visit | | `admin.ticker.example.org` | the admin interface | - | `api.ticker.example.org` | the API, media files and feeds | -!!! note "Why three hostnames?" +!!! note "Where is the API?" - The admin and frontend are separate applications, and the API needs its own name because it - serves attachments at `/media/...` and RSS feeds directly to readers. You can use any names - you like; only their DNS records and your `.env` need to agree. + It has no hostname of its own. Everything it serves — the public endpoints, attachments and RSS + feeds — is reachable under `/api` on both names above. You can use any names you like; only + their DNS records and your `.env` need to agree. ## 1. Get the files @@ -68,7 +67,7 @@ docker compose logs -f traefik Then check that the API is alive: ```shell -curl https://api.ticker.example.org/healthz +curl https://ticker.example.org/healthz # OK ``` @@ -131,16 +130,16 @@ Finally mark the ticker **active**, otherwise the same inactive page is shown. # Through the frontend, exactly as a browser does it curl -s https://ticker.example.org/api/init | jq .data.ticker -# Directly against the API, supplying the origin yourself -curl -s -H 'Origin: https://ticker.example.org' \ - https://api.ticker.example.org/v1/init | jq .data.ticker +# Supplying the origin yourself, the way an RSS reader has to +curl -s 'https://ticker.example.org/api/init?origin=https://ticker.example.org' | jq .data.ticker ``` Both must return your ticker rather than `null`. Now open the frontend, post a message from the admin interface, and confirm it appears **without reloading the page** — that proves the realtime WebSocket connection works. Upload an image to a -message and confirm it renders, which proves `TICKER_UPLOAD_URL` is correct. +message and confirm it renders in the frontend *and* in the admin interface — attachment URLs are +relative, so that proves both hostnames serve `/api` correctly. ## Next steps @@ -163,8 +162,10 @@ so without it the API cannot tell which ticker is being requested and returns th every visitor. It would also collapse its response cache into a single shared entry across all tickers. -The API's own hostname is routed straight through with no rewriting, because `/media/...`, `/feed` -and `/healthz` are served outside the `/v1` prefix. +Everything the API serves lives below `/v1`, attachments at `/v1/media/...` included, so the same +two steps cover images and feeds. The one exception is `/healthz`, which sits at the root and gets +its own small router on the frontend hostname — without it the frontend's single-page fallback would +answer `/healthz` with `index.html` and an uptime monitor would report healthy no matter what. ## Using a different reverse proxy @@ -174,8 +175,9 @@ hostnames: - rewrites `/api/**` to `/v1/**`; - sets `Origin` to the public origin of that hostname; - forwards WebSocket upgrades (`Connection`, `Upgrade`, HTTP/1.1) for `/api/ws`; -- allows request bodies of at least 10 MB, which is the API's own limit; -- and serves the API on its own hostname for `/media/**` and feeds. +- and allows request bodies of at least 10 MB, which is the API's own limit. + +Attachments and feeds need no separate rule; they are below `/v1` like everything else. An nginx equivalent of the `/api/` block: @@ -224,9 +226,9 @@ labels: - traefik.http.routers.admin.middlewares=admin-allow ``` -Note this protects the admin *interface* only. The API hostname must stay public for media and -feeds, so apply the same middleware to the `admin-api` router if you want the API path restricted -too. +Note this protects the admin *interface* only; add the same middleware to the `admin-api` router to +cover its `/api` path as well. The frontend hostname must stay public — that is where readers, RSS +clients and attachments arrive. **Avoid mounting the Docker socket directly.** Traefik reads it to discover containers, and even mounted read-only it is equivalent to root on the host. In a more sensitive setup, put a diff --git a/docs/integrations.md b/docs/integrations.md index 7b624fc..b7326a0 100644 --- a/docs/integrations.md +++ b/docs/integrations.md @@ -147,5 +147,5 @@ logs if a message did not arrive somewhere: docker compose logs ticker | grep bridge_name ``` -Attachments are sent along, using the absolute URLs built from `TICKER_UPLOAD_URL`. If that value is -wrong, posts arrive with broken images — see [Configuration](configuration.md#uploads). +Attachments are sent along as files, read straight from `TICKER_UPLOAD_PATH` — no public URL is +involved, so an integration keeps working even if the interfaces are unreachable. diff --git a/docs/operations.md b/docs/operations.md index 81c23ce..8e66a70 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -82,7 +82,7 @@ gunzip -c ticker-db.sql.gz | docker compose exec -T postgres psql -U ticker tick ## Health and monitoring ```shell -curl https://api.ticker.example.org/healthz +curl https://ticker.example.org/healthz # OK ``` diff --git a/docs/swagger.yaml b/docs/swagger.yaml index d24dda4..788ec6f 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -94,6 +94,30 @@ info: title: Ticker API version: "2.0" paths: + /media/{fileName}: + get: + description: |- + Serves an uploaded attachment. Attachment URLs in other responses are relative to the site + serving them and take the form /api/media/{fileName}; against the API directly the path is + /v1/media/{fileName}. + parameters: + - description: File name of the attachment, e.g. 6a204bd8-9ee6-4478-bcd6-9b3b1e1f0d3a.jpg + in: path + name: fileName + required: true + type: string + produces: + - image/gif + - image/jpeg + - image/png + responses: + "200": + description: OK + "404": + description: Not Found + summary: Retrieves an uploaded attachment + tags: + - public /init: get: consumes: diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 2961438..3660b50 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -23,15 +23,13 @@ Check, in order: ```shell # With an origin: "settings" comes back populated - curl -s -H 'Origin: https://ticker.example.org' \ - https://api.ticker.example.org/v1/init + curl -s 'https://ticker.example.org/api/init?origin=https://ticker.example.org' - # Without one: "settings" is empty — this is what a missing Origin looks like - curl -s https://api.ticker.example.org/v1/init + # Relying on the proxy to add it — this must look the same + curl -s https://ticker.example.org/api/init ``` - If the request through your frontend (`https://ticker.example.org/api/init`) looks like the - second, the proxy is not setting `Origin`. + If the second response has an empty `settings`, the proxy is not setting `Origin`. !!! note "Changes take up to five minutes" @@ -107,21 +105,20 @@ Also make sure the proxy does not time out idle connections aggressively — the ## Images are broken -**Everywhere, including in Telegram or Mastodon posts** — `TICKER_UPLOAD_URL` is wrong. It must be -the API's public base URL, with no `/v1` and no trailing slash: +Attachment URLs are relative — `/api/media/.png` — so they are served by whichever site the +page came from. Check one directly: ```shell -TICKER_UPLOAD_URL=https://api.ticker.example.org +curl -I https://ticker.example.org/api/media/.png ``` -Verify a link resolves: +**Images and the rest of the interface are both broken** — the `/api` path is not routed to the API +at all. See [the empty interface](#everything-in-the-interface-is-empty-with-no-error) above. -```shell -curl -I https://api.ticker.example.org/media/.png -``` - -Note `/media` is served at the API's root, so it is **not** reachable through the `/api` rewrite — -`https://ticker.example.org/api/media/...` is expected to 404. +**Images are broken but everything else works, on a self-built interface image** — the build has an +absolute `TICKER_API_URL` baked in. Requests then go to that address while images resolve against +the site's own origin, which serves no `/api`. Build without it so the relative `/api` default +applies. **Only for older messages** — the uploads directory was not persistent and the files are gone, while the database still references them. Confirm `TICKER_UPLOAD_PATH` points into a named volume, and diff --git a/internal/api/api.go b/internal/api/api.go index e92772d..d90e140 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -125,10 +125,9 @@ func API(config config.Config, store storage.Storage) *Server { public.GET(`/timeline`, ticker.PrefetchTickerFromRequest(store), response_cache.CachePage(inMemoryCache, 10*time.Second, handler.GetTimeline)) public.GET(`/feed`, ticker.PrefetchTickerFromRequest(store), response_cache.CachePage(inMemoryCache, 5*time.Minute, handler.GetFeed)) public.GET(`/ws`, ticker.PrefetchTickerFromRequest(store), handler.HandleWebSocket) + public.GET(`/media/:fileName`, handler.GetMedia) } - r.GET(`/media/:fileName`, handler.GetMedia) - r.GET("/healthz", func(c *gin.Context) { c.String(http.StatusOK, "OK") }) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 45eddb2..a73c699 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -43,6 +43,34 @@ func (s *APITestSuite) TestHealthz() { s.store.AssertExpectations(s.T()) } +func (s *APITestSuite) TestMediaRoute() { + s.Run("is served below /v1", func() { + s.store.On("FindUploadByUUID", mock.Anything).Return(storage.Upload{}, errors.New("not found")).Once() + server := API(s.cfg, s.store) + + req := httptest.NewRequest(http.MethodGet, "/v1/media/uuid.png", nil) + w := httptest.NewRecorder() + server.Router.ServeHTTP(w, req) + + // The route exists; the upload itself does not. + s.Equal(http.StatusNotFound, w.Code) + s.Equal("not found", w.Body.String()) + s.store.AssertExpectations(s.T()) + }) + + s.Run("is not served at the root anymore", func() { + server := API(s.cfg, s.store) + + req := httptest.NewRequest(http.MethodGet, "/media/uuid.png", nil) + w := httptest.NewRecorder() + server.Router.ServeHTTP(w, req) + + s.Equal(http.StatusNotFound, w.Code) + // Gin's own 404, not the handler's: no route matched at all. + s.Equal("404 page not found", w.Body.String()) + }) +} + func (s *APITestSuite) TestLogin() { s.Run("when password is wrong", func() { user, err := storage.NewUser("user@systemli.org", "password") diff --git a/internal/api/media.go b/internal/api/media.go index 51511b9..621e330 100644 --- a/internal/api/media.go +++ b/internal/api/media.go @@ -1,10 +1,8 @@ package api import ( - "fmt" "net/http" "strings" - "time" "github.com/gin-gonic/gin" ) @@ -17,11 +15,16 @@ func (h *handler) GetMedia(c *gin.Context) { return } - expireTime := time.Now().AddDate(0, 1, 0) - cacheControl := fmt.Sprintf("public, max-age=%d", expireTime.Unix()) - expires := expireTime.Format(http.TimeFormat) - - c.Header("Cache-Control", cacheControl) - c.Header("Expires", expires) + // Media is served on the same origin as the admin and frontend. The upload + // handler only accepts JPEG, GIF and PNG, but be explicit about the type and + // forbid sniffing anyway. + c.Header("Content-Type", upload.ContentType) + c.Header("X-Content-Type-Options", "nosniff") + // Rows created before the extension was derived from the content type may + // still carry an arbitrary one, so neutralise them explicitly. + c.Header("Content-Security-Policy", "default-src 'none'; sandbox") + c.Header("Content-Disposition", `inline; filename="`+upload.FileName()+`"`) + // File names contain a UUID, so a response never becomes stale. + c.Header("Cache-Control", "public, max-age=2592000, immutable") c.File(upload.FullPath(h.storage.UploadPath())) } diff --git a/internal/api/media_test.go b/internal/api/media_test.go index 96ab979..0bd110e 100644 --- a/internal/api/media_test.go +++ b/internal/api/media_test.go @@ -4,6 +4,8 @@ import ( "errors" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "github.com/gin-gonic/gin" @@ -26,7 +28,7 @@ func (s *MediaTestSuite) SetupTest() { s.w = httptest.NewRecorder() s.ctx, _ = gin.CreateTestContext(s.w) - s.ctx.Request = httptest.NewRequest(http.MethodGet, "/media", nil) + s.ctx.Request = httptest.NewRequest(http.MethodGet, "/v1/media", nil) s.store = &storage.MockStorage{} s.cfg = config.LoadConfig("") } @@ -42,14 +44,28 @@ func (s *MediaTestSuite) TestGetMedia() { }) s.Run("when upload found", func() { - upload := storage.NewUpload("image.jpg", "image/jpeg", 1) + upload := storage.NewUpload("image/png", 1) + uploadPath := s.T().TempDir() + fullPath := upload.FullPath(uploadPath) + s.NoError(os.MkdirAll(filepath.Dir(fullPath), 0750)) + s.NoError(os.WriteFile(fullPath, []byte("not really a png"), 0600)) + s.store.On("FindUploadByUUID", mock.Anything).Return(upload, nil).Once() - s.store.On("UploadPath").Return("./uploads").Once() + s.store.On("UploadPath").Return(uploadPath).Once() + + w := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(w) + ctx.Request = httptest.NewRequest(http.MethodGet, "/v1/media/"+upload.FileName(), nil) + ctx.AddParam("fileName", upload.FileName()) h := s.handler() - h.GetMedia(s.ctx) + h.GetMedia(ctx) - s.Equal(http.StatusNotFound, s.w.Code) + s.Equal(http.StatusOK, w.Code) + s.Equal("image/png", w.Header().Get("Content-Type")) + s.Equal("nosniff", w.Header().Get("X-Content-Type-Options")) + s.Equal("public, max-age=2592000, immutable", w.Header().Get("Cache-Control")) + s.Contains(w.Header().Get("Content-Security-Policy"), "default-src 'none'") s.store.AssertExpectations(s.T()) }) } diff --git a/internal/api/messages.go b/internal/api/messages.go index 0bc5a25..d9fb89a 100644 --- a/internal/api/messages.go +++ b/internal/api/messages.go @@ -27,7 +27,7 @@ func (h *handler) GetMessages(c *gin.Context) { return } - data := map[string]any{"messages": response.MessagesResponse(messages, h.config)} + data := map[string]any{"messages": response.MessagesResponse(messages)} c.JSON(http.StatusOK, response.SuccessResponse(data)) } @@ -38,7 +38,7 @@ func (h *handler) GetMessage(c *gin.Context) { return } - data := map[string]any{"message": response.MessageResponse(message, h.config)} + data := map[string]any{"message": response.MessageResponse(message)} c.JSON(http.StatusOK, response.SuccessResponse(data)) } @@ -81,7 +81,7 @@ func (h *handler) PostMessage(c *gin.Context) { return } - serializedMessage := response.MessageResponse(message, h.config) + serializedMessage := response.MessageResponse(message) h.realtime.Broadcast(realtime.Message{ Type: "message_created", TickerID: ticker.ID, diff --git a/internal/api/response/message.go b/internal/api/response/message.go index 70b26ed..eccc0c9 100644 --- a/internal/api/response/message.go +++ b/internal/api/response/message.go @@ -1,10 +1,8 @@ package response import ( - "fmt" "time" - "github.com/systemli/ticker/internal/config" "github.com/systemli/ticker/internal/storage" ) @@ -24,12 +22,11 @@ type MessageAttachment struct { ContentType string `json:"contentType"` } -func MessageResponse(message storage.Message, config config.Config) Message { +func MessageResponse(message storage.Message) Message { var attachments []MessageAttachment for _, attachment := range message.Attachments { - name := fmt.Sprintf("%s.%s", attachment.UUID, attachment.Extension) - attachments = append(attachments, MessageAttachment{URL: MediaURL(config.Upload.URL, name), ContentType: attachment.ContentType}) + attachments = append(attachments, MessageAttachment{URL: storage.MediaURL(attachment.FileName()), ContentType: attachment.ContentType}) } return Message{ @@ -44,14 +41,10 @@ func MessageResponse(message storage.Message, config config.Config) Message { } } -func MessagesResponse(messages []storage.Message, config config.Config) []Message { +func MessagesResponse(messages []storage.Message) []Message { msgs := make([]Message, 0) for _, message := range messages { - msgs = append(msgs, MessageResponse(message, config)) + msgs = append(msgs, MessageResponse(message)) } return msgs } - -func MediaURL(uploadURL, name string) string { - return fmt.Sprintf("%s/media/%s", uploadURL, name) -} diff --git a/internal/api/response/message_test.go b/internal/api/response/message_test.go index 8fe29e5..220e4e1 100644 --- a/internal/api/response/message_test.go +++ b/internal/api/response/message_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/stretchr/testify/suite" - "github.com/systemli/ticker/internal/config" "github.com/systemli/ticker/internal/storage" ) @@ -13,11 +12,10 @@ type MessagesResponseTestSuite struct { } func (s *MessagesResponseTestSuite) TestMessagesResponse() { - config := config.Config{Upload: config.Upload{URL: "https://upload.example.com"}} message := storage.NewMessage() message.Attachments = []storage.Attachment{{UUID: "uuid", Extension: "jpg"}} - response := MessagesResponse([]storage.Message{message}, config) + response := MessagesResponse([]storage.Message{message}) s.Equal(1, len(response)) s.Empty(response[0].TelegramURL) @@ -27,7 +25,7 @@ func (s *MessagesResponseTestSuite) TestMessagesResponse() { attachments := response[0].Attachments - s.Equal("https://upload.example.com/media/uuid.jpg", attachments[0].URL) + s.Equal("/api/media/uuid.jpg", attachments[0].URL) } func TestMessagesResponseTestSuite(t *testing.T) { diff --git a/internal/api/response/timeline.go b/internal/api/response/timeline.go index 9645941..3d5d5d2 100644 --- a/internal/api/response/timeline.go +++ b/internal/api/response/timeline.go @@ -1,10 +1,8 @@ package response import ( - "fmt" "time" - "github.com/systemli/ticker/internal/config" "github.com/systemli/ticker/internal/storage" ) @@ -22,13 +20,12 @@ type Attachment struct { ContentType string `json:"contentType"` } -func TimelineResponse(messages []storage.Message, config config.Config) []TimelineEntry { +func TimelineResponse(messages []storage.Message) []TimelineEntry { timeline := make([]TimelineEntry, 0) for _, message := range messages { var attachments []Attachment for _, attachment := range message.Attachments { - name := fmt.Sprintf("%s.%s", attachment.UUID, attachment.Extension) - attachments = append(attachments, Attachment{URL: MediaURL(config.Upload.URL, name), ContentType: attachment.ContentType}) + attachments = append(attachments, Attachment{URL: storage.MediaURL(attachment.FileName()), ContentType: attachment.ContentType}) } timeline = append(timeline, TimelineEntry{ diff --git a/internal/api/response/timeline_test.go b/internal/api/response/timeline_test.go index d9299a9..1960954 100644 --- a/internal/api/response/timeline_test.go +++ b/internal/api/response/timeline_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/stretchr/testify/suite" - "github.com/systemli/ticker/internal/config" "github.com/systemli/ticker/internal/storage" ) @@ -13,18 +12,17 @@ type TimelineTestSuite struct { } func (s *TimelineTestSuite) TestTimelineResponse() { - config := config.Config{Upload: config.Upload{URL: "https://upload.example.com"}} message := storage.NewMessage() message.Attachments = []storage.Attachment{{UUID: "uuid", Extension: "jpg"}} - response := TimelineResponse([]storage.Message{message}, config) + response := TimelineResponse([]storage.Message{message}) s.Equal(1, len(response)) s.Equal(1, len(response[0].Attachments)) attachments := response[0].Attachments - s.Equal("https://upload.example.com/media/uuid.jpg", attachments[0].URL) + s.Equal("/api/media/uuid.jpg", attachments[0].URL) } func TestTimelineTestSuite(t *testing.T) { diff --git a/internal/api/response/upload.go b/internal/api/response/upload.go index be11f85..d9b1775 100644 --- a/internal/api/response/upload.go +++ b/internal/api/response/upload.go @@ -3,7 +3,6 @@ package response import ( "time" - "github.com/systemli/ticker/internal/config" "github.com/systemli/ticker/internal/storage" ) @@ -15,20 +14,20 @@ type Upload struct { ContentType string `json:"contentType"` } -func UploadResponse(upload storage.Upload, config config.Config) Upload { +func UploadResponse(upload storage.Upload) Upload { return Upload{ ID: upload.ID, UUID: upload.UUID, CreatedAt: upload.CreatedAt, - URL: upload.URL(config.Upload.URL), + URL: upload.URL(), ContentType: upload.ContentType, } } -func UploadsResponse(uploads []storage.Upload, config config.Config) []Upload { +func UploadsResponse(uploads []storage.Upload) []Upload { ur := make([]Upload, 0) for _, upload := range uploads { - ur = append(ur, UploadResponse(upload, config)) + ur = append(ur, UploadResponse(upload)) } return ur diff --git a/internal/api/response/upload_test.go b/internal/api/response/upload_test.go index 30c20bd..9070178 100644 --- a/internal/api/response/upload_test.go +++ b/internal/api/response/upload_test.go @@ -1,33 +1,26 @@ package response import ( - "fmt" "testing" "github.com/stretchr/testify/suite" - "github.com/systemli/ticker/internal/config" "github.com/systemli/ticker/internal/storage" ) -var ( - u = storage.NewUpload("image.jpg", "image/jpg", 1) - c = config.Config{ - Upload: config.Upload{URL: "http://localhost:8080"}, - } -) +var u = storage.NewUpload("image/jpeg", 1) type UploadResponseTestSuite struct { suite.Suite } func (s *UploadResponseTestSuite) TestUploadResponse() { - response := UploadResponse(u, c) + response := UploadResponse(u) - s.Equal(fmt.Sprintf("%s/media/%s", c.Upload.URL, u.FileName()), response.URL) + s.Equal("/api/media/"+u.FileName(), response.URL) } func (s *UploadResponseTestSuite) TestUploadsResponse() { - response := UploadsResponse([]storage.Upload{u}, c) + response := UploadsResponse([]storage.Upload{u}) s.Equal(1, len(response)) } diff --git a/internal/api/timeline.go b/internal/api/timeline.go index 5c69abe..4fda275 100644 --- a/internal/api/timeline.go +++ b/internal/api/timeline.go @@ -27,5 +27,5 @@ func (h *handler) GetTimeline(c *gin.Context) { } } - c.JSON(http.StatusOK, response.SuccessResponse(map[string]interface{}{"messages": response.TimelineResponse(messages, h.config)})) + c.JSON(http.StatusOK, response.SuccessResponse(map[string]interface{}{"messages": response.TimelineResponse(messages)})) } diff --git a/internal/api/upload.go b/internal/api/upload.go index 0121e86..2b62712 100644 --- a/internal/api/upload.go +++ b/internal/api/upload.go @@ -15,8 +15,6 @@ import ( "github.com/systemli/ticker/internal/util" ) -var allowedContentTypes = []string{"image/jpeg", "image/gif", "image/png"} - func (h *handler) PostUpload(c *gin.Context) { me, err := helper.Me(c) if err != nil { @@ -65,13 +63,13 @@ func (h *handler) PostUpload(c *gin.Context) { } contentType := util.DetectContentType(file) - if !util.ContainsString(allowedContentTypes, contentType) { + if _, allowed := storage.ExtensionForContentType(contentType); !allowed { log.Error(fmt.Sprintf("%s is not allowed to uploaded", contentType)) c.JSON(http.StatusBadRequest, response.ErrorResponse(response.CodeDefault, "failed to upload")) return } - u := storage.NewUpload(fileHeader.Filename, contentType, ticker.ID) + u := storage.NewUpload(contentType, ticker.ID) err = h.storage.SaveUpload(&u) if err != nil { c.JSON(http.StatusBadRequest, response.ErrorResponse(response.CodeDefault, response.FormError)) @@ -108,7 +106,7 @@ func (h *handler) PostUpload(c *gin.Context) { uploads = append(uploads, u) } - c.JSON(http.StatusOK, response.SuccessResponse(map[string]interface{}{"uploads": response.UploadsResponse(uploads, h.config)})) + c.JSON(http.StatusOK, response.SuccessResponse(map[string]interface{}{"uploads": response.UploadsResponse(uploads)})) } func preparePath(upload storage.Upload, config config.Config) error { diff --git a/internal/config/config.go b/internal/config/config.go index 5e54ca2..71c9f07 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -30,7 +30,6 @@ type Database struct { type Upload struct { Path string `yaml:"path"` - URL string `yaml:"url"` } func defaultConfig() Config { @@ -45,7 +44,6 @@ func defaultConfig() Config { MetricsListen: ":8181", Upload: Upload{ Path: "uploads", - URL: "http://localhost:8080", }, FileBackend: afero.NewOsFs(), } @@ -91,7 +89,7 @@ func LoadConfig(path string) Config { c.Upload.Path = os.Getenv("TICKER_UPLOAD_PATH") } if os.Getenv("TICKER_UPLOAD_URL") != "" { - c.Upload.URL = os.Getenv("TICKER_UPLOAD_URL") + log.Warn("TICKER_UPLOAD_URL is no longer used and can be removed, attachment links are relative to the site serving them") } return c diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3827236..37945ef 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -26,7 +26,6 @@ func (s *ConfigTestSuite) SetupTest() { "TICKER_DATABASE_DSN": "user:password@tcp(localhost:3306)/ticker?charset=utf8mb4&parseTime=True&loc=Local", "TICKER_METRICS_LISTEN": ":9191", "TICKER_UPLOAD_PATH": "/data/uploads", - "TICKER_UPLOAD_URL": "https://example.com", } } @@ -43,7 +42,6 @@ func (s *ConfigTestSuite) TestConfig() { s.Equal("ticker.db", c.Database.DSN) s.Equal(":8181", c.MetricsListen) s.Equal("uploads", c.Upload.Path) - s.Equal("http://localhost:8080", c.Upload.URL) }) s.Run("loads config from env", func() { @@ -61,7 +59,6 @@ func (s *ConfigTestSuite) TestConfig() { s.Equal(s.envs["TICKER_DATABASE_DSN"], c.Database.DSN) s.Equal(s.envs["TICKER_METRICS_LISTEN"], c.MetricsListen) s.Equal(s.envs["TICKER_UPLOAD_PATH"], c.Upload.Path) - s.Equal(s.envs["TICKER_UPLOAD_URL"], c.Upload.URL) for key := range s.envs { os.Unsetenv(key) diff --git a/internal/storage/message.go b/internal/storage/message.go index ee41467..f0162f8 100644 --- a/internal/storage/message.go +++ b/internal/storage/message.go @@ -75,6 +75,10 @@ type Attachment struct { ContentType string } +func (a *Attachment) FileName() string { + return fmt.Sprintf("%s.%s", a.UUID, a.Extension) +} + func (m *Message) AddAttachment(upload Upload) { attachment := Attachment{ UUID: upload.UUID, diff --git a/internal/storage/message_test.go b/internal/storage/message_test.go index 161fd81..3000ae5 100644 --- a/internal/storage/message_test.go +++ b/internal/storage/message_test.go @@ -8,7 +8,7 @@ import ( ) func TestAddAttachments(t *testing.T) { - upload := NewUpload("image.jpg", "image/jped", 1) + upload := NewUpload("image/jpeg", 1) message := NewMessage() message.AddAttachments([]Upload{upload}) diff --git a/internal/storage/upload.go b/internal/storage/upload.go index 75237ba..5efcdcf 100644 --- a/internal/storage/upload.go +++ b/internal/storage/upload.go @@ -2,12 +2,32 @@ package storage import ( "fmt" - "path/filepath" "time" uuid2 "github.com/google/uuid" ) +// MediaPath is the path media is served on, as seen by a browser talking to the +// admin or frontend. +const MediaPath = "/api/media" + +// uploadExtensions maps the content types the API accepts to the extension a file +// is stored and served under. The extension decides how a browser interprets the +// response and media shares an origin with the interfaces, so it is derived from +// the sniffed content type instead of the client-supplied filename. +var uploadExtensions = map[string]string{ + "image/gif": "gif", + "image/jpeg": "jpg", + "image/png": "png", +} + +// ExtensionForContentType returns the extension uploads of this content type are +// stored under, and whether the type is accepted at all. +func ExtensionForContentType(contentType string) (string, bool) { + ext, ok := uploadExtensions[contentType] + return ext, ok +} + type Upload struct { ID int `gorm:"primaryKey"` CreatedAt time.Time @@ -19,10 +39,10 @@ type Upload struct { ContentType string } -func NewUpload(filename, contentType string, tickerID int) Upload { +func NewUpload(contentType string, tickerID int) Upload { now := time.Now() uuid := uuid2.New() - ext := filepath.Ext(filename)[1:] + ext, _ := ExtensionForContentType(contentType) // First version we use a date based directory structure path := fmt.Sprintf("%d/%d", now.Year(), now.Month()) @@ -43,10 +63,13 @@ func (u *Upload) FullPath(uploadPath string) string { return fmt.Sprintf("%s/%s/%s", uploadPath, u.Path, u.FileName()) } -func (u *Upload) URL(uploadPath string) string { - return MediaURL(u.FileName(), uploadPath) +func (u *Upload) URL() string { + return MediaURL(u.FileName()) } -func MediaURL(name string, uploadPath string) string { - return fmt.Sprintf("%s/media/%s", uploadPath, name) +// MediaURL returns the public, host-relative URL of an uploaded file. The API +// serves media below /v1, which the admin and frontend reach through their own +// /api path, so no absolute base URL is needed. +func MediaURL(name string) string { + return fmt.Sprintf("%s/%s", MediaPath, name) } diff --git a/internal/storage/upload_test.go b/internal/storage/upload_test.go index d1bf0cd..dba8cfe 100644 --- a/internal/storage/upload_test.go +++ b/internal/storage/upload_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/assert" ) -var upload = NewUpload("image.jpg", "image/jpeg", 1) +var upload = NewUpload("image/jpeg", 1) func TestUploadFilename(t *testing.T) { fileName := upload.FileName() @@ -20,6 +20,24 @@ func TestUploadFullPath(t *testing.T) { } func TestUploadURL(t *testing.T) { - url := upload.URL("/uploads") - assert.Contains(t, url, "/uploads/media") + assert.Equal(t, "/api/media/"+upload.FileName(), upload.URL()) +} + +func TestExtensionForContentType(t *testing.T) { + ext, ok := ExtensionForContentType("image/png") + assert.True(t, ok) + assert.Equal(t, "png", ext) + + ext, ok = ExtensionForContentType("text/html") + assert.False(t, ok) + assert.Empty(t, ext) +} + +func TestNewUploadIgnoresClientExtension(t *testing.T) { + // A GIF is stored unmodified, so a filename like "evil.html" used to decide + // how the file is served afterwards. + u := NewUpload("image/gif", 1) + + assert.Equal(t, "gif", u.Extension) + assert.Equal(t, u.UUID+".gif", u.FileName()) }