Skip to content

feat(settings): mock service account creation form for #491 - #495

Draft
alukach wants to merge 14 commits into
mainfrom
feat/service-account-form-mock
Draft

feat(settings): mock service account creation form for #491#495
alukach wants to merge 14 commits into
mainfrom
feat/service-account-form-mock

Conversation

@alukach

@alukach alukach commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

What this is

A non-persisting design mock of the service-account creation flow proposed in #491, so the data model can be reviewed in a browser before we build it.

Nothing is saved. Submitting the form renders the database rows the creation would write.

Where to find it

Settings → Service Accounts, on both individual and organization accounts (either can own a service account). Visible to anyone who can manage the account.

What it demonstrates

The four decisions from the epic, in order:

  1. Name — a human-friendly name; the stored id is derived from it and previewed live as svc--<sanitized>. Sanitization collapses double hyphens, so a name can never forge a second reserved prefix (ID_REGEX forbids -- in human account ids, which is what makes the prefix collision-proof).
  2. How software signs in — one or many. A GitHub repository + ref (no secret stored), or an API key that expires in 30/90/365 days or never. Every route reaches the same account with the same access.
  3. What it can reach — the whole owner account, or specific products, each read or read-write.
  4. Which roles it may use — Full access and Read only, both ticked by default. The form notes that both-ticked is a no-op and that unticking Full access is the durable restriction.

Submitting lands on a mock detail view of the integration — the page a real create would redirect to — with an Edit button that returns to the form with every answer intact. If a key sign-in method was chosen, the detail view shows the one-time API key with copy-now framing.

The detail view shows:

  • accounts (existing table) — the new service row with owner_account_id and allowed_roles
  • identity_bindings (new table) — one row per sign-in method, all pointing at the same principal
  • memberships (existing table) — one row per grant; account-wide access collapses to a single row with no repository_id, which is already how hasRole reads it
  • Ready-to-paste config per sign-in method: a GitHub Actions workflow using aws-actions/configure-aws-credentials@v4, or the CLI + env-var form for keys. The role in both changes if Full access is unticked.

Plus the caveats worth seeing next to the model: what doesn't exist yet, that roles only subtract, and that revoking a grant stops new access in ~60s while issued credentials last up to an hour.

Notes for review

  • plan.ts is pure and holds all the model logic; plan.test.ts covers it (14 tests) — namespacing, the GitHub sub claim shape, per-product vs account-wide membership rows, one binding per sign-in method, and the validation rules including the empty-role-set case that would leave an account unable to authenticate at all.
  • The page is gated on PutAccountProfile, not ListAccountMemberships — the latter returns true for any signed-in user (authz.ts:1242-1252), which would have exposed the mock on other people's accounts.
  • No API routes, no server actions, no schema changes. The only edits to existing files are a nav entry and a URL helper.

Two things the mock deliberately surfaces for decision

  • aws-actions/configure-aws-credentials also calls GetCallerIdentity, which /.sts does not implement. Since that action is how most people will first try this, the mock shows it and flags the gap.
  • Keys with no expiry are offered, but Epic: machine identity for unattended workflows #491 currently says keys cannot be issued without one. The mock calls this out as a departure rather than quietly picking a side.

Not included

Key issuance, revocation, the delete permission question, and custom roles. This is the create flow only.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt

Adds a non-persisting design mock of the service-account creation flow to
account and organization settings, so the data model proposed in #491 can be
reviewed before it is built.

The form walks the four decisions from the epic: name, how software signs in
(many per account, GitHub or API key), what it can reach (whole account or
specific products), and which roles it may use. Submitting renders the rows
the creation would write — an `accounts` row, one `identity_bindings` row per
sign-in method, one `memberships` row per grant — plus the environment
variables the workload would then use.

Nothing is saved. `planChanges` is pure and unit-tested; the page is gated on
the same account-management permission that creating a real service account
would require.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
source-cooperative Ready Ready Preview Aug 24, 2026 4:22am

Request Review

@claude

claude Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @alukach's task in 1m 45s —— View job


Review

✅ No blocking issues — safe to merge.

This is a pure, client-side design mock with no persistence, no new API routes, and no schema changes, so the blast radius is small. I checked the areas that matter most for a mock like this:

  • Access gating (guard.ts:13-19, layout.tsx:130-137): correctly gated on Actions.PutAccountProfile, not ListAccountMemberships. Verified against authz.ts:1242-1252 (listAccountMemberships returns true for any signed-in, non-disabled user) vs. authz.ts:639-668 (putAccountProfile requires admin, account owner, or org owner/maintainer). The nav item visibility and the actual page load both use the same check, so there's no way to reach the page via a direct URL that the nav wouldn't have shown.
  • Reserved-id collision safety (plan.ts:83-93): sanitizeName collapses any run of non-[a-z0-9] characters (including repeated hyphens) into a single -, so svc--<name> can never contain a second --. Confirmed ID_REGEX (types/shared.ts:25) independently forbids -- in human-chosen ids, matching the comment's claim. Covered by the evil--thing test case in plan.test.ts:85-87.
  • plan.ts purity: no I/O, no mutation of shared state; planChanges/planDelete/planDisable are deterministic functions over their inputs, matching the "nothing persists" claim in the PR description.
  • XSS/injection surface: all user-controlled strings (name, repo, ref, product ids) are rendered through JSX text nodes / <Code>, never dangerouslySetInnerHTML or raw HTML — no injection vector despite fields like repository and ref being free text.
  • Mock API key (ServiceAccountForm.tsx:32-40): uses crypto.getRandomValues, generated and held only in client state, never transmitted — consistent with "nothing is saved."

No correctness bugs, security issues, or best-practice violations found in the diff.

Follow-up to the service-account mock:

- Submitting now lands on a mock detail view of the integration, with an Edit
  button that returns to the form with every answer intact.
- A one-time API key is shown on that page when a key sign-in method was
  chosen, with the "copy it now, it is only shown once" framing.
- The account id is derived from a human-friendly name rather than typed
  directly. Sanitization collapses double hyphens, so a name can never forge a
  second `svc--` prefix.
- API keys may now be issued with no expiry. #491 currently proposes that every
  key must expire, so the mock flags this as a deliberate departure to decide on.
- The GitHub Actions snippet uses aws-actions/configure-aws-credentials rather
  than hand-rolled environment variables, and notes that the action also calls
  GetCallerIdentity, which /.sts does not implement yet.
- Code samples render in a single <pre> with one container background instead
  of per-line inline code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
Restructures the service-account mock around the flow an admin would
actually use:

- The Service Accounts page is now a list of existing accounts — name and
  derived id, sign-in methods, access, roles, last authenticated, disabled
  state — with a New Service Account button.
- The form moved to /service-accounts/create.
- Each row links to /service-accounts/[name], a detail page whose Edit button
  reopens the form with that account's answers via ?prefill=.
- Editing an existing account exposes a danger zone: disable/enable, and
  delete. Confirming shows the rows the action would touch and, more usefully,
  when access actually stops — deletion does not recall credentials already
  issued, which live out their hour either way.
- Dropped the Owner field and summary row; it is implied by the account whose
  settings you are in. The ownership consequence (org-owned survives a member
  leaving) is kept as a note under the name.

Three fabricated accounts back the list, including one restricted to Read only
and one disabled, so both states are reviewable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
…dary

The list page passed `detailHref` as a function to a client component, which
Next.js refuses to serialize: "Functions cannot be passed directly to Client
Components". Each row now arrives with its own `href`, resolved on the server
via the existing `serviceAccountUrl` helper, so URL construction stays in
lib/urls rather than being rebuilt in the browser.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
…ails>

Everything a real screen would not show — the rows a submit would write, the
caveats, the "this isn't wired up" disclaimer — now sits inside a collapsed
<details>, so the list, form and detail pages read as the product rather than
as an annotated diagram.

Native <details> rather than a Radix accordion: it opens without JavaScript
and is keyboard- and screen-reader-accessible for free. Each summary line
still says "Design mock" while collapsed, so nothing reads as real at a
glance.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
…rows

A table row forced each account's sign-in methods and product grants into a
single truncated cell ("2 products", "GitHub"), which hid exactly the detail
the mock exists to show. Cards give each account room to list every sign-in
method with its repository and ref, every product grant with its permission,
and its roles — without abbreviating.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
There is no longer a separate page per service account. Each card in the list
carries everything the detail page did: sign-in methods, grants, roles, a
collapsed "How to use this" with the workload config, and the collapsed rows
behind it. The only remaining navigation is Edit, which opens the form
prefilled.

Edit, Disable/Enable and Delete now live in a Manage dropdown in the card's
top right, replacing the standalone danger zone. Confirming still reports the
rows the action would touch and when access actually stops.

Also drops the generated `svc--` id from the UI. It is an implementation
detail of the reserved namespace, not something an operator needs to read; it
remains visible in the collapsed model rows where it is actually relevant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
The collapsed "rows behind this service account" disclosure sat between the
card's real content and its footer, which put mock scaffolding in the middle
of a screen meant to read as the product. It is now a modal opened from a
Mock Details entry at the bottom of the Manage dropdown, separated from the
real actions.

The dialog scrolls at 80vh, since a service account with several grants and
sign-in methods produces more rows than fit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
… stdout

The "How to use this" disclosure is now a modal, opened by a Usage example
link on each card, so the card body carries only the account's own facts.

Reworks the API-key snippet on two points:

- The key is stored once in the OS keychain and read from there by the CLI,
  rather than being exported into the environment. The one-time key callout
  now says the same at the moment you'd store it.
- `source-coop token` writes to stdout and the caller redirects it, so the
  path in AWS_WEB_IDENTITY_TOKEN_FILE is the operator's choice. The CLI makes
  no assumption about where the token belongs. The snippet also notes the
  token is short-lived and the SDK re-reads the file, so refreshing it needs
  no restart.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
AWS_ENDPOINT_URL_S3 sits alongside the AWS_ENDPOINT_URL_STS already in the
snippets, so every command in the example inherits the endpoint rather than
each one carrying --endpoint-url. That also removes the line continuation the
YAML example needed.

Renames the placeholder audience from `source-cooperative` to
`source-data-proxy`, matching the convention CI already uses for the GitHub
issuer, and comments what the claim is for — a token minted for anyone else,
real AWS included, is rejected at /.sts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
A hostname names the intended recipient unambiguously: it cannot collide with
another issuer's tokens, it separates environments for free
(data.staging.source.coop will not be accepted at prod), and it matches the
prevailing convention — AWS uses sts.amazonaws.com.

`source-data-proxy` was an internal codename that meant nothing in a workflow
file and would have needed environment suffixes bolted on, as CI's
source-data-proxy-ci already does.

Drops the explanatory comment above the claim.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
@alukach

alukach commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Some design concepts for the cards:

Main@1x StatusRail@1x SpecSheet@1x

Going with option C for now.

…rection

Applies direction C from the design canvas. A tinted identity band carries
the icon, name, status and controls; the body is a three-column grid with
generous spacing; a hairline footer holds the timestamps.

Everything is expressed in Radix theme variables rather than fixed colors, so
the card follows light and dark mode. Two deliberate departures from stock
Radix, both because the theme sets `radius: "none"`:

- Status is a square dot plus a small-caps word, not a soft Badge — Radix's
  tinted badges read as rounded pills and fight square corners.
- Values a machine reads (repository, ref, product, role) render as plain
  mono text in --code-font-family rather than tinted `Code` chips, which at
  this density turned the card into a field of grey blocks.

Also fixes the bucket in both usage snippets: clients address products
path-style as s3://{account}/{product}/, not the registry's internal
account:product bucket name, which never appears in a client URL.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
The theme runs at `scaling: "110%"`, so every Radix size step is multiplied:
`size="1"` renders at 13.2px where the design calls for 11px, and `size="2"`
at 15.4px. The card came out uniformly inflated — labels competing with
values, and mono details rendering at the same size as the text they belong
under, which is what made it read as congested.

Sets the rendered sizes directly through a TYPE constant (label 11, value
14.5, mono value 13, mono detail 12.5, chip 11, meta 11, action 14), and
spaces the header, body and footer at the design's padding rather than the
nearest scaled step. Adds a gap between a sign-in method's name and its
repository lines so the detail sits under the value instead of running into
it.

Also switches Manage from `soft` to `surface`: on the tinted identity band a
soft button reads as a filled grey block rather than the outlined control the
design uses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
…line

Usage example drops highContrast on every card, not just disabled ones. At
gray-12 it competed with the account name beside it; the muted grey reads as
the secondary control it is.

A sign-in method now always renders as a value with its attribute beneath —
GitHub over its repository and ref, API key over its expiry — instead of the
key folding its expiry into the value line with an em dash. The expiry keeps
the sans face since it is prose rather than a machine identifier, but shares
the muted colour and size of the mono attributes beside it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HGZ598AJ1F7NWoyPpgepBt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant