~ knotless Search/
↑↓ move↵ open⇧↵ all results

grain.social/grain

photography-focused photo sharing platform

grain.social
1 collaborator
0
2

A group is started on grain's group host without leaving grain

Closed
Closed chadtmiller.com asked to merge into feature/rust from grain.social/grain:gaps/groups

What changes

social.grain.group.createGroup and getGroupHost are served by grain-serve (crates/grain-serve/src/spaces/group.rs), under the spaces' unshared layer, the write by write::writer. --group-host (GRAIN_GROUP_HOST in deploy) names the group host that has agreed to found groups for grain; getGroupHost answers its URL and the handle domain its describeServer gives, and {} when none is set or the host does not describe itself (logged, and asked again next time).

Starting a group: the founder's PDS signs service auth for fyi.opensocial.provisionGroup; grain posts to it as its confidential OAuth client (a fresh assertion addressed to the host's origin on each try, a DPoP proof of a key made for the session, a nonce answered once) and keeps the session the host hands over as the group's web session (grain::oauth::provisioned, login::provisioned). It is taken only if the host is the group's own authorization server. Then the founder has the host open the pool (fyi.opensocial.createSpace via call_host), grain writes the group's social.grain.actor.profile as the group (write::put, factored out of putRecord, so it is indexed at once), the founder's acceptance is written, and the answer waits up to 5 s for the host's declaration to reach the index. Refusals: NotSupported, InvalidName, HandleUnavailable, CreateFailed, PoolFailed, now declared in the lexicon.

Why

The two methods were the last of the group methods not ported. Behaviour follows the old server's server/xrpc/createGroup.ts, getGroupHost.ts and helpers/groupHost.ts; the lexicons are already under social.grain.group and are not on main, so adding errors and a handleDomain description is not a change to a published schema.

Knotless: there is no counterpart to a session handed over by another server, so oauth::provisioned is built from grain's own oauth.rs (post, proof, Client::fields, server, made's check that the account logs in where it says), which is knotless's oauth.rs. Knotless's space code (Admin::create_of with com.atproto.space.createSpace / memberspace) does not apply to the pool: a group's spaces are made by its host, which fyi.opensocial.createSpace asks, as joinGroup/leaveGroup already do through call_host.

How it was checked

A new spaces test, a_group_is_started_on_the_group_host, against a stand-in founding host (tests/host/mod.rs, Starter): it makes each group an account on the reference PDS, moves its DID document to itself, and as the group's authorization server hands grain a token it trades for the group's session on the way through. It asks for a DPoP nonce first and refuses a client assertion it has seen, and its declaration reaches the index a moment after it answers. The test covers no host and an unanswering host ({}, NotSupported), a bad name, a handle refused, a host handing over (as a whole-body token response) an account it does not log in (CreateFailed), success (handle, what the host was sent, the kept session, the profile and membership on getGroup, the pool readable), and a pool refused. Settings parse --group-host. All five checks pass on the runner.

Shortcomings

  • Not tried against opensocial.fyi itself; the stand-in follows the old server's contract (session or whole-body token response with sub).
  • The 5 s wait for the declaration running out is not exercised by a test.
  • The web still calls social.grain.unspecced.createGroup / getGroupHost, as it does the other group methods.

Review

Start at create_group in spaces/group.rs, then oauth::provisioned.

🤖 Generated with Claude Code

Asked for changes to an earlier version

This ports createGroup and getGroupHost onto grain-serve. create_group has the founder's PDS sign service auth for fyi.opensocial.provisionGroup. It then keeps the session the host hands over, opens the pool, writes the profile as the group, and writes the founder's acceptance. oauth::provisioned accepts the session only if the host is the account's own authorization server, and I found no hole in the flow. Checks were still queued when I looked, and I did not see the diff's test and lexicon files. I read the Rust source only.

One thing should change: in oauth::provisioned the DPoP-nonce resend reuses the client assertion.

  • The problem. client.fields(&at) runs once, before attempt, so the resend after use_dpop_nonce posts the same client_assertion with the same jti. The existing post calls fields inside attempt, so each try gets a new assertion.
  • Why it matters. A host is likely to answer use_dpop_nonce to the first request from a fresh key, since the nonce cache has nothing for it. A host that remembers assertion jtis, as the reference atproto OAuth provider does, may refuse the resend as a replay. Group creation would then fail as CreateFailed against a real host, while the stand-in in the test host does not track jti.
  • The fix. Build the body, including the fields, inside attempt. Have the stand-in host reject a repeated jti so the test covers it.

One smaller point: in spaces/group.rs, the doc comment "The group actor, a DID or handle, names…" now sits above a_group instead of group_named, so it describes the wrong function. Move it back.

crates/grain/src/oauth.rsline 789Outdated
) -> Result<Session> {
let key = P256Key::generate();
let at = origin(url);
let mut sent = body.clone();
review.knotless.dev

fields (and its jti) is computed once here and reused by the nonce resend in attempt(1). Build the body with fresh client.fields(&at) inside attempt, as post does, so a host that rejects replayed assertion jtis accepts the retry.

Approved an earlier version

Both points from my earlier review are done.

  • Client assertion on resend: in oauth::provisioned, client.fields(&at) now runs inside attempt, so the resend after use_dpop_nonce carries a fresh client_assertion and jti. The DPoP proof is also rebuilt on each try and picks up the cached nonce. The stand-in host in tests/host/mod.rs now records jtis and rejects a repeat, so the test covers the resend.
  • Doc comment: the "The group actor…" comment is back above group_named.

I didn't read the whole diff this round. I checked the changed areas and found nothing new. I did not look at the checks.

Approved

Nothing new needs changing since my approval at 4060fd1. I read only the diff from that commit to 6797ea7 and did not look at the rest of the pull again. The checks were still queued, so I don't know whether they pass.

  • starter: it now returns Option<Starter>, and a host that fails to describe itself is logged and not cached. The next call tries again, and both getGroupHost and create_group treat it as "no group host". Behaviour is the same as before and the code is simpler.
  • Test host: the stand-in now hands the stolen account's token back as a whole-body token response with sub, which covers that contract. It also delays the declaration by 500 ms, so the wait for the declaration to reach the index is exercised, though the 5 s timeout is still not.
  • Unreachable host: the test adds a group host that does not answer. It checks {} from getGroupHost and NotSupported from createGroup.

Landed on feature/rust in batch/three (…/3mzbdo7hmjf2b, 236806f), with its head 6797ea7 unchanged.