Skip to content

feat: nostr identity endpoints#95

Merged
bubelov merged 13 commits into
masterfrom
feature/nostr-identity-endpoints
Jun 17, 2026
Merged

feat: nostr identity endpoints#95
bubelov merged 13 commits into
masterfrom
feature/nostr-identity-endpoints

Conversation

@escapedcat

@escapedcat escapedcat commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Nostr identity endpoints (parity follow-up to #89)

Follow-up to the merged Nostr sign-in endpoint (#89). That PR let a Nostr key
mint a token; this one exposes the linked identity and lets an existing
account manage its Nostr link. All new surface is REST under
/v4/users/me; no new DB migration.

What's in here

Expose the linked npub

  • GET /v4/users/me (MeResponse) gains npub: Option<String> (bech32, or
    null). Flows into the create-token and update-username responses too via
    the shared struct.
  • The RPC whoami Res gets the same npub field, so the two "who am I"
    surfaces stay in sync.

Manage the link (new sub-resource /v4/users/me/nostr)

  • GET{ npub } — read the current link (Bearer only).
  • DELETE → clears it, idempotent 200 { npub: null } (Bearer only — clearing
    your own link needs no proof).
  • PUT → links/replaces the pubkey on the account. Returns 200 { npub };
    400 if the npub is already linked to a different account; idempotent re-link
    by the same account → 200.

user.npub already exists (migration 98); select_by_npub/set_npub already
existed (the latter was #[allow(dead_code)], now used). No migration, no
schema.sql change.

Decision that needs your eyes: the PUT dual-credential header

PUT /v4/users/me/nostr needs two credentials at once — a Bearer token
(which account) and a NIP-98 signature (proof you control the pubkey).
Both the Auth and NostrAuth extractors read the Authorization header, so
they can't share it. I resolved this by carrying the proof on a dedicated
header:

Authorization:        Bearer <token>
X-Nostr-Authorization: Nostr <base64(event)>   # signs u=<base>/v4/users/me/nostr,
method PUT

Implementation: factored the NIP-98 verification in nostr_auth.rs into a
shared verified_npub(req, header) helper, and added a thin NostrProof
extractor that reads X-Nostr-Authorization (same ApiBaseUrl-pinned
verification as NostrAuth). The request body stays empty, so there's no
SHA-256 body-binding gap. If you'd prefer a different header name or
mechanism, this is the piece to flag
— happy to change it.

(Deliberately not the #83 approach of a nostr_event body field — nip98.rs
warns body endpoints need body-hash binding, and the client-supplied url
param there was the bot-flagged weakness.)

Conflict check is application-level (no unique index yet)

The PUT conflict check is select_by_npub → compare → set_npub, which has a
documented TOCTOU window: two concurrent PUTs linking the same npub to
different accounts could both pass the check. This is intentional for now and
called out in a code comment — the partial unique index on user.npub (your
in-flight work, closed as #93) is what closes it, after which a set_npub
UNIQUE violation could map to 400. I did not add that index or a migration
here
, to stay off your toes.

Heads-up (not touched): stale comment in the merged sign-in code

src/rest/v4/nostr.rs (merged in #89) has a doc-comment claiming the npub
unique index comes "from migration 101" — but on master, 101 is the
access_token.import_origins migration and no such index exists. The
recovery branch it describes is currently dormant. Flagging only; left it
alone since it's coupled to whenever the index actually lands.

Out of scope

escapedcat and others added 2 commits June 3, 2026 17:20
Add an `npub` field (bech32, nullable) to the two "who am I" responses
so the frontend can show whether the logged-in account has a Nostr
identity linked:

- REST `GET /v4/users/me` (MeResponse) — also flows into the
  create-token and update-username responses via the shared struct.
- RPC `whoami` (Res) — kept in sync with the REST surface.

`user.npub` already exists on the model (migration 98); this is a
purely additive projection change, no query or schema change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two Bearer-authenticated sub-resources for an account's Nostr link:

- GET  /v4/users/me/nostr  -> { npub } (or null) — lets a client poll
  just the link state without the full /me payload.
- DELETE /v4/users/me/nostr -> clears the link via set_npub(None).
  Idempotent: returns 200 { npub: null } even when nothing was linked,
  since clearing your own link needs no NIP-98 proof.

Registered in the v4 users scope. Removes the now-unused #[allow(dead_code)]
on user::queries::set_npub. Linking/replacing a pubkey (PUT) lands
separately since it additionally requires a NIP-98 ownership proof.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This comment was marked as resolved.

escapedcat and others added 4 commits June 3, 2026 17:47
Copilot review on #95 flagged it: get_nostr never touches the pool —
the npub is already on auth.user, and the Auth extractor reads the pool
from app_data itself, so the Data<MainPool> handler param was dead
weight. Removing it makes the signature honest.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot review on #95 flagged the public REST docs as incomplete — the
new identity sub-resource endpoints weren't listed. Add the Available
Endpoints entries plus request/response sections for reading and
clearing the linked npub. (PUT link/replace will be documented when
that endpoint lands.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Links (or replaces) the Nostr pubkey on an already-authenticated
account. This needs TWO credentials at once — a Bearer token to say
which account, and a NIP-98 signature to prove control of the pubkey
being linked — but Auth and NostrAuth both read the Authorization
header. Resolve it by carrying the proof on a dedicated header:

- Factor the NIP-98 verification in nostr_auth.rs into a shared
  `verified_npub(req, header)` helper.
- Add a `NostrProof` extractor that reads `X-Nostr-Authorization`
  (new const `X_NOSTR_AUTHORIZATION`), reusing the same ApiBaseUrl-pinned
  verification as NostrAuth. NostrAuth keeps reading `Authorization`.

Handler: Auth identifies the account, NostrProof proves the pubkey.
Conflict is checked at the application level — if the proven npub is
already linked to a different account, return 400; re-linking your own
npub is an idempotent 200. The empty body keeps the NIP-98 binding to
just `u`+`method` (the nip98 module does no body-hash binding).

There is deliberately NO DB migration: with no UNIQUE index on
user.npub yet, the select-then-set has a documented TOCTOU window that
the maintainer-owned partial unique index is meant to close. Flagged
in code and reserved for that follow-up rather than pre-empted here.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document the link/replace endpoint: the dual-credential requirement
(Bearer + the NIP-98 proof on the X-Nostr-Authorization header), the
exact u/method the proof must sign, and the 200/400/401 responses.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This comment was marked as outdated.

escapedcat and others added 5 commits June 16, 2026 11:58
The link endpoint's conflict check is a best-effort select-then-set with
a documented TOCTOU window (no unique index on user.npub yet). Wrap the
set_npub write so a `user.npub` UNIQUE violation maps to 400 instead of a
generic 500, reusing nostr::is_unique_violation_on (now pub(crate)).

No behavior change against today's schema — no index can fire — but the
endpoint becomes race-safe automatically if the maintainer-owned partial
unique index on user.npub lands later: the concurrent loser then gets the
same 400 as the pre-check path, with no further code change. Extracts the
shared 400 into a small npub_conflict() helper.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Harden the link-endpoint tests so they verify the safety property, not
just the status code:

- conflict-400: a regression that wrote the npub to the claimer *and*
  returned 400 would previously pass. Now reload both users and assert
  the claimer stays unlinked and the owner keeps the npub.
- idempotent-same-user: reload and assert the npub is unchanged (not
  cleared) after a re-link.
- replace test: use a freshly generated key for the prior npub instead
  of the non-bech32 "npub1old" literal, matching what production stores.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migration 101 is the access_token import_origins migration; there is no
unique index on user.npub on master. Three comments (from the merged
sign-in code in #89) claimed such an index exists and enforces
uniqueness. Correct them to reflect reality: uniqueness is enforced at
the call sites today, the index is an optional future add, and the
in-code recovery branch becomes the atomic path only once it lands.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI floats to the latest stable Rust (master removed rust-toolchain.toml),
and 1.96 added/strengthened lints that fail the `clippy -- -D warnings`
gate on pre-existing code:

- unnecessary_sort_by -> sort_by_key(|x| Reverse(..)) in feed/atom.rs,
  rpc/analytics/get_top_clients.rs, rpc/get_area_dashboard.rs,
  rest/v4/activity.rs (all sort keys are Copy)
- useless_conversion -> drop the redundant .into_iter() in
  rpc/set_area_icon.rs, rpc/set_area_tag.rs

None of these are in the Nostr-identity code; they're master's, surfaced
only under 1.96. Verified locally after `rustup update stable` to 1.96.0
to match CI.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This comment was marked as resolved.

Copilot review on #95 flagged the Update Username response example as
stale: update_username returns MeResponse::from(&user), so the response
includes npub — and (via the From impl) always-empty saved_places /
saved_areas. Update the example and field table to the real shape and
note the saved arrays are empty on this endpoint.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@escapedcat escapedcat marked this pull request as ready for review June 16, 2026 12:20
@escapedcat escapedcat requested a review from bubelov June 16, 2026 12:28
@bubelov bubelov merged commit 9b5544f into master Jun 17, 2026
1 check passed
@escapedcat escapedcat deleted the feature/nostr-identity-endpoints branch June 17, 2026 08:58
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.

3 participants