c86c4dc6ece96e17961f048f2ad304cb352fbd29
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  jaap-janandClaude Opus 5
|
a43286ece8 |
Let a team change hands, and be joined by somebody with no account yet
M3 built teams and stopped short of the two operations that decide who controls one. Both were written down as refusals rather than omissions: ADR 0009 listed ownership transfer under "deliberately not built", and design-import-gaps said an invitation needed "a token with a lifetime and an outbound mail path". One of those reasons had expired and the other never applied — an invitation does not need a token if it is not a thing anybody presents. Handing a team over is one write. The member you name becomes owner and you become an admin, in a single transaction, because ownership is sole: promoting first leaves the team owned twice, demoting first leaves it owned by nobody, and there is nobody left with the authority to finish a transfer that stopped in the middle. That is also why it is not two calls to the role endpoint, which refuses Owner outright. The outgoing owner is demoted rather than removed — removing them would revoke their vault key grants and flag every team vault for rekey, which is a far larger act than the one asked for, and somebody handing over a team is usually staying in it. It unblocks the thing that was impossible before: an owner can now leave, by handing the team on first. An invitation is a standing instruction rather than a message. This server has no outbound mail path, so nothing is sent and there is nothing for the invitee to present. The row says the next account signing in with that address joins this team at this role, and telling them to sign in is the caller's job over a channel this server does not carry. A link nobody can deliver would be worse than none. It lives in its own table rather than becoming a membership with MembershipStatus.Invited, and that member stays unwritten for the reason it always was: team_membership.user_id is not nullable and carries a foreign key, so somebody who has never signed in has nothing for that row to point at. Widening it would make the unique index on (team, user) meaningless, because PostgreSQL counts every NULL as distinct. Verification is the security boundary, and nothing in this server read it before. A claim requires the access token to assert email_verified. An invitation decides what the server will serve, so one claimable by anybody able to obtain a token carrying somebody else's address is a way into a team — which is precisely the attack OidcOptions.AllowEmailLinking exists to refuse, and it would have been reintroduced by the back door. There is deliberately no setting that relaxes it: a flag that exists is one somebody turns on for the afternoon their provider is misconfigured. Absence is refused rather than trusted, and logged, because a provider that never sends the claim otherwise leaves every invitation pending with nothing anywhere saying why. Claiming happens at just-in-time provisioning and again on an hourly sweep. The sweep is what makes it recoverable rather than one-shot — an invitation issued between an account being created and that person next signing in would otherwise be stranded for ever — and it shares its rate with the last-seen write because both are housekeeping nobody is waiting on. Archiving is refused while a team owns a vault, and that refusal is the end of the road rather than a step on it. A team vault is readable because of membership, so archiving one that still owned vaults would take them away from everybody holding a key, including the caller, quietly and all at once. Nothing in this product deletes a vault, so no order of operations gets past it today — which is stated with a count of what is in the way, for the reason the SFTP layer refuses a recursive delete: a refusal is visible and a quiet removal is not. It is owner-only, as handing over is; renaming is not, because a rename is visible to everybody and reversible by anybody who can do it. The slug is not renameable at all: it is unique only among live teams, so a rename could take one an archived team is still holding, and that team could then never be restored. LAST ACTIVE is real and coarse on purpose. UserAccount.LastSeenAtUtc is refreshed on ordinary authenticated requests, at most once per account per hour, through ExecuteUpdateAsync — user_account carries the xmin concurrency token, so a read-then-write on the hot path would start losing races between one user's own overlapping requests. An hour is the granularity the question is actually asked at, and the interface draws it to the day rather than the minute so it does not read as a precision that is not there. The remarks in Contracts and in the view model that argued at length for the column's absence are rewritten rather than extended; both had become false. Two endpoints already existed and nothing called them. ChangeTeamMemberRole and ListVaultGrants have been reachable since M3. The role picker refuses Owner itself rather than letting the server do it, since the interface already knew the rule; the key-holder list sits under the vault rather than beside the member, because a grant is per vault and a count on a member row would imply per-item sharing, which is M5. It lists withdrawn and stale grants and says which they are — a list that dropped them would show a departed colleague as merely absent rather than as somebody whose key was taken away — and staleness is decided by comparing generations, since a grant can be Active and still open nothing. ADD MEMBER stopped being a dead end. An address the directory did not know used to end at a sentence telling the user their colleague had to sign in first. It invites them instead, from the same button, because which of the two applies is a fact about the server's account table rather than about what the user is doing; which one happened is reported afterwards, because that decides what they do next. An address that merely has an account is invited rather than refused: refusing would have made the endpoint an oracle for which addresses have accounts here, answerable by anybody willing to create a team first. The phone has a TEAMS screen, behind MORE, and it is the reverse of every other row in design-import-gaps: a shipped screen the design had no slot for. It is there because an invitation is claimed by signing in, so somebody told they are now in a team is at least as likely to be holding a phone — and a membership visible only on a head they never installed is one they cannot see. It draws SHARE KEY and nothing that takes something away: wrapping a key is the one act on that screen a server cannot perform at all, and the desktop guards its revocations with a tooltip, which is a control a touch screen cannot show. Two defects were found by an adversarial pass and both were green against the whole suite at the time. The owner-only check on archiving and handing over had been weakened to the admin check while their messages and comments still said owner — and since nothing behind the archive endpoint re-checks it, an admin the owner had promoted could have archived the team out from under them. And the rename endpoint built its response with a hardcoded Owner role, so an admin who renamed a team was handed a summary claiming they owned it, and a client trusting that instead of re-listing would have offered them the two owner-only buttons the server then refuses. The new table gets its constraints tested rather than merely migrated: live uniqueness per (team, address), the citext proof that an address typed by a person matches one cased by a provider, and reissue after both revocation and acceptance. The teams screen gets its first entries in the layout suite, at the minimum window with every list populated and with each of the two states that cover half of it — it had none, and it just grew four sections and a second line in the member row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
jaap-jan
|
3829217e8a |
Add sync engine: cursors, push/pull, and the advisory-lock ordering proof (M1)
The vault write path. Push is the only way items change — no per-entity POST/PUT/DELETE — so one place enforces revisions, the change log and access control. The concurrency hazard, now proven rather than asserted: bigserial assigns sequence values when the INSERT runs, not at commit, so transaction A can take sequence 5 while B takes 6 and commits first. A reader polling in between sees only 6, advances past 5, and never learns about it. AdvisoryLockOrderingTests reproduces that gap WITHOUT the lock first — otherwise the with-lock test proves nothing, since it would pass just as happily if the interleaving never occurred — then shows pg_advisory_xact_lock removes it, and that 12 concurrent writers produce no gaps. Cursors are opaque and HMAC-tagged, and carry their vault id. 29 unit tests cover the rejections, which are the point: an accepted-but-wrong cursor is silent data loss, strictly worse than an error a client can resync from. Rejected: tampered tag, tampered payload, foreign signing key, a legitimately-issued cursor from another vault, truncation, and hostile input (never throws — cursors come from clients). Push semantics: - 200 even on partial failure, with per-operation status, so one stale item cannot block everything a client queued while offline. - Conflict returns the server's current row for client-side three-way merge. The server cannot merge ciphertext, so never last-writer-wins. - opId receipts make retries exactly-once per operation, not per batch — a client retrying a partially-overlapping batch after a timeout would otherwise double-apply what landed. - A tombstone beats a late upsert, and delete clears hostname/port: leaving the address would keep the server able to resolve a host the user believes they deleted. - Relay field validation mirrors the DB CHECK so a bad request is a clear Invalid rather than a constraint violation surfacing as a 500. Authorization goes through IVaultAccessService, which returns the same answer for "absent" and "forbidden" — distinguishing them is an existence oracle for other tenants' vault ids. Team vaults are explicitly denied until M3 rather than falling through to a permissive default. JIT provisioning keys on (issuer, subject), never email, and handles the concurrent-first-request race via the unique index. Renamed two domain types: Host -> SshHost, because Host collides with Microsoft.Extensions.Hosting.Host in every file of a web project, and SyncChange -> VaultChange to stop it colliding with the Contracts DTO of the same name. Aliasing at every use site would have been permanent friction. Worth noting: `ef migrations has-pending-model-changes` reported clean after those renames even though the snapshot still said "DodoSSH.Domain.Host" — it diffs tables, not CLR type names. The snapshot was regenerated and the emitted DDL diffed against the previous artifacts/schema/v0.1.sql to confirm the rename produced no schema change. Also removed ConfigureAwait(false) from test methods: xUnit1030 flags it as bypassing parallelization limits, which is why MA0004 is suppressed in test projects. Verified: 0 warnings on a clean rebuild, 146 tests pass (up from 122), format clean. Endpoint-level tests are the immediate next step: they need a WireMock OIDC/JWKS stub and real JWT minting, so the "wrong user is denied" matrix does not exist yet for these two routes. The service-layer authorization and the concurrency property are covered. |
||
|
jaap-jan
|
d3b14e6bc0 |
Add configuration, OIDC auth wiring and discovery endpoints (M1)
Options, JWT bearer validation, the /meta and .well-known endpoints, and a dev compose stack with Keycloak. Verified end to end: compose up, migrate, run, both discovery endpoints return correct payloads, and readiness reports the schema current. Configuration: - Strongly-typed options for Server, Oidc, Relay and Sync, all ValidateOnStart. A self-hosted server that boots half-configured and fails later per-request is far harder to diagnose than one that refuses to start and names the bad setting. - Cross-field validation the annotations cannot express: relay needs a WebSocketUrl when enabled, idle timeout must be under max session duration, item payload cap under batch cap. - Startup warnings for combinations that are individually valid but dangerous together: RequireHttpsMetadata false outside Development, and AllowEmailLinking (which turns any token bearing a victim's email into account takeover, hence default false). Auth: - JwtBearer with ClockSkew cut to 30s from the 5-minute default; five minutes of slack on a credential granting vault ciphertext access is more than any clock needs. - IncludeErrorDetails off, and a FallbackPolicy so an endpoint without an explicit policy still requires a caller rather than silently being public. Discovery, per ADR 0002: - /api/v1/meta reports versions, features and push caps. - /.well-known/dodossh-configuration is the onboarding story: the user types one server URL and the client discovers OIDC authority, client id, scopes and relay endpoint. Two environment problems found by actually running the stack: - PostgreSQL 18 changed its data mount point. Mounting /var/lib/postgresql/data — correct through 17 — makes the image refuse to start; 18+ wants a single mount at /var/lib/postgresql with the cluster in a subdirectory. - Keycloak moved to host port 18080. An unrelated Apache Tomcat on this machine holds 127.0.0.1:8080, and a loopback-specific bind beats Docker's 0.0.0.0 publish for "localhost". It presents as Keycloak 404ing every realm while its own log says the import succeeded, which is a genuinely misleading failure. Also: CA1848 is enforced, not advisory — warnings are errors, so the .editorconfig comment claiming otherwise was wrong. Startup and health logging now uses [LoggerMessage]. And a clean rebuild is back to zero warnings; the incremental build had been hiding 40 in test projects (banned Guid.NewGuid, an obsolete Testcontainers constructor, and two analyzer families that are genuinely noise under a test host). Verified: 0 warnings on a clean rebuild, 122 tests pass, format clean. |
||
|
jaap-jan
|
eaf68c86b0 |
Add data model, DbContext and initial migration (M1)
Schema for identity, vaults, grants, hosts and the sync change log, verified against a real PostgreSQL 18 container rather than an in-memory provider: partial unique indexes, CHECK constraints, citext and identity-always columns are all provider behaviour that an in-memory fake would not exercise. Invariants pushed into the database, so they hold even when application code has a bug: - ck_host_relay_target is a security boundary, not tidiness. A host may carry a plaintext hostname and port ONLY when relay is deliberately enabled. Both directions are tested; the important one is that relay-disabled hosts cannot carry an address, since otherwise a bug would silently give the server infrastructure visibility it was never granted. - ck_vault_owner: exactly one of owner_user_id or team_id, or permission resolution would have no defined answer. - ck_vault_key_grant_recipient: member grants name a user; recovery and escrow grants are wrapped to a key and must not. - ck_user_key_wrap_kdf: a password-derived wrap without its parameters is permanently unopenable, so a partial write is rejected outright. Present from the first migration on purpose: - GrantKind (Member/Recovery/Escrow). Recovery cannot be bolted on later — every vault created before it existed would be unrecoverable by design. - team and team_membership, though team features are M3. Adding them later would mean introducing a foreign key on a live vault table. - Host.ContentKeyId, reserved for per-item content keys wrapped to individual users. - user_key as its own table, so key rotation does not require altering the user row. Two things verified rather than assumed: - Npgsql's UseXminAsConcurrencyToken helper no longer exists in EF 10, so xmin is mapped directly in XminConcurrency. The generated migration *looks* like it creates an xmin column; it does not. Confirmed by inspecting pg_attribute (attnum -2, a system column) and by grepping the emitted DDL. A test pins both, because had it created a real column PostgreSQL would have rejected the name. - EF Core is now pinned centrally. The Npgsql provider asks for 10.0.4 while EntityFrameworkCore.Design pulls 10.0.10, and because Design is PrivateAssets=all that higher version does not flow to referencing projects — producing a CS1705 in any test project referencing Infrastructure. Also commits artifacts/schema/v0.1.sql, the idempotent script, as the baseline for future upgrade tests. Verified: 0 warnings, 122 tests pass (27 new against Postgres), format clean. |