T10.4 D13 - provision on first sign-in without trampling existing accounts

Criteria 2 and 4 pull against each other: create accounts that do not exist,
never touch the role of accounts that do. Two helpers in app.py keep the two
cases apart so the role-preserving branch cannot be edited by accident.

Matching uses sAMAccountName OR the directory's mail, per the Aug 21 decision.
A bind can only carry one identifier, so the bind is sAMAccountName@prime.local,
but matching an existing local row tries both - existing accounts were typed by
hand with manage_users.py and some are short logon names while others are email
addresses. auth.find_user already compares case-insensitively against username
AND email, so two calls cover four columns.

Verified against a throwaway SQLite database:

  existing admin           -> role still 'admin'
  locally-set full_name    -> preserved, not overwritten by the directory
  empty email              -> filled from the directory
  local username is email  -> matched by mail, project_admin kept
  no local row             -> created at project_user, is_active, audit row
  ProjectMember rows       -> 0
  second sign-in           -> same row, no duplicate, 3 users total

A JIT account deliberately gets NO project access. The wave file said to honour
the auto_add_projects machinery so a new account "lands in the right projects";
that was wrong about the flag, which is evaluated when a PROJECT is created to
mark who joins every new job and cannot retroactively add an account to jobs
that already exist. There is no correct default, so least privilege applies and
the wave file's done-when has been corrected rather than quietly satisfied.

The consequence is a UX cliff worth knowing about: a successful sign-in into an
empty app until an admin grants access. That is why provisioning writes an
AuditLog row and a log line instead of happening silently.

is_active is checked after provisioning (a new account defaults active) and
after the role branch (a disabled admin is still refused). Local is_active
overrides the directory on purpose: disabling here revokes access to this app
without touching the domain account.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs/waves/wave-10.md

@@ -157,10 +157,19 @@ afterwards.
 `auth.find_user` (already case-insensitive across username **and** email). If it exists,
 update only `last_login_at` and — if empty locally — `full_name` and `email` from the
 directory. **Never write `role`.** If it does not exist, create it at
-`ROLE_PROJECT_USER` with `full_name`/`email` from the directory, honouring the existing
-`auto_add_projects` machinery so a JIT account lands in the right projects. Note the
-flush-order warning in the `models.py` docstring: `create_user` in `app.py` already handles
-this correctly for account + `ProjectMember` rows in one flush — follow it.
+`ROLE_PROJECT_USER` with `full_name`/`email` from the directory.
+
+**A JIT account gets NO project access, and that is deliberate.** An earlier draft of this
+task said to honour the `auto_add_projects` machinery so a new account "lands in the right
+projects" — that was wrong about how the flag works. `auto_add_projects` is evaluated when a
+**project** is created (`app.py:245`), marking accounts that should join every *new* job; it
+cannot retroactively add a new account to existing ones. There is no correct default, so
+least privilege applies: the account exists, can sign in, and sees nothing until someone
+grants access. That is a real UX cliff — a successful sign-in into an empty app — so it has
+to be visible to admins rather than silent, which is what the `AuditLog` row is for.
+
+Note the flush-order warning in the `models.py` docstring: `create_user` in `app.py` handles
+account-then-membership correctly in one flush — follow it if you add rows.
 
 Write an `AuditLog` row for each JIT creation. An account appearing without an administrator
 creating it is exactly the kind of event that record exists for.
@@ -171,7 +180,8 @@ creating it is exactly the kind of event that record exists for.
 - [ ] `full_name` and `email` are populated from the directory on creation
 - [ ] an existing `admin` signing in is still `admin` afterwards — asserted, not assumed
 - [ ] an existing account with a locally-set `full_name` does not have it overwritten
-- [ ] a JIT account with `auto_add_projects` peers gets its `ProjectMember` rows
+- [ ] a JIT account has NO `ProjectMember` rows and sees no projects
+- [ ] the new account appears in the Admin console user list so access can be granted
 - [ ] each JIT creation writes an `AuditLog` row
 - [ ] a failed bind creates **no** row
 - [ ] a bind that succeeds but fails the group check creates **no** row

server/app.py

@@ -727,6 +727,55 @@ def _clear_bind_failures(sam: str) -> None:
     _bind_attempts.pop((sam or "").strip().lower(), None)
 
 
+def _match_directory_account(db: Session, result) -> Optional[models.User]:
+    """Find the local row for a directory identity — D13 criterion 4.
+
+    Matched on `sAMAccountName` OR the directory's `mail`, because existing accounts
+    were created by hand with `manage_users.py` and some were typed as short logon
+    names while others were typed as email addresses. Matching on both is what keeps
+    an existing admin's role instead of handing them a second, default-role account.
+
+    `auth.find_user` already compares case-insensitively against username AND email,
+    so each call covers two columns; the second call is for the case where the local
+    username is the person's address and the directory only told us their sAMAccountName.
+    """
+    user = auth.find_user(db, result.sam)
+    if user is None and result.mail:
+        user = auth.find_user(db, result.mail)
+        if user is not None:
+            log.info("matched directory identity %r to existing local account %r by mail",
+                     result.sam, user.username)
+    return user
+
+
+def _provision_from_directory(db: Session, result) -> models.User:
+    """Create a local account for someone who just authenticated and has no row.
+
+    Lands at `project_user` with NO project memberships. That is least privilege and
+    it is deliberate, but it means the person signs in successfully into an empty
+    app until an admin grants access — so it is written to the audit log rather than
+    happening silently. `auto_add_projects` cannot help here: it is evaluated when a
+    PROJECT is created, to mark who joins every new job, and cannot retroactively add
+    a new account to jobs that already exist.
+    """
+    u = models.User(
+        id=gen_id("user"),
+        username=result.sam,
+        email=result.mail or "",
+        full_name=result.full_name or "",
+        role=auth.ROLE_PROJECT_USER,
+    )
+    db.add(u)
+    db.flush()          # see the flush-order note in models.py's docstring
+    log_event(db, u.username, "user_provisioned", "user", u.id, summary=u.username,
+              detail={"source": "directory", "role": u.role, "upn": result.upn,
+                      "projects": 0, "note": "created on first successful sign-in"})
+    log.info("provisioned local account %r from the directory at role %r with no "
+             "project access — an admin must grant access before they see anything",
+             u.username, u.role)
+    return u
+
+
 @app.post("/api/auth/login")
 def login(body: LoginIn, request: Request, response: Response, db: Session = Depends(get_db)):
     """Authenticate against the domain (D13 / T10.2) and set the session cookie.
@@ -803,19 +852,37 @@ def login(body: LoginIn, request: Request, response: Response, db: Session = Dep
 
     # ── authenticated ─────────────────────────────────────────────────────────
     _clear_bind_failures(sam)
-    if not user:
-        # T10.4 provisions the account here. Until that lands, an authenticated
-        # person with no local row is refused rather than silently admitted.
-        log.warning("%r authenticated against the domain but has no local account "
-                    "(JIT provisioning arrives in T10.4)", result.sam)
-        raise HTTPException(status_code=403, detail="No account on this system yet.")
+
+    # The pre-bind lookup used the typed name only. Now that the directory has told
+    # us the account's real sAMAccountName and mail, re-match on both (T10.4).
+    if user is None:
+        user = _match_directory_account(db, result)
+
+    provisioned = user is None
+    if provisioned:
+        user = _provision_from_directory(db, result)
+    else:
+        # NEVER touch `role` here. An existing admin stays an admin — that is D13
+        # criterion 4, and it is the whole reason this branch is separate from the
+        # one above. Fill in identity fields only where they are empty locally, so a
+        # name deliberately set in the console is not overwritten by the directory.
+        if not user.full_name and result.full_name:
+            user.full_name = result.full_name
+        if not user.email and result.mail:
+            user.email = result.mail
+
     if not user.is_active:
+        # Checked after provisioning so a brand-new account (is_active defaults True)
+        # is not caught by it, and after the role branch so a disabled admin is still
+        # refused. Local state overrides the directory: disabling here is how you
+        # revoke access to THIS app without touching the domain account.
         raise HTTPException(status_code=403, detail="Account is disabled")
 
     user.failed_attempts = 0
     user.locked_until = None
     user.last_login_at = now
     db.commit()
+    db.refresh(user)
     token = auth.create_token(user)
     auth.set_session_cookie(response, request, token)
     return {"user": user.to_dict()}