Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forms on nostr #1190

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

abhay-raizada
Copy link

@abhay-raizada abhay-raizada commented Apr 23, 2024

@abhay-raizada abhay-raizada marked this pull request as draft April 23, 2024 14:26
Copy link
Member

@staab staab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the use of jsonschema, but I think this is too abstract to work well on nostr. Versions should be handled using new kinds, rather than references to schemas (this feels very xmlns ish). A few other comments:

  • Forms should be described using tags. This can help avoid some json-in-json, and I think will make this spec easier for developers to adopt just by reading the NIP.
  • Rather than baking password protection into the spec, I would just re-use NIP 59 for both private forms and private responses (which allows for hiding metadata, not just the response content). You could send the required private key out of band as described, or in any other way, allowing privileged access for whatever use case.
  • Client-specific stuff shouldn't be in a NIP. NIPs are here for interoperability, which is at odds with client-specific details. That stuff can go in NIP 78 instead if needed, or a client's own database.

@abhay-raizada
Copy link
Author

abhay-raizada commented Apr 24, 2024

@staab

Versions should be handled using new kinds, rather than references to schemas (this feels very xmlns ish)

forms would need to have multiple versions in parallel. For example see something like typeform and google forms, both have a separate set of features and need a different template to work with, but both could exist in parallel. I agree i'm not a fan of such namespacing either, but I don't know if new kind for every new template update is a good solution, templates can update based on certain features and updating a nip every time that happens isn't ideal.

Forms should be described using tags. This can help avoid some json-in-json, and I think will make this spec easier for developers to adopt just by reading the NIP.

It'll be very hard to add certain things in tags? like multiple choice options? or even prerequisites on questions? or logic based flows. JSONs are just much more configurable.

Rather than baking password protection into the spec, I would just re-use NIP 59 for both private forms and private responses (which allows for hiding metadata, not just the response content). You could send the required private key out of band as described, or in any other way, allowing privileged access for whatever use case

seems too complicated for a small usecase. just encrypting certain fields with a shared password is a simpler option IMO.

Client-specific stuff shouldn't be in a NIP. NIPs are here for interoperability, which is at odds with client-specific details. That stuff can go in NIP 78 instead if needed, or a client's own database.

agreed, although i think I've wrongly used client specific in the nip, when it should actually be "schema-specific" . Interoperablity doesn't break if clients are able to use schemas interoperab-ly which is why i have tried to collate schemas on the nip. Will update this.

@staab
Copy link
Member

staab commented Apr 24, 2024

JSONs are just much more configurable.

Tags are JSON arrays, which can be used to model associative data structures. It's really no less powerful, and more idiomatic.

just encrypting certain fields with a shared password is a simpler option IMO

It's not though, that's exactly what sharing a private key does, except you have tooling already available in clients to do encryption with secp256k1.

@abhay-raizada
Copy link
Author

abhay-raizada commented May 1, 2024

Tags are JSON arrays, which can be used to model associative data structures. It's really no less powerful, and more idiomatic.

Hmm, you have a point, I'll evaluate on it and iterate if i see no reason for a push-back here.

It's not though, that's exactly what sharing a private key does, except you have tooling already available in clients to do encryption with secp256k1.

passwords make for a better UI, than sharing an entire key, I agree that it makes it less secure, but i think in this particular usecase (hiding the form template) it makes sense, also passwords would give flexibility to obfuscate only certain fields in a form.

EDIT: after going through #1189 I realize it makes much more sense to share forms via p tags itself. Incorporating that into this nip

@abhay-raizada abhay-raizada requested a review from staab May 9, 2024 05:28
@abhay-raizada abhay-raizada changed the title NIP-101: Forms on nostr Forms on nostr May 9, 2024
Copy link
Member

@staab staab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a big improvement. I would even go further and flatten out the settings tag rather than nesting json there, but that's a nitpick.

@abhay-raizada
Copy link
Author

I would even go further and flatten out the settings tag rather than nesting json there, but that's a nitpick.

We can flatten some keys from there like "description", but an optional settings pram still needs to exist.

@abhay-raizada abhay-raizada requested a review from staab May 13, 2024 05:21
101.md Outdated
the mechanism to share the keys is by encrypting the keys and adding it to the p-tag of the event.
The p-tag for `kind:30168` events, should look like.

`["p","<pubkey for the user>", "<optional relays>", "<Encrypted-View-Key>", "<Encrypted-Signing-key>"]`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting choice of putting the two keys in every tag. It does reveal to the public who can edit and who can view. But maybe that's not a problem. Is the goal related to performance? To not decrypt all the other tags as in #1228 at the expense of a bigger event?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes in part, it also made semantical sense while writing the client code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does make simpler. We need to reflect on @mikedilger's concern on #1228 about not reusing the p tag for any of this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought about it as well, doesn't seem good to overload the p-tag. I thought of an agreed upon p<kindno> tags to solve this, but not using p-tags also risks users not being notified, since how many kinds do clients look for to notify the user?

EDIT: nvm, saw your comment on the PR, duplication works, tad wasteful.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIP 72 is not a great spec, it should have used moderator or something rather than p tags

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's from the times everyone was moving to markers and away from positional tags. Now everyone is moving away from markers and into custom tags, like the q thing. :)

@staab
Copy link
Member

staab commented May 13, 2024

All the encryption stuff seems very ad-hoc to me, with the downside that private forms will always leak metadata. Why not wrap forms and send them using something more like NIP 17 or #875?

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented May 13, 2024

We could generalize kind:24 to work for any event (not only for kind:35834) and include the possibility of containing two keys: one to edit the event (signing/encrypting key) and another to decrypt the event (viewing key).

Otherwise having to create a 35834 to define the group + a wrapped kind:24 for each viewing and editing member + merge all operations of kind:27 so editors know who the other editors are for each replaceable/form seems like an overkill. For rotating keys, it will be important for clients to sync with multiple kind 24s and 27s with their respective versions of the replaceable. The design of this PR and #1228 is not as private (I am not sure if we need wrapping levels of privacy for this) but it does solve all of these needs inside a single event (no version <-> permission syncing needs).

@abhay-raizada abhay-raizada marked this pull request as ready for review May 13, 2024 15:28
@staab
Copy link
Member

staab commented May 13, 2024

We could generalize kind:24 to work for any event (not only for kind:35834) and include the possibility of containing two keys: one to edit the event (signing/encrypting key) and another to decrypt the event (viewing key).

That's sort of what I was thinking. Some wrapper that handles the key management in a way that's orthogonal to the thing being wrapped seems ideal.

@vitorpamplona
Copy link
Collaborator

That's sort of what I was thinking. Some wrapper that handles the key management in a way that's orthogonal to the thing being wrapped seems ideal.

Hum.. we might also not need wrapping of kind 24 if participants are public, like in this and the Spreadsheet specs.

@vitorpamplona
Copy link
Collaborator

Cool! Are we moving to key then?

@abhay-raizada
Copy link
Author

Cool! Are we moving to key then?

yes :P

@stokebreakup
Copy link
Contributor

stokebreakup commented Jun 1, 2024

Working version here: https://deploy-preview-151--hilarious-cupcake-5ce684.netlify.app/#/c

This could be an interesting alternative to LinkedIn in Nostr.

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.

None yet

4 participants