diff --git a/.dockerignore b/.dockerignore index e019ef4..5de4667 100644 --- a/.dockerignore +++ b/.dockerignore @@ -17,6 +17,9 @@ docs/ **/.pytest_cache/ **/.mypy_cache/ **/.ruff_cache/ -backend/tests/ +# backend/tests/ is intentionally NOT excluded: the `dev` build stage +# needs it in the build context to run the suite in CI/locally. The +# `runtime` stage never copies it in (see backend/Dockerfile), so the +# production image stays test-free regardless. *.log *.zip diff --git a/.env.example b/.env.example index 276f197..fd96c21 100644 --- a/.env.example +++ b/.env.example @@ -12,6 +12,10 @@ DJANGO_SECRET_KEY=change-me-to-a-64-character-random-string-before-any-real-depl DJANGO_DEBUG=false DJANGO_ALLOWED_HOSTS=localhost,127.0.0.1,backend DJANGO_ADMIN_URL=admin/ +# Serve /api/docs/, /api/redoc/ and /api/schema/. Handy locally; set to false +# on any public deployment, since they describe every endpoint and payload in +# the service and are reachable without authenticating. +SERVE_API_DOCS=true ENVIRONMENT=development SERVICE_NAME=linkedin-search-api SECURE_SSL_REDIRECT=false diff --git a/.gitignore b/.gitignore index 9ea3ccb..e22b677 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,9 @@ # Environment / secrets -.env -.env.local +# Glob, not just ".env": local backups (.env.backup-*, .env.prod, .env.bak) +# hold the same credentials and must never be committed. .env.example is the +# only member of the family that is safe, so it is re-included explicitly. +.env* +!.env.example *.pem *.key @@ -22,6 +25,12 @@ node_modules/ frontend/dist/ npm-debug.log* +# Test artefacts +frontend/test-results/ +frontend/playwright-report/ +frontend/blob-report/ +frontend/coverage/ + # Editors / OS .vscode/ .idea/ diff --git a/Makefile b/Makefile index 10eee7e..d21f594 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,11 @@ help: ## Show this help # --- Lifecycle --------------------------------------------------------------- .PHONY: env env: ## Create .env from the template (never overwrites an existing one) - @test -f .env || (cp .env.example .env && echo "Created .env - review the secrets before deploying.") + @test -f .env || (cp .env.example .env && echo "Created .env" && ./scripts/generate_secrets.sh) + +.PHONY: secrets +secrets: ## Replace any remaining change-me placeholders in .env with real secrets + @./scripts/generate_secrets.sh .PHONY: up up: env ## Build and start the whole stack @@ -27,16 +31,32 @@ up: env ## Build and start the whole stack dev: env ## Start the stack with hot reload (Vite on :5173, runserver on :8000) $(COMPOSE_DEV) up --build +.PHONY: prod-volumes +prod-volumes: ## Create the external data volumes the production overlay expects + @for v in postgres elasticsearch logstash; do \ + name="$${DATA_VOLUME_PREFIX:-linkedin-search}-$${v}-data"; \ + docker volume inspect "$$name" >/dev/null 2>&1 \ + && echo " exists $$name" \ + || (docker volume create "$$name" >/dev/null && echo " created $$name"); \ + done + +.PHONY: prod-migrate +prod-migrate: ## Apply database migrations once, before rolling out a new image + $(COMPOSE_PROD) run --rm backend-migrate + .PHONY: prod -prod: env ## Start with the production overlay +prod: env prod-volumes ## Start with the production overlay $(COMPOSE_PROD) up --build -d + @echo "Run 'make prod-migrate' to apply schema changes - production containers do not migrate on boot." .PHONY: down down: ## Stop the stack (keeps volumes) $(COMPOSE) down .PHONY: clean -clean: ## Stop the stack and delete all volumes +clean: ## Stop the stack and delete all volumes (DESTRUCTIVE - dev stack only) + @echo "This deletes the database, the search index and the log queue of the *default* stack." + @printf 'Type "yes" to continue: ' && read ans && [ "$$ans" = "yes" ] $(COMPOSE) down -v --remove-orphans .PHONY: logs @@ -79,7 +99,7 @@ kibana-setup: ## Register the app-logs data view in Kibana # --- Quality ----------------------------------------------------------------- .PHONY: test test: ## Run the backend test suite inside the stack - $(COMPOSE) exec -e DJANGO_SETTINGS_MODULE=config.settings.test backend pytest + $(COMPOSE) run --rm backend-test .PHONY: test-local test-local: ## Run the backend test suite against a local PostgreSQL diff --git a/backend/.dockerignore b/backend/.dockerignore index 544a759..c312f22 100644 --- a/backend/.dockerignore +++ b/backend/.dockerignore @@ -10,6 +10,7 @@ htmlcov/ .pytest_cache/ .mypy_cache/ .ruff_cache/ -tests/ +# tests/ is intentionally not excluded here - see the root .dockerignore +# for why (the `dev` build stage needs it in the context). *.sqlite3 *.log diff --git a/backend/Dockerfile b/backend/Dockerfile index 44a347e..59f7ef4 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -50,16 +50,33 @@ RUN pip install --no-index --find-links=/wheels -r /tmp/requirements/base.txt \ && rm -rf /wheels /tmp/requirements WORKDIR /app -COPY --chown=app:app backend/ /app/ +# Explicit list, not `COPY backend/ /app/`: the runtime image must not ship +# test code (or stray repo files like this Dockerfile itself), only what the +# server needs to run. `tests/` is added back for the `dev` stage below. +COPY --chown=app:app backend/apps/ /app/apps/ +COPY --chown=app:app backend/config/ /app/config/ +COPY --chown=app:app backend/manage.py backend/entrypoint.sh backend/gunicorn.conf.py backend/pyproject.toml /app/ COPY --chown=app:app data/ /data/ # Static assets are only needed for the admin and the API docs, but baking them -# in keeps the container stateless. -RUN DJANGO_SECRET_KEY=build-only-not-a-real-secret-000000000000000000000000000000000000 \ +# in keeps the container stateless. The step runs against the production +# settings on purpose: they are the ones this image ships with, and the dev +# module pulls in django-extensions which is not part of base.txt. +# WORKDIR creates /app as root before the COPY above populates it, and +# --chown on COPY only touches what it copies - the directory entry itself +# stays root-owned, which is enough to make anything that writes a *new* file +# straight into /app (e.g. pytest-cov's .coverage) fail with EACCES as the +# app user. +# The secret is generated per build and thrown away with the layer: nothing +# secret-shaped is ever committed to the repo or baked into the image, and +# collectstatic only needs *a* valid key, never a specific one. (A literal +# placeholder here would also trip the guard in config/settings/prod.py.) +RUN chown app:app /app \ + && chmod +x /app/entrypoint.sh \ + && DJANGO_SECRET_KEY="$(python -c 'import secrets; print(secrets.token_urlsafe(64))')" \ DATABASE_URL=postgres://build:build@localhost:5432/build \ - DJANGO_SETTINGS_MODULE=config.settings.dev \ - python manage.py collectstatic --noinput --clear \ - && chmod +x /app/entrypoint.sh + DJANGO_SETTINGS_MODULE=config.settings.prod \ + python manage.py collectstatic --noinput --clear USER app EXPOSE 8000 @@ -69,3 +86,25 @@ HEALTHCHECK --interval=30s --timeout=5s --start-period=40s --retries=3 \ ENTRYPOINT ["/app/entrypoint.sh"] CMD ["gunicorn", "config.wsgi:application", "--config", "/app/gunicorn.conf.py"] + +# ----------------------------------------------------------------------------- +# Stage 3 - development +# +# Same layout as the runtime image plus the tooling from dev.txt (pytest, +# django-extensions, ...) so `config.settings.dev` can actually be imported. +# Selected through `target: dev` in docker-compose.dev.yml. +# ----------------------------------------------------------------------------- +FROM runtime AS dev + +USER root + +ENV DJANGO_SETTINGS_MODULE=config.settings.dev + +COPY backend/requirements/ /tmp/requirements/ +RUN pip install -r /tmp/requirements/dev.txt \ + && rm -rf /tmp/requirements + +# The one directory the runtime stage deliberately left out. +COPY --chown=app:app backend/tests/ /app/tests/ + +USER app diff --git a/backend/apps/search/backends/elastic.py b/backend/apps/search/backends/elastic.py index 073ccd3..e044d20 100644 --- a/backend/apps/search/backends/elastic.py +++ b/backend/apps/search/backends/elastic.py @@ -80,6 +80,18 @@ FILTER_FIELDS = { "companies": "company.keyword", } +#: Maps a facet (as returned to the frontend) to the request attribute that +#: selects values *within that same facet*. Used to exclude a facet's own +#: filter from its own aggregation - see ``_build_aggregations``. +FACET_TO_FILTER_ATTR = { + "skills": "skills", + "job_title_role": "roles", + "seniority": "seniority", + "country": "countries", + "industry": "industries", + "company": "companies", +} + def build_query(request: SearchRequest) -> dict[str, Any]: """Translate a search request into an Elasticsearch request body.""" @@ -118,20 +130,26 @@ def build_query(request: SearchRequest) -> dict[str, Any]: else: must.append({"match_all": {}}) + active_filters = _active_filters(request) + range_filter = _range_filter(request) + body: dict[str, Any] = { - "query": { - "bool": { - "must": must, - "should": should, - "filter": _build_filters(request), - } - }, + "query": {"bool": {"must": must, "should": should}}, "from": request.offset, "size": request.page_size, "track_total_hits": True, "sort": _build_sort(request), } + # Filters live in ``post_filter``, not ``query.bool.filter``: post_filter + # narrows the hits and the total count *after* aggregations are computed, + # which is what lets a multi-select facet stay populated with its own + # sibling values - see ``_build_aggregations`` for the other half of the + # trick (each facet additionally excludes its own filter). + all_filters = [*active_filters.values(), *([range_filter] if range_filter else [])] + if all_filters: + body["post_filter"] = {"bool": {"filter": all_filters}} + if request.q: body["highlight"] = { "pre_tags": [""], @@ -140,33 +158,35 @@ def build_query(request: SearchRequest) -> dict[str, Any]: } if request.include_facets: - body["aggs"] = _build_aggregations() + body["aggs"] = _build_aggregations(active_filters, range_filter) return body -def _build_filters(request: SearchRequest) -> list[dict[str, Any]]: - filters: list[dict[str, Any]] = [] - +def _active_filters(request: SearchRequest) -> dict[str, dict[str, Any]]: + """One ``terms`` clause per attribute that actually has a value selected.""" + filters: dict[str, dict[str, Any]] = {} for attribute, field_name in FILTER_FIELDS.items(): values = getattr(request, attribute) if values: # ``terms`` is OR within one filter; separate filters are ANDed, # which matches how users read the UI: "python OR django" skills, # AND located in Germany. - filters.append({"terms": {field_name: list(values)}}) - - if request.min_experience is not None or request.max_experience is not None: - bounds: dict[str, int] = {} - if request.min_experience is not None: - bounds["gte"] = request.min_experience - if request.max_experience is not None: - bounds["lte"] = request.max_experience - filters.append({"range": {"years_experience": bounds}}) - + filters[attribute] = {"terms": {field_name: list(values)}} return filters +def _range_filter(request: SearchRequest) -> dict[str, Any] | None: + if request.min_experience is None and request.max_experience is None: + return None + bounds: dict[str, int] = {} + if request.min_experience is not None: + bounds["gte"] = request.min_experience + if request.max_experience is not None: + bounds["lte"] = request.max_experience + return {"range": {"years_experience": bounds}} + + def _build_sort(request: SearchRequest) -> list[Any]: if request.sort == SORT_NAME: return [{"full_name.keyword": "asc"}, {"id": "asc"}] @@ -177,12 +197,35 @@ def _build_sort(request: SearchRequest) -> list[Any]: return ["_score", {"id": "asc"}] # deterministic tie-break -def _build_aggregations() -> dict[str, Any]: +def _build_aggregations( + active_filters: dict[str, dict[str, Any]], range_filter: dict[str, Any] | None +) -> dict[str, Any]: + """ + One ``terms`` aggregation per facet, each scoped by every *other* active + filter - the standard multi-select faceting pattern. Without this, a + facet's own filter (applied globally through ``post_filter``) would + already have narrowed the document set before the aggregation runs, so + ticking "Engineering" under Job family would make every other job family + option vanish from that same list instead of just narrowing the results. + The experience range applies to every facet unconditionally: it is not a + facet with its own checkboxes, so it is never excluded. + """ size = settings.SEARCH["FACET_SIZE"] - return { - name: {"terms": {"field": field_name, "size": size, "order": {"_count": "desc"}}} - for name, field_name in FACET_FIELDS.items() - } + aggs: dict[str, Any] = {} + for name, field_name in FACET_FIELDS.items(): + own_attribute = FACET_TO_FILTER_ATTR[name] + other_filters = [ + clause for attribute, clause in active_filters.items() if attribute != own_attribute + ] + if range_filter is not None: + other_filters.append(range_filter) + + terms_agg = {"terms": {"field": field_name, "size": size, "order": {"_count": "desc"}}} + if other_filters: + aggs[name] = {"filter": {"bool": {"filter": other_filters}}, "aggs": {name: terms_agg}} + else: + aggs[name] = terms_agg + return aggs class ElasticsearchBackend(SearchBackend): @@ -269,11 +312,16 @@ class ElasticsearchBackend(SearchBackend): @staticmethod def _to_facets(aggregations: dict[str, Any]) -> dict[str, list[FacetBucket]]: - return { - name: [ + facets: dict[str, list[FacetBucket]] = {} + for name, payload in aggregations.items(): + # A facet scoped by other filters is wrapped in an outer `filter` + # aggregation (see ``_build_aggregations``); unwrap it to reach the + # `terms` buckets. When no other filter was active it is the bare + # `terms` aggregation already. + terms_payload = payload.get(name, payload) + facets[name] = [ FacetBucket(key=str(bucket["key"]), count=bucket["doc_count"]) - for bucket in payload.get("buckets", []) + for bucket in terms_payload.get("buckets", []) if bucket["key"] != "" ] - for name, payload in aggregations.items() - } + return facets diff --git a/backend/apps/search/backends/postgres.py b/backend/apps/search/backends/postgres.py index 26eacab..73755db 100644 --- a/backend/apps/search/backends/postgres.py +++ b/backend/apps/search/backends/postgres.py @@ -42,14 +42,26 @@ _TOKEN_RE = re.compile(r"[\w']+", re.UNICODE) _FACET_LIMIT = 25 _MIN_TRIGRAM_SIMILARITY = 0.3 +#: Maps a facet (as returned to the frontend) to the request attribute that +#: selects values *within that same facet*. Used to exclude a facet's own +#: filter from its own count - see ``_facets``. +_FACET_TO_FILTER_ATTR = { + "skills": "skills", + "job_title_role": "roles", + "seniority": "seniority", + "country": "countries", + "industry": "industries", + "company": "companies", +} + class PostgresBackend(SearchBackend): name = "postgres" def search(self, request: SearchRequest) -> SearchResult: started = time.perf_counter() - queryset = self._apply_filters(Profile.objects.all(), request) - queryset = self._apply_keyword(queryset, request.q) + keyword_queryset = self._apply_keyword(Profile.objects.all(), request.q) + queryset = self._apply_filters(keyword_queryset, request) total = queryset.count() page = ( @@ -60,7 +72,7 @@ class PostgresBackend(SearchBackend): ] ) hits = [self._to_hit(profile, request.q) for profile in page] - facets = self._facets(queryset) if request.include_facets else {} + facets = self._facets(keyword_queryset, request) if request.include_facets else {} return SearchResult( total=total, @@ -87,23 +99,35 @@ class PostgresBackend(SearchBackend): # -- Query construction ------------------------------------------------- @staticmethod - def _apply_filters(queryset: QuerySet[Profile], request: SearchRequest) -> QuerySet[Profile]: - if request.skills: + def _apply_filters( + queryset: QuerySet[Profile], request: SearchRequest, *, exclude: str | None = None + ) -> QuerySet[Profile]: + """ + Apply every active filter except ``exclude``. + + ``exclude`` is how a facet's own count query avoids narrowing itself: + pass e.g. ``exclude="roles"`` when computing the job-family facet so + ticking "Engineering" does not make "Human resources" disappear from + that same list. The main hits queryset calls this with no exclusion. + """ + if request.skills and exclude != "skills": # Every selected skill widens the result set (OR), mirroring the # ``terms`` filter used by the Elasticsearch backend. queryset = queryset.filter( skills__slug__in=[Skill.make_slug(s) for s in request.skills] ) - if request.roles: + if request.roles and exclude != "roles": queryset = queryset.filter(job_title_role__in=request.roles) - if request.seniority: + if request.seniority and exclude != "seniority": queryset = queryset.filter(seniority_levels__overlap=list(request.seniority)) - if request.countries: + if request.countries and exclude != "countries": queryset = queryset.filter(location_country__in=request.countries) - if request.industries: + if request.industries and exclude != "industries": queryset = queryset.filter(industry__in=request.industries) - if request.companies: + if request.companies and exclude != "companies": queryset = queryset.filter(current_company__name__in=request.companies) + # The experience range is not a facet with its own checkboxes, so it + # always applies - it is never the thing being excluded. if request.min_experience is not None: queryset = queryset.filter(years_experience__gte=request.min_experience) if request.max_experience is not None: @@ -152,53 +176,65 @@ class PostgresBackend(SearchBackend): return queryset.order_by("-rank", "full_name", "id") # -- Facets ------------------------------------------------------------- - def _facets(self, queryset: QuerySet[Profile]) -> dict[str, list[FacetBucket]]: + def _facets( + self, keyword_queryset: QuerySet[Profile], request: SearchRequest + ) -> dict[str, list[FacetBucket]]: + """ + One count per facet, each computed on a queryset that excludes that + facet's *own* filter. + + This is what keeps a multi-select facet fully populated once one of + its values is ticked: the job-family facet must still list "Human + resources" after "Engineering" is selected, while every *other* facet + (country, seniority, ...) does narrow to the engineering-only result + set. Six independent queries instead of one is a deliberate + trade-off - see the module docstring on why this backend does not aim + to match Elasticsearch's scale, only its contract. + """ + return { + name: self._facet_counts( + name, self._apply_filters(keyword_queryset, request, exclude=attribute) + ) + for name, attribute in _FACET_TO_FILTER_ATTR.items() + } + + @staticmethod + def _facet_counts(facet_name: str, queryset: QuerySet[Profile]) -> list[FacetBucket]: ids = list(queryset.values_list("id", flat=True)) if not ids: - return { - name: [] - for name in ( - "skills", - "job_title_role", - "seniority", - "country", - "industry", - "company", + return [] + + if facet_name == "skills": + counter = Counter( + Skill.objects.filter(profile_skills__profile_id__in=ids).values_list( + "name", flat=True ) - } + ) + elif facet_name == "seniority": + counter = Counter() + for levels in Profile.objects.filter(id__in=ids).values_list( + "seniority_levels", flat=True + ): + for level in levels or []: + counter[level] += 1 + else: + column = { + "job_title_role": "job_title_role", + "country": "location_country", + "industry": "industry", + "company": "current_company__name", + }[facet_name] + counter = Counter( + value or "" for value in Profile.objects.filter(id__in=ids).values_list( + column, flat=True + ) + ) - skills = Skill.objects.filter(profile_skills__profile_id__in=ids).values_list( - "name", flat=True - ) - rows = Profile.objects.filter(id__in=ids).values( - "job_title_role", "location_country", "industry", "current_company__name", - "seniority_levels", - ) # fmt: skip - - counters: dict[str, Counter] = { - "skills": Counter(skills), - "job_title_role": Counter(), - "seniority": Counter(), - "country": Counter(), - "industry": Counter(), - "company": Counter(), - } - for row in rows: - counters["job_title_role"][row["job_title_role"]] += 1 - counters["country"][row["location_country"]] += 1 - counters["industry"][row["industry"]] += 1 - counters["company"][row["current_company__name"] or ""] += 1 - for level in row["seniority_levels"] or []: - counters["seniority"][level] += 1 - - return { - name: [ - FacetBucket(key=key, count=count) - for key, count in counter.most_common(_FACET_LIMIT) - if key - ] - for name, counter in counters.items() - } + return [ + FacetBucket(key=key, count=count) + for key, count in counter.most_common(_FACET_LIMIT) + if key + ] # -- Result mapping ----------------------------------------------------- @classmethod diff --git a/backend/apps/search/indexing.py b/backend/apps/search/indexing.py index 2a3e54c..1dc5f6e 100644 --- a/backend/apps/search/indexing.py +++ b/backend/apps/search/indexing.py @@ -30,7 +30,7 @@ from datetime import UTC, datetime from typing import Any from django.conf import settings -from elasticsearch import Elasticsearch +from elasticsearch import BadRequestError, Elasticsearch from elasticsearch.helpers import bulk from apps.profiles.models import Profile @@ -227,8 +227,33 @@ def _default_queryset(): # noqa: ANN202 ) +# Creating an index waits for the master to ack and the shard to be allocated, +# which can take far longer than the per-request timeout used for searches. +INDEX_CREATE_TIMEOUT = 120 + + def create_index(client: Elasticsearch, name: str) -> None: - client.indices.create(index=name, settings=INDEX_SETTINGS, mappings=INDEX_MAPPINGS) + """ + Create the physical index, tolerating a retried request. + + ``indices.create`` is not idempotent, but the client runs with + ``retry_on_timeout``: when the cluster acks the creation more slowly than + the request timeout, the retry comes back as + ``resource_already_exists_exception`` and the rebuild fails even though the + index is there. The name carries a timestamp generated moments earlier, so + an index already under that name can only be our own first attempt. + """ + try: + client.options(request_timeout=INDEX_CREATE_TIMEOUT).indices.create( + index=name, settings=INDEX_SETTINGS, mappings=INDEX_MAPPINGS + ) + except BadRequestError as exc: + if exc.error != "resource_already_exists_exception": + raise + logger.warning( + "index_create_retried", + extra={"event": "index_create_retried", "index": name}, + ) def rebuild_index(client: Elasticsearch | None = None) -> dict[str, Any]: @@ -245,13 +270,19 @@ def rebuild_index(client: Elasticsearch | None = None) -> dict[str, Any]: create_index(client, new_index) logger.info("index_created", extra={"event": "index_created", "index": new_index}) - succeeded, errors = bulk( - client, - ({"_index": new_index, **doc} for doc in iter_documents()), - raise_on_error=False, - stats_only=False, - ) - client.indices.refresh(index=new_index) + # Anything that goes wrong from here on leaves an index nothing points at; + # drop it rather than accumulating one orphan per failed attempt. + try: + succeeded, errors = bulk( + client, + ({"_index": new_index, **doc} for doc in iter_documents()), + raise_on_error=False, + stats_only=False, + ) + client.indices.refresh(index=new_index) + except Exception: + client.indices.delete(index=new_index, ignore_unavailable=True) + raise previous = _current_indices(client, alias) actions: list[dict[str, Any]] = [{"add": {"index": new_index, "alias": alias}}] diff --git a/backend/apps/search/types.py b/backend/apps/search/types.py index 994f3dc..9a93944 100644 --- a/backend/apps/search/types.py +++ b/backend/apps/search/types.py @@ -23,7 +23,7 @@ FACET_FIELDS = { "job_title_role": "job_title_role", "seniority": "seniority_levels", "country": "location_country", - "industry": "industry", + "industry": "industry.keyword", "company": "company.keyword", } diff --git a/backend/config/settings/base.py b/backend/config/settings/base.py index d06f44b..ecb25f1 100644 --- a/backend/config/settings/base.py +++ b/backend/config/settings/base.py @@ -64,6 +64,9 @@ INSTALLED_APPS = [ "django_filters", "corsheaders", "drf_spectacular", + # Serves the Swagger UI / ReDoc bundles from our own origin instead of a + # CDN, which is what keeps the strict CSP in frontend/nginx.conf working. + "drf_spectacular_sidecar", # Local "apps.core", "apps.accounts", @@ -212,8 +215,21 @@ SPECTACULAR_SETTINGS = { "COMPONENT_SPLIT_REQUEST": True, "SCHEMA_PATH_PREFIX": "/api/v1", "SERVE_PERMISSIONS": ["rest_framework.permissions.AllowAny"], + # Serve the UI assets from this origin (drf-spectacular-sidecar) rather + # than the default cdn.jsdelivr.net "@latest" URLs: the CSP blocks + # third-party scripts, so the CDN version renders a blank page. + "SWAGGER_UI_DIST": "SIDECAR", + "SWAGGER_UI_FAVICON_HREF": "SIDECAR", + "REDOC_DIST": "SIDECAR", } +# Whether /api/docs/, /api/redoc/ and /api/schema/ are routed at all. They +# describe every endpoint, parameter and response shape in the service, so +# production keeps them off by default and opts in explicitly (behind an +# authenticating proxy) rather than publishing the full API surface to +# anonymous callers - see config/urls.py. +SERVE_API_DOCS = env.bool("SERVE_API_DOCS", default=True) + # --------------------------------------------------------------------------- # CORS / CSRF # --------------------------------------------------------------------------- diff --git a/backend/config/settings/prod.py b/backend/config/settings/prod.py index 80463f2..3227843 100644 --- a/backend/config/settings/prod.py +++ b/backend/config/settings/prod.py @@ -7,7 +7,7 @@ back to an insecure default. """ from .base import * # noqa: F403 -from .base import DEBUG, SECRET_KEY, env +from .base import DATABASES, DEBUG, SECRET_KEY, env # --------------------------------------------------------------------------- # Fail fast on unsafe configuration @@ -18,11 +18,36 @@ if DEBUG: # pragma: no cover - configuration guard if len(SECRET_KEY) < 50: # pragma: no cover - configuration guard raise RuntimeError("DJANGO_SECRET_KEY must be at least 50 characters in production.") +# A length check alone is false comfort: the placeholder shipped in +# .env.example is 68 characters long and would sail straight through it. +_PLACEHOLDER_MARKERS = ("change-me", "changeme", "build-only", "not-a-real-secret") + + +def _reject_placeholder(name: str, value: str) -> None: # pragma: no cover - guard + lowered = value.lower() + if any(marker in lowered for marker in _PLACEHOLDER_MARKERS): + raise RuntimeError( + f"{name} still holds a placeholder value from .env.example. " + "Generate a real secret before deploying: " + 'python -c "import secrets; print(secrets.token_urlsafe(64))"' + ) + + +_reject_placeholder("DJANGO_SECRET_KEY", SECRET_KEY) +_reject_placeholder("POSTGRES_PASSWORD", DATABASES["default"].get("PASSWORD") or "") +_reject_placeholder("ELASTICSEARCH_PASSWORD", env("ELASTICSEARCH_PASSWORD", default="")) + # --------------------------------------------------------------------------- # Transport security # --------------------------------------------------------------------------- SECURE_SSL_REDIRECT = env.bool("SECURE_SSL_REDIRECT", default=True) SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") + +# Container/orchestrator probes hit the API over plain HTTP on localhost, so +# without an exemption every probe gets a 301 to https. `curl -fsS` treats a +# 3xx as success, which means a completely broken container would still report +# itself healthy - the probe has to reach the real view to be worth anything. +SECURE_REDIRECT_EXEMPT = [r"^api/v1/health/"] SECURE_HSTS_SECONDS = 31_536_000 # one year SECURE_HSTS_INCLUDE_SUBDOMAINS = True SECURE_HSTS_PRELOAD = True @@ -40,3 +65,8 @@ ADMIN_URL = env("DJANGO_ADMIN_URL", default="admin/") # Deny by default; CORS_ALLOWED_ORIGINS must be set explicitly. CORS_ALLOW_ALL_ORIGINS = False + +# The interactive docs describe every endpoint and payload in the service and +# are served unauthenticated. Off unless the operator explicitly opts in (for +# an internal-only origin, say) - see config/urls.py. +SERVE_API_DOCS = env.bool("SERVE_API_DOCS", default=False) diff --git a/backend/config/urls.py b/backend/config/urls.py index 0efac05..09c94af 100644 --- a/backend/config/urls.py +++ b/backend/config/urls.py @@ -25,16 +25,23 @@ api_v1 = [ urlpatterns = [ path(getattr(settings, "ADMIN_URL", "admin/"), admin.site.urls), path("api/v1/", include((api_v1, "api"), namespace="v1")), - # OpenAPI schema and the two documentation renderers. - path("api/schema/", SpectacularAPIView.as_view(), name="schema"), - path( - "api/docs/", - SpectacularSwaggerView.as_view(url_name="schema"), - name="swagger-ui", - ), - path("api/redoc/", SpectacularRedocView.as_view(url_name="schema"), name="redoc"), ] +# OpenAPI schema and the two documentation renderers. They are served without +# authentication, and together they document every endpoint, parameter and +# response shape in the service - useful everywhere except on a public +# production origin, where SERVE_API_DOCS defaults to false. +if getattr(settings, "SERVE_API_DOCS", True): + urlpatterns += [ + path("api/schema/", SpectacularAPIView.as_view(), name="schema"), + path( + "api/docs/", + SpectacularSwaggerView.as_view(url_name="schema"), + name="swagger-ui", + ), + path("api/redoc/", SpectacularRedocView.as_view(url_name="schema"), name="redoc"), + ] + admin.site.site_header = "LinkedIn Profile Search" admin.site.site_title = "LinkedIn Profile Search" admin.site.index_title = "Operations" diff --git a/backend/requirements/base.txt b/backend/requirements/base.txt index ff71ba1..748d8e4 100644 --- a/backend/requirements/base.txt +++ b/backend/requirements/base.txt @@ -19,8 +19,14 @@ psycopg[binary]~=3.2.3 djangorestframework-simplejwt~=5.3.1 argon2-cffi~=23.1.0 -# API schema & interactive documentation +# API schema & interactive documentation. +# The sidecar ships the Swagger UI / ReDoc bundles as static files. Without +# it drf-spectacular points the browser at cdn.jsdelivr.net, which the CSP in +# frontend/nginx.conf ("script-src 'self'") blocks, leaving a blank page - and +# the CDN URL is pinned to "@latest", which is a supply-chain risk in its own +# right. Self-hosting fixes both and lets the docs work air-gapped. drf-spectacular~=0.27.2 +drf-spectacular-sidecar~=2026.9.1 # Cross origin access for the SPA django-cors-headers~=4.4.0 diff --git a/backend/tests/test_query_builder.py b/backend/tests/test_query_builder.py index 26b287f..5c2e882 100644 --- a/backend/tests/test_query_builder.py +++ b/backend/tests/test_query_builder.py @@ -11,15 +11,22 @@ from __future__ import annotations from apps.search.backends.elastic import build_query from apps.search.types import SORT_EXPERIENCE_DESC, SORT_NAME, SearchRequest +FACET_NAMES = ("skills", "job_title_role", "seniority", "country", "industry", "company") + def _bool(request: SearchRequest) -> dict: return build_query(request)["query"]["bool"] +def _post_filters(request: SearchRequest) -> list[dict]: + body = build_query(request) + return body.get("post_filter", {}).get("bool", {}).get("filter", []) + + def test_empty_query_matches_everything() -> None: - clause = _bool(SearchRequest()) - assert clause["must"] == [{"match_all": {}}] - assert clause["filter"] == [] + body = build_query(SearchRequest()) + assert body["query"]["bool"]["must"] == [{"match_all": {}}] + assert "post_filter" not in body def test_keyword_query_uses_fuzzy_multi_match_and_phrase_boost() -> None: @@ -32,23 +39,35 @@ def test_keyword_query_uses_fuzzy_multi_match_and_phrase_boost() -> None: assert clause["should"][0]["multi_match"]["type"] == "phrase" -def test_filters_are_terms_clauses_in_filter_context() -> None: +def test_filters_do_not_narrow_the_scored_query() -> None: + """ + Filters must live in ``post_filter``, never in ``query.bool`` - that is + what lets a facet's own aggregation stay unaffected by its own selection. + See ``test_a_facets_own_selection_does_not_narrow_its_own_aggregation`` + for the behaviour this enables. + """ clause = _bool( SearchRequest(skills=("python", "django"), countries=("germany",), seniority=("senior",)) ) - assert {"terms": {"skills.keyword": ["python", "django"]}} in clause["filter"] - assert {"terms": {"location_country": ["germany"]}} in clause["filter"] - assert {"terms": {"seniority_levels": ["senior"]}} in clause["filter"] - # Filters must not leak into the scoring clauses. assert clause["must"] == [{"match_all": {}}] + assert "filter" not in clause + + +def test_filters_are_terms_clauses_in_the_post_filter() -> None: + filters = _post_filters( + SearchRequest(skills=("python", "django"), countries=("germany",), seniority=("senior",)) + ) + assert {"terms": {"skills.keyword": ["python", "django"]}} in filters + assert {"terms": {"location_country": ["germany"]}} in filters + assert {"terms": {"seniority_levels": ["senior"]}} in filters def test_experience_range_is_translated_to_a_range_filter() -> None: - clause = _bool(SearchRequest(min_experience=5, max_experience=10)) - assert {"range": {"years_experience": {"gte": 5, "lte": 10}}} in clause["filter"] + filters = _post_filters(SearchRequest(min_experience=5, max_experience=10)) + assert {"range": {"years_experience": {"gte": 5, "lte": 10}}} in filters - open_ended = _bool(SearchRequest(min_experience=5)) - assert {"range": {"years_experience": {"gte": 5}}} in open_ended["filter"] + open_ended = _post_filters(SearchRequest(min_experience=5)) + assert {"range": {"years_experience": {"gte": 5}}} in open_ended def test_pagination_maps_to_from_and_size() -> None: @@ -74,3 +93,62 @@ def test_highlight_only_requested_with_a_keyword() -> None: def test_facets_can_be_switched_off() -> None: assert "aggs" in build_query(SearchRequest()) assert "aggs" not in build_query(SearchRequest(include_facets=False)) + + +# -- Multi-select faceting --------------------------------------------------- +# +# The bug this section pins down: ticking one value in a facet (e.g. "senior" +# under Seniority) must not make the other values in *that same facet* +# disappear. Elasticsearch has no built-in notion of "OR within a facet, +# ignore this facet's own filter" - it has to be built by hand as one `filter` +# aggregation per facet, scoped by every *other* active filter. These tests +# exist so a future refactor of ``build_query``/``_build_aggregations`` cannot +# silently regress back to the single-facet-collapses-to-one-value bug. + + +def test_a_facets_own_selection_does_not_narrow_its_own_aggregation() -> None: + aggs = build_query(SearchRequest(roles=("engineering",)))["aggs"] + # `job_title_role` is the facet driven by `roles` - its own filter must be + # absent from its own aggregation, so unrelated role buckets still appear. + assert aggs["job_title_role"] == { + "terms": {"field": "job_title_role", "size": 25, "order": {"_count": "desc"}} + } + + +def test_a_facets_selection_does_narrow_every_other_facet() -> None: + aggs = build_query(SearchRequest(roles=("engineering",)))["aggs"] + for facet in ("skills", "seniority", "country", "industry", "company"): + wrapper = aggs[facet] + assert wrapper["filter"] == { + "bool": {"filter": [{"terms": {"job_title_role": ["engineering"]}}]} + } + # The terms aggregation itself is nested one level deeper, under its + # own name, once it is wrapped in the scoping `filter` aggregation. + assert facet in wrapper["aggs"] + + +def test_two_active_facets_each_exclude_only_their_own_filter() -> None: + aggs = build_query(SearchRequest(roles=("engineering",), countries=("germany",)))["aggs"] + + role_filters = aggs["job_title_role"]["filter"]["bool"]["filter"] + assert role_filters == [{"terms": {"location_country": ["germany"]}}] + + country_filters = aggs["country"]["filter"]["bool"]["filter"] + assert country_filters == [{"terms": {"job_title_role": ["engineering"]}}] + + # A facet with no selection of its own is scoped by both. + skills_filters = aggs["skills"]["filter"]["bool"]["filter"] + assert {"terms": {"job_title_role": ["engineering"]}} in skills_filters + assert {"terms": {"location_country": ["germany"]}} in skills_filters + + +def test_experience_range_scopes_every_facet_and_is_never_excluded() -> None: + """The years-of-experience range has no facet of its own, so it always + applies - including to a facet that shares no other active filter.""" + aggs = build_query(SearchRequest(min_experience=5))["aggs"] + for facet in FACET_NAMES: + wrapper = aggs[facet] + assert wrapper["filter"]["bool"]["filter"] == [ + {"range": {"years_experience": {"gte": 5}}} + ] + diff --git a/backend/tests/test_search_api.py b/backend/tests/test_search_api.py index 4a56b21..c15e013 100644 --- a/backend/tests/test_search_api.py +++ b/backend/tests/test_search_api.py @@ -90,6 +90,50 @@ def test_facets_reflect_the_filtered_result_set( assert response.data["facets"]["skills"] +def test_selecting_a_facet_value_keeps_its_own_sibling_values_visible( + auth_client, analyst, sample_profiles +) -> None: # noqa: ANN001 + """ + Multi-select facets: ticking "Engineering" under Job family must not make + "Human resources" disappear from that same checkbox list - a facet is + scoped by every *other* active filter, never by its own. Every other + facet (country, in ``test_facets_reflect_the_filtered_result_set`` above) + keeps narrowing to the selection as before. + """ + response = search(auth_client(analyst), roles="engineering") + assert sorted(names(response)) == ["maria frontend", "sara python"] + + roles = {bucket["key"]: bucket["count"] for bucket in response.data["facets"]["job_title_role"]} + assert roles == {"engineering": 2, "human_resources": 1} + + +def test_self_exclusion_does_not_ignore_other_active_filters( + auth_client, analyst, sample_profiles +) -> None: # noqa: ANN001 + """ + Excluding a facet's own filter must not turn into excluding *all* + filters: with ``roles=engineering`` AND ``countries=germany`` both active, + the job-family facet drops only its own ``roles`` filter and keeps + ``countries=germany``. "human_resources" (john, based in the United + States) correctly stays absent here - not because the facet is narrowed by + its own selection, but because no German profile has that role at all. + Contrast with ``test_selecting_a_facet_value_keeps_its_own_sibling_values_visible``, + where the same "human_resources" sibling *does* reappear once + ``countries`` is not also constraining the result set. + """ + response = search(auth_client(analyst), roles="engineering", countries="germany") + assert names(response) == ["sara python"] + + roles = {bucket["key"] for bucket in response.data["facets"]["job_title_role"]} + assert roles == {"engineering"} + + countries = {bucket["key"] for bucket in response.data["facets"]["country"]} + # Scoped by roles=engineering (the other active filter) but not by its + # own countries=germany selection, so spain (maria, also engineering) + # still shows up. + assert countries == {"germany", "spain"} + + def test_sorting_by_experience(auth_client, analyst, sample_profiles) -> None: # noqa: ANN001 assert names(search(auth_client(analyst), sort="-experience"))[0] == "john recruiter" assert names(search(auth_client(analyst), sort="experience"))[0] == "maria frontend" diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index b84ebe8..68e4c5f 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -11,7 +11,9 @@ services: backend: build: - target: runtime + # The dev stage adds dev.txt (django-extensions, pytest, ...) on top of + # the runtime image; config.settings.dev cannot be imported without it. + target: dev command: - python - manage.py diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 88c860a..b5b9df6 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -53,16 +53,48 @@ services: DJANGO_SETTINGS_MODULE: config.settings.prod DJANGO_DEBUG: "false" SECURE_SSL_REDIRECT: "true" + # Off unless deliberately opted in: /api/docs/ and /api/schema/ publish + # the entire API surface to anonymous callers. + SERVE_API_DOCS: ${SERVE_API_DOCS:-false} ELASTICSEARCH_USERNAME: elastic ELASTICSEARCH_PASSWORD: ${ELASTIC_PASSWORD} # A production container must never mutate data on boot. SEED_DATASET: "false" SEED_USERS: "false" + # Nor schema-migrate on boot: with `replicas: 2` below, both containers + # would start `manage.py migrate` at the same moment and race each other + # through the same DDL. Migrations are a deliberate, one-at-a-time + # deploy step instead - `make prod-migrate`, which runs the one-shot + # `backend-migrate` service defined below. + RUN_MIGRATIONS: "false" deploy: replicas: 2 resources: limits: { cpus: "1.0", memory: 768m } + # --------------------------------------------------------------------------- + # One-shot schema migration, run by `make prod-migrate` before rolling out a + # new image. Kept out of the default set by the "migrate" profile so a plain + # `up` never starts it. + # --------------------------------------------------------------------------- + backend-migrate: + extends: + file: docker-compose.yml + service: backend + profiles: ["migrate"] + restart: "no" + entrypoint: [] + command: ["python", "manage.py", "migrate", "--noinput"] + environment: + DJANGO_SETTINGS_MODULE: config.settings.prod + DJANGO_DEBUG: "false" + DJANGO_SECRET_KEY: ${DJANGO_SECRET_KEY:?DJANGO_SECRET_KEY is required} + DATABASE_URL: postgres://${POSTGRES_USER:-linkedin}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB:-linkedin} + # Migrating must not depend on the search cluster being reachable. + SEARCH_BACKEND: postgres + SECURE_SSL_REDIRECT: "false" + deploy: !reset null + frontend: deploy: resources: @@ -72,3 +104,28 @@ services: deploy: resources: limits: { memory: 1g } + +# ============================================================================= +# Data durability +# +# The base compose file declares these as ordinary project volumes, which +# `docker compose down -v` (and therefore `make clean`) deletes without +# asking. Marking them external in production flips that guarantee: Compose +# treats them as owned by the operator, never creates them and - crucially - +# never removes them, so a mistyped teardown cannot destroy the database or +# the search index. +# +# They must exist before the first `make prod`; `make prod-volumes` creates +# them. Back them up with your usual volume/snapshot tooling - external only +# means Compose will not delete them, not that they are backed up. +# ============================================================================= +volumes: + postgres-data: + external: true + name: ${DATA_VOLUME_PREFIX:-linkedin-search}-postgres-data + elasticsearch-data: + external: true + name: ${DATA_VOLUME_PREFIX:-linkedin-search}-elasticsearch-data + logstash-data: + external: true + name: ${DATA_VOLUME_PREFIX:-linkedin-search}-logstash-data diff --git a/docker-compose.yml b/docker-compose.yml index 2311a62..9bc482e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -93,7 +93,9 @@ services: ELASTICSEARCH_HOSTS: http://elasticsearch:9200 ELASTICSEARCH_USERNAME: ${ELASTICSEARCH_USERNAME:-} ELASTICSEARCH_PASSWORD: ${ELASTICSEARCH_PASSWORD:-} - xpack.monitoring.enabled: "false" + # No `xpack.monitoring.enabled` here on purpose: any logstash.* variable + # makes the image entrypoint rewrite config/logstash.yml, which is + # mounted read-only below. The setting already lives in that file. volumes: - ./elk/logstash/pipeline:/usr/share/logstash/pipeline:ro - ./elk/logstash/templates:/usr/share/logstash/templates:ro @@ -142,6 +144,9 @@ services: build: context: . dockerfile: backend/Dockerfile + # Explicit: the Dockerfile has a later `dev` stage that must not leak + # into the default stack. + target: runtime <<: *service-defaults environment: DJANGO_SETTINGS_MODULE: ${DJANGO_SETTINGS_MODULE:-config.settings.prod} @@ -150,6 +155,10 @@ services: DJANGO_ALLOWED_HOSTS: ${DJANGO_ALLOWED_HOSTS:-localhost,127.0.0.1,backend} DJANGO_ADMIN_URL: ${DJANGO_ADMIN_URL:-admin/} SECURE_SSL_REDIRECT: ${SECURE_SSL_REDIRECT:-false} + # This stack is the local/demo one, so the interactive API docs are on. + # The production overlay turns them off - they document every endpoint + # and are served unauthenticated. + SERVE_API_DOCS: ${SERVE_API_DOCS:-true} ENVIRONMENT: ${ENVIRONMENT:-development} SERVICE_NAME: ${SERVICE_NAME:-linkedin-search-api} CORS_ALLOWED_ORIGINS: ${CORS_ALLOWED_ORIGINS:-http://localhost:8080} @@ -197,6 +206,37 @@ services: - no-new-privileges:true networks: [backend-net, frontend-net] + # --------------------------------------------------------------------------- + # Test runner - never started by `up`/`dev`/`prod` (the "test" profile keeps + # it out of the default set); invoked only via `make test`, which runs + # `docker compose run --rm backend-test`. + # + # The `backend` service above intentionally runs the `runtime` build stage, + # which ships no test tooling - that is what keeps the production image + # minimal. `pytest`, `django-extensions` etc. live only in the `dev` stage, + # so the test runner builds that stage instead. It talks to the same + # Postgres the rest of the stack uses; pytest-django creates and migrates + # its own throwaway `test_` database there, and `config.settings.test` + # forces the search backend to Postgres, so Elasticsearch/Redis are not + # dependencies here. + # --------------------------------------------------------------------------- + backend-test: + build: + context: . + dockerfile: backend/Dockerfile + target: dev + profiles: ["test"] + entrypoint: [] + command: ["pytest"] + environment: + DJANGO_SETTINGS_MODULE: config.settings.test + DJANGO_SECRET_KEY: ${DJANGO_SECRET_KEY:?DJANGO_SECRET_KEY is required} + DATABASE_URL: postgres://${POSTGRES_USER:-linkedin}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB:-linkedin} + depends_on: + postgres: + condition: service_healthy + networks: [backend-net] + frontend: build: context: . @@ -209,10 +249,12 @@ services: security_opt: - no-new-privileges:true read_only: true + # A tmpfs mount is created root:root 0755 and shadows whatever the image + # chowned at build time, so nginx (uid 101) needs the mode set explicitly. tmpfs: - - /tmp - - /var/run - - /var/cache/nginx + - /tmp:mode=1777 + - /var/run:mode=1777 + - /var/cache/nginx:mode=1777 networks: [frontend-net] networks: diff --git a/frontend/Dockerfile b/frontend/Dockerfile index a21720b..b6807cc 100644 --- a/frontend/Dockerfile +++ b/frontend/Dockerfile @@ -29,6 +29,9 @@ FROM nginx:1.27-alpine AS runtime RUN rm -rf /usr/share/nginx/html/* /etc/nginx/conf.d/default.conf COPY frontend/nginx.conf /etc/nginx/conf.d/app.conf +# Included by the location blocks in app.conf - see each file's own comment. +COPY frontend/security-headers.conf /etc/nginx/conf.d/security-headers.conf +COPY frontend/proxy-backend.conf /etc/nginx/conf.d/proxy-backend.conf COPY --from=builder /app/dist /usr/share/nginx/html # nginx:alpine already ships an unprivileged "nginx" user; the official @@ -39,5 +42,7 @@ RUN touch /var/run/nginx.pid \ USER nginx EXPOSE 8080 +# 127.0.0.1, not localhost: nginx listens on IPv4 only, and busybox wget +# resolves localhost to ::1 first without falling back to IPv4. HEALTHCHECK --interval=30s --timeout=4s --start-period=10s --retries=3 \ - CMD wget -q --spider http://localhost:8080/healthz || exit 1 + CMD wget -q --spider http://127.0.0.1:8080/healthz || exit 1 diff --git a/frontend/contract/schemas.ts b/frontend/contract/schemas.ts new file mode 100644 index 0000000..8f1fa9a --- /dev/null +++ b/frontend/contract/schemas.ts @@ -0,0 +1,79 @@ +import { z } from 'zod'; + +/** + * Runtime mirrors of the TypeScript interfaces in `src/types/index.ts`. + * + * A TS interface is a compile-time-only promise: nothing stops the Django + * side from renaming a field, changing its nullability, or dropping a facet, + * and the frontend would only find out at runtime, in production, as a blank + * screen or a silently-wrong render. These schemas are validated against the + * real API response in `search-response.contract.test.ts`, so that class of + * drift fails a test instead. + */ + +export const FacetBucketSchema = z.object({ + key: z.string(), + count: z.number().int().nonnegative(), +}); + +export const FACET_NAMES = [ + 'skills', + 'job_title_role', + 'seniority', + 'country', + 'industry', + 'company', +] as const; + +export const SearchHitSchema = z.object({ + id: z.number(), + score: z.number(), + full_name: z.string(), + headline: z.string(), + job_title: z.string(), + job_title_role: z.string(), + seniority_levels: z.array(z.string()), + company: z.string(), + industry: z.string(), + location_name: z.string(), + location_country: z.string(), + years_experience: z.number().nullable(), + linkedin_url: z.string(), + skills: z.array(z.string()), + highlight: z.record(z.string(), z.array(z.string())), +}); + +export const SearchResponseSchema = z.object({ + count: z.number().int().nonnegative(), + page: z.number().int().positive(), + page_size: z.number().int().positive(), + total_pages: z.number().int().nonnegative(), + took_ms: z.number().int().nonnegative(), + backend: z.string(), + results: z.array(SearchHitSchema), + facets: z.record(z.enum(FACET_NAMES), z.array(FacetBucketSchema)), +}); + +export const UserSchema = z.object({ + id: z.number(), + email: z.string(), + full_name: z.string(), + role: z.enum(['admin', 'analyst', 'viewer']), + can_view_pii: z.boolean(), + date_joined: z.string(), +}); + +export const TokenPairSchema = z.object({ + access: z.string(), + refresh: z.string(), + user: UserSchema, +}); + +export const ApiErrorSchema = z.object({ + error: z.object({ + code: z.string(), + message: z.string(), + details: z.unknown().optional(), + request_id: z.string(), + }), +}); diff --git a/frontend/contract/search-response.contract.test.ts b/frontend/contract/search-response.contract.test.ts new file mode 100644 index 0000000..9068b91 --- /dev/null +++ b/frontend/contract/search-response.contract.test.ts @@ -0,0 +1,92 @@ +/** + * Contract test: hits the real, running API (not a mock) and validates the + * response against the same shape `src/types/index.ts` promises the rest of + * the frontend. This is the third kind of test in this suite, distinct from + * the other two: + * + * - unit tests (Vitest + Testing Library, `src/**\/*.test.tsx`) check + * frontend logic in isolation with a fake backend; + * - the e2e test (Playwright, `e2e/`) checks the full user-facing flow + * through a real browser against the real stack; + * - this one checks that the *data shape* the two sides agree on has not + * silently drifted - a Django field rename, a nullability change, or a + * dropped facet breaks this test even though it would type-check fine on + * both sides in isolation (the frontend's `SearchResponse` interface is + * just an assertion about what the backend sends, never verified against + * the backend itself anywhere else in this codebase). + * + * Requires the docker compose stack to be running (`make up`); not part of + * `npm test`, run separately via `npm run test:contract`. + */ +import { describe, expect, it } from 'vitest'; + +import { ApiErrorSchema, FACET_NAMES, SearchResponseSchema, TokenPairSchema } from './schemas'; +import { API_BASE_URL, DEMO_ANALYST } from './support/env'; + +async function login(): Promise { + const response = await fetch(`${API_BASE_URL}/auth/login/`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(DEMO_ANALYST), + }); + const body = await response.json(); + const parsed = TokenPairSchema.parse(body); + return parsed.access; +} + +describe('search response contract', () => { + it('matches the SearchResponse shape the frontend types promise', async () => { + const token = await login(); + const response = await fetch(`${API_BASE_URL}/search/?roles=engineering&page_size=5`, { + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.status).toBe(200); + + const body = await response.json(); + const result = SearchResponseSchema.safeParse(body); + + if (!result.success) { + throw new Error(`API response no longer matches SearchResponse:\n${result.error.message}`); + } + }); + + it('always returns every facet the frontend renders a filter group for', async () => { + // FilterPanel.tsx hard-codes these six facet names; a backend response + // missing one would leave that whole filter group silently empty. + const token = await login(); + const response = await fetch(`${API_BASE_URL}/search/?page_size=1`, { + headers: { Authorization: `Bearer ${token}` }, + }); + const body = SearchResponseSchema.parse(await response.json()); + + for (const facet of FACET_NAMES) { + expect(body.facets).toHaveProperty(facet); + expect(Array.isArray(body.facets[facet])).toBe(true); + } + }); + + it('returns real results for the seeded demo dataset', async () => { + const token = await login(); + const response = await fetch(`${API_BASE_URL}/search/?page_size=5`, { + headers: { Authorization: `Bearer ${token}` }, + }); + const body = SearchResponseSchema.parse(await response.json()); + + expect(body.count).toBeGreaterThan(0); + expect(body.results.length).toBeGreaterThan(0); + }); + + it('an unauthorized request matches the ApiError envelope the frontend error handler expects', async () => { + // api/client.ts's `toError()` reaches into `error.response.data.error.*` + // unconditionally - if the backend ever changed this envelope, every + // error message in the UI would silently become empty. + const response = await fetch(`${API_BASE_URL}/search/`); + expect(response.status).toBe(401); + + const body = await response.json(); + const result = ApiErrorSchema.safeParse(body); + if (!result.success) { + throw new Error(`401 response no longer matches ApiError:\n${result.error.message}`); + } + }); +}); diff --git a/frontend/contract/support/env.ts b/frontend/contract/support/env.ts new file mode 100644 index 0000000..879ae5c --- /dev/null +++ b/frontend/contract/support/env.ts @@ -0,0 +1,26 @@ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +/** Same idea as e2e/support/env.ts - read the seeded demo credentials from `.env` rather than hard-coding them. */ +function readDotEnv(): Record { + const path = fileURLToPath(new URL('../../../.env', import.meta.url)); + const text = readFileSync(path, 'utf-8'); + const values: Record = {}; + for (const line of text.split('\n')) { + const match = /^([A-Z0-9_]+)=(.*)$/.exec(line.trim()); + const key = match?.[1]; + if (key !== undefined) values[key] = match?.[2] ?? ''; + } + return values; +} + +const env = readDotEnv(); + +export const DEMO_ANALYST = { + email: env.DEMO_ANALYST_EMAIL ?? 'analyst@example.com', + password: env.DEMO_ANALYST_PASSWORD ?? 'Analyst-Passw0rd!', +}; + +export const API_BASE_URL = process.env.E2E_BASE_URL + ? `${process.env.E2E_BASE_URL}/api/v1` + : 'http://localhost:8080/api/v1'; diff --git a/frontend/e2e/multi-select-facets.spec.ts b/frontend/e2e/multi-select-facets.spec.ts new file mode 100644 index 0000000..d9d0f10 --- /dev/null +++ b/frontend/e2e/multi-select-facets.spec.ts @@ -0,0 +1,119 @@ +import { expect, test } from '@playwright/test'; + +import { DEMO_ANALYST } from './support/env'; + +/** + * End-to-end regression test for the bug reported against the running app: + * ticking one value in a facet (e.g. "Job family") made every other value in + * that same checkbox list disappear, because the backend's aggregations were + * computed on the already-filtered result set. Fixed in + * apps/search/backends/{elastic,postgres}.py (self-exclusion aggregations) + * and covered at the unit/API level by test_query_builder.py and + * test_search_api.py. This spec drives the real browser against the real, + * Elasticsearch-backed stack (`docker compose up`) end to end, so it also + * catches anything a backend-only test cannot: wiring in FacetGroup/ + * FilterPanel, the network round trip, and URL state. + */ + +async function signIn(page: import('@playwright/test').Page) { + await page.goto('/login'); + await page.getByLabel('Email').fill(DEMO_ANALYST.email); + await page.getByLabel('Password').fill(DEMO_ANALYST.password); + await page.getByRole('button', { name: 'Sign in' }).click(); + await expect(page.getByRole('button', { name: /^Job family/ })).toBeVisible(); +} + +/** The raw facet value lives in the label's `title` attribute (see FacetGroup.tsx). */ +async function facetValues(group: import('@playwright/test').Locator): Promise { + const labels = group.locator('.facet__label'); + const count = await labels.count(); + const values: string[] = []; + for (let i = 0; i < count; i += 1) { + values.push((await labels.nth(i).getAttribute('title')) ?? ''); + } + return values; +} + +test.describe('multi-select facets', () => { + test('ticking one job-family value keeps every sibling value in the list', async ({ page }) => { + await signIn(page); + + const jobFamily = page.locator('.facet').filter({ has: page.getByRole('button', { name: /^Job family/ }) }); + const before = await facetValues(jobFamily); + expect(before.length).toBeGreaterThan(1); + + const firstCheckbox = jobFamily.getByRole('checkbox').first(); + const firstValue = await jobFamily.locator('.facet__label').first().getAttribute('title'); + + const responsePromise = page.waitForResponse((response) => response.url().includes('/api/v1/search/')); + await firstCheckbox.check(); + await responsePromise; + + await expect(firstCheckbox).toBeChecked(); + + // The exact bug: this used to collapse to just the ticked value. + const after = await facetValues(jobFamily); + expect(after.sort()).toEqual(before.sort()); + + // The URL is the source of truth for the filter state. + expect(page.url()).toContain(`roles=${encodeURIComponent(firstValue ?? '')}`); + }); + + test('ticking a second job-family value accumulates instead of replacing the first', async ({ page }) => { + await signIn(page); + + const jobFamily = page.locator('.facet').filter({ has: page.getByRole('button', { name: /^Job family/ }) }); + const checkboxes = jobFamily.getByRole('checkbox'); + const labels = jobFamily.locator('.facet__label'); + + const firstValue = await labels.nth(0).getAttribute('title'); + const secondValue = await labels.nth(1).getAttribute('title'); + + let responsePromise = page.waitForResponse((response) => response.url().includes('/api/v1/search/')); + await checkboxes.nth(0).check(); + await responsePromise; + + responsePromise = page.waitForResponse((response) => response.url().includes('/api/v1/search/')); + await checkboxes.nth(1).check(); + await responsePromise; + + await expect(checkboxes.nth(0)).toBeChecked(); + await expect(checkboxes.nth(1)).toBeChecked(); + + const url = new URL(page.url()); + const roles = (url.searchParams.get('roles') ?? '').split(','); + expect(roles).toContain(firstValue); + expect(roles).toContain(secondValue); + + // Ticking the second one must not have removed the first from the DOM. + const after = await facetValues(jobFamily); + expect(after).toContain(firstValue); + expect(after).toContain(secondValue); + + // "Clear all" must be able to undo both selections in one click. + await page.getByRole('button', { name: /clear all/i }).click(); + await expect(checkboxes.nth(0)).not.toBeChecked(); + }); + + test('a different facet still narrows when job family is filtered (no over-relaxing)', async ({ page }) => { + await signIn(page); + + const jobFamily = page.locator('.facet').filter({ has: page.getByRole('button', { name: /^Job family/ }) }); + const country = page.locator('.facet').filter({ has: page.getByRole('button', { name: /^Country/ }) }); + + // Country is collapsed by default (only the first three facets start + // open) - expand it so its checkbox list is actually in the DOM. + await page.getByRole('button', { name: /^Country/ }).click(); + const countryBefore = await facetValues(country); + expect(countryBefore.length).toBeGreaterThan(0); + + const responsePromise = page.waitForResponse((response) => response.url().includes('/api/v1/search/')); + await jobFamily.getByRole('checkbox').first().check(); + await responsePromise; + + const countryAfter = await facetValues(country); + // A facet with no selection of its own must still be scoped by the + // active filter - it should not also stay untouched. + expect(countryAfter.length).toBeLessThanOrEqual(countryBefore.length); + }); +}); diff --git a/frontend/e2e/support/env.ts b/frontend/e2e/support/env.ts new file mode 100644 index 0000000..5856928 --- /dev/null +++ b/frontend/e2e/support/env.ts @@ -0,0 +1,26 @@ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +/** + * Reads the demo analyst credentials straight out of the repo's `.env` + * (created by `make env` from `.env.example`) instead of hard-coding them, so + * the e2e suite keeps working if the seeded demo password ever changes. + */ +function readDotEnv(): Record { + const path = fileURLToPath(new URL('../../../.env', import.meta.url)); + const text = readFileSync(path, 'utf-8'); + const values: Record = {}; + for (const line of text.split('\n')) { + const match = /^([A-Z0-9_]+)=(.*)$/.exec(line.trim()); + const key = match?.[1]; + if (key !== undefined) values[key] = match?.[2] ?? ''; + } + return values; +} + +const env = readDotEnv(); + +export const DEMO_ANALYST = { + email: env.DEMO_ANALYST_EMAIL ?? 'analyst@example.com', + password: env.DEMO_ANALYST_PASSWORD ?? 'Analyst-Passw0rd!', +}; diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 8435ec9..9bd942b 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -4,7 +4,21 @@ # Putting both behind one origin means the browser never issues a cross-origin # request, so no CORS pre-flight, no third-party cookie rules, and the API can # keep its allow-list empty in production. +# +# Every location includes security-headers.conf rather than relying on a +# server-level declaration - see the comment in that file for why inheritance +# cannot be trusted here. Proxied locations additionally include +# proxy-backend.conf, which forwards the client details and drops the +# duplicate security headers Django emits. # ----------------------------------------------------------------------------- + +# The SPA's policy: no inline script, nothing loaded off-origin. The bundle is +# fully self-hosted, so this can stay strict. ('unsafe-inline' for styles is +# still required by the inline style attributes React emits.) +map $host $spa_csp { + default "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'; font-src 'self'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'"; +} + server { listen 8080; server_name _; @@ -12,14 +26,6 @@ server { root /usr/share/nginx/html; index index.html; - # --- Security headers ---------------------------------------------------- - add_header X-Content-Type-Options "nosniff" always; - add_header X-Frame-Options "DENY" always; - add_header Referrer-Policy "strict-origin-when-cross-origin" always; - add_header Permissions-Policy "camera=(), microphone=(), geolocation=(), interest-cohort=()" always; - # The bundle is fully self-hosted, so the policy can stay strict. - add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'; font-src 'self'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'" always; - server_tokens off; client_max_body_size 2m; @@ -29,40 +35,59 @@ server { location = /healthz { access_log off; + include /etc/nginx/conf.d/security-headers.conf; add_header Content-Type text/plain; return 200 'ok'; } # Hashed assets never change under the same name - cache them hard. location /assets/ { + include /etc/nginx/conf.d/security-headers.conf; + add_header Content-Security-Policy $spa_csp always; expires 1y; add_header Cache-Control "public, immutable"; try_files $uri =404; } - location /api/ { + # --- API documentation --------------------------------------------------- + # Swagger UI and ReDoc bootstrap themselves from an inline