Allowing attachment points to be pulled from descriptions on sync

CLAUDE.md

@@ -1,26 +1,41 @@
 # CLAUDE.md
 
-Guidance for working in this repo. Read alongside `README.md` (user-facing setup).
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+Guidance for working in this repo. Read alongside `README.md` (user-facing setup
+for the original FastAPI skeleton this app was built from).
 
 ## What this is
 
-A FastAPI skeleton serving **both** a JSON API (`/api/*`) and server-rendered
+**Fitguide**: a parts-compatibility guide for Armament scopes and accessories,
-HTML via **Jinja2 + HTMX** (`/`, `/admin/*`). Async SQLAlchemy 2.0 ORM, JWT auth
+built on a FastAPI skeleton that serves **both** a JSON API (`/api/*`) and
-with refresh-token revocation, and RBAC (users → roles → permissions).
+server-rendered HTML via **Jinja2 + HTMX** (`/`, `/scopes`, `/admin/*`). Async
+SQLAlchemy 2.0 ORM, JWT auth with refresh-token revocation (plus optional LDAP
+direct-bind login), and RBAC (users → roles → permissions). The domain layer on
+top of the skeleton tracks parts, their attachment points, and which parts fit
+together — see "Domain model" below.
 
 ## Commands
 
 ```bash
 uv sync --extra dev                    # install (incl. dev tools)
+uv run alembic upgrade head            # apply migrations
 uv run python -m scripts.seed          # seed perms/roles/superuser (idempotent)
 uv run uvicorn app.main:app --reload   # dev server on :8000
 uv run pytest                          # tests (in-memory SQLite)
+uv run pytest tests/test_foo.py::test_bar   # single test
 uv run ruff check . [--fix]            # lint
-uv run alembic upgrade head            # apply migrations
+uv run black .                         # format (ruff does not format here)
 uv run alembic revision --autogenerate -m "msg"   # new migration after model changes
 docker compose up --build              # app + Postgres, auto-seeded
 ```
 
+`Makefile` wraps the common ones (`make sync|seed|dev|test|fmt|migrate`).
+`make clean` deletes the local `app.db`, re-migrates, and reseeds — the quick
+way to get a clean dev database. `make seed` runs `alembic upgrade head` before
+seeding, i.e. day-to-day dev relies on migrations, not just
+`AUTO_CREATE_TABLES` (see "Database").
+
 ## Architecture / layering
 
 Request → router (`app/api/` JSON or `app/web/` HTML) → `app/crud/` (data access)
@@ -28,40 +43,106 @@ Request → router (`app/api/` JSON or `app/web/` HTML) → `app/crud/` (data ac
 token logic in `app/services/auth.py`. Keep route handlers thin; put DB logic in
 `crud/`, not in routers.
 
+`app/api/` is one router module per resource, all mounted under `/api` by
+`app/api/router.py`. `app/web/` is organized differently: `pages.py` (dashboard/
+login), `scopes.py` (the public-facing `/scopes` fit guide), and **`admin.py`**
+— a single ~3000-line router covering every admin screen (users, roles, part
+taxonomy tables, imported parts, augmented parts + detail + nested attachment
+points/fits, audit log, etc.), organized into `# --- Section ---` banner
+comments per resource rather than split into files. When adding an admin screen,
+follow the existing banner-section convention there rather than starting a new
+web router module.
+
+## Domain model
+
+The business domain, independent of the auth/RBAC skeleton:
+
+- **`AugmentedPart`** (`augmented_parts`) is the generic "part" entity — a
+  scope, flip cover, ARD, polarizer, adapter ring, etc. — distinguished by
+  `PartType`/`SubPartType`, with `PartColour`, `Manufacturer`, and an optional
+  `ScopeModel`. "Scope" is just one `PartType` value (see `SCOPE_TYPE_NAME` in
+  `app/services/scope_guide.py`), not a separate table.
+- **`ScopeInfo`** is a 1:1 extension of `AugmentedPart` (magnification,
+  objective thread, model suffix) populated only for scope-type parts.
+- **`AttachmentPoint`** belongs to one `AugmentedPart`, is named via
+  `AttachmentPointName` (e.g. "Ocular", "Objective"), and connects via
+  *exactly one* of `IsoThread` / `UncThread` / `ClampConnection` /
+  `StaticConnection` — that "exactly one" invariant is enforced in the app
+  layer (CRUD/API validation), not by a DB constraint.
+- **`Fit`** (`app/models/fit.py`) pairs two `AttachmentPoint`s (canonical
+  ordering enforced in `app/crud/fit.py` so a pair only ever has one row),
+  labeled `possible` / `exact` / `none`, with threaded `FitNote`s.
+  `app/services/fit_matching.py` computes `possible` fits automatically
+  (matching ISO/UNC thread specs exactly, or a static connection's dimension
+  falling inside a clamp connection's range); `exact`/`none` are user-asserted
+  and never touched by the algorithm.
+- **`ImportedPart`** (`imported_parts`, PK is the string `InventoryID` — not an
+  int id) holds raw rows from an external inventory feed, fetched via
+  `app/services/odata.py` (OData v4, paginated). `app/services/
+  augmented_part_sync.py` reconciles these into `AugmentedPart` +
+  `AttachmentPoint` rows: infers `PartType` by substring match on the
+  description, parses `TBXThreadAttribute` against ISO/UNC regexes (falling
+  back to an OD-derived `StaticConnection`), and prunes `AugmentedPart`s whose
+  backing `ImportedPart`s have disappeared.
+- **`app/services/scope_import.py`** is a separate, manually-triggered bulk
+  importer: reads an uploaded `.xlsx` of third-party scopes, upserting by
+  `product_number` (→ `AugmentedPart.sku`) and creating `ScopeModel`,
+  `ScopeInfo`, up to three `AttachmentPoint`s, and up to two `Fit`s per row
+  (see the module docstring for the full column mapping).
+- **`app/services/scope_guide.py`** assembles the `/scopes` page: one row per
+  Scope-type `AugmentedPart`, one column per other `PartType`, each cell
+  listing parts of that type with a recorded `Fit`.
+
 ## Conventions that matter
 
 - **Everything async.** Engine, sessions, CRUD, routes, tests. Use `await`; never
   add a sync DB call. `get_db` (in `app/core/database.py`) yields the request session.
-- **Relationships use `lazy="selectin"`** (`User.roles`, `Role.permissions`) so
+- **Relationships use `lazy="selectin"`** (`User.roles`, `Role.permissions`,
-  attributes are safe to read after the session closes — required under async.
+  `AttachmentPoint.iso_thread`, etc.) so attributes are safe to read after the
-  Preserve this when adding relationships that templates/schemas read.
+  session closes — required under async. Preserve this when adding relationships
+  that templates/schemas read.
 - **Config** is `app/core/config.py` (`pydantic-settings`). Add new settings there;
   they're auto-read from env vars (uppercased field name) or `.env`. Env wins.
 - **RBAC**: guard API routes with `Depends(require_permission("resource:action"))`;
   guard web routes with `web_require(...)` in `app/web/admin.py`. Superusers bypass
   all checks (`User.is_superuser` / `User.has_permission`). Add new permission
   strings to `scripts/seed.py`'s `PERMISSIONS`/`ROLES`.
+- **HTMX UI-level write gating**: on top of route-level RBAC, pages that mix
+  read-only and read-write viewers (e.g. `augmented_part_detail.html`'s Fits and
+  Attachment Points sections) compute a `can_write_<thing>` bool from the
+  current user's permissions and wrap add/edit/delete controls in
+  `{% if can_write_<thing> %}` in the template, so read-only users see data but
+  not controls they'd get a 403 on. The route-level `web_require(...)` check
+  remains the real security boundary; the template flag is only for UX.
 - **Auth, two transports**: API uses `Authorization: Bearer`; the HTMX UI uses an
   httpOnly cookie (`settings.cookie_name`). `app/deps.py` reads both. Web auth
-  failures raise `NotAuthenticated`, handled in `main.py` as a redirect to `/login`.
+  failures raise `NotAuthenticated`, handled in `main.py` as a redirect to `/login`
+  (preserving the original path as `?next=`).
 - **Refresh tokens** are stored server-side (hash + `jti` + `revoked`) in the
   `refresh_tokens` table. `services/auth.rotate_refresh_token` revokes-then-reissues.
   Note: JWT payload `exp` is a tz-aware `datetime` (PyJWT only epoch-encodes inside
   the token) — pass it straight to the model, don't `fromtimestamp` it.
+- **LDAP login** (`app/services/ldap.py`) is optional (`settings.ldap_enabled`),
+  direct-bind, and just-in-time provisions a local `User` on first successful
+  login. `ldap3` is sync, so its calls run via `run_in_threadpool`.
 
 ## Database
 
 One `DATABASE_URL` drives SQLite (`sqlite+aiosqlite://`, the default) or Postgres
 (`postgresql+asyncpg://`). `AUTO_CREATE_TABLES=true` calls `Base.metadata.create_all`
-on startup for dev convenience; production should use Alembic instead. `init_db()`
+on startup for dev convenience, but the day-to-day `make seed`/`make clean` flow
-imports `app.models` inside the function to avoid a circular import.
+runs `alembic upgrade head` first — treat Alembic migrations as the source of
+truth and keep them in sync with model changes. `init_db()` imports `app.models`
+inside the function to avoid a circular import.
 
 ## Tests
 
 `tests/conftest.py` builds an in-memory SQLite engine with `StaticPool`, creates
 tables, and overrides `get_db`. Fixtures: `client` (httpx ASGI), `seeded`
-(perms+roles), `make_user`. `asyncio_mode = "auto"` — write `async def test_*`
+(perms+roles from `scripts.seed.PERMISSIONS`/`ROLES`), `make_user`, and
-without decorators.
+`db_sessionmaker` (used directly in tests to seed domain rows — models, fits,
+attachment points — beyond what `seeded`/`make_user` cover). `asyncio_mode =
+"auto"` — write `async def test_*` without decorators.
 
 ## When extending
 

app/main.py

@@ -1,5 +1,6 @@
 from contextlib import asynccontextmanager
 from pathlib import Path
+from urllib.parse import quote
 
 from fastapi import FastAPI, Request
 from fastapi.middleware.cors import CORSMiddleware
@@ -45,7 +46,15 @@ def create_app() -> FastAPI:
 
     @app.exception_handler(NotAuthenticated)
     async def _redirect_to_login(request: Request, exc: NotAuthenticated):
-        return RedirectResponse(url="/login", status_code=302)
+        # Preserve the originally requested URL so we can return there after
+        # login. Only meaningful for GET navigations (the ones users deep-link).
+        login_url = "/login"
+        if request.method == "GET" and request.url.path not in ("/", "/login"):
+            target = request.url.path
+            if request.url.query:
+                target += "?" + request.url.query
+            login_url = f"/login?next={quote(target, safe='')}"
+        return RedirectResponse(url=login_url, status_code=302)
 
     @app.get("/healthz", tags=["health"])
     async def healthz():

app/services/augmented_part_sync.py

@@ -55,7 +55,7 @@ MM_PER_INCH = 25.4
 #   M50x.75    - No leading digit on floats
 #  Float pattern: \d*\.?\d+
 _ISO_THREAD_RE = re.compile(
-    r"^M\s*(?P<diameter>\d*\.?\d+)\s*[xX-]\s*P?(?P<pitch>\d*\.?\d+)(?P<extra>.*)$"
+    r"M\s*(?P<diameter>\d*\.?\d+)\s*[xX-]\s*P?(?P<pitch>\d*\.?\d+)(?P<extra>.*)"
 )
 
 # e.g.
@@ -65,8 +65,8 @@ _ISO_THREAD_RE = re.compile(
 #
 # Leading digit / period, one or more digits, a dash, then
 _UNC_THREAD_RE = re.compile(
-    r"^(?P<diameter>\d*\.?\d+)\s*-\s*(?P<tpi>\d*\.?\d+)"
+    r"(?P<diameter>\d*\.?\d+)\s*-\s*(?P<tpi>\d*\.?\d+)"
-    r"\s*(?P<series>UNEF|UNC|UNF|UNS|UN)?$",
+    r"\s*(?P<series>UNEF|UNC|UNF|UNS|UN)?",
     re.IGNORECASE,
 )
 
@@ -111,7 +111,9 @@ async def _get_or_create_attachment_point_name(db: AsyncSession, name: str):
     return apn
 
 
-async def _add_attachment_point(db: AsyncSession, ap: AugmentedPart, ip: ImportedPart) -> None:
+async def _add_attachment_point(
+    db: AsyncSession, ap: AugmentedPart, ip: ImportedPart
+) -> None:
     """Infer and create an attachment point for a newly-synced AugmentedPart.
 
     Preference order: TBXThreadAttribute (ISO/UNC thread) then ProductODmm/in
@@ -119,7 +121,11 @@ async def _add_attachment_point(db: AsyncSession, ap: AugmentedPart, ip: Importe
     attribute is present but unparseable.
     """
     thread_value = (ip.TBXThreadAttribute or "").strip()
-    parsed = _parse_thread_attribute(thread_value) if thread_value else None
+    parsed = None
+    if thread_value:
+        parsed = _parse_thread_attribute(thread_value)
+    else:
+        parsed = _parse_thread_attribute(ip.Description or "")
 
     if parsed is not None:
         kind, fields = parsed
@@ -127,7 +133,9 @@ async def _add_attachment_point(db: AsyncSession, ap: AugmentedPart, ip: Importe
 
         if kind == "iso":
             depth_mm = _parse_float(ip.ProductMaxThreadDepthmm)
-            thread = await iso_thread_crud.get_by_values(db, depth_mm=depth_mm, **fields)
+            thread = await iso_thread_crud.get_by_values(
+                db, depth_mm=depth_mm, **fields
+            )
             if thread is None:
                 thread = await iso_thread_crud.create(db, depth_mm=depth_mm, **fields)
             await attachment_point_crud.create(
@@ -138,7 +146,9 @@ async def _add_attachment_point(db: AsyncSession, ap: AugmentedPart, ip: Importe
             )
         elif kind == "unc":
             depth_in = _parse_float(ip.ProductMaxThreadDepthin)
-            thread = await unc_thread_crud.get_by_values(db, depth_in=depth_in, **fields)
+            thread = await unc_thread_crud.get_by_values(
+                db, depth_in=depth_in, **fields
+            )
             if thread is None:
                 thread = await unc_thread_crud.create(db, depth_in=depth_in, **fields)
             await attachment_point_crud.create(
@@ -236,7 +246,9 @@ async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
     # ------------------------------------------------------------------ phase 1
 
     # IDs referenced in existing associations
-    result = await db.execute(select(augmented_part_imported_parts.c.imported_part_id).distinct())
+    result = await db.execute(
+        select(augmented_part_imported_parts.c.imported_part_id).distinct()
+    )
     all_linked_ids: set[str] = {row[0] for row in result.all()}
 
     # IDs that still exist in imported_parts
@@ -250,7 +262,9 @@ async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
         # Which AugmentedParts are affected?
         result = await db.execute(
             select(augmented_part_imported_parts.c.augmented_part_id).where(
-                augmented_part_imported_parts.c.imported_part_id.in_(orphaned_import_ids)
+                augmented_part_imported_parts.c.imported_part_id.in_(
+                    orphaned_import_ids
+                )
             )
         )
         orphaned_ap_ids: set[int] = {row[0] for row in result.all()}
@@ -258,7 +272,9 @@ async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
         # Remove the stale association rows
         await db.execute(
             augmented_part_imported_parts.delete().where(
-                augmented_part_imported_parts.c.imported_part_id.in_(orphaned_import_ids)
+                augmented_part_imported_parts.c.imported_part_id.in_(
+                    orphaned_import_ids
+                )
             )
         )
         await db.flush()
@@ -281,7 +297,9 @@ async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
     # ------------------------------------------------------------------ phase 2
 
     # Re-query linked IDs after phase 1 cleanup
-    result = await db.execute(select(augmented_part_imported_parts.c.imported_part_id).distinct())
+    result = await db.execute(
+        select(augmented_part_imported_parts.c.imported_part_id).distinct()
+    )
     now_linked_ids: set[str] = {row[0] for row in result.all()}
     unlinked_ids = current_ids - now_linked_ids
 
@@ -289,7 +307,9 @@ async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
     skipped = 0
 
     if not unlinked_ids:
-        return AugmentedPartSyncResult(created=created, deleted=deleted, skipped=skipped)
+        return AugmentedPartSyncResult(
+            created=created, deleted=deleted, skipped=skipped
+        )
 
     result = await db.execute(
         select(ImportedPart).where(ImportedPart.InventoryID.in_(unlinked_ids))

app/templates/admin/_augmented_part_ap_row.html

@@ -22,6 +22,7 @@
   <span class="ap-item-name">{{ apoint.attachment_point_name.name }}</span>
   <span class="ap-item-type">{{ conn_type }}</span>
   <code class="ap-item-conn">{{ conn_label }}</code>
+  {% if can_write_attachment_points %}
   <div class="row-actions">
     <button class="btn btn-ghost"
             hx-get="/admin/augmented-parts/{{ apoint.augmented_part_id }}/attachment-points/{{ apoint.id }}/edit"
@@ -33,4 +34,5 @@
             hx-swap="outerHTML"
             hx-confirm="Delete this attachment point?">Delete</button>
   </div>
+  {% endif %}
 </div>

app/templates/admin/_augmented_part_attachment_points.html

@@ -7,6 +7,7 @@
     <p class="muted" style="font-size:0.85rem;margin:0 0 0.5rem">No attachment points yet.</p>
   {% endfor %}
 
+  {% if can_write_attachment_points %}
   <form hx-post="/admin/augmented-parts/{{ augmented_part_id }}/attachment-points"
         hx-target="#ap-{{ augmented_part_id }}-attachment-points"
         hx-swap="outerHTML"
@@ -35,4 +36,5 @@
     <div id="ap-{{ augmented_part_id }}-add-connection-options"></div>
     <button type="submit" class="btn btn-primary">Add</button>
   </form>
+  {% endif %}
 </div>

app/templates/admin/augmented_part_detail.html

@@ -3,20 +3,6 @@
 {% block body %}
 {% include "_nav.html" %}
 
-{%- macro conn_desc(point) -%}
-  {%- if point.iso_thread -%}
-    ISO M{{ point.iso_thread.diameter_mm }}×{{ point.iso_thread.pitch_mm }}
-  {%- elif point.unc_thread -%}
-    UNC {{ point.unc_thread.diameter_fraction }}-{{ point.unc_thread.tpi }} {{ point.unc_thread.series.value }}
-  {%- elif point.clamp_connection -%}
-    Clamp {{ point.clamp_connection.min_mm if point.clamp_connection.min_mm is not none else point.clamp_connection.min_in }}–{{ point.clamp_connection.max_mm if point.clamp_connection.max_mm is not none else point.clamp_connection.max_in }}
-  {%- elif point.static_connection -%}
-    Static {{ point.static_connection.od_mm }} mm OD
-  {%- else -%}
-    —
-  {%- endif -%}
-{%- endmacro -%}
-
 <main class="container">
   <p><a href="/admin/augmented-parts" class="btn btn-ghost">← Augmented Parts</a></p>
 
@@ -62,19 +48,13 @@
       </dl>
     </div>
     {% endif %}
+  </div>
 
+  {% if can_read_attachment_points %}
   <div class="card">
-      <h2>Attachment points</h2>
+    {% include "admin/_augmented_part_attachment_points.html" %}
-      {% for point in attachment_points %}
-        <div class="ap-item">
-          <span class="ap-item-name">{{ point.attachment_point_name.name }}</span>
-          <code class="ap-item-conn">{{ conn_desc(point) }}</code>
-        </div>
-      {% else %}
-        <p class="muted">No attachment points recorded.</p>
-      {% endfor %}
-    </div>
   </div>
+  {% endif %}
 
   {% if can_read_fits %}
   <div class="card">

app/templates/login.html

@@ -9,6 +9,7 @@
     {% if ldap_enabled %}
 
       <form method="post" action="/login/ldap">
+        <input type="hidden" name="next" value="{{ next_url or '' }}">
         {% if ldap_error %}<div class="alert">{{ ldap_error }}</div>{% endif %}
         <label>Username
           <input type="text" name="username" value="{{ username or '' }}" required autofocus>
@@ -22,6 +23,7 @@
       <details class="local-login-panel" {% if local_error %}open{% endif %}>
         <summary>Use a local account instead</summary>
         <form method="post" action="/login">
+          <input type="hidden" name="next" value="{{ next_url or '' }}">
           {% if local_error %}<div class="alert">{{ local_error }}</div>{% endif %}
           <label>Email
             <input type="email" name="email" value="{{ email or '' }}" required>
@@ -36,6 +38,7 @@
     {% else %}
 
       <form method="post" action="/login">
+        <input type="hidden" name="next" value="{{ next_url or '' }}">
         {% if local_error %}<div class="alert">{{ local_error }}</div>{% endif %}
         <label>Email
           <input type="email" name="email" value="{{ email or '' }}" required autofocus>

app/web/admin.py

@@ -1792,17 +1792,26 @@ async def augmented_part_detail(
     if ap is None:
         raise HTTPException(status_code=404, detail="Augmented part not found")
     can_read_fits = user.is_superuser or "fits:read" in user.permissions
+    can_read_attachment_points = (
+        user.is_superuser or "attachment_points:read" in user.permissions
+    )
     ctx = {
         "user": user,
         "ap": ap,
         "scope_info": await scope_info_crud.get_by_augmented_part(
             db, augmented_part_id
         ),
-        "attachment_points": await attachment_point_crud.list_for_augmented_part(
-            db, augmented_part_id
-        ),
         "can_read_fits": can_read_fits,
+        "can_read_attachment_points": can_read_attachment_points,
     }
+    if can_read_attachment_points:
+        ctx.update(
+            await _augmented_part_ap_ctx(
+                db,
+                augmented_part_id,
+                can_write=_can_write_attachment_points(user),
+            )
+        )
     if can_read_fits:
         ctx.update(
             await _augmented_part_fits_ctx(
@@ -1909,7 +1918,13 @@ async def delete_augmented_part(
     return Response(status_code=status.HTTP_200_OK)
 
 
-async def _augmented_part_ap_ctx(db: AsyncSession, augmented_part_id: int) -> dict:
+def _can_write_attachment_points(user: User) -> bool:
+    return user.is_superuser or "attachment_points:write" in user.permissions
+
+
+async def _augmented_part_ap_ctx(
+    db: AsyncSession, augmented_part_id: int, *, can_write: bool = True
+) -> dict:
     return {
         "augmented_part_id": augmented_part_id,
         "attachment_points_list": await attachment_point_crud.list_for_augmented_part(
@@ -1920,6 +1935,7 @@ async def _augmented_part_ap_ctx(db: AsyncSession, augmented_part_id: int) -> di
         "unc_threads": await unc_thread_crud.list_all(db),
         "clamp_connections": await clamp_connection_crud.list_all(db),
         "static_connections": await static_connection_crud.list_all(db),
+        "can_write_attachment_points": can_write,
     }
 
 
@@ -1931,9 +1947,11 @@ async def augmented_part_attachment_points(
     request: Request,
     augmented_part_id: int,
     db: AsyncSession = Depends(get_db),
-    _: User = Depends(web_require("attachment_points:read")),
+    user: User = Depends(web_require("attachment_points:read")),
 ):
-    ctx = await _augmented_part_ap_ctx(db, augmented_part_id)
+    ctx = await _augmented_part_ap_ctx(
+        db, augmented_part_id, can_write=_can_write_attachment_points(user)
+    )
     return templates.TemplateResponse(
         request, "admin/_augmented_part_attachment_points.html", ctx
     )
@@ -1976,7 +1994,9 @@ async def create_augmented_part_attachment_point(
             "AttachmentPoint", apoint.id, f"part {augmented_part_id}"
         ),
     )
-    ctx = await _augmented_part_ap_ctx(db, augmented_part_id)
+    ctx = await _augmented_part_ap_ctx(
+        db, augmented_part_id, can_write=_can_write_attachment_points(actor)
+    )
     return templates.TemplateResponse(
         request, "admin/_augmented_part_attachment_points.html", ctx
     )
@@ -2022,13 +2042,18 @@ async def augmented_part_attachment_point_row(
     augmented_part_id: int,
     attachment_point_id: int,
     db: AsyncSession = Depends(get_db),
-    _: User = Depends(web_require("attachment_points:read")),
+    user: User = Depends(web_require("attachment_points:read")),
 ):
     apoint = await attachment_point_crud.get(db, attachment_point_id)
     if apoint is None:
         raise HTTPException(status_code=404, detail="Attachment point not found")
     return templates.TemplateResponse(
-        request, "admin/_augmented_part_ap_row.html", {"apoint": apoint}
+        request,
+        "admin/_augmented_part_ap_row.html",
+        {
+            "apoint": apoint,
+            "can_write_attachment_points": _can_write_attachment_points(user),
+        },
     )
 
 
@@ -2075,7 +2100,12 @@ async def update_augmented_part_attachment_point(
         ),
     )
     return templates.TemplateResponse(
-        request, "admin/_augmented_part_ap_row.html", {"apoint": apoint}
+        request,
+        "admin/_augmented_part_ap_row.html",
+        {
+            "apoint": apoint,
+            "can_write_attachment_points": _can_write_attachment_points(actor),
+        },
     )
 
 
@@ -2103,7 +2133,9 @@ async def delete_augmented_part_attachment_point(
                 "AttachmentPoint", attachment_point_id, f"part {augmented_part_id}"
             ),
         )
-    ctx = await _augmented_part_ap_ctx(db, augmented_part_id)
+    ctx = await _augmented_part_ap_ctx(
+        db, augmented_part_id, can_write=_can_write_attachment_points(actor)
+    )
     return templates.TemplateResponse(
         request, "admin/_augmented_part_attachment_points.html", ctx
     )

app/web/pages.py

@@ -1,4 +1,4 @@
-from fastapi import APIRouter, Depends, Form, HTTPException, Request, status
+from fastapi import APIRouter, Depends, Form, HTTPException, Query, Request, status
 from fastapi.responses import HTMLResponse, RedirectResponse, Response
 from sqlalchemy.ext.asyncio import AsyncSession
 
@@ -14,6 +14,21 @@ from app.web.templates import templates
 router = APIRouter(include_in_schema=False)
 
 
+def _safe_next(next_url: str) -> str:
+    """Return next_url only if it's a safe same-site path, else "/".
+
+    Guards against open-redirect: rejects absolute URLs and protocol-relative
+    ("//host", "/\\host") targets, and never bounces back to the auth pages.
+    """
+    if (
+        next_url.startswith("/")
+        and not next_url.startswith(("//", "/\\"))
+        and not next_url.startswith(("/login", "/logout"))
+    ):
+        return next_url
+    return "/"
+
+
 def _set_auth_cookie(response: Response, access_token: str) -> None:
     response.set_cookie(
         key=settings.cookie_name,
@@ -35,9 +50,11 @@ async def dashboard(
 
 
 @router.get("/login", response_class=HTMLResponse)
-async def login_page(request: Request):
+async def login_page(request: Request, next_url: str = Query("", alias="next")):
     return templates.TemplateResponse(
-        request, "login.html", {"ldap_enabled": settings.ldap_enabled}
+        request,
+        "login.html",
+        {"ldap_enabled": settings.ldap_enabled, "next_url": _safe_next(next_url)},
     )
 
 
@@ -46,6 +63,7 @@ async def login_submit(
     request: Request,
     email: str = Form(...),
     password: str = Form(...),
+    next_url: str = Form("", alias="next"),
     db: AsyncSession = Depends(get_db),
 ):
     user = await user_crud.authenticate(db, email, password)
@@ -57,11 +75,14 @@ async def login_submit(
                 "local_error": "Incorrect email or password",
                 "email": email,
                 "ldap_enabled": settings.ldap_enabled,
+                "next_url": _safe_next(next_url),
             },
             status_code=status.HTTP_401_UNAUTHORIZED,
         )
     access, _ = await issue_token_pair(db, user)
-    response = RedirectResponse(url="/", status_code=status.HTTP_303_SEE_OTHER)
+    response = RedirectResponse(
+        url=_safe_next(next_url), status_code=status.HTTP_303_SEE_OTHER
+    )
     _set_auth_cookie(response, access)
     return response
 
@@ -71,6 +92,7 @@ async def login_ldap_submit(
     request: Request,
     username: str = Form(...),
     password: str = Form(...),
+    next_url: str = Form("", alias="next"),
     db: AsyncSession = Depends(get_db),
 ):
     if not settings.ldap_enabled:
@@ -84,11 +106,14 @@ async def login_ldap_submit(
                 "ldap_error": "Incorrect username or password",
                 "username": username,
                 "ldap_enabled": True,
+                "next_url": _safe_next(next_url),
             },
             status_code=status.HTTP_401_UNAUTHORIZED,
         )
     access, _ = await issue_token_pair(db, user)
-    response = RedirectResponse(url="/", status_code=status.HTTP_303_SEE_OTHER)
+    response = RedirectResponse(
+        url=_safe_next(next_url), status_code=status.HTTP_303_SEE_OTHER
+    )
     _set_auth_cookie(response, access)
     return response
 

tests/test_augmented_part_detail.py

@@ -91,6 +91,36 @@ async def test_detail_shows_attributes_and_fits_readonly(
     assert "Add fit" not in body
     assert "Mark Exact" not in body
     assert "Recompute possible fits" not in body
+    # ...and attachment-point add/edit/delete controls are hidden too
+    assert 'name="attachment_point_name_id"' not in body
+    assert "Delete this attachment point?" not in body
+
+
+@pytest.mark.asyncio
+async def test_detail_write_controls_for_attachment_point_editor(
+    client, seeded, make_user, db_sessionmaker
+):
+    async with db_sessionmaker() as db:
+        ids = await _seed_part_with_fit(db)
+        perms = seeded["perms"]
+        editor = await role_crud.create(
+            db,
+            name="ap-editor",
+            permission_ids=[
+                perms["augmented_parts:read"].id,
+                perms["attachment_points:read"].id,
+                perms["attachment_points:write"].id,
+            ],
+        )
+        editor_id = editor.id
+    await make_user("apeditor@example.com", role_ids=[editor_id])
+    await _web_login(client, "apeditor@example.com")
+
+    resp = await client.get(f"/admin/augmented-parts/{ids['scope_id']}")
+    assert resp.status_code == 200
+    body = resp.text
+    assert 'name="attachment_point_name_id"' in body
+    assert "Delete this attachment point?" in body
 
 
 @pytest.mark.asyncio

tests/test_login_redirect.py

@@ -0,0 +1,73 @@
+import pytest
+
+
+@pytest.mark.asyncio
+async def test_deep_link_redirects_to_login_with_next(client):
+    resp = await client.get("/scopes")
+    assert resp.status_code == 302
+    assert resp.headers["location"] == "/login?next=%2Fscopes"
+
+
+@pytest.mark.asyncio
+async def test_deep_link_preserves_query_string(client):
+    resp = await client.get("/scopes?description=alpha")
+    assert resp.status_code == 302
+    assert resp.headers["location"] == "/login?next=%2Fscopes%3Fdescription%3Dalpha"
+
+
+@pytest.mark.asyncio
+async def test_root_redirect_has_no_next(client):
+    resp = await client.get("/")
+    assert resp.status_code == 302
+    assert resp.headers["location"] == "/login"
+
+
+@pytest.mark.asyncio
+async def test_login_page_renders_next_field(client):
+    resp = await client.get("/login?next=/scopes")
+    assert resp.status_code == 200
+    assert 'name="next" value="/scopes"' in resp.text
+
+
+@pytest.mark.asyncio
+async def test_login_redirects_to_next_on_success(client, make_user):
+    await make_user("u@example.com")
+    resp = await client.post(
+        "/login",
+        data={"email": "u@example.com", "password": "secret123", "next": "/scopes"},
+    )
+    assert resp.status_code == 303
+    assert resp.headers["location"] == "/scopes"
+
+
+@pytest.mark.asyncio
+async def test_login_defaults_to_root_without_next(client, make_user):
+    await make_user("u2@example.com")
+    resp = await client.post(
+        "/login", data={"email": "u2@example.com", "password": "secret123"}
+    )
+    assert resp.status_code == 303
+    assert resp.headers["location"] == "/"
+
+
+@pytest.mark.asyncio
+async def test_login_rejects_unsafe_next(client, make_user):
+    await make_user("u3@example.com")
+    for bad in ["https://evil.com", "//evil.com", "/\\evil.com", "/login"]:
+        resp = await client.post(
+            "/login",
+            data={"email": "u3@example.com", "password": "secret123", "next": bad},
+        )
+        assert resp.status_code == 303
+        assert resp.headers["location"] == "/", bad
+
+
+@pytest.mark.asyncio
+async def test_failed_login_preserves_next(client, make_user):
+    await make_user("u4@example.com")
+    resp = await client.post(
+        "/login",
+        data={"email": "u4@example.com", "password": "wrong", "next": "/scopes"},
+    )
+    assert resp.status_code == 401
+    assert 'name="next" value="/scopes"' in resp.text

tests/test_scope_guide.py

@@ -238,4 +238,5 @@ async def test_scopes_page_forbidden_without_permission(client, seeded, make_use
 async def test_scopes_page_redirects_when_anonymous(client):
     resp = await client.get("/scopes")
     assert resp.status_code == 302
-    assert resp.headers["location"] == "/login"
+    # Deep link is preserved so the user lands back here after logging in.
+    assert resp.headers["location"] == "/login?next=%2Fscopes"