Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,26 @@ DATA_UPLOAD_MAX_MEMORY_SIZE=67108864
SITE_ADDRESS=:80

# Mailbox shown to users on error pages and in the footer for support
# requests.
# requests — and the sole recipient of the software-request notification
# email (nothing ever emails a requester; the app stores no user addresses).
SUPPORT_EMAIL=support@ilifu.ac.za

# --- Email ------------------------------------------------------------------
# The SMTP relay the request notification goes out through. Dev and test
# never open an SMTP socket (console and locmem backends), so everything
# here can stay commented locally. Production selects the real SMTP backend
# and requires EMAIL_HOST and DEFAULT_FROM_EMAIL — the other five keep
# defaults matching smtp-relay.gmail.com in IP-allowlist mode (587,
# STARTTLS, no credentials).
# EMAIL_HOST=smtp-relay.gmail.com
# EMAIL_PORT=587
# EMAIL_USE_TLS=1
# EMAIL_USE_SSL=0
# EMAIL_HOST_USER=
# EMAIL_HOST_PASSWORD=
# DEFAULT_FROM_EMAIL=catalogue@ilifu.ac.za
# EMAIL_TIMEOUT=10

# --- OIDC -----------------------------------------------------------------
# Left commented deliberately: the defaults in settings/base.py already point
# at `ilifu_catalog.devauth`, the fake in-process issuer mounted only when
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ jobs:
walking.py
client.py
cli.py
# Matches BOTH tasks.py files (find matches by basename): the
# ingest apply and the support notification. Same silent-failure
# profile as the rest of this list — a bug in either loses work
# (a snapshot, a notification) without erroring where anyone
# looks.
tasks.py
)

found_files=()
Expand Down
3 changes: 3 additions & 0 deletions ANSIBLE_DEPLOY.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ quietly running on a shared placeholder.
| `OIDC_OP_USER_ENDPOINT` | back-channel |
| `OIDC_OP_JWKS_ENDPOINT` | back-channel |
| `ILIFU_ADMIN_EMAILS` | comma-separated email addresses; parsed as a plain string like the hosts list. **Nobody can reach the admin screen without this**, which is why it has no default in production |
| `EMAIL_HOST` | SMTP relay for the software-request notification (e.g. `smtp-relay.gmail.com`); port/TLS/credentials have working defaults for that relay's IP-allowlist mode |
| `DEFAULT_FROM_EMAIL` | the notification's From address — relays reject Django's `webmaster@localhost` fallback, so a deployment must state a real one |
| `EMAIL_USE_SSL` | only for an implicit-SSL relay (usually port 465) — and then set **both** `EMAIL_USE_SSL=1` *and* `EMAIL_USE_TLS=0`. TLS defaults on for the 587+STARTTLS mode, and the pair together is refused **at boot**: setting only `EMAIL_USE_SSL=1` takes the whole site down, not just email |

Against real Keycloak all four endpoints are externally reachable and the front/back-channel
distinction disappears; it only matters against the in-process development issuer.
Expand Down
9 changes: 6 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ that lacks it silently tests the renderer only against a fake.
**Two coverage gates, both enforced in CI.** 95% project-wide, plus a second pass requiring
**100%** on `ingest.py`, `manpages.py`, `search.py`, `categories.py`, `claims.py`,
`backends.py`, `permissions.py`, `modules.py`, `containers.py`, `middleware.py`, `lmod.py`,
`libraries.py`, `walking.py`, `client.py` and `cli.py` (the last two are the collector's
transport — the code that carries the bearer token). Reaching that with assertion-free tests
`libraries.py`, `walking.py`, `client.py`, `cli.py` (the last two are the collector's
transport — the code that carries the bearer token) and `tasks.py` (matched by basename, so
both of them: the ingest apply and the support notification). Reaching that with assertion-free tests
defeats the purpose; the gate exists to make those modules trustworthy — they are the ones
whose bugs are silent, losing most of a scan without erroring, or quietly granting or
refusing access. The authoritative list is `domain_module_names` in
Expand Down Expand Up @@ -175,7 +176,9 @@ which is the difference between reading the code and arguing with it.
licences). No build step, no CDN — the cluster environment can be offline. Fonts are
self-hosted `.woff2` too; nothing loads from Google Fonts.
- **Celery + Redis**, not django-q. `beat` runs one job: the nightly public-list cache
regeneration at 02:00. Ingest applies are the other task.
regeneration at 02:00. The workers run two more: ingest applies, and the
software-request support notification (`requests_app/tasks.py`) — the one whose failure
is user-visible, surfacing as the admin queue's un-notified `!` marker.
- **mozilla-django-oidc against Keycloak.** Any user the realm authenticates can sign in and
gets an account on first login. **Admin is an email allow-list in this app, not a Keycloak
group** — `ILIFU_ADMIN_EMAILS`, compared against the `email` claim at each login, with the
Expand Down
20 changes: 16 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,9 @@ for the annotated list.

Every variable has a development default, which is why the stack runs on a fresh clone.
**Production removes the defaults that must not be shared** — `ProductionSettings` makes the
secret key, the allowed hosts, all six OIDC fields and the admin allow-list required, so a
deployment that omits one fails to boot rather than quietly running on a placeholder.
secret key, the allowed hosts, all six OIDC fields, the admin allow-list and the mail relay
pair (`EMAIL_HOST`, `DEFAULT_FROM_EMAIL`) required, so a deployment that omits one fails to
boot rather than quietly running on a placeholder.

| Variable | Purpose | Required in prod |
|---|---|---|
Expand All @@ -231,7 +232,12 @@ deployment that omits one fails to boot rather than quietly running on a placeho
| `REDIS_MAXMEMORY` | ceiling for the Redis container; sized to hold a full staged snapshot alongside the cache | no |
| `DATA_UPLOAD_MAX_MEMORY_SIZE` | largest request body Django will read. Django's 2.5 MB default is far below one ingest batch — see `.env.example` | no |
| `SITE_ADDRESS` | Caddy site address; `:80` in dev, real hostname in prod | yes |
| `SUPPORT_EMAIL` | shown to users; receives software requests | no |
| `SUPPORT_EMAIL` | shown to users, and the **sole recipient** of the software-request notification email | no |
| `EMAIL_HOST` | SMTP relay for the request notification | **yes** |
| `EMAIL_PORT` / `EMAIL_USE_TLS` / `EMAIL_USE_SSL` | relay port and encryption mode; defaults are 587 + STARTTLS | no |
| `EMAIL_HOST_USER` / `_PASSWORD` | relay credentials; empty for a relay in IP-allowlist mode | no |
| `DEFAULT_FROM_EMAIL` | the notification's From address — relays reject Django's `webmaster@localhost` fallback | **yes** |
| `EMAIL_TIMEOUT` | seconds before a hung SMTP send is abandoned; Django's own backend would wait forever | no |
| `OIDC_RP_CLIENT_ID` / `_SECRET` | Keycloak client credentials | **yes** |
| `OIDC_OP_AUTHORIZATION_ENDPOINT` | front-channel: the browser is redirected here | **yes** |
| `OIDC_OP_TOKEN_ENDPOINT` | back-channel: fetched by the app server | **yes** |
Expand Down Expand Up @@ -345,7 +351,13 @@ rather than overlooked.
apply never gets picked up inside that window is gone, and the collector has already been
told 202. `tasks.py` logs it at ERROR so it is visible rather than silent, and the next
scan re-uploads — but closing it properly means staging somewhere durable.
10. **`docker compose up` needs a `.env` despite the settings defaults.**
10. **A permanently failed support notification has no resend control.** The admin queue's
`!` marker says the notification never went out, but the only way to act on it is a
shell (`notify_support_of_request.delay(pk)`); a resend control on the triage form
would close the loop the marker opens. Deliberately deferred: the marker itself was the
fix for a silent failure, and a control that re-emails support belongs with a broader
decision about what else triage should be able to trigger.
11. **`docker compose up` needs a `.env` despite the settings defaults.**
`settings/base.py` defaults every variable so the stack runs on a fresh clone, but
`docker/entrypoint.sh` reads `os.environ['DATABASE_URL']` directly, with no default, so
without a `.env` the web container loops on `postgres not ready yet ('DATABASE_URL')`
Expand Down
2 changes: 1 addition & 1 deletion catalog/src/ilifu_catalog/accounts/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
def _admin_emails() -> Collection[str]:
"""`settings.ILIFU_ADMIN_EMAILS`, read through `getattr` for django-stubs' sake.

See `software.views_catalog._support_email`'s docstring for why the ignore
See `ilifu_catalog.support.support_email`'s docstring for why the ignore
is narrowed to a single accessor rather than repeated at every call site:
django-stubs types `django.conf.settings` against Django's own
`global_settings`, and this project's settings modules build their
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 6.0.7 on 2026-08-01 13:48

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
('requests_app', '0001_initial'),
]

operations = [
migrations.AddField(
model_name='softwarerequest',
name='support_notified_at',
field=models.DateTimeField(blank=True, default=None, null=True),
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from django.db import migrations
from django.db.models import F


def stamp_rows_the_synchronous_code_notified(apps, schema_editor):
"""Stamp every un-stamped row with its own `created_at`.

Rows that predate 0002 were notified synchronously — the pre-Celery code
emailed support inside the request cycle, before the response went out —
so their NULL stamp is an artefact of the column arriving later, not a
fact about support. Left NULL, each would wear the admin queue's
"support not notified" marker forever, a permanent false alarm on the
one screen where the marker is meant to prompt action. `created_at` is
an approximation and looks like one: it is visibly older than any send
the Celery task will ever record.

Runs on whatever rows exist at migration time, which on a fresh install
is none — the marker's real semantics start with the first row created
after this deploy.
"""
software_request_model = apps.get_model('requests_app', 'SoftwareRequest')
software_request_model.objects.filter(support_notified_at__isnull=True).update(
support_notified_at=F('created_at')
)


class Migration(migrations.Migration):
dependencies = [
('requests_app', '0002_softwarerequest_support_notified_at'),
]

operations = [
# Reverse is a no-op rather than re-NULLing: rolling back the *code*
# to the synchronous version makes the column unread, and un-stamping
# rows would destroy real send records if the migration is ever
# reversed after the task has run.
migrations.RunPython(stamp_rows_the_synchronous_code_notified, migrations.RunPython.noop),
]
8 changes: 8 additions & 0 deletions catalog/src/ilifu_catalog/requests_app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ class SoftwareRequest(models.Model):
)
admin_note = models.TextField(blank=True, default='')
created_at = models.DateTimeField(auto_now_add=True)
# When `requests_app.tasks.notify_support_of_request` got its email out
# to SUPPORT_EMAIL. A nullable timestamp, not a state machine: NULL means
# "support has not been told" whether the send is still pending or has
# been given up on — retries are Celery's business, not the schema's —
# and the admin queue's marker only asks that binary question. Stamped
# via a queryset `.update()`, never instance `.save()`, so the task
# cannot clobber a concurrent admin triage with a stale row.
support_notified_at = models.DateTimeField(null=True, blank=True, default=None)
# Nullable and `SET_NULL`, not `CASCADE`: a `SoftwareRequest` is the
# record that someone asked, and that record must survive even if the
# package it was eventually fulfilled by is later removed from the
Expand Down
114 changes: 114 additions & 0 deletions catalog/src/ilifu_catalog/requests_app/tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
"""The support notification, off the request cycle.

`views._save_and_notify` saves the `SoftwareRequest` row and enqueues this
task; the row is the record of truth and this email is best-effort delivery
of the news. That split is the whole design: the requester's submit never
waits on SMTP and never fails because of it — a relay outage surfaces as
the admin queue's "support not notified" marker (`support_notified_at`
still NULL) instead of a 500 on a request that was in fact saved.

Retry policy: three retries at a fixed sixty seconds — four attempts over
roughly three minutes covers a relay blip without pretending to be a mail
queue. Exhaustion logs ERROR and returns rather than raising: by the time
the budget is spent there is no requester left to hand an error to, and a
FAILURE state in a result backend nobody polls helps nobody. (Under
`CELERY_TASK_ALWAYS_EAGER` the *retry* path does raise `Retry` straight
back into the submitting request — the view's guarded enqueue is what
absorbs it; see `views._save_and_notify`.)
"""

from __future__ import annotations

import logging

from django.core.mail import send_mail
from django.utils import timezone

from ilifu_catalog.celery import app
from ilifu_catalog.support import support_email

from .models import SoftwareRequest

logger = logging.getLogger(__name__)


# Bound to `ilifu_catalog.celery.app` explicitly rather than `@shared_task`,
# for the reason spelled out at `software.tasks.apply_pending_snapshot`; the
# `type: ignore` is the same celery-ships-no-py.typed caveat as there.
@app.task( # type: ignore[untyped-decorator]
name='ilifu_catalog.requests_app.notify_support_of_request',
bind=True,
max_retries=3,
default_retry_delay=60,
)
def notify_support_of_request(self, request_id: int) -> None: # type: ignore[no-untyped-def]
"""Email SUPPORT_EMAIL that `request_id` landed, and stamp the row.

Logs and returns — rather than raising — when the row no longer exists:
by the time this runs there is no HTTP requester left to hand an error
back to, and a retry can never make a deleted row reappear.

The requester is named by `get_username()` — Keycloak's
`preferred_username` — explicitly rather than by interpolating the user
object and relying on `User.__str__` to be the same thing. It has
silently been the OIDC subject UUID before.

There is deliberately no reply-to address: this application stores none
(see `accounts.claims`), so the last line says where to look the
username up instead of implying support can just hit reply.
"""
software_request = SoftwareRequest.objects.select_related('user').filter(pk=request_id).first()
if software_request is None:
logger.error('request %s no longer exists; support was never notified', request_id)
return
# A redelivered message (worker restart mid-task, a retry whose first
# delivery did complete) must not become a duplicate email in support's
# inbox or a fresher stamp over the real send time — the same
# running-twice posture `software.tasks.apply_pending_snapshot` takes.
if software_request.support_notified_at is not None:
return

username = software_request.user.get_username()
try:
send_mail(
subject=(
f'[ilifu software] request: {software_request.name} {software_request.version}'
),
message=(
f'{username} requested {software_request.name} {software_request.version}.\n\n'
f'Source: {software_request.source_url}\n'
f'Licence: {software_request.get_licence_display()}\n'
f'Delivery preference: {software_request.get_delivery_pref_display()}\n\n'
f'{software_request.justification}\n\n'
f'To contact the requester, look up the Keycloak user {username!r} — '
f'the catalogue stores no email addresses.'
),
from_email=None,
recipient_list=[support_email()],
fail_silently=False,
)
except Exception as exc:
# Give up *before* calling retry once the budget is spent: on
# exhaustion `Task.retry(exc=...)` re-raises the original exception
# rather than `MaxRetriesExceededError`, so "catch the exhaustion
# error" is a branch that never runs. Counting attempts ourselves is
# the version with no such trapdoor.
if self.request.retries >= self.max_retries:
logger.error(
'support was never notified of request %s after %s attempts: %s '
'(it shows as un-notified on the admin queue)',
request_id,
self.request.retries + 1,
exc,
)
return
raise self.retry(exc=exc) from exc

# A queryset update, not instance `.save()`: an admin may have triaged
# the request while the email was in flight, and this task must not
# write its stale copy of those fields back over theirs. The isnull
# filter keeps the *first* send's timestamp if two deliveries of one
# message ever race past the check above.
SoftwareRequest.objects.filter(pk=request_id, support_notified_at__isnull=True).update(
support_notified_at=timezone.now()
)
Loading