nostr-protocol / nips

Nostr Implementation Possibilities
2.39k stars 582 forks source link

NIP-46: Missing Details on Metadata Usage in Client-Initiated Connection #1596

Open buttercat1791 opened 19 hours ago

buttercat1791 commented 19 hours ago

The specification for the client-provided connection string fails to explain the metadata parameter.

Currently, the NIP states the following:

Direct connection initiated by the client

client provides a connection token in the form:

nostrconnect://<client-pubkey>?relay=<wss://relay-to-connect-on>&metadata=<json metadata: {"name":"...", "url": "...", "description": "...", "perms": "..."}>&secret=<required-secret-value>

user passes this token to remote-signer, which then sends connect response event to the client-pubkey via the specified relays. Client discovers remote-signer-pubkey from connect response author. secret value MUST be provided to avoid connection spoofing, client MUST validate the secret returned by connect response.

The metadata query param in the client connection string is mentioned once, here, then never fully explained. The only detail given is in the section on permissions in the connect request, where it states:

Same permission format may be used for perms field of metadata in nostrconnect:// string.

The expected usage of the name, url, and description fields is never given.

Is there anyone who has implemented this client-initiated connection handshake and can explain how the metadata parameter is used?

If there is no expected usage, I propose we replace the metadata param in the client connection string with just an optional perms parameter.

SilberWitch commented 19 hours ago

@fiatjaf can you help us clarify this? Or do you know, who can?

greenart7c3 commented 18 hours ago

Usually clients prefer using name=example&perms=nip04_encryption instead of the json metadata to make the qrcode smaller I support both ways in amber. Dont know about other signers Also i made all those fields optional so you can send only the perms in the json if you want I think nostrudel, coracle and nostr-login widget has the nostrconnect uri

fiatjaf commented 18 hours ago

@staab @nostrband

buttercat1791 commented 18 hours ago

Usually clients prefer using name=example&perms=nip04_encryption instead of the json metadata to make the qrcode smaller I support both ways in amber. Dont know about other signers Also i made all those fields optional so you can send only the perms in the json if you want I think nostrudel, coracle and nostr-login widget has the nostrconnect uri

That's helpful!

Can we define that in NIP-46? It should be clear what sorts of values are expected for name and perms, and we should remove any values from the spec that aren't being used by any clients.

buttercat1791 commented 18 hours ago

@alltheseas

alltheseas commented 18 hours ago

@brugeman

nostrband commented 18 hours ago

Is there anyone who has implemented this client-initiated connection handshake and can explain how the metadata parameter is used?

Metadata is used by signer to show to the user some info about the client that is trying to connect, i.e. "nostrudel.ninja icon is requesting connection, and requesting these perms". All fields are optional, I would say web apps SHOULD provide url and icon, native apps - name and icon.

Usually clients prefer using name=example&perms=nip04_encryption instead of the json metadata to make the qrcode smaller I support both ways in amber. Dont know about other signers Also i made all those fields optional so you can send only the perms in the json if you want I think nostrudel, coracle and nostr-login widget has the nostrconnect uri.

The original spec (or at least every version I looked at) mentions metadata as json. I saw that nostrudel (the only one that supported nostrconnect a while ago) passes it as query-string, so I implemented both ways in nsec.app. nostr-login uses json. I'm fine with switching to query string.

It should be clear what sorts of values are expected for name and perms, and we should remove any values from the spec that aren't being used by any clients.

I don't use description, and I don't think it's useful. The rest are useful and pretty obvious, perms is defined. Maybe something like "Client's metadata is presented by signer to the user to improve UX" might be added to the NIP.

staab commented 17 hours ago

Yeah, this is annoyingly under-specified. I had to look at nostr-login's source to get something to work. It doesn't follow the spec, but instead puts all the params directly into the uri, which is much more sane than json encoding everything. I checked with @greenart7c3 and he said he'd be fine to support that. (I know both guys have already chimed in, just explaining how I got to where I am) So Coracle does this:

const params = new URLSearchParams({secret, url, name, image, perms})

for (const relay of relays) {
  params.append('relay', relay)
}

const nostrconnect = `nostrconnect://${clientPubkey}?${params.toString()}`

@nostrband is referring to icon rather than image, so there are obviously different ideas floating around. We should open a PR to clarify.

staab commented 16 hours ago

https://github.com/nostr-protocol/nips/pull/1597