Oren Castellan

Hire (track 03) · session 15 of 26 · model claude-sonnet-5 · Sep 13, 2026 6:15 AM PDT – Sep 13, 2026 6:17 AM PDT
Sep 13, 6:15 AM PDTLead
Review PR #40 on github.com/wtvamp/loginid-takehome, branch feature/LT-41, for track 03-engineering-delivery. This implements LT-41 (Q3): the idp-connector binary's POST /auth and POST /identity handlers against a stubbed third-party vendor. Worktree at /Users/warrenthompson/Source/LoginID-worktrees/LT-41 (based on origin/main). Context: refinement/LT-41.md and 02-ai-security-architecture/connector-security.md are the design docs (both worth reading in full — connector-security.md especially, since most of this story's requirements come directly from its §1-§6 and its attack-tree table at the bottom). Key points: - The assignment's fixed contract: POST /auth takes {"username","password"}, returns access_token; POST /identity takes {"phone","name"}, returns PII (name, phone, street_address, locality, region, postal_code, country). - No real ABC/XYZ vendor exists (explicit non-goal) — a stub is fine. - Attack-tree leaf 1: a fresh, isolated Authorization header must be built per outbound vendor request, never inherited from HTTP connection/keep-alive reuse. - Attack-tree leaf 4: the vendor token variable must be scoped to the narrowest function possible, never passed up the call stack; excluded from any generic request/response dump used in panic traces/debug output. - Attack-tree leaf 5: outbound vendor calls rate-limited per calling-API-client identity AND per source IP independently, with a stricter threshold on vendor-auth failure ratio specifically (not just volume) — this PR reuses the already-built api.GrantLimiter (PR #30/#34) rather than building a new component, per this story's own non-goal ("no shared rate-limit store... this story's own in-process rate limiting is sufficient, matching the pattern already used for RateLimiter/TouchCounter/GrantLimiter"). - /identity must fail closed with a generic error that doesn't distinguish "no token"/"bad token"/"vendor rejected it" in response CONTENT, plus (Marcus Ilori's ruling on a Tobias Lindqvist timing-oracle objection) apply a ~100ms uniform floor across those three failure branches — explicitly a partial mitigation, not full closure, since a real vendor round-trip's latency isn't controllable. - The connector's own /auth and /identity must validate an inbound JWT with a DISTINCT audience (idp-connector-service, not api-service's loginid-api-service) and scope (connector:identity-lookup) — a new Scope constant was added to internal/api/auth.go and internal/api/jwtmiddleware.go's singleScope recognized-scope switch. - Never log the vendor password, vendor access_token, or raw vendor error response bodies, in any environment or error path. Files to review (all new unless noted): - internal/connector/vendorclient.go — VendorClient interface (Authenticate, FetchIdentity), Identity struct matching the assignment's PII field list exactly. - internal/connector/stubvendorclient.go — StubVendorClient: in-process fake, Seed(username,password,identity), Authenticate/FetchIdentity backed by simple maps with a mutex. - internal/connector/httpvendorclient.go — HTTPVendorClient: NewHTTPVendorClient requires an https:// baseURL (errors otherwise); a module-level defaultVendorHTTPClient with a bounded Transport (MaxIdleConns/MaxIdleConnsPerHost/MaxConnsPerHost/IdleConnTimeout) and an explicit http.Client.Timeout; Authenticate/FetchIdentity each build a fresh *http.Request via http.NewRequestWithContext with the token set directly on that request's header (never a client-level default), assume the vendor shares this project's own JSON shape (a stated simplifying assumption), and collapse every failure mode (dial error, non-200 status, decode failure) to one of two sentinel errors (ErrVendorAuthFailed / ErrVendorIdentityNotFound) without ever reading/logging the raw response body on failure. - internal/connector/handlers.go — NewAuthHandler, NewIdentityHandler, Deps struct (Vendor, Limiter, AuditLog), a local unexported vendorLimiter interface (satisfied by api.GrantLimiter). NewIdentityHandler: rate-limit gate first (before touching anything else, keyed by AuthContext.Sub + source IP), then body decode, then the vendor token read from the X-Vendor-Access-Token header, then FetchIdentity — every failure path routes through waitForFloor(start) + writeIdentityFailure(w), the single call site for the byte-identical response. NewAuthHandler: rate-limit gate, decode body, Authenticate, generic failure on any error. - internal/app/connector_router.go — NewConnectorRouter: mounts /healthz, POST /auth, POST /identity behind NewJWTMiddleware(keys, cfg.AuthJWTIssuer, cfg.ConnectorJWTAudience, auditLog) + NewDeadlineMiddleware, constructs one shared api.InProcessGrantLimiter instance for both handlers. - cmd/idp-connector/main.go — chooses HTTPVendorClient (if cfg.IDPABCBaseURL set) or a StubVendorClient seeded with one demo identity (otherwise); builds a JWKSCache from cfg.AuthJWKSURL. - internal/api/auth.go, internal/api/jwtmiddleware.go — added ScopeConnectorIdentityLookup and recognized it in singleScope's switch (shared scope vocabulary/JWT verification mechanism reused across both binaries). - Tests: internal/connector/handlers_test.go, httpvendorclient_test.go, stubvendorclient_test.go, internal/app/connector_router_test.go. Please run `go build ./...`, `go vet ./...`, and `go test ./... -race` yourself in that worktree, then focus on: 1. Does the "fresh header per request, never inherited from connection reuse" property actually hold under real connection pooling — read httpvendorclient.go's Authenticate/FetchIdentity carefully; is there any shared/client-level state (a header map, a captured request object) that could leak across concurrent or sequential calls with different tokens, beyond what the existing test already covers? 2. Is the token variable's scope in handlers.go's NewIdentityHandler actually as narrow as claimed — does `token` (or `accessToken` in NewAuthHandler) ever get assigned to something with a wider lifetime, logged, or passed somewhere unexpected? 3. Check every error path in httpvendorclient.go and handlers.go for a raw vendor response body, raw dial/DNS error, or any other potentially-sensitive detail leaking into a log call, an audit event, or the HTTP response written to the connector's own caller. 4. Is reusing api.GrantLimiter for this purpose actually sound, or does anything about GrantLimiter's original design (built for the token *issuance* endpoint, PR #30/#34) not transfer cleanly to gating *vendor calls*? E.g., does keying by AuthContext.Sub (the CALLING API CLIENT's identity, not any end-user identity) match what leaf 5 actually asks for? 5. The rehash of the timing-floor logic — is waitForFloor actually correctly excluded from the success path and the 429/400 paths (which the PR claims are NOT part of the timing-oracle concern), and included on all three of (and only) the three named failure branches? 6. Anything else: error handling, nullability, off-by-one, idiom, naming, doc-comment accuracy, your usual review lens. Report your findings in plain prose to Renata Cole (03 lead), as you'd normally report a review. If everything holds up, say so plainly — don't manufacture findings.
Sep 13, 6:15 AM PDTOren Castellan
Bash pwd
input
{
  "command": "pwd"
}
result
/Users/warrenthompson/Source/LoginID/03-engineering-delivery
Read /Users/warrenthompson/Source/LoginID/CLAUDE.md
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID/CLAUDE.md"
}
result
1	# LoginID Take-Home POC — Project Root
2	
3	## Role
4	
5	You are the project manager agent for this repository. This file is the shared context every track refers back to. Each of the five track directories below has its own CLAUDE.md that can be worked independently by a scoped subagent, but that subagent should read this root file first for the assignment, the overall narrative, and how its track fits with the other four.
6	
7	When you spin up agents or subagents to do work in a track, point them at that track's CLAUDE.md as their brief, and tell them this root file exists for cross-track context. Don't let a track's agent silently duplicate scope another track owns — check the "Owns / does not own" boundaries below first.
8	
9	## What this repository is
10	
11	This is a take-home assignment for a LoginID interview, submitted as a full directory. The goal is not just to answer three coding questions — it's to demonstrate capability as an AI architect: how you scope a problem, use AI tooling deliberately, reason about security and data, and produce something that reads as an intentional, well-run engineering effort rather than a quick script.
12	
13	Code does not need to work or be fully complete. LoginID has said explicitly they care more about the design and the reasoning behind it, and about which AI tools/frameworks were used and how. Every track should document its reasoning, not just its output.
14	
15	## The assignment (verbatim from LoginID)
16	
17	Provide code solutions for the following questions in Go (preferably) or other languages — describe any tool, framework, or AI used.
18	
19	1. Code of a database DAO that can store and retrieve `user_profile` (name, address, phone) and `user_credential` (username, method, password), with support for multiple databases (PostgreSQL or CockroachDB, SQLite, etc).
20	2. Design a RESTful API web service to search and retrieve the above user profile data, plus security API authentication.
21	3. Design a service connector to third-party identity providers (referred to generically as ABC or XYZ) to retrieve personal data from vendors, with the following endpoints:
22	   - `/auth` — retrieves an `access_token` based on a POST JSON body `{"username": "<string>", "password": "<string>"}`
23	   - `/identity` — retrieves user PII (name, phone, address: `street_address`, `locality`, `region`, `postal_code`, `country`) based on a POST JSON body `{"phone": "<string>", "name": "<string>"}`
24	
25	Go is the preferred language. Other languages are acceptable if justified.
26	
27	## Track structure
28	
29	Five directories, each independently workable, each owning a distinct slice of the problem:
30	
31	| Directory | Track | Owns |
32	|---|---|---|
33	| `01-product-industry-research-design/` | Product & Industry Research (incl. Design) | Market/industry framing (IAM, passwordless auth, IDP aggregation space), personas and use cases, API and data contract design rationale, UX of the surfaces that would sit on top of this |
34	| `02-ai-security-architecture/` | AI Architecture & Security Architecture | The AI tooling/workflow used to produce this submission, threat modeling, auth/authz design for the API, credential and PII handling, secrets management, third-party connector security |
35	| `03-engineering-delivery/` | Engineering & Delivery | The actual Go code for all three questions, project structure, test strategy, delivery documentation |
36	| `04-infra-devops/` | Infra & DevOps | How this would be built, containerized, deployed, and operated — CI/CD, environments, multi-database configuration, observability |
37	| `05-data-ops/` | Data Ops | Data modeling for `user_profile` / `user_credential`, multi-database abstraction strategy, migrations, PII data governance and retention |
38	
39	Directories are numbered for reading order in the final submission, not for sequencing of work — tracks can be worked in any order or in parallel.
40	
41	## Working principles for every track
42	
43	- State your reasoning, not just your conclusion. LoginID is grading the "why" as much as the "what."
44	- Name the AI tool/workflow you used for that track's output (this project is being built with Claude Code; say so, and say how — e.g., subagents scoped per track, a given skill, a given model).
45	- Keep each track's CLAUDE.md self-contained enough that a fresh agent with no other context can pick it up and do correct work from it alone, plus this root file.
46	- Cross-reference other tracks by relative path (e.g., `../05-data-ops/CLAUDE.md`) rather than restating their content.
47	- Implementation is gated per story, not project-wide: Warren gave the implementation go on 2026-09-12 (evening) as an agile loop. Code (`.go`, `.sql`, `go.mod`, Dockerfiles, CI config) may be written only for a story the Product Owner has pulled, refined with the research team, and handed to engineering — recorded in `<track>/refinement/<LT-key>.md`. Work happens on a `feature/<LT-key>` branch, never directly on `main`. A story is done only when it is deployed to the public URL and the Product Owner has reviewed it there.
48	- **Attribution rule (Warren, 2026-09-12): every commit, PR, and Jira ticket says who did the work.** *Commits:* the git author is the persona doing the work — set once per worktree with `git config --worktree user.name "<Persona Name> (<track> <role>, <model>)"` and `git config --worktree user.email "<slug>@agents.loginid-takehome.invalid"` (`extensions.worktreeConfig` is enabled repo-wide) — and every commit message ends with trailers: `Story: LT-nn`, `Work-By: <persona> (<seat>); <persona> (<seat>)…` naming each hire who contributed and what they did (implementation, tests, review, objection seat), then `Co-Authored-By: Claude <model> <noreply@anthropic.com>`. *PRs:* title `LT-nn: <summary>`; body has a **Who did the work** section (lead, hires and seats, reviewers incl. 02's independent reviewer where applicable, models), plus links to the refinement doc and the Jira issue. *Jira:* every transition and every PR link is a comment beginning `Work-By: …` with the same names; the story's final comment before Done names who deployed and who reviewed. The PM applies the same rule to its own commits. Persona names, not "Claude" or "the team".
49	- **Shared working tree rule.** Every lead session shares this one checkout, so `git checkout`/`git switch` here changes everyone's tree — **never run them in this directory.** Feature branches live in worktrees outside the repo: `git worktree add /Users/warrenthompson/Source/LoginID-worktrees/<LT-key> -b feature/<LT-key> main`; work, commit, and push from that directory; the root tree stays on `main`. Remote: `origin` = `«email»:wtvamp/loginid-takehome.git` (public). Go code lives at the repo root (`cmd/`, `internal/`, `go.mod`); track directories hold documents.
50	
51	## Org, hiring, and orchestration
52	
53	The project is run as an org: this PM agent (Dana Whitfield) orchestrates five standing track-lead agents, and each lead hires a small sub-team of specialist agents with deliberately varied personalities. **Each track lead runs as its own top-level Claude Code session**, launched from its track directory in its own tmux window — so it loads its own track `CLAUDE.md` and persona natively, owns and spawns its hires as named teammates in that window, and talks to the PM session through cross-session messaging. Hires belong to their lead's session, not the PM's. The governing files, all at the root:
54	
55	- `PLAN.md` — the approved org plan for the planning phase (phases, gates, verification). `~/.claude/plans/` is scratch; this is the record.
56	- `PLANNING.md` — the thin cross-track board: one row per track, one writer per row, plus the service-boundaries section 03 owns and 04 reads.
57	- `CASTING.md` — archetype palette, org-wide variance rules, hire sizing, the persona-writing standard, the orchestration-pattern catalog (adversarial pair, red/blue, three hats, rotating devil's advocate, structured written debate, newcomer's question), the deep-research rule, the hire-creation procedure, and the registry of every hire.
58	- `LOG.md` — dated, append-only journal of what happened and why (team-lead writes it; it is part of the submission).
59	- `.claude/agents/` — one agent definition per hire (model pinned; a once-only per-agent `PreToolUse` hook displays its own persona on its first tool call). `.claude/AGENT_TEMPLATE.md` is the starting point (kept outside the agents directory so it is not registered as an agent type itself).
60	- `02-ai-security-architecture/planning-approach.md` — the agreed Claude-architecture approach: model/effort by role, coordination rules, hand-off standard, dependency order.
61	
62	Rules that hold across every track:
63	- Every agent — lead or hire — has a profile-gen persona (name, temperament, professional history, technical background, stated blind spot). Hires are created text-first with `--output file`; portraits arrive later from `scripts/portrait-queue.sh`. Hires are **never** added as `profile-gen` markers in any `CLAUDE.md` — only the six markers that exist today.
64	- Hires do not spawn subagents. Only track leads (and team-lead) authorize or run deep research (patents, whitepapers, standards, primary sources); hires request it from their lead.
65	- Tracks never modify root-level state (this file, the root persona, root config); flag it to team-lead instead.
66	- **Decisions already made are not re-asked.** Warren's rulings reach tracks through the PM and are recorded in `LOG.md` and this file. A lead must not raise an `AskUserQuestion` to Warren for a decision that is already on the record (e.g., the deployment target, the public repo, the code go) — it asks the PM by message, which answers from the record or escalates. A dialog left open in a lead's session blocks that lead and everything queued behind it (this cost DevOps an hour on 2026-09-12). Genuinely new decisions that only Warren can make still go to Warren — through the PM.
67	- Hand-offs are files by relative path, written so the receiver can act without re-deriving the sender's reasoning; chat messages signal readiness, they do not carry content.
68	- **Before declaring anything handed off, merged-ready, or done, run `git status` and `git diff origin/main -- <your track>/`.** Four times in this project a finished artifact sat unpushed or on a dead branch while its author reported it complete (2026-09-12/13). The author is the worst-placed person to notice, because they can see the work; the reviewer looking at `origin/main` sees the truth. Also check `gh pr view <n> --json state` before pushing to a story branch — once the PM merges, that branch is dead and pushes to it go nowhere.
69	- Edit `CLAUDE.md` files at turn boundaries only (prompt-cache stability). Milestone `git` commits at the end of each phase; no mid-task commits.
70	
71	## Status
72	
73	Planning (Phases 0–4) closed 2026-09-12: five track plans, 18 hires, three cross-track consistency passes (`decisions/cross-track-consistency.md`), and a Jira backlog (`LT-1`…`LT-52`). Implementation began the same evening as an agile loop (`LOG.md` from "Implementation go" onward). As of 2026-09-13 06:00 PDT: **Done end to end** (deployed to the public URL and PO-reviewed) — LT-34 walking skeleton, LT-32 issuer split-Deployment, LT-52 in-namespace Postgres + migrations. **Live:** `https://loginid-takehome.uplifttech.org/healthz` (build SHA == `main`, `db: ok`); `/profiles/search` and `/profiles/{id}` deployed behind RS256 JWT verification and failing closed; `/auth/token` a 501 placeholder until LT-51. **In Test** awaiting one joint live review with a real token: LT-36/37 (Postgres/CockroachDB and SQLite DAOs), LT-38 (conformance suite), LT-39 (REST search/read), LT-40 (auth middleware). **In dev:** LT-51 (client-credentials issuance; PRs #34 + #36), LT-41 (idp-connector, Q3). PRs #1–#37 merged. Teardown runs only on Warren's instruction (`04-infra-devops/teardown.md`).
74	
75	<!-- profile-gen:start slug=dana-whitfield -->
76	@profiles/dana-whitfield/dana-whitfield.md
77	<!-- profile-gen:end slug=dana-whitfield -->
78	
Sep 13, 6:16 AM PDTOren Castellan
Read /Users/warrenthompson/Source/LoginID/03-engineering-delivery/CLAUDE.md
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID/03-engineering-delivery/CLAUDE.md"
}
result
1	# Track: Engineering & Delivery
2	
3	Refer back to `../CLAUDE.md` for the full assignment text and how this track fits with the other four.
4	
5	## Mission
6	
7	Write the actual code that answers all three assignment questions, in Go, implementing the designs the other tracks produce rather than re-deciding them here. Code does not need to be complete or working end-to-end — LoginID cares about the design and reasoning — but it should be structured and readable enough to demonstrate real engineering judgment.
8	
9	## Owns
10	
11	- Question 1: the DAO layer for `user_profile` (name, address, phone) and `user_credential` (username, method, password), written to support multiple backing databases (PostgreSQL/CockroachDB, SQLite, etc.) behind a common interface, per the strategy in `../05-data-ops/CLAUDE.md`.
12	- Question 2: the RESTful API service to search/retrieve `user_profile` data, wired to the authentication mechanism designed in `../02-ai-security-architecture/CLAUDE.md`.
13	- Question 3: the third-party IDP connector service implementing `/auth` and `/identity` against the contracts specified in the assignment, following the connector security design in `../02-ai-security-architecture/CLAUDE.md`.
14	- Project structure, module layout, and inline documentation of design decisions and trade-offs made during implementation.
15	- Test strategy (unit/integration scope, what's stubbed vs. real) — tests don't need full coverage, but the approach should be stated.
16	
17	## Does not own
18	
19	- Deciding the API auth mechanism or threat model — that's `../02-ai-security-architecture/CLAUDE.md`; implement what it specifies.
20	- Deciding the multi-database abstraction strategy or schema — that's `../05-data-ops/CLAUDE.md`; implement what it specifies.
21	- Deployment, containerization, CI/CD — that's `../04-infra-devops/CLAUDE.md`.
22	- Product/industry rationale for why the API is shaped this way — that's `../01-product-industry-research-design/CLAUDE.md`; reference it, don't re-derive it.
23	
24	## Deliverables
25	
26	- Go source implementing questions 1, 2, and 3.
27	- A short README in this directory explaining what's implemented, what's stubbed/mocked, and why, plus which AI tool/workflow was used to produce the code and how.
28	- Any test files demonstrating the stated test strategy.
29	
30	## AI tooling note
31	
32	Record which AI tool/workflow generated this track's code (this project is being built with Claude Code) and how much was AI-authored vs. human-reviewed/edited.
33	
34	## Status
35	
36	Planning complete (2026-09-12): 4 hires spawned and live, `PLAN.md` approved, Service boundaries published in root `PLANNING.md`, four implementation-call decisions ruled (`decisions/go-layout-debate.md`, `error-semantics.md`, `context-propagation.md`, `test-double-strategy.md`), 05's DAO contract accepted (including a second-pass review and 05's resulting `CreateProfileWithCredential` fix), 02's `handoff-03-auth.md` v3 accepted, `backlog.md` written (11 stories, Epic `LT-3`). No `.go`/`.sql`/`go.mod` exists yet — still waiting on Warren's explicit implementation go-ahead.
37	
38	<!-- profile-gen:start slug=renata-cole -->
39	@profiles/renata-cole/renata-cole.md
40	<!-- profile-gen:end slug=renata-cole -->
41	
Read /Users/warrenthompson/Source/LoginID/03-engineering-delivery/profiles/oren-castellan/oren-castellan.md
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID/03-engineering-delivery/profiles/oren-castellan/oren-castellan.md"
}
result
1	---
2	schema_version: 1
3	name: "Oren Castellan"
4	slug: "oren-castellan"
5	image: "profiles/oren-castellan/oren-castellan.png"
6	
7	nsfw: false
8	display:
9	  image: true
10	  name: true
11	  autostart: false
12	generation:
13	  backend: "comfyui"
14	  model: "comfyui-workflow.json"
15	  prompt: "Oren Castellan, a meticulous and understated man in his late 30s with short neat hair, wire-rim glasses, wearing a plain navy button-down shirt, calm focused expression, head-and-shoulders portrait, centered, neutral light gray studio background, professional corporate headshot photography, soft studio lighting, sharp focus, photorealistic"
16	  negative_prompt: blurry, low quality, distorted, extra limbs, watermark, text, nsfw, nude, nudity, explicit, sexual content
17	  seed: null
18	  gif_mode: null
19	  created_at: "2026-09-12T00:00:00Z"
20	---
21	
22	# Oren Castellan
23	
24	![Oren Castellan](profiles/oren-castellan/oren-castellan.png)
25	
26	
27	## Personality
28	
29	Meticulous and understated; in review he goes quiet, then produces a numbered list nobody else caught. Decides by tracing every input to its type and zero value before trusting a code path. Pushes back on "the happy path handles it" and on errors swallowed with `_`. Eight years reviewing Go services at a logistics company, two more doing contract review for a fintech's PCI-scoped repos. Deep in `go vet`/`staticcheck`, table-driven tests, and nil-vs-empty-slice distinctions. Blind spot: he can spend an hour on a rare nullability edge case while missing that the function solves the wrong problem — pair him with someone who checks the solution's shape first. In disagreement he stays procedural, names the specific line and failure, and drops it once satisfied.
30	
31	
Read /Users/warrenthompson/Source/LoginID/03-engineering-delivery/PLAN.md
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID/03-engineering-delivery/PLAN.md"
}
result
1	# Track 03 (Engineering & Delivery) — Phase 1 Plan
2	
3	## Context
4	
5	Team-lead has asked every track lead to plan in plan mode before any hiring or code. This
6	track (Renata Cole) owns the actual Go implementation of all three assignment questions —
7	the DAO, the REST API, and the IDP connector — so its plan needs to name concrete,
8	Jira-shaped deliverables, propose a 4-hire roster with real personality variance (per
9	`CASTING.md`), decide which decisions in this track deserve a debate pattern versus which are
10	waste, state the dependency order this track already committed to in
11	`../02-ai-security-architecture/planning-approach.md`, and produce the "Service boundaries"
12	section of root `PLANNING.md` that 04 (Theo) is blocked on reading. Nothing here is new
13	strategy — it operationalizes what's already agreed in `planning-approach.md` and the root
14	`CLAUDE.md`'s org section. No code is written in this phase.
15	
16	## 1. Deliverables & tasks (one Jira Story each; Epic = this track)
17	
18	*This table is the Phase 1 planning snapshot (2026-09-12). `backlog.md` (11 stories, Jira Epic `LT-3`) is the current, authoritative, PR-sized expansion — its story descriptions and acceptance criteria supersede this table's "What"/"Acceptance standard" columns wherever they differ. Consult `backlog.md`, not this table, for current story shape, hand-off versions, or acceptance criteria.*
19	
20	| Story | What | Hand-off consumed | Hand-off produced | Acceptance standard |
21	|---|---|---|---|---|
22	| S1. Scaffolding | Go module layout, `cmd/` entrypoints, route wiring, stub handlers, config loading | none (assignment text only) | `PLANNING.md` Service boundaries (this plan, §4 below) | Buildable skeleton; 04 can read the boundaries doc without asking a follow-up question |
23	| S2. DAO per backend | `internal/dao` `Repository` interface + Postgres/CockroachDB/SQLite implementations for `user_profile`, `user_credential` | 05's Go-shaped contract in `../05-data-ops/multi-db-strategy.md` (interface signatures, field types/nullability, credential-method-as-lookup-table note) | none outbound | 03 implements directly from the contract with no re-derivation of schema or interface decisions; if the contract is prose instead of Go signatures, this story is blocked and kicked back to 05 |
24	| S3. REST handlers + auth middleware | Search/retrieve `user_profile` endpoints (Q2) + middleware enforcing 02's scheme | 02's receiver-shaped auth hand-off (token scheme, validation point — the deferred follow-up to `api-auth-design.md` noted in 02's own `CLAUDE.md` Status) | none outbound | Middleware implements the scheme as specified; no alternatives re-litigated in code |
25	| S4. Connector `/auth` + `/identity` | Go client + handlers per the assignment's fixed JSON contracts | 02's `connector-security.md` (token lifecycle: no-cache-by-default, fetch per `/identity` call, discard on return — TTL/refresh apply only to how quickly a re-fetch happens, not to any stored token) | none outbound | Token-lifecycle logic matches 02's spec exactly; endpoint paths/bodies are the assignment's, not re-designed |
26	| S5. Tests | Table-driven unit tests for DAO (mocked driver), handlers (mocked DAO), connector (mocked HTTP), **plus the cross-backend conformance suite 05's per-driver ruling is conditional on (`multi-db-strategy.md` §7) — one suite, written once against the `Repository` interface, run against every backend** (consistency-pass finding F25: this was a requirement with no named deliverable until now; `backlog.md`'s S5 is the current, authoritative expansion of this story) | S1–S4 outputs | none | Stated strategy: unit-heavy, integration deliberately stubbed (assignment doesn't require full end-to-end); documented, not silently absent |
27	| S6. README | What's implemented vs. stubbed/mocked and why; AI tool/workflow used (this plan's own hiring + patterns); test-strategy section must state the hand-written-fake rule from `decisions/test-double-strategy.md` (same-PR fake update on interface change) since that's where LoginID reads the test approach | all of the above | the track's README | A cold reader can tell what runs vs. what's a stub without asking |
28	
29	## 2. Hire roster — 4 hires (suggested cast from `CASTING.md`, adopted as-is)
30	
31	All four: **model sonnet** (planning/design/implementation tier per `planning-approach.md`;
32	opus is reserved for the connector token-lifecycle *implementation* seat in a later phase, not
33	planning — none of these four need it now). No hire spawns subagents; no CLAUDE.md marker;
34	profile-gen text-first, after "hiring go".
35	
36	**Oren Castellan — Detail Hawk, code reviewer.** *Function:* second-pass review on DAO/handler/connector Go for error semantics, nullability, off-by-one — the review-seat Renata asked for on her own output, plus the independent pass 02 called for on security-graded surfaces. *Tags:* serious, cautious. *Pattern/role:* rotating devil's advocate seat (implementation calls); second-pass reviewer on S3/S4.
37	
38	*Persona:* Meticulous and understated; in review he goes quiet, then produces a numbered list nobody else caught. Decides by tracing every input to its type and zero value before trusting a code path. Pushes back on "the happy path handles it" and on errors swallowed with `_`. Eight years reviewing Go services at a logistics company, two more doing contract review for a fintech's PCI-scoped repos. Deep in `go vet`/`staticcheck`, table-driven tests, and nil-vs-empty-slice distinctions. Blind spot: he can spend an hour on a rare nullability edge case while missing that the function solves the wrong problem — pair him with someone who checks the solution's shape first. In disagreement he stays procedural, names the specific line and failure, and drops it once satisfied. (129 words)
39	
40	**Nolan Reyes — Veteran, ops/migrations/token-lifecycle sanity.** *Function:* pattern-matches implementation and layout decisions against real production failure modes; pessimist hat in the Go-layout three-hats run. *Tags:* serious, cautious. *Pattern/role:* three-hats pessimist (Go layout); rotating devil's advocate participant.
41	
42	*Persona:* Dry and unhurried; opens reviews with what broke last time he saw this pattern in production. Decides from incident history more than documentation. Pushes back on any "this will just work" claim about retries, timeouts, or token refresh under load. Eleven years in platform ops — six at a regional bank's card-issuing platform, five running SRE for a logistics SaaS — paged once for an expired-token cascade and once for a migration that locked a table for forty minutes; never forgot either. Deep in Go's `context` package, connection pooling, and graceful shutdown. Blind spot: he over-indexes on failures he personally lived through and under-weights new ones. Argues by scenario, not principle, and concedes fast when shown a failure mode he hasn't seen. (134 words)
43	
44	**Marisol Ferran — Enthusiast, Renata's temperament opposite (required).** *Function:* pushes for trying the promising-but-unproven tool (codegen, generics) rather than the safe default; optimist hat in three-hats. *Tags:* playful, bold. *Pattern/role:* three-hats optimist (Go layout); counterweight voice on S2's driver-abstraction choice.
45	
46	*Persona:* Bright and quick to say "let's just try it" before the room finishes weighing options; energized by tools that remove boilerplate. Decides by prototyping fast and showing the result rather than arguing it abstractly. Pushes back on "we've always done it this way" and on process that delays the first line of code by a week. Four years building internal tools at a logistics startup, two more at a small dev-tools shop evaluating codegen and ORM options for clients. Comfortable with `sqlc`, Go generics, and OpenAPI-driven scaffolding. Blind spot: she underweights the years-long operational cost of a clever tool and can talk a team into a dependency nobody wants to own. In disagreement she stays upbeat, offers to build the smaller version on the spot, and yields once the maintenance cost is spelled out. (141 words)
47	
48	**Ines Dabrowski — Systems Cartographer, Go layout / service topology.** *Function:* owns the diagramming behind the Service boundaries decision below; draws the package/dependency map before anyone commits to prose. *Tags:* serious, bold. *Pattern/role:* drafts the Service boundaries doc (§4); non-hat contributor to three-hats as tie-break if optimist/pessimist stall.
49	
50	*Persona:* Calm and visual; sketches a box-and-arrow diagram before committing an opinion to words. Decides by mapping every boundary and dependency first, then asking what crosses it and why. Pushes back on two services sharing a package for convenience rather than a real shared concern. Seven years as a platform engineer at a multi-tenant SaaS company, three designing service boundaries for a payments integration team. Strong on Go module layout, dependency direction, and where an interface should live versus where it's implemented. Blind spot: she can over-abstract a boundary that would have been fine as one package at this project's size, mistaking cleanliness for necessity. In disagreement she draws the alternative rather than argues it, and updates the diagram in front of the room when persuaded. (135 words)
51	
52	Roster spans both required axes (serious/playful via Marisol; cautious/bold via Ines and
53	Marisol), includes Renata's required opposite (Marisol), and duplicates no archetype from
54	another track's *function* (Detail Hawk here is a code reviewer; 05's Detail Hawk is a DDL
55	reviewer, a different function — allowed per variance rule 2).
56	
57	## 3. Debate plan
58	
59	- **Three hats — Go layout (one binary vs. two).** Marisol (optimist: ship one binary, less
60	  ceremony) / Nolan (pessimist: two binaries, because a shared-everything binary is how a
61	  card-issuing platform he worked on ended up with the connector's outbound vendor calls
62	  taking down the client-facing API during an incident) / **Renata takes the pragmatist hat
63	  herself** rather than hiring a fifth (catalog explicitly allows this) — she's the
64	  Minimalist-leaning lead per `CASTING.md` §7. Ines drafts the resulting diagram either way.
65	  Decision recorded in `decisions/go-layout-debate.md`.
66	- **Rotating devil's advocate — implementation calls.** Error semantics (wrap vs. sentinel
67	  errors), context propagation into the DAO layer, test-double strategy (interface fakes vs.
68	  generated mocks). Seat rotates Oren → Nolan → Marisol → Ines per decision so no one hire
69	  becomes "the objector." Cheapest pattern; run on every non-trivial implementation call in
70	  S2–S4.
71	
72	  *(Consistency-pass finding F47: this section named "Callum" as the pessimist/rotation-2 hire
73	  through Phase 1 while the actual roster in §2 above was already "Nolan Reyes" — Callum
74	  Reyes was renamed to clear a first-name collision with 04's Callum Ferreira before hiring;
75	  this section simply hadn't been updated to match. Fixed above; no roster change.)*
76	- **Newcomer's-question pass on hand-offs — run by Renata herself, not a hired Newcomer.**
77	  This track wasn't allocated a Newcomer seat (04 has one; hiring a second here would exceed
78	  the intent of a 4-hire cap for marginal gain). Renata reads 05's `multi-db-strategy.md` and
79	  02's receiver-shaped hand-offs cold, the moment they land, and lists anything she can't act
80	  on without re-deriving the sender's reasoning — same test the pattern exists to run, just
81	  performed by the actual receiver instead of a proxy.
82	- **Explicit waste:** the `/auth` and `/identity` endpoint paths and JSON bodies are fixed by
83	  the assignment text — never debated. Red team/blue team is 02's pattern for the token
84	  lifecycle's security properties, not this track's to re-run; 03 only debates *how* it
85	  implements what 02 specifies (error handling, retries), never *whether* the scheme is right.
86	
87	## 4. Dependencies (and the Service boundaries output for 04)
88	
89	**Split start**, consistent with `planning-approach.md` §2: S1 (scaffolding) starts as soon as
90	hiring completes — no upstream blocker. S2 (DAO) is blocked on 05's Go-shaped contract landing
91	*stable* (not polished — unlikely to be rewritten). S3's middleware is blocked on 02's
92	receiver-shaped auth hand-off; S3's route wiring and non-auth handler logic can start earlier.
93	S4 is blocked on 02's `connector-security.md` token-lifecycle spec, which already exists in
94	prose form — implementable now, refined if 02's deferred receiver-shaped version changes
95	anything.
96	
97	**Service boundaries (written into root `PLANNING.md` alongside this plan):**
98	
99	*This subsection is the Phase 1 snapshot (2026-09-12, before hiring). Root `PLANNING.md`'s
100	Service boundaries section is the current, authoritative version — it has since gained
101	`internal/app` (shared bootstrap, `decisions/go-layout-debate.md`), `DB_DSN_FILE` (02's
102	`handoff-04-secrets.md` S7), literal `AUTH_JWT_ISSUER`/`AUTH_JWT_AUDIENCE` values, and the
103	connector's own audience/client-credential variables (consistency-pass F15). Consult
104	`PLANNING.md`, not this snapshot, for what 04 or anyone else should build against.*
105	
106	- **Two binaries**, not one: `cmd/api-service` (Q1 DAO + Q2 REST API, client-facing, bearer-
107	  token auth) and `cmd/idp-connector` (Q3, outbound-only to vendors). Reasoning: different
108	  threat models and auth surfaces per 02's track, independent scaling/deploy needs, and it
109	  mirrors the assignment's own Q2/Q3 split rather than inventing a new boundary.
110	- **Package layout:**
111	  ```
112	  cmd/api-service/main.go
113	  cmd/idp-connector/main.go
114	  internal/dao/        # Repository interface + postgres/cockroachdb/sqlite implementations
115	  internal/model/      # shared structs: UserProfile, UserCredential
116	  internal/api/         # REST handlers, router, auth middleware
117	  internal/connector/   # /auth, /identity client + handlers
118	  internal/config/      # env-var config loading, shared by both binaries
119	  ```
120	- **Config surface:** environment variables only (12-factor, container-friendly for 04) —
121	  `DB_DRIVER` (postgres|cockroachdb|sqlite), `DB_DSN`, `HTTP_ADDR`, `AUTH_JWT_ISSUER` /
122	  `AUTH_JWT_AUDIENCE` (names pending 02's final scheme), `IDP_ABC_BASE_URL`,
123	  `IDP_ABC_CLIENT_ID` / `IDP_ABC_CLIENT_SECRET`. No flags, no config files.
124	- **DAO driver-selection mechanism:** a factory `dao.New(driver, dsn string) (dao.Repository, error)`
125	  chosen at startup from `DB_DRIVER`; every backend satisfies one `Repository` interface
126	  (exact method signatures finalized once 05's contract lands — the factory *shape* is stable
127	  now, its return type's method set is not).
128	- **What 04 may treat as stable:** the two-binary split, the `cmd/` entrypoint names, the
129	  env-var config approach and the variable names above, and the `dao.New(driver, dsn)` factory
130	  signature. **Not yet stable:** internal DAO implementation details, ORM/query-builder choice,
131	  and handler-package internals — those can still shift as 05's and 02's hand-offs land.
132	
133	## Amendment: QA function (2026-09-12, Warren's agile-loop go-ahead)
134	
135	**Oren Castellan leads QA** (unit, integration, and the end-to-end/UX acceptance tests written from the PO's review script); **Nolan Reyes supports**. 02's independent second review still applies on top of this for every `security-graded` story — QA and 02's review are not the same gate.
136	
137	## Verification
138	
139	- `PLAN.md` for this track matches this file verbatim once copied.
140	- `PLANNING.md` row 03 updated to "plan drafted — awaiting hiring go"; Service boundaries
141	  section replaces its "TBD" with the content in §4 above.
142	- No `.go`, `.sql`, `go.mod`, or agent-definition files exist yet — only prose changed.
143	- Roster (4 hires) checked against `CASTING.md` variance rules before messaging team-lead:
144	  no archetype repeated within this team; Renata's required opposite (Enthusiast) present;
145	  two temperament axes spanned; no first name collides with any lead.
146	
Bash ls /Users/warrenthompson/Source/LoginID/03-engineering-delivery/refinement/ 2>/dev/null; find /Users
input
{
  "command": "ls /Users/warrenthompson/Source/LoginID/03-engineering-delivery/refinement/ 2>/dev/null; find /Users/warrenthompson/Source/LoginID/03-engineering-delivery/refinement -iname \"*LT-41*\" 2>/dev/null"
}
result
LT-32.md
LT-34.md
LT-35.md
LT-36-LT-37.md
LT-38.md
LT-39.md
LT-40.md
LT-41.md
LT-51.md
/Users/warrenthompson/Source/LoginID/03-engineering-delivery/refinement/LT-41.md
Sep 13, 6:16 AM PDTOren Castellan
Read /Users/warrenthompson/Source/LoginID/03-engineering-delivery/refinement/LT-41.md
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID/03-engineering-delivery/refinement/LT-41.md"
}
result
1	# LT-41 refinement — S8: connector `/auth` + `/identity` (Q3)
2	
3	**Product Owner:** Naomi Voss (01). **Research team:** Imogen Hale, Desmond Okafor, Tobias Lindqvist. **Engineering:** Renata Cole (03). **Source:** `03-engineering-delivery/backlog.md` S8 (LT-41); `02-ai-security-architecture/connector-security.md` (including its attack-tree table), `decisions/connector-token-lifecycle-redblue.md`, `handoff-03-auth.md` v6.
4	
5	**Who did the work:** Refined by Naomi Voss (PO); claims check Desmond Okafor; objection Tobias Lindqvist; engineering counterpart Renata Cole.
6	
7	## Why this one, now
8	
9	Q3 (the third assignment question) has no code yet, and it's genuinely independent of LT-51's in-flight work: the connector only *validates* a `connector:identity-lookup`-scoped token presented to it — it doesn't need to know how `api-service` obtains that token in the first place (that's LT-33, 02's still-open story, on `api-service`'s outbound side). Pulling this now keeps refinement from being the bottleneck while Renata and Theo build LT-51.
10	
11	**Checked the dependency graph rather than assuming:** LT-33 (02, `handoff-04-secrets.md` row 10 + `api-service`'s client credential) depends in part on "03's connector-client implementation," but that's the *outbound caller* inside `api-service`, not this story — this story is `cmd/idp-connector` itself, the inbound side. LT-41 can be built, refined, and merged without LT-33 closing first; LT-33 is what wires `api-service` to call it correctly, a separate piece.
12	
13	## Correction to `backlog.md`'s own S8 text, found before drafting
14	
15	**Three real controls from `connector-security.md`'s attack-tree table (S3 red/blue, already run and ruled) needed fixing relative to `backlog.md`'s S8 acceptance criteria — one truly missing, one named in prose but never promoted to a checkable criterion, one already fixed in this doc but not credited in this section (Desmond's claims check corrected all three of these framings).** The table is the authoritative, already-ratified source:
16	
17	1. **Leaf 1 (connection-reuse token leak) — named in prose, not promoted to a criterion.** `backlog.md`'s S8 **Description** paragraph already says, almost verbatim, "fresh isolated `Authorization` header per outbound request, never inherited from pooling/keep-alive" — so this isn't an undocumented gap, it's a requirement stated in prose that never became its own checkable acceptance criterion with an enforcement site. The criteria list only has the generic timeout/pool/backoff bullet. Promoted below.
18	2. **Leaf 5 (credential-stuffing oracle) — a genuine, total gap.** Rate limiting on outbound vendor calls, keyed per calling-API-client **and** per source IP, with a stricter threshold specifically on vendor-auth **failure ratio** (not just volume), paired with exponential backoff per identity attempted. Confirmed by reading `backlog.md`'s S8 text directly: no rate-limiting language anywhere, not even in prose.
19	3. **Leaf 4 (memory/crash-dump exposure), narrow-scoping half — already fixed in this doc's criteria below, just not credited here until now.** Two sub-controls beyond "never log the token" (which S8's existing criteria already cover): the token excluded from generic request/response dumps used in panic traces/debug output, and the token variable scoped to the narrowest function possible, never passed up the call stack. Both are already present as their own bullets below; this section previously undercounted the fix as covering two gaps instead of three.
20	
21	Both are added below as acceptance criteria, since the attack-tree table already ruled on them (Residual L on both, after blue's response) — this refinement is transcribing an already-ratified control, not inventing a new one.
22	
23	## Acceptance criteria (restated from `backlog.md` S8, plus the two additions above)
24	
25	- [ ] Implements 02's token-lifecycle spec (`connector-security.md`, `decisions/connector-token-lifecycle-redblue.md`) without re-litigating the no-cache-by-default posture.
26	- [ ] No vendor token is persisted or cached across requests by default; if a future exception is ever justified it requires re-review, not a silent default change — enforcement site: no storage/cache call in the token-fetch path.
27	- [ ] Every outbound vendor call sets `context.WithTimeout`; the outbound HTTP client uses a bounded connection pool; token-refresh retries back off rather than tight-looping — enforcement site: the HTTP client construction plus a test asserting a timeout fires.
28	- [ ] **Added (attack-tree leaf 1):** a fresh, isolated `Authorization` header is constructed per outbound vendor request — never inherited from HTTP connection/keep-alive reuse. Enforcement site: a test making two sequential vendor calls with two different presented tokens through the same pooled client, asserting the second call's outbound header matches its own token, not the first call's.
29	- [ ] **Added (attack-tree leaf 5):** outbound vendor calls are rate-limited keyed per calling-API-client identity **and** per source IP independently, with a stricter threshold on vendor-auth failure ratio specifically (not just raw volume), paired with exponential backoff per identity attempted. Enforcement site: the rate-limit/backoff component plus a test asserting a high-failure-ratio pattern trips the stricter threshold before a high-volume-but-mostly-successful pattern would.
30	- [ ] The connector's own `/auth`/`/identity` handlers validate an inbound JWT with `aud: idp-connector-service` and `scope: connector:identity-lookup`, distinct from `api-service`'s audience — enforcement site: this handler's own middleware instance, with a test asserting a `loginid-api-service`-audience token is rejected here.
31	- [ ] `/identity` accepts the vendor token as a request header (not a body field, since the assignment's fixed `{"phone","name"}` body has no credential field), fails closed with a generic error that doesn't distinguish "no token" from "bad token" from "vendor rejected it" in **response content** (same user-enumeration discipline as elsewhere in this project) — per `connector-security.md`'s F3 resolution.
32	- [ ] **Resolved by 02's ruling (Tobias's objection, routed to Marcus):** apply a uniform response-time floor (~100ms, well under any `context.WithTimeout` budget elsewhere in the chain) across all three failure branches (no-token, bad-token, vendor-rejected) — narrows the trivially-observable timing gap into something requiring statistical analysis over many requests, not full closure. **The remainder is an accepted residual risk, stated explicitly in `connector-security.md` §4, not engineered away further** — full closure (delaying every request to match the vendor's worst-case latency) would be a real availability cost disproportionate to what it protects, since what's disclosed even in the worst case is only that a presented token's shape reached the vendor, never any subject's identity, existence, or PII (§4 already normalizes vendor-rejection reasons so *which* check failed doesn't leak either). This is a different, and correctly different, resolution from LT-51's dummy-Argon2id fix: that leak was a fully-controlled local computation makeable genuinely constant-time; this one involves a third-party network call whose latency distribution isn't this connector's to control. Enforcement site: `connector-security.md` §4's stated floor and residual-risk language, plus a test asserting all three branches return within a bounded time delta of each other (not "both fast" or "both slow" — comparably close, per the floor).
33	- [ ] The token variable is scoped to the narrowest function possible and never passed up the call stack beyond the vendor-call function itself — per attack-tree leaf 4's control (accepted that Go gives no guaranteed memory zeroing, but the exposure window is narrowed to one function call, not eliminated).
34	- [ ] Never logs, in any environment or error path: the end-user's vendor password, any vendor `access_token` or full `Authorization` header value, or raw vendor error response bodies (per `handoff-04-secrets.md`'s never-log list) — enforcement site: log-call review against that list, plus confirming the token field is excluded from any generic request/response dump used in panic traces or debug output, not only application logs (leaf 4's "same discipline as application logs" extension).
35	
36	## Non-goals
37	
38	- No implementation of `api-service`'s outbound call to this connector, or how `api-service` obtains its own `connector:identity-lookup` token — that's LT-33 (02, still open), a separate story.
39	- No real ABC/XYZ vendor integration — a stubbed vendor per the assignment's generic framing; this story's job is the connector's own contract and security properties, not a specific vendor's API quirks.
40	- No change to `handoff-03-auth.md`'s scope vocabulary or claim scheme — already fixed (v6).
41	- No shared rate-limit store/edge mechanism — attack-tree leaf 5 names that as 04's mechanism if enforced at a gateway; this story's own in-process rate limiting is sufficient for this take-home's scale, matching the pattern already used for `RateLimiter`/`TouchCounter`/`GrantLimiter`.
42	
43	## Flagged gap for the joint LT-34/39/40/51 live-URL review script (not this story's own scope)
44	
45	**Per team-lead's request to update the joint review script** (mint a token, create a profile via whatever write path exists, search it masked, read it full with a `reason_code`, hit the 429 cap): **checked directly — there is no write/create path anywhere in `internal/api`.** `backlog.md`'s S6 (LT-39) only implements search/retrieve (Q2's explicit scope); no `POST /profiles` handler exists, and none is currently planned as a story. The migration `Job`'s `shared/` directory only seeds the `auth_method` lookup row (per `migration-approach.md`), not `user_profile`/`user_credential` test data — there's no seeding mechanism for a reviewable profile either.
46	
47	**This is a real gap in the joint review script's own plan, not something this story can quietly work around.** Two options, neither decided here: (a) a small, explicit "seed one test profile" step — either a one-off manual `INSERT` against the deployed Postgres instance (out-of-band, like the credential bootstraps) for review purposes only, clearly not a production data-entry path, or (b) a minimal `POST /profiles` handler as a new, small story if profile creation is expected to exist at all for a complete Q1/Q2 demonstration. Flagging both to team-lead rather than picking one — this is a product/scope decision (does the submission need a write path at all?), not an engineering detail.
48	
49	## PO review script
50	
51	1. Confirm the fresh-header-per-request test (leaf 1) exists and passes.
52	2. Confirm the failure-ratio rate-limit test (leaf 5) exists and passes.
53	3. Confirm the connector's inbound middleware rejects a `loginid-api-service`-audience token (wrong audience for this service).
54	4. Confirm `/identity`'s generic failure response doesn't distinguish no-token/bad-token/vendor-rejected cases.
55	5. Read the vendor-call code path directly: confirm the token variable's scope doesn't escape the vendor-call function, and confirm no log call anywhere references the token, the vendor password, or a raw vendor error body.
56	6. Once deployed (this connector has no public route — reachable only from `api-service` via the existing `NetworkPolicy`): no direct live-URL check is possible for this story on its own; its behavior is exercised indirectly once `api-service`'s outbound call (LT-33) actually invokes it as part of the onboarding flow. Not part of the LT-34/39/40/51 joint review, which only covers Q1/Q2.
57	
58	## Research team review
59	
60	**Desmond Okafor — claims check:** Independently re-verified the "no write path exists" claim against `origin/main`'s actual `internal/app/verifier_router.go` — only `/healthz`, `POST /profiles/search`, `GET /profiles/{id}` registered; `CreateProfileWithCredential` appears only in test fakes, never called from a handler. Verified leaf 5's quote and "total gap" framing exactly. Corrected leaf 1's framing: the fresh-header requirement was already in `backlog.md`'s S8 *Description* prose, just never promoted to its own checkable acceptance criterion — not an undocumented gap, a documented-but-unenforceable one. Caught that leaf 4's narrow-scoping/no-dump-leak sub-controls were already fixed in this doc's criteria but not credited in the correction section — three gaps closed, not two. All fixes applied above.
61	
62	**Tobias Lindqvist — objection:** F3's identical-response-content requirement for `/identity`'s fail-closed error doesn't close a timing side-channel — the three failure branches (no-token, bad-token, vendor-rejected) have inherently different latencies, the last requiring an actual network round-trip to the vendor. Unlike LT-51's dummy-Argon2id case, this can't be cleanly equalized (one branch is a genuinely variable network call). Routed to Marcus for a ruling rather than deciding unilaterally — his own proposed floor is honestly partial, not a full close, which is exactly why this needs a security-owner decision on the trade-off, not a refinement-doc default.
63	
64	**Naomi Voss — ruling:** All fixes applied, including Marcus's ruling on the timing-oracle question (a ~100ms uniform floor, remainder accepted as a named residual risk in `connector-security.md` §4 — a correctly different resolution from LT-51's fully-closable case, not a half-measure). This refinement is now fully resolved and accepted, ready for Renata.
65	
66	## Update — write-path gap resolved (team-lead + Priya, on LT-39's record)
67	
68	**Decision:** no `POST /profiles` story — Q2 is search/retrieve, and a write endpoint would be scope creep beyond the assignment. Review data for the joint LT-34/39/40/51 URL review enters via a version-controlled file, `deploy/demo-data/001-review-profiles.sql` — idempotent, run under the `migrator` role, **never placed under `migrations/`**, obviously fictional values in reserved E.164 ranges, governed by the same discipline as real rows. Applied by Theo as a documented runbook step, after LT-51 lands. See the corrected joint review script in `refinement/LT-51.md` (and `LT-39.md`/`LT-40.md`'s cross-references) — no create step; seed via runbook, then mint → search → read → cap.
69	
Read /Users/warrenthompson/Source/LoginID/02-ai-security-architecture/connector-security.md
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID/02-ai-security-architecture/connector-security.md"
}
result
1	# IDP Connector Security Design — Question 3
2	
3	Owner: Marcus Ilori (`02-ai-security-architecture`). Covers the third-party identity-provider connector (generic vendors "ABC"/"XYZ") exposing `/auth` and `/identity`, per the assignment. Implementation target: `../03-engineering-delivery/CLAUDE.md`.
4	
5	## Contract recap (from the assignment, verbatim in `../CLAUDE.md`)
6	
7	- `POST /auth` — body `{"username": "<string>", "password": "<string>"}` → returns an `access_token`.
8	- `POST /identity` — body `{"phone": "<string>", "name": "<string>"}` → returns PII (name, phone, address fields).
9	
10	Read literally, this connector is a thin proxy in front of ABC/XYZ: it takes vendor credentials, gets a vendor token, and uses that token to pull PII from the vendor. Every security question here is "what does this service do with a vendor secret and vendor PII while it's in our custody," since we don't control the vendor's own security posture.
11	
12	## Threat surface specific to this connector
13	
14	This extends the third-party-token row in `threat-model.md`; this document is the concrete design against those threats.
15	
16	### 1. What happens to the vendor `access_token` once retrieved
17	
18	- **Never logged.** The `/auth` request body contains a vendor password; the response contains a bearer token. A naive "log the request and response for debugging" habit puts both in the same log line — this is called out explicitly because it's the single most common way a connector like this leaks a credential in practice, not a hypothetical.
19	- **No persistent or cached token, by default — updated for F3's caller-holds-the-token model (seam re-run regression fix).** The S3 ruling's no-cache posture stands, but its original wording assumed the connector itself calls `/auth` immediately before every `/identity` call, chaining the two internally. That's superseded by F3: our own `/auth` and `/identity` are genuinely separate calls from our caller's perspective (matching the assignment's fixed two-endpoint contract), and it is the **caller** (`api-service`, on the onboarding flow's behalf) that calls our `/auth`, receives the vendor token, and presents it as a header on the following `/identity` call. What the connector still never does: cache or persist the token, write it to any datastore, or hold it any longer than the single request currently being handled. The connector's `/identity` handler uses whatever token its caller presents for that one call and discards it when the call returns — the "no cache" discipline now describes the connector's own handling of a caller-presented token, not a connector-managed fetch-then-cache cycle. Caching is still an explicitly-justified exception requiring its own re-review if it's ever proposed for either side of this exchange; the exception conditions (immutable cache key, TTL, KMS fail-closed) from the S3 ruling are unchanged if that question is ever reopened.
20	- **Fetch-use-zeroize boundary, restated for the caller-holds-the-token model:** on the `/identity` side, the presented token's scope runs from the point the handler reads it off the request header to the end of that single call, and no further — it is not cached, not passed to any other request, not logged. On the `/auth` side, the token returned to the caller is not retained by the connector after the response is sent. **Retry behavior is now the caller's responsibility, not the connector's**: if `api-service` retries a `/identity` call, whether it re-presents the same token or first calls `/auth` again is its own policy decision based on the token's likely freshness — the connector's only obligation is to treat each presented token as freshly supplied and reject it cleanly (fail closed, generic error) if the vendor rejects it, never silently retrying the vendor call with a stale token on the connector's own initiative. Go gives no guaranteed memory zeroing without unsafe/cgo, so a crash dump taken mid-call could still capture the token in the handler's stack; that residual is accepted given the window is one request, not a cache TTL. The token field is excluded from any generic request/response dump used in error handling — the same log-hygiene discipline below applies to panic traces and debug dumps, not just application logs.
21	- **The outbound HTTP client to the vendor must construct a fresh, isolated `Authorization` header per request, never inherited from connection reuse.** Connection pooling/keep-alive is the normal default in any HTTP client library; a pooled client that reuses a request-context object or fails to reset the auth header per call can attach request A's vendor token to request B's outbound call — the same cross-tenant leak as a cache-key collision, with no cache required. This is a required implementation discipline, not an assumption left implicit.
22	- **Scoped narrowly if the vendor supports it** — request only the scope needed for `/identity`, not a broad token, so a leaked token's blast radius is bounded by the vendor's own authorization model as well as ours.
23	- **Returned once, to the authenticated internal caller only — never to end users, never persisted server-side (resolved by the cross-track consistency pass, F3).** Our own `/auth` returns the vendor `access_token` to whichever authenticated `connector:identity-lookup`-scoped caller invoked it (in this project's shape, `cmd/api-service` acting on an onboarding flow's behalf) — this is what the assignment's `/auth` contract actually requires ("retrieves an `access_token`"), and it is what `../01-product-industry-research-design/ux-notes.md`'s onboarding flow depends on to bridge its two user-facing steps. Because our own `/identity` request body is fixed by the assignment to `{"phone", "name"}` with no credential field, the vendor token travels as a request header on the subsequent `/identity` call (e.g. `X-Vendor-Access-Token`) — this doesn't violate the fixed JSON contract, which governs the body only, not headers. The connector itself still never caches or persists the token (S3's no-cache ruling stands unchanged): each `/identity` call's presented token is used once and discarded, exactly as if it had been freshly fetched, and it is the caller's responsibility — not the connector's — to hold the token for the short span between the two calls. If a caller presents no token, or an expired one, `/identity` fails closed with a generic error that does not distinguish "no token" from "bad token" from "vendor rejected it" (same user-enumeration discipline as §4's other failure-handling rules).
24	
25	### 2. How `/identity` calls are authenticated to the vendor
26	
27	- Standard bearer-token usage: the vendor `access_token`, presented by our caller as a request header on our own `/identity`, is forwarded as an `Authorization: Bearer` header to the vendor's identity endpoint (or whatever the vendor's own contract specifies — this connector adapts to the vendor, not the reverse).
28	- **Token refresh/re-auth on expiry is the caller's responsibility, not the connector's (corrected for F3 — seam re-run regression fix).** The earlier text here said the connector handles this "transparently" so a caller "should not need to know or manage vendor token lifecycle" — that assumed the connector itself owned the fetch-then-use cycle. Under F3's model the caller holds the token between our `/auth` and `/identity` calls, so the caller necessarily manages when to re-authenticate: if our `/identity` rejects a token as expired, it fails closed with a generic error (per §1) and it is the caller's decision to call our `/auth` again, not a retry the connector performs on its own initiative.
29	- **Vendor calls over TLS only**, certificate validation never disabled, including in local/dev environments (repeating the threat model's point because it's the most common place a "temporary" dev bypass ships to production unnoticed).
30	- **Our own credentials to call the vendor's `/auth`** (this connector's client id/secret with ABC/XYZ, if the vendor requires connector-level registration in addition to the end-user's own username/password) are runtime-injected secrets, never in source or config committed to the repo — requirement set here, mechanism owned by `../04-infra-devops/CLAUDE.md`.
31	
32	### 3. What this service should and shouldn't log or persist
33	
34	| Data | Log it? | Persist it? |
35	|---|---|---|
36	| End-user's vendor username | No (identifier, but low sensitivity relative to password — still avoid in plaintext logs; use it only for correlation via a hash if audit trails need it) | Only if a legitimate business reason exists; not by default |
37	| End-user's vendor password (`/auth` request body) | **Never, under any circumstance, including error paths and stack traces** | **Never** |
38	| Vendor `access_token` | **Never** | Only as short-TTL encrypted cache per above; never in plaintext, never in application logs |
39	| PII returned by `/identity` (name, phone, address fields) | Never the field values; log only the *fact* of a lookup (caller identity, timestamp, success/failure, which record was resolved by opaque id) | Retention/caching policy for this PII is `../05-data-ops/CLAUDE.md`'s call — this track's requirement is that whatever is persisted follows the same encryption-at-rest and access-control bar as `user_profile` data, no weaker because it arrived via a connector rather than direct input |
40	| Vendor error responses | Log status/error-code only; do not log raw vendor response bodies by default, since a vendor error payload can itself echo back submitted PII or partial credentials | N/A |
41	
42	### 4. Failure handling as a security control, not just reliability
43	
44	- **Vendor auth failures must not leak which part of the credential was wrong** (username vs. password) in whatever this connector surfaces upward — same user-enumeration concern as the threat model's credential-stuffing row, now facing outward at whoever calls our `/auth` passthrough.
45	- **Retry/backoff on vendor calls** should be rate-limited on our side independent of the vendor's own limits, so a compromised or careless caller can't use our connector to hammer the vendor (which is both an availability risk to us and a way to get our connector's vendor credentials rate-limited or banned).
46	- **Circuit-breaking**: if the vendor is failing or behaving anomalously (e.g., returning tokens for identities that don't match the request), fail closed — do not fall back to a cached or stale identity result silently, since that risks serving stale PII as if freshly verified.
47	- **Timing side-channel on `/identity`'s fail-closed error, partially mitigated, accepted residual (found by Tobias Lindqvist during LT-41 refinement, ruled 2026-09-13).** The identical-error requirement for "no token" / "bad token" / "vendor rejected it" (§1) covers response *body and status code*, but the three paths have inherently different latencies: no-token and bad-token fail locally (microseconds), vendor-rejected requires an actual round-trip to the third-party IDP (tens–hundreds of ms, and variable — the vendor's latency is not under our control). An attacker can distinguish "malformed/missing token" from "well-formed enough to reach the vendor" via timing alone, regardless of identical bodies.
48	
49	  **This is a materially lower-severity leak than the classic timing side-channel this track has otherwise guarded against (e.g., password/account-existence timing, which this design closes by always running the same fixed-cost hash regardless of outcome).** What's disclosed here is only that a presented string has a shape that reached the vendor — not whether any specific identity or account exists, not PII, and not which part of a credential was wrong (§4's existing rule already normalizes vendor-rejection reasons). It's recon on token *shape*, not on any subject's identity.
50	
51	  **Mitigation applied, not full closure: a response-time floor (~100ms, well under any `context.WithTimeout` budget in the call chain, so it doesn't interact with the timeout requirements elsewhere) applied uniformly across all three failure branches** — padding the fast local-fail paths up to roughly the floor narrows the trivially-observable microseconds-vs-hundreds-of-ms gap into something requiring statistical analysis across many requests rather than one request telling an attacker everything instantly. **Full closure is not pursued and is accepted as a residual**, because the vendor round-trip's latency *distribution* (not just its average) varies with real-world network and vendor conditions in ways a fixed floor cannot fully mask — unlike a local hash computation, which this design can and does make constant-time-equivalent regardless of outcome, a third-party network call's tail latency isn't ours to control. Closing this further would mean either delaying every request to match the vendor's worst observed latency (a real availability cost for a low-value information leak) or removing the network call from the timing entirely (impossible — we must actually ask the vendor). Accepted for this system on the grounds that the disclosed information is low-value token-shape recon, not identity or PII exposure, and the cost of full closure is disproportionate to that value.
52	
53	### 5. Authentication and authorization of calls *into* this connector
54	
55	Found by Ingrid Solano's S8 independent review — §1–§4 above are entirely about protecting the *vendor* token and the *vendor* call; none of them state who is allowed to call *our* `/auth` and `/identity` in the first place. Fixed here rather than left as a silent assumption:
56	
57	- **Callers of `cmd/idp-connector`'s own `/auth` and `/identity` must be authenticated**, using the same OAuth2 client-credentials → JWT bearer mechanism as `api-auth-design.md`, but with a **distinct audience** (`aud: idp-connector-service`, separate from `loginid-api-service`) and its own scope (`connector:identity-lookup`) — this is a different threat model and a different, smaller caller set (internal services, not question 2's broader API clientele), so it gets its own audience rather than reusing the search API's. Only a service that legitimately mediates an end-user or business flow needing vendor identity data (in this project's shape, `cmd/api-service` acting on a caller's behalf) should hold this credential — it is not exposed to arbitrary external callers.
58	- **Why this matters concretely:** without it, an unauthenticated caller could hit our `/auth` passthrough with arbitrary vendor credentials all day and use this connector as exactly the credential-testing oracle §4's rate-limit control is meant to prevent — but "rate-limited per calling-API-client" has nothing to bind "per calling-API-client" to if no caller identity is required to reach the endpoint at all.
59	- **Audit logging** for calls into this connector follows the same discipline as §3's table: caller `sub`, scope, timestamp, outcome — never the vendor credentials or PII in the request/response bodies.
60	
61	### 6. What a vendor token authorizes at `/identity` — an assumption, stated explicitly
62	
63	Also found by S8 review. This design's token-handling discipline (§1) is entirely about *custody* of the vendor token — encryption, memory scope, never-log. It says nothing about what the token is allowed to *do*: specifically, whether a vendor `access_token` obtained from one person's `/auth` call authorizes an `/identity` query for a *different* person's `name`/`phone`, or only for the token-holder's own identity.
64	
65	This connector cannot enforce that boundary itself — the assignment's generic `/identity` contract (`{"phone": "...", "name": "..."}`, no scoping field) puts that decision entirely on the vendor's own API. **Stated assumption, not a resolved control:** this design assumes a real ABC/XYZ-shaped vendor binds `/identity` queries to the authenticated principal at the vendor side (i.e., the vendor's own API rejects a query for an identity other than the token-holder's). If that assumption doesn't hold for a given real vendor, this connector becomes a lookup oracle for anyone who can obtain one valid vendor token, and no control in §1–§5 touches that risk, because none of them govern token *authority* rather than token *custody*. This is the same class of question as the search API's object-level authorization (`api-auth-design.md`, S4) and belongs on `../01-product-industry-research-design/CLAUDE.md`'s list when evaluating which real vendor ABC/XYZ represents — this track cannot resolve it without knowing a concrete vendor's actual token semantics.
66	
67	## What this design does not resolve
68	
69	- Data retention duration for cached `/identity` results — `../05-data-ops/CLAUDE.md`.
70	- Which secrets manager holds this connector's own vendor credentials — `../04-infra-devops/CLAUDE.md`.
71	- Vendor selection/evaluation (which real-world providers "ABC"/"XYZ" might represent) — that's product framing in `../01-product-industry-research-design/CLAUDE.md`, not a security question.
72	
73	## Sources
74	
75	- OWASP API Security Top 10 2023, API7 (Server Side Request Forgery) and API8 (Security Misconfiguration) — both relevant to a service that makes outbound calls to a third party on a user's behalf: https://owasp.org/API-Security/editions/2023/en/0x11-t10/
76	- OWASP Logging Cheat Sheet (what not to log — credentials, tokens, PII): https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
77	
78	## Attack-tree table — connector token lifecycle (S3 red/blue, 2026-09-12)
79	
80	Full positions, rebuttals, and ruling: `decisions/connector-token-lifecycle-redblue.md`. Table shape per `planning-approach.md` Appendix A.
81	
82	| # | Leaf (attack path) | Precondition | Likelihood | Impact | Blue response: Control *or* Accepted risk | Owner | Residual | Source |
83	|---|---|---|---|---|---|---|---|---|
84	| 1 | Cache-key confusion → cross-tenant token/PII leak via request-derived cache key | Persistent cache (eliminated by default posture below) | M | H | **Eliminated by design:** no cache by default (Felix Adebayo, alternatives seat) — no persistent cache means no key to collide on. **Control, exception-path only:** if caching is ever re-opened, key must derive from immutable vendor-issued ids, never request-supplied `name`/`phone`. **Control (this run, Tomasz's rebuttal):** the connection-reuse variant of this leaf survives even with no cache — a pooled HTTP client that doesn't reset `Authorization` per call can attach one request's token to another. Closed with a required control: fresh, isolated auth header constructed per outbound request, never inherited from connection reuse. | 03 (client construction; cache-key derivation if exception ever opened) | **L** — storage variant eliminated; connection-reuse variant closed by an explicit, checkable implementation requirement | OWASP API8:2023; judgment (connection-reuse variant) |
85	| 2 | Fail-open on KMS unavailability during a caching window | Persistent cache with KMS-backed encryption at rest (eliminated by default posture) | M | H | **Eliminated by design:** no cache by default removes the write-time KMS dependency from the hot path. **Control, exception-path only:** if caching is ever re-opened, KMS-unreachable fails closed to "no cache, fetch live," never to an unencrypted write. Tomasz conceded this leaf outright — no hole found. | 03/04 (KMS mechanism is 04's) | **L** | NIST SP 800-57 |
86	| 3 | Vendor token replay — no proof-of-possession/session binding once decrypted for use | Token exfiltrated by any other means within its exposure window | M | H | **Accepted risk.** Bearer tokens under RFC 6750 are possession-equals-authorization by design; PoP/binding (DPoP, RFC 9449; mTLS-bound tokens, RFC 8705) needs vendor-side support this project cannot assume for a generic ABC/XYZ vendor the assignment never specifies. Acceptable *for
… truncated (20 KB cap)
Sep 13, 6:16 AM PDTOren Castellan
Now let's set up the worktree and check out the code.
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 2>/dev/null && pwd && git log --oneline -5 &
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 2>/dev/null && pwd && git log --oneline -5 && git status"
}
result
/Users/warrenthompson/Source/LoginID-worktrees/LT-41
dd433e0 LT-41: idp-connector /auth + /identity against a stubbed ABC/XYZ vendor
2ea4b5d Merge PR #34: LT-51 OAuth2 client-credentials issuance — Postgres client store, JWKS endpoint, GrantLimiter
b7db08e LT-51: OAuth2 client-credentials grant, Postgres-backed client store
00f4194 Root CLAUDE.md Status: implementation loop state as of 2026-09-13 06:00 (was stale: 'no code until go-ahead')
43e62db Root rule: verify against origin/main before declaring work handed off; check PR state before pushing to a story branch
On branch feature/LT-41
Your branch is up to date with 'origin/feature/LT-41'.

nothing to commit, working tree clean
Bash gh pr view 40 --repo wtvamp/loginid-takehome --json title,state,headRefName,baseRefName 2>&1
input
{
  "command": "gh pr view 40 --repo wtvamp/loginid-takehome --json title,state,headRefName,baseRefName 2>&1"
}
result
{"baseRefName":"main","headRefName":"feature/LT-41","state":"OPEN","title":"LT-41: idp-connector /auth + /identity (Q3, stubbed vendor)"}
Sep 13, 6:16 AM PDTOren Castellan
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && go build ./... && echo BUILD_OK && go vet
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && go build ./... && echo BUILD_OK && go vet ./... && echo VET_OK"
}
result
BUILD_OK
VET_OK
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && go test ./... -race 2>&1 | tail -60
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && go test ./... -race 2>&1 | tail -60"
}
result
?   	loginid-takehome/cmd/api-service	[no test files]
?   	loginid-takehome/cmd/idp-connector	[no test files]
ok  	loginid-takehome/internal/api	(cached)
ok  	loginid-takehome/internal/app	(cached)
ok  	loginid-takehome/internal/config	(cached)
ok  	loginid-takehome/internal/connector	(cached)
ok  	loginid-takehome/internal/dao	(cached)
ok  	loginid-takehome/internal/dao/conformance	(cached)
ok  	loginid-takehome/internal/dao/postgres	(cached)
ok  	loginid-takehome/internal/dao/sqlite	(cached)
?   	loginid-takehome/internal/model	[no test files]
Sep 13, 6:16 AM PDTOren Castellan
Build, vet, and race tests all pass. Now the actual code review.
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/httpvendorclient.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/httpvendorclient.go"
}
result
1	package connector
2	
3	import (
4		"bytes"
5		"context"
6		"encoding/json"
7		"fmt"
8		"io"
9		"net/http"
10		"net/url"
11		"time"
12	)
13	
14	// vendorCallTimeout bounds every outbound vendor call — belt-and-braces
15	// alongside the caller's own context.WithTimeout, the same
16	// defense-in-depth pattern LT-40's JWKS fetch client uses (an explicit
17	// http.Client.Timeout independent of context, since a context deadline
18	// bug elsewhere must not leave this client waiting forever).
19	const vendorCallTimeout = 10 * time.Second
20	
21	// defaultVendorHTTPClient is a bounded-pool client — connector-security.md's
22	// "bounded connection pool" requirement. Deliberately module-level so
23	// every HTTPVendorClient constructed without an explicit client shares
24	// one bounded pool rather than each accidentally getting Go's unbounded
25	// http.DefaultTransport.
26	var defaultVendorHTTPClient = &http.Client{
27		Timeout: vendorCallTimeout,
28		Transport: &http.Transport{
29			MaxIdleConns:        20,
30			MaxIdleConnsPerHost: 5,
31			MaxConnsPerHost:     10,
32			IdleConnTimeout:     30 * time.Second,
33		},
34	}
35	
36	// HTTPVendorClient calls a real (or real-shaped) vendor over HTTPS,
37	// demonstrating connector-security.md's outbound discipline: TLS only,
38	// a bounded connection pool, and a fresh, isolated Authorization header
39	// built inside each call rather than held as client-level state —
40	// httpClient's connection pooling can reuse a TCP connection across
41	// calls, but never a header, since baseURL/httpClient are the only
42	// fields this type holds; the token is a per-call parameter, never
43	// stored.
44	//
45	// No real ABC/XYZ vendor exists for this take-home (LT-41's own
46	// non-goal) — this implementation assumes a vendor that happens to
47	// share this project's own JSON request/response shape, a stated
48	// simplifying assumption for demonstrating the connector's own outbound
49	// security properties, not a claim about any real vendor's actual API.
50	type HTTPVendorClient struct {
51		baseURL    string
52		httpClient *http.Client
53	}
54	
55	// NewHTTPVendorClient requires an https:// baseURL — connector-security.md
56	// §2: "Vendor calls over TLS only ... including in local/dev
57	// environments." httpClient defaults to defaultVendorHTTPClient (bounded
58	// pool + timeout) if nil.
59	func NewHTTPVendorClient(baseURL string, httpClient *http.Client) (*HTTPVendorClient, error) {
60		u, err := url.Parse(baseURL)
61		if err != nil {
62			return nil, fmt.Errorf("connector: parsing vendor base URL: %w", err)
63		}
64		if u.Scheme != "https" {
65			return nil, fmt.Errorf("connector: vendor base URL %q must use https, per connector-security.md's TLS-only requirement", baseURL)
66		}
67		if httpClient == nil {
68			httpClient = defaultVendorHTTPClient
69		}
70		return &HTTPVendorClient{baseURL: baseURL, httpClient: httpClient}, nil
71	}
72	
73	type vendorAuthRequest struct {
74		Username string `json:"username"`
75		Password string `json:"password"`
76	}
77	
78	type vendorAuthResponse struct {
79		AccessToken string `json:"access_token"`
80	}
81	
82	func (c *HTTPVendorClient) Authenticate(ctx context.Context, username, password string) (string, error) {
83		reqBody, err := json.Marshal(vendorAuthRequest{Username: username, Password: password})
84		if err != nil {
85			return "", fmt.Errorf("connector: encoding vendor auth request: %w", err)
86		}
87	
88		// A fresh *http.Request built from scratch on every call — no
89		// client-level default header exists to leak across calls, and this
90		// request object is never retained past this function returning.
91		req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+"/auth", bytes.NewReader(reqBody))
92		if err != nil {
93			return "", fmt.Errorf("connector: building vendor auth request: %w", err)
94		}
95		req.Header.Set("Content-Type", "application/json")
96	
97		resp, err := c.httpClient.Do(req)
98		if err != nil {
99			// Never wrap or surface the raw error to our own caller —
100			// connector-security.md §4: vendor errors must not distinguish
101			// which part of the credential was wrong, and a raw dial/DNS
102			// error can itself carry sensitive detail (host, IP). The
103			// caller-facing generic failure is this package's caller's job
104			// (internal/connector's handlers); this method's contract is
105			// simply "success or ErrVendorAuthFailed", nothing richer.
106			return "", ErrVendorAuthFailed
107		}
108		defer func() { _, _ = io.Copy(io.Discard, resp.Body); _ = resp.Body.Close() }()
109	
110		if resp.StatusCode != http.StatusOK {
111			// Do not read/log the response body — connector-security.md §3:
112			// "do not log raw vendor response bodies by default, since a
113			// vendor error payload can itself echo back submitted PII or
114			// partial credentials."
115			return "", ErrVendorAuthFailed
116		}
117	
118		var body vendorAuthResponse
119		if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&body); err != nil || body.AccessToken == "" {
120			return "", ErrVendorAuthFailed
121		}
122		return body.AccessToken, nil
123	}
124	
125	type vendorIdentityRequest struct {
126		Phone string `json:"phone"`
127		Name  string `json:"name"`
128	}
129	
130	type vendorIdentityResponse struct {
131		Name          string `json:"name"`
132		Phone         string `json:"phone"`
133		StreetAddress string `json:"street_address"`
134		Locality      string `json:"locality"`
135		Region        string `json:"region"`
136		PostalCode    string `json:"postal_code"`
137		Country       string `json:"country"`
138	}
139	
140	func (c *HTTPVendorClient) FetchIdentity(ctx context.Context, accessToken, phone, name string) (Identity, error) {
141		reqBody, err := json.Marshal(vendorIdentityRequest{Phone: phone, Name: name})
142		if err != nil {
143			return Identity{}, fmt.Errorf("connector: encoding vendor identity request: %w", err)
144		}
145	
146		req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+"/identity", bytes.NewReader(reqBody))
147		if err != nil {
148			return Identity{}, fmt.Errorf("connector: building vendor identity request: %w", err)
149		}
150		req.Header.Set("Content-Type", "application/json")
151		// The one place accessToken is used — set on this request only,
152		// never on a client-level default header, never retained after this
153		// function returns.
154		req.Header.Set("Authorization", "Bearer "+accessToken)
155	
156		resp, err := c.httpClient.Do(req)
157		if err != nil {
158			return Identity{}, ErrVendorIdentityNotFound
159		}
160		defer func() { _, _ = io.Copy(io.Discard, resp.Body); _ = resp.Body.Close() }()
161	
162		if resp.StatusCode != http.StatusOK {
163			return Identity{}, ErrVendorIdentityNotFound
164		}
165	
166		var body vendorIdentityResponse
167		if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&body); err != nil {
168			return Identity{}, ErrVendorIdentityNotFound
169		}
170		return Identity{
171			Name:          body.Name,
172			Phone:         body.Phone,
173			StreetAddress: body.StreetAddress,
174			Locality:      body.Locality,
175			Region:        body.Region,
176			PostalCode:    body.PostalCode,
177			Country:       body.Country,
178		}, nil
179	}
180	
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/vendorclient.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/vendorclient.go"
}
result
1	// Package connector implements Q3 — cmd/idp-connector's outbound calls
2	// to a generic third-party identity provider ("ABC"/"XYZ" in the
3	// assignment's own generic framing), per
4	// 02-ai-security-architecture/connector-security.md and
5	// refinement/LT-41.md. No real vendor integration exists or is asked
6	// for (LT-41's own non-goals) — VendorClient is the seam a real vendor
7	// adapter would implement; this package ships a stub (stubvendorclient.go)
8	// good enough to demonstrate the connector's own contract and security
9	// properties end to end, and an HTTP-based implementation
10	// (httpvendorclient.go) that demonstrates the outbound security
11	// discipline connector-security.md requires, callable against any vendor
12	// that happens to share this project's own JSON shape (a stated
13	// simplifying assumption, not a claim about any real vendor's actual
14	// API).
15	package connector
16	
17	import "context"
18	
19	// Identity is the PII a vendor's /identity endpoint returns, matching
20	// the assignment's own field list verbatim — name, phone, and address
21	// (street_address, locality, region, postal_code, country).
22	type Identity struct {
23		Name          string
24		Phone         string
25		StreetAddress string
26		Locality      string
27		Region        string
28		PostalCode    string
29		Country       string
30	}
31	
32	// VendorClient is this connector's seam to a third-party IDP. Both
33	// methods take the vendor bearer token as an explicit parameter, never a
34	// client-level field — connector-security.md §1's fetch-use-zeroize
35	// boundary requires the token's scope to run from the point it's read
36	// off the request to the end of that single call, which a client-level
37	// field would silently violate by keeping it alive for the client's own
38	// lifetime instead.
39	type VendorClient interface {
40		// Authenticate exchanges an end-user's vendor username/password for
41		// a vendor access token, per the assignment's own POST /auth
42		// contract. Returns a generic error on any failure — the specific
43		// reason (unknown username vs. wrong password vs. vendor
44		// unreachable) is for this package's own internal handling only,
45		// never surfaced to our caller (connector-security.md §4:
46		// "must not leak which part of the credential was wrong").
47		Authenticate(ctx context.Context, username, password string) (accessToken string, err error)
48	
49		// FetchIdentity looks up PII by phone/name using an already-obtained
50		// vendor accessToken, per the assignment's own POST /identity
51		// contract. accessToken is used for this one call only and is never
52		// retained by the implementation past this call returning.
53		FetchIdentity(ctx context.Context, accessToken, phone, name string) (Identity, error)
54	}
55	
Sep 13, 6:16 AM PDTOren Castellan
This checks out fine for #1 and #3 mostly. Now the handlers, router, and rate-limiter reuse.
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/handlers.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/handlers.go"
}
result
1	package connector
2	
3	import (
4		"context"
5		"encoding/json"
6		"io"
7		"net"
8		"net/http"
9		"strings"
10		"time"
11	
12		"loginid-takehome/internal/api"
13	)
14	
15	// identityFailureFloor is /identity's uniform response-time floor across
16	// its three failure branches (no-token, bad-token, vendor-rejected) —
17	// Marcus Ilori's (02) ruling on Tobias Lindqvist's timing-oracle
18	// objection (refinement/LT-41.md), recorded in full in
19	// connector-security.md §4. ~100ms, well under any context.WithTimeout
20	// budget elsewhere in the chain (vendorCallTimeout is 10s) so it never
21	// competes with those deadlines. This narrows, but does not fully close,
22	// the gap between a local-only failure (microseconds) and a
23	// vendor-round-trip failure (tens-hundreds of ms) — full closure is an
24	// accepted non-goal per that ruling.
25	const identityFailureFloor = 100 * time.Millisecond
26	
27	// identityErrorBody is the one JSON shape every /identity failure
28	// returns, regardless of which of the three branches produced it —
29	// content-identical by construction (a single writeIdentityFailure call
30	// site below), not by three call sites happening to independently agree.
31	type identityErrorBody struct {
32		Error string `json:"error"`
33	}
34	
35	// vendorLimiter is the interface these handlers depend on — satisfied
36	// directly by api.GrantLimiter (PR #30/#34's already-built component,
37	// reused here rather than rebuilt per LT-41's own non-goals: "this
38	// story's own in-process rate limiting is sufficient, matching the
39	// pattern already used for RateLimiter/TouchCounter/GrantLimiter").
40	// Declared locally so this package's own tests can supply a minimal fake
41	// satisfying only the calls actually made here, per
42	// decisions/test-double-strategy.md.
43	type vendorLimiter interface {
44		Allow(ctx context.Context, clientID, sourceIP string) (bool, error)
45		RecordFailure(ctx context.Context, clientID, sourceIP string)
46		RecordSuccess(ctx context.Context, clientID, sourceIP string)
47	}
48	
49	// Deps bundles NewAuthHandler/NewIdentityHandler's collaborators.
50	type Deps struct {
51		Vendor   VendorClient
52		Limiter  vendorLimiter
53		AuditLog api.AuditLogger
54	}
55	
56	func sourceIPFrom(r *http.Request) string {
57		host, _, err := net.SplitHostPort(r.RemoteAddr)
58		if err != nil {
59			return r.RemoteAddr
60		}
61		return host
62	}
63	
64	func writeJSON(w http.ResponseWriter, status int, v any) {
65		w.Header().Set("Content-Type", "application/json")
66		w.WriteHeader(status)
67		_ = json.NewEncoder(w).Encode(v)
68	}
69	
70	// writeIdentityFailure is the single call site for every /identity
71	// failure response — the one place that guarantees byte-identical
72	// content across the no-token, bad-token, and vendor-rejected branches
73	// (connector-security.md §1/F3), never three call sites independently
74	// trying to agree.
75	func writeIdentityFailure(w http.ResponseWriter) {
76		writeJSON(w, http.StatusUnauthorized, identityErrorBody{Error: "identity_lookup_failed"})
77	}
78	
79	// waitForFloor sleeps out the remainder of identityFailureFloor since
80	// start, if any — applied uniformly across /identity's three failure
81	// branches so none of them is distinguishable from the others by
82	// response latency alone (partial mitigation per connector-security.md
83	// §4's own accepted-residual language; not applied to the success path,
84	// which was never part of the timing-oracle concern).
85	func waitForFloor(start time.Time) {
86		if remaining := identityFailureFloor - time.Since(start); remaining > 0 {
87			time.Sleep(remaining)
88		}
89	}
90	
91	// authRequestBody is the assignment's own fixed POST /auth contract.
92	type authRequestBody struct {
93		Username string `json:"username"`
94		Password string `json:"password"`
95	}
96	
97	type authResponseBody struct {
98		AccessToken string `json:"access_token"`
99	}
100	
101	// NewAuthHandler implements POST /auth. The caller (in this project's
102	// shape, cmd/api-service acting on an onboarding flow's behalf) must
103	// already be authenticated with a connector:identity-lookup-scoped,
104	// idp-connector-service-audience token — enforced by whatever middleware
105	// wraps this handler (internal/app.NewConnectorRouter), not by this
106	// function; AuthContextFromContext here is read-only identification for
107	// rate-limiting and audit purposes.
108	func NewAuthHandler(deps Deps) http.HandlerFunc {
109		return func(w http.ResponseWriter, r *http.Request) {
110			ctx := r.Context()
111			ac, _ := api.AuthContextFromContext(ctx)
112			sourceIP := sourceIPFrom(r)
113	
114			allowed, err := deps.Limiter.Allow(ctx, ac.Sub, sourceIP)
115			if err != nil {
116				writeJSON(w, http.StatusInternalServerError, identityErrorBody{Error: "server_error"})
117				return
118			}
119			if !allowed {
120				writeJSON(w, http.StatusTooManyRequests, identityErrorBody{Error: "slow_down"})
121				return
122			}
123	
124			var body authRequestBody
125			if err := json.NewDecoder(io.LimitReader(r.Body, 1<<16)).Decode(&body); err != nil {
126				writeJSON(w, http.StatusBadRequest, identityErrorBody{Error: "invalid_request"})
127				return
128			}
129	
130			// accessToken is scoped to this block only — read from the
131			// vendor call's return value, written straight into the
132			// response, never assigned to any wider-scoped variable or
133			// logged. This IS the narrowest this value can be scoped given
134			// /auth's own contract: its job is delivering the token to the
135			// caller, so it necessarily reaches the response writer, but
136			// nothing else.
137			accessToken, err := deps.Vendor.Authenticate(ctx, body.Username, body.Password)
138			if err != nil {
139				deps.Limiter.RecordFailure(ctx, ac.Sub, sourceIP)
140				logAuditOutcome(ctx, deps.AuditLog, ac.Sub, api.AuditAuthnFailure)
141				// Generic failure regardless of *why* the vendor rejected
142				// it (unknown username vs. wrong password vs. vendor
143				// unreachable) — connector-security.md §4: "must not leak
144				// which part of the credential was wrong."
145				writeJSON(w, http.StatusUnauthorized, identityErrorBody{Error: "vendor_authentication_failed"})
146				return
147			}
148			deps.Limiter.RecordSuccess(ctx, ac.Sub, sourceIP)
149			logAuditOutcome(ctx, deps.AuditLog, ac.Sub, api.AuditSuccess)
150	
151			writeJSON(w, http.StatusOK, authResponseBody{AccessToken: accessToken})
152		}
153	}
154	
155	// identityRequestBody is the assignment's own fixed POST /identity
156	// contract — no credential field, since the vendor token travels as a
157	// request header instead (connector-security.md §1).
158	type identityRequestBody struct {
159		Phone string `json:"phone"`
160		Name  string `json:"name"`
161	}
162	
163	type identityResponseBody struct {
164		Name          string `json:"name"`
165		Phone         string `json:"phone"`
166		StreetAddress string `json:"street_address"`
167		Locality      string `json:"locality"`
168		Region        string `json:"region"`
169		PostalCode    string `json:"postal_code"`
170		Country       string `json:"country"`
171	}
172	
173	// vendorAccessTokenHeader is the header /identity accepts the vendor's
174	// access_token on — a header, not a body field, since the assignment's
175	// fixed {"phone","name"} body has no credential field (LT-41 acceptance
176	// criteria).
177	const vendorAccessTokenHeader = "X-Vendor-Access-Token"
178	
179	// NewIdentityHandler implements POST /identity. Every failure path
180	// (missing token, malformed-looking token, vendor rejection) returns
181	// through writeIdentityFailure after waitForFloor — the two calls
182	// together are what make the three branches content- and
183	// (approximately) time-identical, per connector-security.md §1/§4.
184	func NewIdentityHandler(deps Deps) http.HandlerFunc {
185		return func(w http.ResponseWriter, r *http.Request) {
186			start := time.Now()
187			ctx := r.Context()
188			ac, _ := api.AuthContextFromContext(ctx)
189			sourceIP := sourceIPFrom(r)
190	
191			allowed, err := deps.Limiter.Allow(ctx, ac.Sub, sourceIP)
192			if err != nil {
193				writeJSON(w, http.StatusInternalServerError, identityErrorBody{Error: "server_error"})
194				return
195			}
196			if !allowed {
197				// Not one of the three timing-oracle branches (it's an
198				// overt, openly-disclosed 429, not a covert signal) — no
199				// floor applied.
200				writeJSON(w, http.StatusTooManyRequests, identityErrorBody{Error: "slow_down"})
201				return
202			}
203	
204			var body identityRequestBody
205			if err := json.NewDecoder(io.LimitReader(r.Body, 1<<16)).Decode(&body); err != nil {
206				// A malformed body is a caller bug, not one of the three
207				// named credential/token-validity branches — ordinary 400,
208				// no floor.
209				writeJSON(w, http.StatusBadRequest, identityErrorBody{Error: "invalid_request"})
210				return
211			}
212	
213			token := strings.TrimSpace(r.Header.Get(vendorAccessTokenHeader))
214			if token == "" {
215				deps.Limiter.RecordFailure(ctx, ac.Sub, sourceIP)
216				logAuditOutcome(ctx, deps.AuditLog, ac.Sub, api.AuditAuthnFailure)
217				waitForFloor(start)
218				writeIdentityFailure(w)
219				return
220			}
221	
222			// token is scoped to this function only, passed by value into
223			// FetchIdentity, and never assigned to any variable outside
224			// this block or returned — attack-tree leaf 4's control
225			// (connector-security.md §1/Attack-tree row 4).
226			identity, err := deps.Vendor.FetchIdentity(ctx, token, body.Phone, body.Name)
227			if err != nil {
228				deps.Limiter.RecordFailure(ctx, ac.Sub, sourceIP)
229				logAuditOutcome(ctx, deps.AuditLog, ac.Sub, api.AuditAuthnFailure)
230				waitForFloor(start)
231				writeIdentityFailure(w)
232				return
233			}
234			deps.Limiter.RecordSuccess(ctx, ac.Sub, sourceIP)
235			logAuditOutcome(ctx, deps.AuditLog, ac.Sub, api.AuditSuccess)
236	
237			// Success is not part of the timing-oracle concern — no floor.
238			writeJSON(w, http.StatusOK, identityResponseBody{
239				Name:          identity.Name,
240				Phone:         identity.Phone,
241				StreetAddress: identity.StreetAddress,
242				Locality:      identity.Locality,
243				Region:        identity.Region,
244				PostalCode:    identity.PostalCode,
245				Country:       identity.Country,
246			})
247		}
248	}
249	
250	// logAuditOutcome logs caller sub/scope/timestamp/outcome only — never
251	// the vendor username/password, vendor access_token, or any PII field
252	// value, per connector-security.md §3/§5's audit discipline.
253	func logAuditOutcome(ctx context.Context, auditLog api.AuditLogger, sub string, kind api.AuditEventKind) {
254		if auditLog == nil {
255			return
256		}
257		auditLog.Log(ctx, api.AuditEvent{
258			Sub:       sub,
259			Scope:     api.ScopeConnectorIdentityLookup,
260			Kind:      kind,
261			Timestamp: time.Now().UTC(),
262		})
263	}
264	
Sep 13, 6:16 AM PDTOren Castellan
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -rl "GrantLimiter" internal/api/ | h
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -rl \"GrantLimiter\" internal/api/ | head"
}
result
internal/api/grantlimiter_test.go
internal/api/tokenhandler_test.go
internal/api/clientstore.go
internal/api/grantlimiter.go
internal/api/tokenhandler.go
internal/api/issuer_verifier_integration_test.go
Sep 13, 6:16 AM PDTOren Castellan
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/api/grantlimiter.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/api/grantlimiter.go"
}
result
1	package api
2	
3	import (
4		"context"
5		"sync"
6		"time"
7	)
8	
9	// GrantLimiter enforces the token-issuing endpoint's own failed-grant
10	// rate limiting and progressive backoff — handoff-03-auth.md v3:
11	// "Token-endpoint brute-force protection, separate from the resource-API
12	// rate limits ... failed-grant rate limiting and progressive backoff per
13	// client_id and per source IP on the token-issuing endpoint itself, plus
14	// alerting on a sustained run of failed grants against one client_id."
15	//
16	// This is genuinely separate from RateLimiter (ratelimit.go): that type
17	// gates the resource API by an already-authenticated sub, at a fixed
18	// per-minute rate; this one gates the endpoint that issues the
19	// credential in the first place, keyed independently by client_id AND
20	// by source IP — a distributed brute force (many IPs, one client_id)
21	// and a single-source credential-stuffing run (one IP, many client_ids)
22	// are both attack shapes this is meant to slow down, and either key
23	// exceeding its own backoff blocks the attempt.
24	//
25	// Not wired to a live endpoint by this story: /auth/token's real grant
26	// logic is LT-51's scope (this story's non-goals). This is the
27	// ready-to-use component LT-51 constructs and calls from the real
28	// handler — building it now, tested in isolation, means LT-51 doesn't
29	// also have to design a backoff/alerting scheme under its own deadline.
30	type GrantLimiter interface {
31		// Allow reports whether a grant attempt for clientID from sourceIP
32		// may proceed right now — false while either key is in backoff.
33		Allow(ctx context.Context, clientID, sourceIP string) (bool, error)
34		// RecordFailure records a failed grant attempt for both clientID and
35		// sourceIP, advancing each key's backoff state independently.
36		// Crossing the alert threshold for a given client_id (not source IP
37		// — the requirement names client_id specifically) fires alert
38		// exactly once per crossing, not once per subsequent failure.
39		RecordFailure(ctx context.Context, clientID, sourceIP string)
40		// RecordSuccess clears backoff state for both clientID and
41		// sourceIP — a successful grant resets the count, standard
42		// exponential-backoff practice: this deters sustained brute force,
43		// it does not permanently degrade a legitimate client that mistyped
44		// a secret once.
45		RecordSuccess(ctx context.Context, clientID, sourceIP string)
46	}
47	
48	// grantBackoffState is one key's (a client_id's, or a source IP's)
49	// consecutive-failure bookkeeping.
50	type grantBackoffState struct {
51		consecutiveFailures int
52		blockedUntil        time.Time
53		alerted             bool
54	}
55	
56	// InProcessGrantLimiter is an in-process implementation. As with
57	// RateLimiter/TouchCounter's in-process implementations, a shared store
58	// is the multi-instance replacement behind the same interface — the
59	// mechanism is this track's call, per handoff-03-auth.md v3.
60	//
61	// Both maps are swept periodically (see sweepLocked) rather than left to
62	// grow forever: this component's own doc comment names "a distributed
63	// brute force (many IPs, one client_id)" as a defended attack shape, and
64	// that is exactly the traffic pattern that would otherwise grow
65	// bySourceIP without bound — every distinct attacking IP earns a
66	// permanent entry that only a matching RecordSuccess would remove, which
67	// an attacking IP by definition never triggers (Nolan Reyes, PR #30
68	// review — the same unbounded-growth shape an earlier PR's TouchCounter
69	// review already found and fixed once).
70	type InProcessGrantLimiter struct {
71		mu             sync.Mutex
72		byClientID     map[string]*grantBackoffState
73		bySourceIP     map[string]*grantBackoffState
74		baseBackoff    time.Duration
75		maxBackoff     time.Duration
76		alertThreshold int
77		alertFn        func(clientID string, consecutiveFailures int)
78		now            func() time.Time
79		lastSweep      time.Time
80	}
81	
82	// grantSweepInterval and grantEntryTTL bound how long a key's state
83	// survives with no further failures against it. A blocked key is never
84	// swept before its own backoff has elapsed (sweeping mid-block would
85	// silently un-block it early); grantEntryTTL is generous past any
86	// realistic maxBackoff so a legitimately still-escalating attacker isn't
87	// reset early either.
88	const (
89		grantSweepInterval = time.Minute
90		grantEntryTTL      = time.Hour
91	)
92	
93	// NewInProcessGrantLimiter constructs a limiter with exponential backoff
94	// starting at baseBackoff (doubling per consecutive failure, capped at
95	// maxBackoff) and alertFn called once per client_id crossing
96	// alertThreshold consecutive failures. alertFn may be nil (no alerting —
97	// tests use this; production wiring passes a real sink).
98	func NewInProcessGrantLimiter(baseBackoff, maxBackoff time.Duration, alertThreshold int, alertFn func(clientID string, consecutiveFailures int)) *InProcessGrantLimiter {
99		return &InProcessGrantLimiter{
100			byClientID:     make(map[string]*grantBackoffState),
101			bySourceIP:     make(map[string]*grantBackoffState),
102			baseBackoff:    baseBackoff,
103			maxBackoff:     maxBackoff,
104			alertThreshold: alertThreshold,
105			alertFn:        alertFn,
106			now:            time.Now,
107		}
108	}
109	
110	func (l *InProcessGrantLimiter) Allow(_ context.Context, clientID, sourceIP string) (bool, error) {
111		l.mu.Lock()
112		defer l.mu.Unlock()
113		now := l.now()
114		l.sweepLocked(now)
115		return !isBlocked(l.byClientID[clientID], now) && !isBlocked(l.bySourceIP[sourceIP], now), nil
116	}
117	
118	// sweepLocked drops any entry whose blockedUntil is at least
119	// grantEntryTTL in the past — called with l.mu already held, at most
120	// once per grantSweepInterval, so it adds no per-call cost beyond a
121	// single time comparison on every other call.
122	func (l *InProcessGrantLimiter) sweepLocked(now time.Time) {
123		if now.Sub(l.lastSweep) < grantSweepInterval {
124			return
125		}
126		l.lastSweep = now
127		sweepMap(l.byClientID, now)
128		sweepMap(l.bySourceIP, now)
129	}
130	
131	func sweepMap(m map[string]*grantBackoffState, now time.Time) {
132		for key, s := range m {
133			if now.Sub(s.blockedUntil) >= grantEntryTTL {
134				delete(m, key)
135			}
136		}
137	}
138	
139	func isBlocked(s *grantBackoffState, now time.Time) bool {
140		return s != nil && now.Before(s.blockedUntil)
141	}
142	
143	func (l *InProcessGrantLimiter) RecordFailure(_ context.Context, clientID, sourceIP string) {
144		l.mu.Lock()
145		defer l.mu.Unlock()
146		now := l.now()
147	
148		cs := l.advance(l.byClientID, clientID, now)
149		l.advance(l.bySourceIP, sourceIP, now)
150	
151		if cs.consecutiveFailures >= l.alertThreshold && !cs.alerted {
152			cs.alerted = true
153			if l.alertFn != nil {
154				l.alertFn(clientID, cs.consecutiveFailures)
155			}
156		}
157	}
158	
159	// advance increments key's consecutive-failure count and sets its
160	// exponential backoff window, creating the state entry if this is its
161	// first failure.
162	func (l *InProcessGrantLimiter) advance(m map[string]*grantBackoffState, key string, now time.Time) *grantBackoffState {
163		s, ok := m[key]
164		if !ok {
165			s = &grantBackoffState{}
166			m[key] = s
167		}
168		s.consecutiveFailures++
169		backoff := l.baseBackoff << uint(s.consecutiveFailures-1) // 2^(n-1) * base
170		if backoff > l.maxBackoff || backoff <= 0 {               // overflow guard: a left-shifted duration can wrap negative
171			backoff = l.maxBackoff
172		}
173		s.blockedUntil = now.Add(backoff)
174		return s
175	}
176	
177	func (l *InProcessGrantLimiter) RecordSuccess(_ context.Context, clientID, sourceIP string) {
178		l.mu.Lock()
179		defer l.mu.Unlock()
180		delete(l.byClientID, clientID)
181		delete(l.bySourceIP, sourceIP)
182	}
183	
Sep 13, 6:16 AM PDTOren Castellan
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/handlers_test.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/handlers_test.go"
}
result
1	package connector
2	
3	import (
4		"context"
5		"encoding/json"
6		"net/http"
7		"net/http/httptest"
8		"strings"
9		"testing"
10		"time"
11	
12		"loginid-takehome/internal/api"
13	)
14	
15	// fakeVendorClient implements VendorClient with configurable funcs, per
16	// decisions/test-double-strategy.md.
17	type fakeVendorClient struct {
18		authFn     func(ctx context.Context, username, password string) (string, error)
19		identityFn func(ctx context.Context, accessToken, phone, name string) (Identity, error)
20	}
21	
22	func (f *fakeVendorClient) Authenticate(ctx context.Context, username, password string) (string, error) {
23		return f.authFn(ctx, username, password)
24	}
25	func (f *fakeVendorClient) FetchIdentity(ctx context.Context, accessToken, phone, name string) (Identity, error) {
26		return f.identityFn(ctx, accessToken, phone, name)
27	}
28	
29	func withAuthContext(sub string) func(*http.Request) *http.Request {
30		return func(r *http.Request) *http.Request {
31			return r.WithContext(api.WithAuthContext(r.Context(), api.AuthContext{Sub: sub, Scope: api.ScopeConnectorIdentityLookup}))
32		}
33	}
34	
35	func TestNewAuthHandler_Success(t *testing.T) {
36		vendor := &fakeVendorClient{
37			authFn: func(_ context.Context, username, password string) (string, error) {
38				if username == "u" && password == "p" {
39					return "vendor-token-abc", nil
40				}
41				return "", ErrVendorAuthFailed
42			},
43		}
44		limiter := api.NewInProcessGrantLimiter(time.Second, time.Minute, 5, nil)
45		h := NewAuthHandler(Deps{Vendor: vendor, Limiter: limiter, AuditLog: api.StdoutAuditLogger{}})
46	
47		body := strings.NewReader(`{"username":"u","password":"p"}`)
48		req := withAuthContext("caller-1")(httptest.NewRequest(http.MethodPost, "/auth", body))
49		w := httptest.NewRecorder()
50		h.ServeHTTP(w, req)
51	
52		if w.Code != http.StatusOK {
53			t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String())
54		}
55		var resp authResponseBody
56		if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
57			t.Fatalf("decoding response: %v", err)
58		}
59		if resp.AccessToken != "vendor-token-abc" {
60			t.Errorf("access_token = %q, want vendor-token-abc", resp.AccessToken)
61		}
62	}
63	
64	func TestNewAuthHandler_VendorRejects_GenericFailure(t *testing.T) {
65		vendor := &fakeVendorClient{
66			authFn: func(context.Context, string, string) (string, error) { return "", ErrVendorAuthFailed },
67		}
68		limiter := api.NewInProcessGrantLimiter(time.Second, time.Minute, 5, nil)
69		h := NewAuthHandler(Deps{Vendor: vendor, Limiter: limiter, AuditLog: api.StdoutAuditLogger{}})
70	
71		req := withAuthContext("caller-1")(httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(`{"username":"u","password":"wrong"}`)))
72		w := httptest.NewRecorder()
73		h.ServeHTTP(w, req)
74	
75		if w.Code != http.StatusUnauthorized {
76			t.Fatalf("status = %d, want 401", w.Code)
77		}
78		var body identityErrorBody
79		if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
80			t.Fatalf("decoding body: %v", err)
81		}
82		if body.Error != "vendor_authentication_failed" {
83			t.Errorf("error = %q — must not distinguish unknown-username from wrong-password (connector-security.md §4)", body.Error)
84		}
85	}
86	
87	// TestNewIdentityHandler_ThreeFailureBranches_ContentIdenticalAndTimingClose
88	// is the enforcement site for both F3 (content) and Marcus Ilori's
89	// ~100ms floor ruling (timing) — refinement/LT-41.md's own named test
90	// shape: no-token, bad-token, and vendor-rejected must produce
91	// byte-identical response bodies/status, and must return within a
92	// bounded time delta of each other, not merely "both fast" or "both
93	// slow" independently.
94	func TestNewIdentityHandler_ThreeFailureBranches_ContentIdenticalAndTimingClose(t *testing.T) {
95		// "bad token" — a local, near-instant rejection.
96		badTokenVendor := &fakeVendorClient{
97			identityFn: func(context.Context, string, string, string) (Identity, error) {
98				return Identity{}, ErrVendorIdentityNotFound
99			},
100		}
101		// "vendor rejected it" — simulates an actual network round-trip's
102		// latency (well under the floor, so the floor is what equalizes it).
103		vendorRejectedVendor := &fakeVendorClient{
104			identityFn: func(context.Context, string, string, string) (Identity, error) {
105				time.Sleep(15 * time.Millisecond)
106				return Identity{}, ErrVendorIdentityNotFound
107			},
108		}
109	
110		// Each call gets its own fresh limiter — this test is about content/
111		// timing equivalence across failure branches, not rate-limiting
112		// interaction, so a shared limiter's backoff from an earlier call in
113		// this same test must not affect a later one.
114		runCase := func(vendor VendorClient, sendToken bool) (int, []byte, time.Duration) {
115			limiter := api.NewInProcessGrantLimiter(time.Second, time.Minute, 5, nil)
116			h := NewIdentityHandler(Deps{Vendor: vendor, Limiter: limiter, AuditLog: api.StdoutAuditLogger{}})
117			req := withAuthContext("caller-1")(httptest.NewRequest(http.MethodPost, "/identity", strings.NewReader(`{"phone":"+15555550100","name":"Jane"}`)))
118			if sendToken {
119				req.Header.Set(vendorAccessTokenHeader, "some-token")
120			}
121			w := httptest.NewRecorder()
122			start := time.Now()
123			h.ServeHTTP(w, req)
124			elapsed := time.Since(start)
125			return w.Code, w.Body.Bytes(), elapsed
126		}
127	
128		noTokenStatus, noTokenBody, noTokenElapsed := runCase(badTokenVendor, false)
129		badTokenStatus, badTokenBody, badTokenElapsed := runCase(badTokenVendor, true)
130		vendorRejectedStatus, vendorRejectedBody, vendorRejectedElapsed := runCase(vendorRejectedVendor, true)
131	
132		if noTokenStatus != http.StatusUnauthorized || badTokenStatus != http.StatusUnauthorized || vendorRejectedStatus != http.StatusUnauthorized {
133			t.Fatalf("statuses = %d, %d, %d, want all 401", noTokenStatus, badTokenStatus, vendorRejectedStatus)
134		}
135		if string(noTokenBody) != string(badTokenBody) || string(badTokenBody) != string(vendorRejectedBody) {
136			t.Errorf("response bodies differ: no-token=%q bad-token=%q vendor-rejected=%q — must be byte-identical (connector-security.md §1/F3)",
137				noTokenBody, badTokenBody, vendorRejectedBody)
138		}
139	
140		// All three must land close to the ~100ms floor (identityFailureFloor),
141		// not just "all fast" or "all slow" independently — the floor pads
142		// the fast paths UP, it doesn't cap the slow path down, so every
143		// branch should be at least the floor and not wildly divergent from
144		// it (a generous upper bound accounts for test-environment jitter).
145		for name, elapsed := range map[string]time.Duration{
146			"no-token":        noTokenElapsed,
147			"bad-token":       badTokenElapsed,
148			"vendor-rejected": vendorRejectedElapsed,
149		} {
150			if elapsed < identityFailureFloor {
151				t.Errorf("%s branch returned in %v, want at least the %v floor", name, elapsed, identityFailureFloor)
152			}
153			if elapsed > identityFailureFloor+50*time.Millisecond {
154				t.Errorf("%s branch returned in %v, want within ~50ms of the %v floor", name, elapsed, identityFailureFloor)
155			}
156		}
157	}
158	
159	func TestNewIdentityHandler_Success_NoFloorApplied(t *testing.T) {
160		limiter := api.NewInProcessGrantLimiter(time.Second, time.Minute, 5, nil)
161		vendor := &fakeVendorClient{
162			identityFn: func(context.Context, string, string, string) (Identity, error) {
163				return Identity{Name: "Jane Demo", Phone: "+15555550100", Country: "US"}, nil
164			},
165		}
166		h := NewIdentityHandler(Deps{Vendor: vendor, Limiter: limiter, AuditLog: api.StdoutAuditLogger{}})
167	
168		req := withAuthContext("caller-1")(httptest.NewRequest(http.MethodPost, "/identity", strings.NewReader(`{"phone":"+15555550100","name":"Jane"}`)))
169		req.Header.Set(vendorAccessTokenHeader, "good-token")
170		w := httptest.NewRecorder()
171		start := time.Now()
172		h.ServeHTTP(w, req)
173		elapsed := time.Since(start)
174	
175		if w.Code != http.StatusOK {
176			t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String())
177		}
178		if elapsed >= identityFailureFloor {
179			t.Errorf("success path took %v — the failure floor must not apply to a success response", elapsed)
180		}
181		var body identityResponseBody
182		if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
183			t.Fatalf("decoding response: %v", err)
184		}
185		if body.Name != "Jane Demo" || body.Country != "US" {
186			t.Errorf("unexpected identity in response: %+v", body)
187		}
188	}
189	
190	// TestVendorCallLimiter_FailureRatioTripsBeforeVolume is LT-41's own
191	// named enforcement site for attack-tree leaf 5: a caller with a high
192	// FAILURE ratio must be blocked before a caller with high VOLUME but
193	// mostly successful calls ever would be. Reuses api.GrantLimiter (PR
194	// #30/#34) directly, per this story's non-goal ("no shared rate-limit
195	// store... this story's own in-process rate limiting is sufficient,
196	// matching the pattern already used for RateLimiter/TouchCounter/
197	// GrantLimiter") — GrantLimiter's backoff is driven purely by
198	// CONSECUTIVE failures (RecordSuccess resets it to zero), so a caller
199	// who never has two failures in a row can generate unlimited volume
200	// without ever tripping the limiter, while a caller failing repeatedly
201	// trips within a handful of attempts.
202	func TestVendorCallLimiter_FailureRatioTripsBeforeVolume(t *testing.T) {
203		limiter := api.NewInProcessGrantLimiter(10*time.Millisecond, time.Hour, 100, nil)
204		ctx := context.Background()
205	
206		// High-failure-ratio caller: fails every attempt in a row.
207		blockedAfter := -1
208		for i := 0; i < 10; i++ {
209			allowed, err := limiter.Allow(ctx, "attacker", "10.0.0.1")
210			if err != nil {
211				t.Fatalf("Allow: %v", err)
212			}
213			if !allowed {
214				blockedAfter = i
215				break
216			}
217			limiter.RecordFailure(ctx, "attacker", "10.0.0.1")
218		}
219		if blockedAfter == -1 {
220			t.Fatalf("high-failure-ratio caller was never blocked across 10 consecutive failures")
221		}
222	
223		// High-volume-but-mostly-successful caller: never blocked, no matter
224		// how many attempts, because every attempt succeeds (no consecutive
225		// failures ever accumulate).
226		for i := 0; i < 1000; i++ {
227			allowed, err := limiter.Allow(ctx, "legit-client", "10.0.0.2")
228			if err != nil {
229				t.Fatalf("Allow: %v", err)
230			}
231			if !allowed {
232				t.Fatalf("high-volume-but-successful caller was blocked after %d attempts, want never (volume alone must not trip this limiter)", i)
233			}
234			limiter.RecordSuccess(ctx, "legit-client", "10.0.0.2")
235		}
236	
237		if blockedAfter >= 1000 {
238			t.Fatalf("failure-ratio caller (blocked at attempt %d) did not trip before the 1000-attempt volume caller (never tripped) — want the opposite", blockedAfter)
239		}
240	}
241	
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/httpvendorclient_test.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/httpvendorclient_test.go"
}
result
1	package connector
2	
3	import (
4		"context"
5		"encoding/json"
6		"net/http"
7		"net/http/httptest"
8		"sync"
9		"testing"
10		"time"
11	)
12	
13	func TestNewHTTPVendorClient_RejectsNonHTTPS(t *testing.T) {
14		if _, err := NewHTTPVendorClient("http://vendor.example.com", nil); err == nil {
15			t.Errorf("NewHTTPVendorClient accepted a plain http:// base URL, want an error (connector-security.md §2: TLS only)")
16		}
17		if _, err := NewHTTPVendorClient("not-a-url", nil); err == nil {
18			t.Errorf("NewHTTPVendorClient accepted a garbage base URL, want an error")
19		}
20	}
21	
22	// TestHTTPVendorClient_FetchIdentity_FreshAuthorizationHeaderPerRequest
23	// is attack-tree leaf 1's own named enforcement site (refinement/LT-41.md):
24	// two sequential calls through the SAME pooled *http.Client, each
25	// presenting a DIFFERENT vendor token, must each produce an outbound
26	// Authorization header matching ITS OWN token — never the previous
27	// call's, which a client that held a shared/default header (instead of
28	// building it fresh per call) could otherwise leak across requests via
29	// connection reuse.
30	func TestHTTPVendorClient_FetchIdentity_FreshAuthorizationHeaderPerRequest(t *testing.T) {
31		var mu sync.Mutex
32		var observedHeaders []string
33	
34		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
35			mu.Lock()
36			observedHeaders = append(observedHeaders, r.Header.Get("Authorization"))
37			mu.Unlock()
38			w.Header().Set("Content-Type", "application/json")
39			_ = json.NewEncoder(w).Encode(vendorIdentityResponse{Name: "ok"})
40		}))
41		defer srv.Close()
42	
43		// Construct directly (bypassing NewHTTPVendorClient's https-only
44		// check, which httptest.Server's plain-http URL would otherwise
45		// fail) — this file is in the same package, so it can set the
46		// unexported fields directly rather than needing a second,
47		// test-only constructor in non-test code.
48		client := &HTTPVendorClient{baseURL: srv.URL, httpClient: srv.Client()}
49	
50		if _, err := client.FetchIdentity(context.Background(), "token-A", "+15555550100", "Jane"); err != nil {
51			t.Fatalf("first FetchIdentity call: %v", err)
52		}
53		if _, err := client.FetchIdentity(context.Background(), "token-B", "+15555550100", "Jane"); err != nil {
54			t.Fatalf("second FetchIdentity call: %v", err)
55		}
56	
57		if len(observedHeaders) != 2 {
58			t.Fatalf("server observed %d requests, want 2", len(observedHeaders))
59		}
60		if observedHeaders[0] != "Bearer token-A" {
61			t.Errorf("first request's Authorization = %q, want %q", observedHeaders[0], "Bearer token-A")
62		}
63		if observedHeaders[1] != "Bearer token-B" {
64			t.Errorf("second request's Authorization = %q, want %q — must not inherit the first call's token via connection reuse", observedHeaders[1], "Bearer token-B")
65		}
66	}
67	
68	func TestHTTPVendorClient_Authenticate_SendsCredentialsAndReturnsToken(t *testing.T) {
69		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
70			var body vendorAuthRequest
71			if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
72				t.Errorf("server: decoding request: %v", err)
73			}
74			if body.Username != "u" || body.Password != "p" {
75				w.WriteHeader(http.StatusUnauthorized)
76				return
77			}
78			w.Header().Set("Content-Type", "application/json")
79			_ = json.NewEncoder(w).Encode(vendorAuthResponse{AccessToken: "minted-token"})
80		}))
81		defer srv.Close()
82	
83		client := &HTTPVendorClient{baseURL: srv.URL, httpClient: srv.Client()}
84		token, err := client.Authenticate(context.Background(), "u", "p")
85		if err != nil {
86			t.Fatalf("Authenticate: %v", err)
87		}
88		if token != "minted-token" {
89			t.Errorf("token = %q, want minted-token", token)
90		}
91	}
92	
93	func TestHTTPVendorClient_Authenticate_VendorRejects_GenericError(t *testing.T) {
94		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
95			w.WriteHeader(http.StatusUnauthorized)
96			_, _ = w.Write([]byte(`{"error":"invalid_grant","hint":"password field was 'hunter2'"}`))
97		}))
98		defer srv.Close()
99	
100		client := &HTTPVendorClient{baseURL: srv.URL, httpClient: srv.Client()}
101		_, err := client.Authenticate(context.Background(), "u", "wrong")
102		if err != ErrVendorAuthFailed {
103			t.Errorf("err = %v, want ErrVendorAuthFailed — the raw vendor response body must never surface (connector-security.md §3)", err)
104		}
105	}
106	
107	func TestHTTPVendorClient_RespectsContextTimeout(t *testing.T) {
108		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
109			time.Sleep(200 * time.Millisecond)
110			w.WriteHeader(http.StatusOK)
111		}))
112		defer srv.Close()
113	
114		client := &HTTPVendorClient{baseURL: srv.URL, httpClient: srv.Client()}
115		ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond)
116		defer cancel()
117	
118		_, err := client.Authenticate(ctx, "u", "p")
119		if err == nil {
120			t.Fatalf("Authenticate did not return an error despite a 20ms context timeout against a 200ms-slow server")
121		}
122	}
123	
Sep 13, 6:16 AM PDTOren Castellan
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/app/connector_router.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/app/connector_router.go"
}
result
1	package app
2	
3	import (
4		"log"
5		"net/http"
6		"time"
7	
8		"loginid-takehome/internal/api"
9		"loginid-takehome/internal/config"
10		"loginid-takehome/internal/connector"
11	)
12	
13	// connectorJWKSCacheTTL matches api-service's own verifier JWKS TTL
14	// (internal/app/verifier_router.go) — both binaries verify tokens minted
15	// by the same issuer, so there's no reason for a different refresh
16	// cadence.
17	const connectorJWKSCacheTTL = 15 * time.Minute
18	
19	// connectorVendorLimiterBaseBackoff/MaxBackoff/AlertThreshold mirror
20	// NewIssuerRouter's own GrantLimiter constants — same reasoning
21	// (handoff-03-auth.md names the mechanism, not specific numbers), reused
22	// here for the vendor-call rate limiter per LT-41's attack-tree leaf 5
23	// control rather than inventing a second set of magic numbers.
24	const (
25		connectorVendorLimiterBaseBackoff    = 1 * time.Second
26		connectorVendorLimiterMaxBackoff     = 5 * time.Minute
27		connectorVendorLimiterAlertThreshold = 5
28	)
29	
30	// NewConnectorRouter builds cmd/idp-connector's real router — LT-41's
31	// /auth and /identity handlers behind the connector's own inbound JWT
32	// verification (a distinct audience, connector-security.md §5: aud =
33	// cfg.ConnectorJWTAudience, scope = connector:identity-lookup — NOT
34	// api-service's audience, so a profile:*-scoped api-service token
35	// cannot be replayed against this connector and vice versa).
36	//
37	// vendor is the outbound seam to the third-party IDP — either a real
38	// (HTTPVendorClient) or stub (StubVendorClient) implementation;
39	// cmd/idp-connector/main.go decides which based on whether
40	// IDP_ABC_BASE_URL is configured.
41	func NewConnectorRouter(cfg config.Config, keys api.KeySource, vendor connector.VendorClient) http.Handler {
42		mux := http.NewServeMux()
43		mux.HandleFunc("/healthz", healthHandler("idp-connector"))
44	
45		auditLog := api.StdoutAuditLogger{}
46		authMiddleware := api.NewJWTMiddleware(keys, cfg.AuthJWTIssuer, cfg.ConnectorJWTAudience, auditLog)
47		protect := func(h http.Handler) http.Handler {
48			return api.NewDeadlineMiddleware(authMiddleware(h))
49		}
50	
51		// One shared limiter instance across both handlers — a caller
52		// hammering /auth and a caller hammering /identity are the same
53		// abuse signal against the same vendor, keyed by the same
54		// (calling-API-client sub, source IP) pair (LT-41 attack-tree
55		// leaf 5). Reuses api.GrantLimiter (PR #30/#34) rather than
56		// building a new rate-limit type, per this story's own non-goals.
57		limiter := api.NewInProcessGrantLimiter(
58			connectorVendorLimiterBaseBackoff, connectorVendorLimiterMaxBackoff, connectorVendorLimiterAlertThreshold,
59			func(clientID string, consecutiveFailures int) {
60				log.Printf("idp-connector: ALERT sustained failed vendor calls from caller sub=%s (%d consecutive)", clientID, consecutiveFailures)
61			},
62		)
63	
64		deps := connector.Deps{Vendor: vendor, Limiter: limiter, AuditLog: auditLog}
65		mux.Handle("POST /auth", protect(connector.NewAuthHandler(deps)))
66		mux.Handle("POST /identity", protect(connector.NewIdentityHandler(deps)))
67	
68		return mux
69	}
70	
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/stubvendorclient.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/connector/stubvendorclient.go"
}
result
1	package connector
2	
3	import (
4		"context"
5		"crypto/rand"
6		"encoding/hex"
7		"errors"
8		"sync"
9	)
10	
11	// ErrVendorAuthFailed is returned by StubVendorClient.Authenticate for
12	// any unrecognized username/password pair — deliberately a single
13	// sentinel, not distinguishing "unknown username" from "wrong password",
14	// matching connector-security.md §4's own rule for a real vendor
15	// integration (this stub follows the same discipline it will eventually
16	// front for).
17	var ErrVendorAuthFailed = errors.New("connector: vendor authentication failed")
18	
19	// ErrVendorIdentityNotFound is returned by StubVendorClient.FetchIdentity
20	// for an unrecognized or expired token, or a phone/name pair with no
21	// matching stub record.
22	var ErrVendorIdentityNotFound = errors.New("connector: vendor identity lookup failed")
23	
24	// StubVendorClient is a self-contained, in-process fake vendor —
25	// LT-41's own non-goal list explicitly excludes real ABC/XYZ
26	// integration, so this is what cmd/idp-connector wires by default
27	// (IDP_ABC_BASE_URL unset) to demonstrate the connector's full /auth ->
28	// /identity flow without a real third party. Deterministic and
29	// data-free beyond what's seeded via Seed — no real end-user data ever
30	// lives in this type.
31	type StubVendorClient struct {
32		mu          sync.Mutex
33		credentials map[string]string   // username -> password
34		identities  map[string]Identity // username -> identity
35		tokens      map[string]string   // token -> username
36	}
37	
38	// NewStubVendorClient returns an empty stub — call Seed to add a
39	// demo/test credential.
40	func NewStubVendorClient() *StubVendorClient {
41		return &StubVendorClient{
42			credentials: make(map[string]string),
43			identities:  make(map[string]Identity),
44			tokens:      make(map[string]string),
45		}
46	}
47	
48	// Seed registers one fake vendor end-user: username/password
49	// authenticate successfully, and the resulting token resolves to
50	// identity via FetchIdentity — regardless of what phone/name the caller
51	// actually requested, matching a real vendor's own binding-to-the-token
52	// behavior (connector-security.md §6's stated assumption).
53	func (s *StubVendorClient) Seed(username, password string, identity Identity) {
54		s.mu.Lock()
55		defer s.mu.Unlock()
56		s.credentials[username] = password
57		s.identities[username] = identity
58	}
59	
60	func (s *StubVendorClient) Authenticate(_ context.Context, username, password string) (string, error) {
61		s.mu.Lock()
62		defer s.mu.Unlock()
63		want, ok := s.credentials[username]
64		if !ok || want != password {
65			return "", ErrVendorAuthFailed
66		}
67		token, err := randomToken()
68		if err != nil {
69			return "", err
70		}
71		s.tokens[token] = username
72		return token, nil
73	}
74	
75	func (s *StubVendorClient) FetchIdentity(_ context.Context, accessToken, _, _ string) (Identity, error) {
76		s.mu.Lock()
77		defer s.mu.Unlock()
78		username, ok := s.tokens[accessToken]
79		if !ok {
80			return Identity{}, ErrVendorIdentityNotFound
81		}
82		identity, ok := s.identities[username]
83		if !ok {
84			return Identity{}, ErrVendorIdentityNotFound
85		}
86		return identity, nil
87	}
88	
89	func randomToken() (string, error) {
90		b := make([]byte, 16)
91		if _, err := rand.Read(b); err != nil {
92			return "", err
93		}
94		return hex.EncodeToString(b), nil
95	}
96	
Sep 13, 6:17 AM PDTOren Castellan
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -n "ScopeConnectorIdentityLookup\|si
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -n \"ScopeConnectorIdentityLookup\\|singleScope\\|ConnectorJWTAudience\" internal/api/auth.go internal/api/jwtmiddleware.go internal/config/*.go 2>/dev/null"
}
result
internal/api/auth.go:34:	// ScopeConnectorIdentityLookup is cmd/idp-connector's own inbound
internal/api/auth.go:45:	ScopeConnectorIdentityLookup Scope = "connector:identity-lookup"
internal/config/config.go:74:	ConnectorJWTAudience  string
internal/config/config.go:143:		ConnectorJWTAudience:  os.Getenv("CONNECTOR_JWT_AUDIENCE"),
internal/api/jwtmiddleware.go:89:	scope, err := singleScope(c.Scope)
internal/api/jwtmiddleware.go:106:// singleScope requires the token's scope claim to name exactly one
internal/api/jwtmiddleware.go:125:func singleScope(raw string) (Scope, error) {
internal/api/jwtmiddleware.go:130:		case ScopeReadOwn, ScopeReadAny, ScopeSearch, ScopeConnectorIdentityLookup:
Sep 13, 6:17 AM PDTOren Castellan
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/api/jwtmiddleware.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/internal/api/jwtmiddleware.go"
}
result
1	// jwtmiddleware.go is LT-40's core: verify the client-credentials JWT
2	// scheme from handoff-03-auth.md v3/v4 and populate AuthContext for
3	// LT-39's already-built handlers. This is the actual integration seam
4	// with S6's work, not new response-shape design — a verification failure
5	// uses the exact same writeUnauthorized this package's handlers already
6	// call for a missing AuthContext.
7	package api
8	
9	import (
10		"context"
11		"errors"
12		"net/http"
13		"strings"
14		"time"
15	
16		"github.com/golang-jwt/jwt/v5"
17	)
18	
19	// jwtAlgorithm is the one signing algorithm this verifier ever accepts.
20	// Passed explicitly to jwt.WithValidMethods on every parse — never left
21	// to the library's default, per handoff-03-auth.md v3's explicit
22	// instruction ("check your JWT library's default behavior — several
23	// popular libraries have shipped this vulnerability by not doing this by
24	// default") and RFC 8725 §3.1. This is what makes alg:none and an
25	// HMAC-with-the-public-key-as-secret downgrade both fail before the
26	// library ever calls KeySource — jwt/v5's WithValidMethods rejects a
27	// token whose header alg isn't in this allowlist ahead of signature
28	// verification, not as a secondary check after.
29	const jwtAlgorithm = "RS256"
30	
31	// claims is this service's JWT claim set (handoff-03-auth.md v3's
32	// minimum: sub, scope, exp, iat, aud, plus iss and kid via the header).
33	// RegisteredClaims gives exp/iat/iss/aud validation for free from the
34	// library — this verifier still explicitly re-checks Audience below
35	// because jwt/v5's built-in audience check (via WithAudience) does not
36	// distinguish "no aud claim at all" from "aud present but empty," and a
37	// token with no audience claim must not silently pass as if it matched.
38	type claims struct {
39		Scope string `json:"scope"`
40		jwt.RegisteredClaims
41	}
42	
43	// jwtVerifier is the collaborator NewJWTMiddleware wraps — an interface
44	// so a test can supply a fake instead of a real *JWKSCache doing network
45	// I/O, mirroring this package's existing test-double approach
46	// (decisions/test-double-strategy.md).
47	type jwtVerifier struct {
48		keys     KeySource
49		issuer   string
50		audience string
51	}
52	
53	// verify parses and fully validates tokenString, returning the populated
54	// AuthContext on success. Every failure returns a generic error — the
55	// specific reason (expired, wrong audience, bad signature, unknown kid,
56	// disallowed algorithm) is never distinguishable to the caller of this
57	// function beyond "verification failed," which is what makes
58	// writeUnauthorized's single, undifferentiated 401 body correct here
59	// without an extra translation step.
60	func (v *jwtVerifier) verify(ctx context.Context, tokenString string) (AuthContext, error) {
61		var c claims
62		token, err := jwt.ParseWithClaims(tokenString, &c, func(t *jwt.Token) (any, error) {
63			kid, _ := t.Header["kid"].(string)
64			if kid == "" {
65				return nil, errors.New("token header has no kid")
66			}
67			return v.keys.KeyForKid(ctx, kid)
68		}, jwt.WithValidMethods([]string{jwtAlgorithm}), jwt.WithIssuer(v.issuer))
69		if err != nil {
70			return AuthContext{}, err
71		}
72		if !token.Valid {
73			return AuthContext{}, errors.New("token invalid")
74		}
75	
76		// Audience checked explicitly, not via jwt.WithAudience above: v5's
77		// audience option treats an all-nil/absent Audience claim
78		// inconsistently across versions in a way we don't want to depend
79		// on — asserting it here directly means this check's behavior is
80		// this file's own to guarantee, not the library's default to trust.
81		if !hasAudience(c.Audience, v.audience) {
82			return AuthContext{}, errors.New("token audience mismatch")
83		}
84	
85		if c.Subject == "" {
86			return AuthContext{}, errors.New("token has no sub claim")
87		}
88	
89		scope, err := singleScope(c.Scope)
90		if err != nil {
91			return AuthContext{}, err
92		}
93	
94		return AuthContext{Sub: c.Subject, Scope: scope}, nil
95	}
96	
97	func hasAudience(aud jwt.ClaimStrings, want string) bool {
98		for _, a := range aud {
99			if a == want {
100				return true
101			}
102		}
103		return false
104	}
105	
106	// singleScope requires the token's scope claim to name exactly one
107	// distinct recognized scope. handoff-03-auth.md v3 describes the claim
108	// as "space-delimited, values below" without describing how a handler
109	// disambiguates a multi-scope token per request, and AuthContext (LT-39)
110	// carries exactly one Scope by design ("handoff-03-auth.md v3 doesn't
111	// describe multi-scope tokens being disambiguated per-request, so one
112	// request carries exactly one Scope"). Rather than guess which of
113	// several granted scopes a request is "really" using — silently picking
114	// one is exactly the kind of authorization ambiguity this project's
115	// contracts have consistently ruled against — a token naming zero or
116	// more than one DISTINCT recognized scope is rejected outright. This
117	// matches the provisioning model in decisions/search-authz-scoping.md: a
118	// client is provisioned for the one use case its credential exists for.
119	//
120	// Deduplicated by distinct value, not by occurrence count: a malformed
121	// but unambiguous claim like "profile:search profile:search" names one
122	// scope repeated, not two different ones to disambiguate between, and
123	// rejecting it as "more than one" would contradict this function's own
124	// reasoning above (Oren Castellan, PR #30 review).
125	func singleScope(raw string) (Scope, error) {
126		fields := strings.Fields(raw)
127		found := make(map[Scope]bool, len(fields))
128		for _, f := range fields {
129			switch Scope(f) {
130			case ScopeReadOwn, ScopeReadAny, ScopeSearch, ScopeConnectorIdentityLookup:
131				found[Scope(f)] = true
132			}
133		}
134		switch len(found) {
135		case 0:
136			return "", errors.New("token scope claim names no recognized scope")
137		case 1:
138			for s := range found {
139				return s, nil
140			}
141			panic("unreachable")
142		default:
143			return "", errors.New("token scope claim names more than one distinct recognized scope, which this verifier does not disambiguate per request")
144		}
145	}
146	
147	// NewJWTMiddleware verifies every request's bearer token and populates
148	// AuthContext on success, per handoff-03-auth.md v3/v4: public-key-only
149	// verification (keys never includes signing-key material — enforced
150	// structurally, since KeySource's real implementation, JWKSCache, only
151	// ever holds *rsa.PublicKey values parsed from a JWKS response), pinned
152	// RS256, kid-based key selection supporting a rotation overlap window (a
153	// KeySource backed by the issuer's full published key set, not a single
154	// cached key), and issuer/audience checks. Verification failure returns
155	// 401 via writeUnauthorized — the same path LT-39's handlers already use
156	// for a missing AuthContext, not a new response shape — with one
157	// exception: a KeySource failing closed on exceeded JWKS staleness
158	// (ErrJWKSStale) logs AuditJWKSUnavailable first, since that's a
159	// token-infrastructure degradation, not an ordinary bad-credential
160	// attempt, and worth distinguishing in the audit trail (Tomasz Wrede, 02
161	// cold review). audit may be nil (tests that don't care about audit
162	// output); production wiring always passes a real AuditLogger.
163	func NewJWTMiddleware(keys KeySource, issuer, audience string, audit AuditLogger) func(http.Handler) http.Handler {
164		v := &jwtVerifier{keys: keys, issuer: issuer, audience: audience}
165		return func(next http.Handler) http.Handler {
166			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
167				ctx := r.Context()
168				tokenString, ok := bearerToken(r)
169				if !ok {
170					logAuthnFailure(ctx, audit, nil)
171					writeUnauthorized(w)
172					return
173				}
174				ac, err := v.verify(ctx, tokenString)
175				if err != nil {
176					logAuthnFailure(ctx, audit, err)
177					writeUnauthorized(w)
178					return
179				}
180				r = r.WithContext(WithAuthContext(ctx, ac))
181				next.ServeHTTP(w, r)
182			})
183		}
184	}
185	
186	func logAuthnFailure(ctx context.Context, audit AuditLogger, err error) {
187		if audit == nil {
188			return
189		}
190		kind := AuditAuthnFailure
191		if errors.Is(err, ErrJWKSStale) {
192			kind = AuditJWKSUnavailable
193		}
194		audit.Log(ctx, AuditEvent{Kind: kind, Timestamp: time.Now().UTC()})
195	}
196	
197	// bearerToken's scheme match is case-insensitive ("Bearer", "bearer",
198	// "BEARER" all accepted) per RFC 7235 §2.1, which defines the
199	// auth-scheme token as case-insensitive — a prior case-sensitive match
200	// would 401 a structurally valid request from any client that happened
201	// to send a lowercase scheme (Oren Castellan, PR #30 review).
202	func bearerToken(r *http.Request) (string, bool) {
203		h := r.Header.Get("Authorization")
204		const prefixLen = len("Bearer ")
205		if len(h) <= prefixLen || !strings.EqualFold(h[:prefixLen], "Bearer ") {
206			return "", false
207		}
208		tok := strings.TrimSpace(h[prefixLen:])
209		if tok == "" {
210			return "", false
211		}
212		return tok, true
213	}
214	
215	// deadlineBudget is the context.WithTimeout duration this middleware
216	// applies to every request before it reaches a handler — the concrete
217	// number decisions/context-propagation.md's ruling required ("this gets
218	// a concrete number attached when [the middleware] is written, not left
219	// as 'some deadline'"). Sized to cover realistic p99 DAO latency
220	// (single-digit milliseconds for an indexed Postgres/CockroachDB point
221	// query or trigram search, per 05-data-ops/research-cockroachdb-postgres-semantics.md)
222	// plus this middleware's own JWKS verification path (a cache hit is
223	// sub-millisecond; a cache-miss fetch to the issuer's in-cluster Service
224	// is bounded by deadlineBudget itself, not a separate timeout, since it
225	// happens inside the same request). 5 seconds leaves an order of
226	// magnitude of headroom over expected p99 without ever handing the DAO
227	// layer a near-zero remaining deadline as a matter of routine operation.
228	const deadlineBudget = 5 * time.Second
229	
230	// NewDeadlineMiddleware applies deadlineBudget to every request's
231	// context before it reaches a handler — the enforcement site
232	// decisions/context-propagation.md's ruling named ("a concrete number
233	// attached when [the middleware] is written"). Composed outermost (see
234	// router wiring), so the budget covers JWT verification (including a
235	// JWKS cache-miss fetch) and the handler's own DAO call under one
236	// deadline, not two independently-sized timeouts that could disagree.
237	func NewDeadlineMiddleware(next http.Handler) http.Handler {
238		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
239			ctx, cancel := context.WithTimeout(r.Context(), deadlineBudget)
240			defer cancel()
241			next.ServeHTTP(w, r.WithContext(ctx))
242		})
243	}
244	
Read /Users/warrenthompson/Source/LoginID-worktrees/LT-41/cmd/idp-connector/main.go
input
{
  "file_path": "/Users/warrenthompson/Source/LoginID-worktrees/LT-41/cmd/idp-connector/main.go"
}
result
1	// Command idp-connector is the outbound-only binary (Q3: /auth, /identity
2	// against third-party IDPs). Per decisions/go-layout-debate.md, this file
3	// is a thin wrapper — route registration and config loading live in
4	// internal/app and internal/config, not here.
5	package main
6	
7	import (
8		"log"
9		"net/http"
10		"time"
11	
12		"loginid-takehome/internal/api"
13		"loginid-takehome/internal/app"
14		"loginid-takehome/internal/config"
15		"loginid-takehome/internal/connector"
16	)
17	
18	func main() {
19		cfg, err := config.Load()
20		if err != nil {
21			log.Fatalf("idp-connector: loading config: %v", err)
22		}
23	
24		addr := cfg.HTTPAddr
25		if addr == "" {
26			addr = ":8081"
27		}
28	
29		// No real ABC/XYZ vendor exists for this take-home (LT-41's own
30		// non-goal) — IDP_ABC_BASE_URL unset (the common case in this
31		// environment) wires the self-contained StubVendorClient instead,
32		// seeded with one demo identity so the connector's full /auth ->
33		// /identity flow is exercisable end to end without a real vendor. If
34		// IDP_ABC_BASE_URL IS set, a real (or real-shaped) vendor is assumed
35		// reachable over HTTPS and HTTPVendorClient is used instead — this
36		// process does not silently fall back to the stub if that vendor
37		// turns out to be unreachable at request time; NewHTTPVendorClient
38		// only fails loudly at startup, on a non-https URL.
39		var vendor connector.VendorClient
40		if cfg.IDPABCBaseURL != "" {
41			httpVendor, err := connector.NewHTTPVendorClient(cfg.IDPABCBaseURL, nil)
42			if err != nil {
43				log.Fatalf("idp-connector: configuring vendor client: %v", err)
44			}
45			vendor = httpVendor
46			log.Printf("idp-connector: using HTTP vendor client at %s", cfg.IDPABCBaseURL)
47		} else {
48			stub := connector.NewStubVendorClient()
49			stub.Seed("demo-user", "demo-password", connector.Identity{
50				Name:          "Jane Demo",
51				Phone:         "+15555550100",
52				StreetAddress: "1 Demo Way",
53				Locality:      "Demoville",
54				Region:        "CA",
55				PostalCode:    "94000",
56				Country:       "US",
57			})
58			vendor = stub
59			log.Printf("idp-connector: IDP_ABC_BASE_URL not set — using in-process stub vendor (LT-41 non-goal: no real vendor integration)")
60		}
61	
62		// Same JWKS-fetch mechanism as api-service's verifier
63		// (internal/app/verifier_router.go) — both binaries verify tokens
64		// minted by the same issuer, just checking a different audience.
65		keys := api.NewJWKSCache(cfg.AuthJWKSURL, 15*time.Minute, nil)
66	
67		log.Printf("idp-connector: listening on %s", addr)
68		if err := http.ListenAndServe(addr, app.NewConnectorRouter(cfg, keys, vendor)); err != nil {
69			log.Fatalf("idp-connector: %v", err)
70		}
71	}
72	
Sep 13, 6:17 AM PDTOren Castellan
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -n "func Test" internal/app/connecto
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -n \"func Test\" internal/app/connector_router_test.go"
}
result
47:func TestNewConnectorRouter_HealthzUnaffectedByAuth(t *testing.T) {
59:func TestNewConnectorRouter_NoToken_401(t *testing.T) {
84:func TestNewConnectorRouter_WrongAudience_Rejected(t *testing.T) {
106:func TestNewConnectorRouter_CorrectAudienceAndScope_ReachesAuthHandler(t *testing.T) {
128:func TestNewConnectorRouter_IdentityEndToEnd(t *testing.T) {
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -n "func AuthContextFromContext\|fun
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && grep -n \"func AuthContextFromContext\\|func WithAuthContext\" -A6 internal/api/*.go"
}
result
internal/api/auth.go:64:func WithAuthContext(ctx context.Context, ac AuthContext) context.Context {
internal/api/auth.go-65-	return context.WithValue(ctx, authContextKey{}, ac)
internal/api/auth.go-66-}
internal/api/auth.go-67-
internal/api/auth.go-68-// AuthContextFromContext retrieves the AuthContext WithAuthContext
internal/api/auth.go-69-// attached, ok=false if none was ever set (e.g. this package's handlers
internal/api/auth.go-70-// were reached without going through auth middleware — a caller bug, not
--
internal/api/auth.go:72:func AuthContextFromContext(ctx context.Context) (AuthContext, bool) {
internal/api/auth.go-73-	ac, ok := ctx.Value(authContextKey{}).(AuthContext)
internal/api/auth.go-74-	return ac, ok
internal/api/auth.go-75-}
internal/api/auth.go-76-
internal/api/auth.go-77-// ReasonCode is the closed enum profile:read:any requires on every
internal/api/auth.go-78-// request (handoff-03-auth.md v3). Adding a value is a documented change
Sep 13, 6:17 AM PDTOren Castellan
Good — that matches the existing project-wide idiom (LT-39/40 handlers do the same `ac, _ :=`). Not a new issue introduced by this PR. Now let me double check ErrVendorAuthFailed/ErrVendorIdentityNotFound aren't imported oddly and check git log message for attribution trailers.
Bash cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && git log -1 --format="%B"
input
{
  "command": "cd /Users/warrenthompson/Source/LoginID-worktrees/LT-41 && git log -1 --format=\"%B\""
}
result
LT-41: idp-connector /auth + /identity against a stubbed ABC/XYZ vendor

Q3's connector: POST /auth ({"username","password"} -> access_token) and
POST /identity ({"phone","name"} -> PII), per connector-security.md and
refinement/LT-41.md.

internal/connector (new): VendorClient is the seam to a third-party IDP.
StubVendorClient is a self-contained in-process fake (no real ABC/XYZ
vendor exists — this story's own non-goal); HTTPVendorClient demonstrates
the outbound security discipline for real: TLS-only at construction,
a bounded connection pool + context timeout, and a fresh Authorization
header built inside each call from an explicit per-call token parameter
— never a client-level field, so connection reuse can't leak one
request's vendor token onto another's outbound call (attack-tree leaf 1,
enforced by TestHTTPVendorClient_FetchIdentity_FreshAuthorizationHeaderPerRequest
against a real pooled http.Client over two sequential calls with
different tokens).

Handlers reuse PR #30/#34's already-built GrantLimiter (per this story's
own non-goal: no new rate-limit mechanism) keyed by calling-API-client
sub and source IP for vendor-call abuse control (attack-tree leaf 5) —
TestVendorCallLimiter_FailureRatioTripsBeforeVolume demonstrates a
high-failure-ratio caller trips within a handful of consecutive
failures while a high-volume-but-mostly-successful caller never trips
at all, since GrantLimiter's backoff is driven purely by consecutive
failures.

/identity's three failure branches (no token, bad/expired token, vendor
rejected it) return through one call site (writeIdentityFailure) for
byte-identical content, and a ~100ms uniform floor (Marcus Ilori's
ruling on Tobias Lindqvist's timing-oracle objection, connector-security.md
§4 — a partial mitigation, full closure explicitly not pursued since a
real vendor round-trip's latency isn't ours to control) —
TestNewIdentityHandler_ThreeFailureBranches_ContentIdenticalAndTimingClose
asserts both content identity and that all three land within a bounded
delta of the floor, not just "all fast" or "all slow" independently.
The vendor-presented token is scoped to FetchIdentity's own call only,
never assigned to a wider-scoped variable (attack-tree leaf 4).

Both /auth and /identity require their own inbound JWT — a distinct
audience (idp-connector-service, ConnectorJWTAudience) and scope
(connector:identity-lookup, added to internal/api's scope vocabulary)
from api-service's profile:* scopes, so a token minted for one service
cannot be replayed against the other (connector-security.md §5) —
TestNewConnectorRouter_WrongAudience_Rejected proves a
loginid-api-service-audience token is rejected here.

cmd/idp-connector/main.go wires the real NewConnectorRouter, choosing
HTTPVendorClient when IDP_ABC_BASE_URL is configured or the in-process
stub (seeded with one demo identity) otherwise — no config additions
needed, IDPABCBaseURL/ConnectorJWTAudience/AuthJWKSURL etc. were already
in the config surface from earlier stories.

Story: LT-41
Work-By: Renata Cole (03-engineering-delivery lead)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Cuba4JUvrsQCrTdUj4yzk
Sep 13, 6:17 AM PDTOren Castellan
I've completed the review. Build, vet, and race-enabled tests all pass clean in the worktree at `/Users/warrenthompson/Source/LoginID-worktrees/LT-41`. **Report to Renata Cole — PR #40 (LT-41) second-pass review** Went through this line by line against the six points you asked about, plus `refinement/LT-41.md` and `connector-security.md`'s attack-tree table. 1. **Fresh header, no connection-pool leakage** — holds. `HTTPVendorClient` has exactly two fields (`baseURL`, `httpClient`); no header map, no captured `*http.Request`, no client-level default header anywhere. Both `Authenticate` and `FetchIdentity` build a brand-new `*http.Request` via `http.NewRequestWithContext` every call and set `Authorization` directly on that request. `TestHTTPVendorClient_FetchIdentity_FreshAuthorizationHeaderPerRequest` proves it against a real pooled `http.Client` (two sequential calls, two different tokens, server observes the correct header each time). I traced it for concurrent use too — since the token lives only in a local `req` variable per goroutine, there's no shared mutable state a race could hit; `go test -race` agrees. 2. **Token scope in handlers.go** — as narrow as claimed. In `NewIdentityHandler`, `token` is read off the header, passed by value into `FetchIdentity`, and never assigned to anything outside that block. In `NewAuthHandler`, `accessToken` is the vendor call's return value, written straight to the response encoder; the file's own comment is honest that this is the practical floor given `/auth`'s job is literally to deliver the token to its caller. Neither ever reaches a log call or a wider-scoped variable. 3. **No sensitive leakage in error paths** — checked every branch in both files. `HTTPVendorClient.Authenticate`/`FetchIdentity` never read the response body on a non-200 or decode failure (collapsed straight to the two sentinel errors), dial/DNS errors are discarded rather than wrapped or logged, and response bodies are drained-and-closed via `io.Copy(io.Discard, ...)` without inspection. `logAuditOutcome` only logs sub/scope/kind/timestamp. `TestHTTPVendorClient_Authenticate_VendorRejects_GenericError` explicitly asserts a vendor body containing a password hint doesn't surface. Confirmed clean. 4. **GrantLimiter reuse for vendor-call gating** — sound, with one thing worth naming plainly rather than glossing over. GrantLimiter's blocking mechanism is *consecutive-failure* backoff (reset to zero by `RecordSuccess`), not a literal separate "failure-ratio threshold" vs. "volume threshold." That's not the same statement as the acceptance criterion's prose, but it produces the same effect: a mostly-failing caller trips fast, a mostly-succeeding-but-high-volume caller never trips, which is exactly what `TestVendorCallLimiter_FailureRatioTripsBeforeVolume` demonstrates. Keying by `AuthContext.Sub` is correct — that's the calling API client's identity (`api-service`'s own credential), matching leaf 5's "per calling-API-client identity," not conflated with any end-user identity. One real caveat: "per source IP" here is the IP of whatever calls this connector's HTTP endpoint — in this deployment that's `api-service`'s pod IP, not any external attacker's IP, since the connector has no public route. That's not a defect in this PR (it's implementing the design doc as written), but it means the per-IP half of leaf 5's control does less work than the design's prose implies once you account for the actual network topology. Flagging it for the record, not blocking. 5. **waitForFloor placement** — correct on all three fronts. Applied on: no-token (empty header), and vendor-call error (covering both "bad token" and "vendor rejected it," which collapse to the same code path since `FetchIdentity` returns one sentinel either way — that's the right design, not a missed branch, since the connector genuinely can't distinguish them). Not applied on: the 429 rate-limit path (explicit comment: "not one of the three timing-oracle branches"), the 400 malformed-body path (explicit comment: "caller bug, not one of the three named branches"), or the success path. `TestNewIdentityHandler_ThreeFailureBranches_ContentIdenticalAndTimingClose` and `TestNewIdentityHandler_Success_NoFloorApplied` both assert this directly. 6. **Everything else** — nothing else rises to a finding. Distinct audience/scope wiring (`ScopeConnectorIdentityLookup`, `ConnectorJWTAudience`) is correctly threaded through `singleScope`'s switch and `NewConnectorRouter`, with `TestNewConnectorRouter_WrongAudience_Rejected` covering it. `NewHTTPVendorClient`'s https-only check is a hard construction-time error, not a runtime warning. One very minor nit, not worth a fix cycle on its own: `StubVendorClient.tokens` grows unbounded (never evicted) — fine for a stub with no vendor round trip and a take-home's lifetime, but if anyone ever reaches for this stub in a longer-running context it's worth a TTL/sweep like `InProcessGrantLimiter` already has. Everything here holds up. No blocking findings — this is a clean pass. If you want item 4's IP-topology observation and the stub's unbounded map addressed, they're both small and non-blocking; otherwise this is ready as reviewed. Files reviewed (all under `/Users/warrenthompson/Source/LoginID-worktrees/LT-41`): `internal/connector/vendorclient.go`, `stubvendorclient.go`, `httpvendorclient.go`, `handlers.go`, `internal/app/connector_router.go`, `cmd/idp-connector/main.go`, `internal/api/auth.go`, `internal/api/jwtmiddleware.go`, `internal/api/grantlimiter.go`, and the four test files (`internal/connector/handlers_test.go`, `httpvendorclient_test.go`, `stubvendorclient_test.go`, `internal/app/connector_router_test.go`).