Skip to content

feat: link enrolled controllers to a site - #40

Merged
lemarier merged 2 commits into
lemarier/delete-the-account-from-the-appfrom
lemarier/link-enrolled-controllers-to-a-site-after-sign-i
Sep 25, 2026
Merged

lemarier merged 2 commits into
lemarier/delete-the-account-from-the-appfrom
lemarier/link-enrolled-controllers-to-a-site-after-sign-i

Conversation

@lemarier

@lemarier lemarier commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Closes #35. Stacked on #38, which adds the cloud client and the fresh sign-in path; merge that first.

Change

A signed-in account can now link the controllers this phone is paired with to a site it owns. Account › Sites lists the account's sites and the pairings this account sees on the phone, marks which are linked, creates a site, and links a controller under a name the person can edit. The link request carries only deviceId, epoch and the name, never the setup code or the enrolment key. When the cloud answers reauthentication_required, the app signs in again through AuthKit and retries once, only if the same account is still signed in before and after that sign-in.

  • Epoch from the core. kept_generation (UniFFI) returns the device_id and epoch of a kept enrolment and clears the bytes. km43's StoredEnrolment::decode validates them first; km43 exposes neither field, so both are read from its documented encoding layout.
  • Listing pairings. EnrolmentStore gains storedDeviceIDs() throws. AccountEnrolmentStore lists its own and the signed-out controllers once each, and generations(reader:) drops entries that do not decode or are filed under another controller. A Keychain that cannot be read, as on a locked phone, shows as an error, not as no controllers.
  • Sites API. CloudClient.sites(), createSite(named:) and link(_:named:to:authenticate:) follow the @origin89/cloud 0.1.0 contract. Site IDs must be UUIDs because they go back into request paths, and names are 1 to 80 characters once trimmed.

Trust boundary

Site authority and generation provenance are the cloud's to enforce; this PR only supplies identifiers and the bearer token. From the cloud code at origin89hq/cloud@f554459 (not tested against the deployed Worker): the link insert requires an owner membership of the site for the token's user (apps/cloud/src/store.ts), and cloud#4 reports admin and non-member tests answering 404. Provenance is not enforced: the cloud cannot check that the caller is enrolled with the controller, so anyone who has seen a device_id and epoch can claim that generation first and block the owner's link with 409. That is origin89hq/cloud#3, open, and it must be fixed before the cloud accepts readings. A link grants no controller access; the controller stays the only authority.

Sites shows only in builds with a cloud. Only production is deployed, so Debug builds, which sign in to the staging WorkOS environment, have none until a staging Worker exists.

Validation

just check passes: rustfmt, Clippy, Rust tests, 212 Swift tests, the unsigned simulator build with no warnings, and the bench build.

  • Rust: the generation from a real pairing, one paired at epoch u32::MAX, and undecodable bytes (empty, cut, too long, unknown format, zero epoch).
  • Swift: listing with unreadable and misfiled entries, an account's own and signed-out pairings merged, site decoding including a non-UUID ID and an unknown role, name bounds, a link (201) and a repeated link (200) with the exact request body, generation_linked, stale_epoch and not_found, a fresh sign-in then retry, a cancelled sign-in, a second stale answer that is not retried, a sign-in as another account that links nothing, a signed-out account, and an unreadable pairing list.

Not done: no link has been made against cloud.origin89.com from a device. That needs a Release build signed in with a production WorkOS account.

Copilot AI lite review requested due to automatic review settings September 25, 2026 15:49
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

The change exposes controller generation data from kept enrolments, adds cloud APIs for listing and creating sites and linking controllers, and adds a sites screen to the signed-in iOS account view. The screen displays sites and paired controllers and supports site creation and controller linking.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to d1ad7

If sites fail to load, the screen can appear stuck loading without an obvious retry action. This is a bounded issue that should be fixed or accepted before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to d1ad7

Linking is limited to a signed-in account and does not send pairing secrets. The remaining risk is whether the cloud verifies who may claim a controller generation, and whether account changes during an in-flight request leave the app showing the right account’s result.

Retained concerns

  • Medium · security · inferred: Controller linking depends on cloud-side site authorization and generation provenance that this change cannot verify. The client supplies identifiers and a bearer token, not proof of possession of the kept enrolment; an incorrect cloud claim policy would affect persistent controller ownership.
  • Medium · security · inferred: An ordinary successful link has no final account check, and the subsequent sites reload has no account or response-order guard. If the account changes while these requests are in flight and the view remains active, a result can be treated as current after its initiating identity has changed.
Security review details

Security Blast Radius

  • inferred — The independently affected asset is a controller generation linked into a cloud site, with site membership determining who can act on the resulting association. The available evidence does not establish cloud-wide or cross-tenant access.

Security Findings and Attack Paths

  • inferred — A caller able to supply a device ID and epoch reaches the authenticated link endpoint; whether those identifiers suffice to claim another party’s generation depends on cloud policy not verified here. No exploitable authorization bypass is established by the client evidence alone.

Trust Boundaries and Controls

  • observed — The UI presents owner sites for linking, the client validates display names, and its transport sends bearer authentication. These client controls do not substitute for cloud-side authorization of the site and controller claim.

Resilience and Maintainability Implications

  • observed — The linking sheet prevents simultaneous submissions and retains non-cancellation errors; the client’s same-account checks specifically cover reauthentication, not completion of an ordinary request.

Hardening Proposals

  • proposed — Verify the deployed cloud endpoint’s site-membership and generation-claim rules before relying on the new link path; bind in-flight link and reload results to the initiating account, and establish a reconciliation or idempotency contract for uncertain site-creation outcomes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 58 functions across 16 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR meets the coding requirements in #35. Rust exposes device_id and epoch through kept_generation, and Swift maps stored enrolments to ControllerGeneration. CloudClient.sites() lists sites…
Out of Scope Changes check ✅ Passed The changes stay within #35. The Sites UI, local enrolment-generation plumbing, cloud site operations, Rust decoding, storage enumeration, and related tests directly support linking enrolled controlle…
Description check ✅ Passed The description includes the required Change and Validation sections, explains the resulting behavior and trust boundary, lists validation commands and meaningful test cases, and states the unperforme…
Title check ✅ Passed The title clearly and concisely describes the primary change: linking enrolled controllers to a site.
  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

@lemarier
lemarier added this pull request to stack #41 September 25, 2026 15:49
@lemarier
lemarier force-pushed the lemarier/link-enrolled-controllers-to-a-site-after-sign-i branch from b562a98 to a7e90da Compare September 25, 2026 16:16
@lemarier
lemarier force-pushed the lemarier/link-enrolled-controllers-to-a-site-after-sign-i branch from a7e90da to d1ad7b5 Compare September 25, 2026 16:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
apps/ios/Origin89/SitesView.swift-115-123 (1)

115-123: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the site list visible after a failed reload

reload() sets failure when cloud.sites() fails. It leaves sites unchanged. On the first load, sites is still nil. The screen then shows the error and a ProgressView that never ends. The person can only retry by pull-to-refresh, and nothing on screen says that is possible. Two fixes are possible. You can show a retry button in the failure section. You can also stop showing the spinner once failure is set.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Advanced

Run ID: f301eddf-36f1-4935-bda0-f78e303d3657

📥 Commits

Reviewing files that changed from the base of the PR and between b562a98 and d1ad7b5.

📒 Files selected for processing (11)
  • apps/ios/Origin89/AccountView.swift
  • apps/ios/Origin89/SetupView.swift
  • apps/ios/Origin89/SitesView.swift
  • apps/ios/SetupBench/Sources/SetupBench/main.swift
  • apps/ios/SetupKit/Sources/SetupKit/AccountEnrolmentStore.swift
  • apps/ios/SetupKit/Sources/SetupKit/Contract.swift
  • apps/ios/SetupKit/Sources/SetupKit/SetupFlow.swift
  • apps/ios/SetupKit/Sources/SetupKit/Sites.swift
  • apps/ios/SetupKit/Tests/SetupKitTests/GenerationTests.swift
  • apps/ios/SetupKit/Tests/SetupKitTests/KeptEnrolmentTests.swift
  • apps/ios/SetupKit/Tests/SetupKitTests/SitesTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

@lemarier
lemarier merged commit 124a8cc into main Sep 25, 2026
2 checks passed
@lemarier
lemarier deleted the lemarier/link-enrolled-controllers-to-a-site-after-sign-i branch September 25, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Link enrolled controllers to a site after sign-in

2 participants