NIP-46 auth

[riccardobl]

Description

Solves #864 .
This PR updates the behavior of the "Login with Nostr" button by adding an intermediary page that presents the following login options:

• Login with extension: the current behavior using the browser extension eg. alby
• Login by pasting a bunker:// token
• Login by using a nip-05 address
• Login by copying or scanning the nostrconnect:/// url with the signer

Screenshots

Additional Context

• I've updated the nostr-tools dependency to the latest version

Still wip

Checklist

Are your changes backwards compatible? Please answer below:

yes

Did you QA this? Could we deploy this straight to production? Please answer below:

yes

For frontend changes: Tested on mobile? Please answer below:

Yes

Did you introduce any new environment variables? If so, call them out explicitly here:

no

[huumn]

@SocketSecurity ignore npm/[email protected]

[ekzyis]

Hi, thanks for the PR and integrating it with my relay code!

I left some suggestions regarding the reconnect logic.

Will look at Relay.subscribe and the rest tomorrow.

[ekzyis]

Hey @riccardobl, sorry for the delayed review.

I tested your code today with nsec.app and pasted the bunker:// string into SN and it works well! I haven't tested nostrconnect:// though. I expected clicking on the QR code would open nsec.app but it didn't. Guess it doesn't register for this scheme?

Anyway, looking through the code, I have two questions:

1. Have you looked into the NIP-46 implementation from NDK¹? Maybe I missed something or forgot the reason, but I wonder why we don't simply use NDK's implementation. That should work just as well with less lines of code we have to maintain ourselves, no?

2. I am currently confused why you need long-living connections with a subscription pool (this.subs) for your implementation. Was it not possible to implement NIP-46 with the existing implementation of relay.fetch? I wrote it for NWC and afaict, NIP-46 has a similar request-response flow so I wonder why it needed changes. Is it because we need to handle connections to multiple relays at once? That's something my relay code is lacking.

Footnotes

1. https://github.com/nostr-dev-kit/ndk/tree/master/ndk/src/signers/nip46 ↩

[riccardobl]

I expected clicking on the QR code would open nsec.app but it didn't. Guess it doesn't register for this scheme?

Clicking or scanning the QR is for the nip-46 nostrconnect:// flow , currently this is not supported by many signers. The only one that i've found that is production looking is nostrum , but it seems it diverges from the spec ( nostr-connect/nostrum#22 ) so it doesn't work properly atm.
I made sure to implement the full nip-46 despite the current status of the various signers.

Have you looked into the NIP-46 implementation from NDK1? Maybe I missed something or forgot the reason, but I wonder why we don't simply use NDK's implementation. That should work just as well with less lines of code we have to maintain ourselves, no?

Mainly because it is not a dependency that is currently available in stacker news and it didn't feel right to add a new dependency just for this, also ndk doesn't support nostrconnect:// flow from what i can tell

I am currently confused why you need long-living connections with a subscription pool (this.subs) for your implementation. Was it not possible to implement NIP-46 with the existing implementation of relay.fetch? I wrote it for NWC and afaict, NIP-46 has a similar request-response flow so I wonder why it needed changes. Is it because we need to handle connections to multiple relays at once? That's something my relay code is lacking.

Mostly because we would need to re-subscribe at least 3 times to the same filter to use the fetch logic, it would be slower and provide no benefit imo, i think it makes sense to use fetch where you don't have a back and forth communication. Also the nostrconnect:// flow needs to listen for an app initiating the connection, so it can't timeout.

subscription pool (this.subs)

The reason to track subscription is to close them once the connection is established in one of the supported methods or if the signer is closed

[ekzyis]

Hi @riccardobl, thanks again for your PR!

We've decided to merge it into an intermediate branch¹ since we're looking into using only NDK for all our nostr code instead of the library mess of nostr-tools and nostr that we currently have. We will do this ourselves in this branch before merging it into master since it's out of the scope of this PR. Your changes look good and you couldn't have known that we will be looking into NDK (we didn't even know that ourselves before I reviewed this PR) so it's okay that you didn't use NDK.

I will approve this PR and @huumn will send you your sats for #864. Looking forward to more contributions from you!

Footnotes

1. I will merge the bugfixes you provided for existing relay code separately into master ↩