nostr-protocol / nips

Nostr Implementation Possibilities
2.39k stars 582 forks source link

nip29: support for unmanaged groups, top-level relay-local groups and invite codes #1496

Closed fiatjaf closed 3 weeks ago

fiatjaf commented 2 months ago

This basically allows NIP-29 to be used without any moderation in normal relays and the usage of these relays as groups. These can later upgrade to fully moderated relays.

It is a counterproposal to https://github.com/nostr-protocol/nips/pull/1495.

This PR subsumes https://github.com/nostr-protocol/nips/pull/1493.

staab commented 2 months ago

NIP 29 needed less complexity, not more!

kehiy commented 2 months ago

any updates on this??

staab commented 2 months ago

I've written my own counter-proposal on #1495

kehiy commented 2 months ago

got it, will check it. thanks.

vitorpamplona commented 1 month ago

I really wish NIP-29 was designed for just one application in mind (say... slack XOR discord XOR telegram), instead of a generic group scheme for all event types. In that way, supporters of this NIP could have a fixed scope of event kinds to support and avoid UX issues of some things appear in one client but don't in another.

In the way it's written, it suggests that only super apps can code NIP-29 or users will always complain of things missing.

ismyhc commented 1 month ago

I really wish NIP-29 was designed for just one application in mind (say... slack XOR discord XOR telegram), instead of a generic group scheme for all event types. In that way, supporters of this NIP could have a fixed scope of event kinds to support and avoid UX issues of some things appear in one client but don't in another.

In the way it's written, it suggests that only super apps can code NIP-29 or users will always complain of things missing.

Im not sure I understand what you mean? It's fairly specific now. There aren't many overlapping kinds. It's quite similar to basic telegram as I understand it. Or at least that's how I've coded my client/server implementations.

vitorpamplona commented 1 month ago

Doesn't it have chats + text notes? Those are different things in my mind. There is no reason to require clients to do both in order to have a Telegram client. Separating two and not allowing any other event kind (like a Calendar event, or a live stream) to be part of the group will be beneficial to the NIP.

ismyhc commented 1 month ago

chats + text notes

kind 11 (text root note) is not a great description imo. As I understand it:

Kind 9 chat Kind 11 forum post

No client has to support both. You can just support chat style, or you can just support forum style, or you can do both.

Nip29 feels very specific to me. I guess it's possible we could separate the "forum style", but I like the idea of a nip29 only client that combines group chat functionality with forum style bulletin board functionality.

Also, in the nip29 strfry write policy im working on, I limit the nips quite a lot so that the relay is mainly used for nip29. This is up to the relay operator of course, but I only care about basic metadata (profile info) and nip29 on my relay.

vitorpamplona commented 1 month ago

No client has to support both. You can just support chat style, or you can just support forum style, or you can do both.

That's a problem. You can't just find a "NIP-29 client" to use. You have to find one that implements the features you need for a given group. It is very confusing for users.

I am definitely in favor of breaking this NIP into two even though the group-management stuff is just copy-paste from one another.

ismyhc commented 1 month ago

No client has to support both. You can just support chat style, or you can just support forum style, or you can do both.

That's a problem. You can't just find a "NIP-29 client" to use. You have to find one that implements the features you need for a given group. It is very confusing for users.

I am definitely in favor of breaking this NIP into two even though the group-management stuff is just copy-paste from one another.

Maybe so, but I don't see it as a problem at all. I believe most clients will build out all of the functionality of nip29.

max21dev commented 3 weeks ago

My perspective:

1. Clarify Group Creation Process: Initially, the NIP indicates no direct method for creating groups. However, we have a "create-group" event that any user can send. To clarify this process, it’s important to outline a straightforward set of rules to enable users and clients to easily form groups, leaving it to the relays to decide if they will accept or limit these events, thereby granting or restricting group creation access.

2. Empower "create-group" Event: It’s recommended to include all metadata fields in a single "create-group" event, allowing the relay to decide whether to create the group. Relays may impose certain restrictions on group creation, such as limiting by pubkey, specific words in the group name, or other criteria, or even fully denying group creation through the "create-group" event.

3. Handle “h” tags for “create-group” Event: Currently, there’s some ambiguity around how to handle "h" tags for "create-group" events. Most relays expect these tags for all types of moderation events, including "create-group," yet before a group is created, there’s no "h" tag or group ID available.

4. Defining Relationships Between Roles and Permissions: Initially, permissions were included, but this PR removed them, leaving only roles without linked permissions. It’s crucial to clearly define roles, permissions, and their relationships. Each user may have multiple roles, with each role granting specific, predefined permissions. We need to clarify the structure of roles, permissions, and their connections to users, and streamline the process of adding or removing roles and permissions through well-defined events. Note that using the "add-user" event to handle roles can lead to confusion and misuse and is considered a bad practice.

fiatjaf commented 3 weeks ago

1. Clarify Group Creation Process: Initially, the NIP indicates no direct method for creating groups. However, we have a "create-group" event that any user can send. To clarify this process, it’s important to outline a straightforward set of rules to enable users and clients to easily form groups, leaving it to the relays to decide if they will accept or limit these events, thereby granting or restricting group creation access.

We can add this later if necessary. I don't think this NIP should specify rules for creation of groups. In fact I am against the existence of the create-group, I think groups should be created on a proprietary relay HTML page or manually by relay admins on behalf of manual requests from users. But having an unspecified create-group is a good compromise. Relays decide what to allow what to not allow.

2. Empower "create-group" Event: It’s recommended to include all metadata fields in a single "create-group" event, allowing the relay to decide whether to create the group. Relays may impose certain restrictions on group creation, such as limiting by pubkey, specific words in the group name, or other criteria, or even fully denying group creation through the "create-group" event.

It's bad to have multiple ways of doing the same thing -- such as setting group metadata. Also relays shouldn't be denying group creation from their name, that would be very easy to escape these restrictions if they existed, so I don't think we should do that.

3. Handle “h” tags for “create-group” Event: Currently, there’s some ambiguity around how to handle "h" tags for "create-group" events. Most relays expect these tags for all types of moderation events, including "create-group," yet before a group is created, there’s no "h" tag or group ID available.

If I'm not mistaken I believe create-group should include an h tag with the desired group id. The user or client creating the group decides on that, the relay decides if it allows that or not.

4. Defining Relationships Between Roles and Permissions: Initially, permissions were included, but this PR removed them, leaving only roles without linked permissions. It’s crucial to clearly define roles, permissions, and their relationships. Each user may have multiple roles, with each role granting specific, predefined permissions. We need to clarify the structure of roles, permissions, and their connections to users, and streamline the process of adding or removing roles and permissions through well-defined events. Note that using the "add-user" event to handle roles can lead to confusion and misuse and is considered a bad practice.

The reason why this PR is removing those hardcoded permissions is that they are super annoying and shouldn't be specified in the NIP. It's not possible to specify all the nuances of permissions in the NIP, they are way too many. Ultimately the relay will have to decide what the permission "set-role" means, for example, and there are many grey areas. Even a half-assed specification like we had here was already way too cumbersome to implement on the relay side. It's much better to just not specify anything and let each relay make up their own rules, so they can be very simple, none or super complicated as per their wish.

fiatjaf commented 3 weeks ago

I'd like to merge this soon if no one objects.

sepehr-safari commented 3 weeks ago

We can add this later if necessary

Many devs are working on their NIP29 clients right now, and most of them are on their early stages, better to reach an agreement sooner 🤔

max21dev commented 3 weeks ago

The reason why this PR is removing those hardcoded permissions is that they are super annoying and shouldn't be specified in the NIP. It's not possible to specify all the nuances of permissions in the NIP, they are way too many. Ultimately the relay will have to decide what the permission "set-role" means, for example, and there are many grey areas. Even a half-assed specification like we had here was already way too cumbersome to implement on the relay side. It's much better to just not specify anything and let each relay make up their own rules, so they can be very simple, none or super complicated as per their wish.

I believe without having some minimum standard of the roles and permissions, it is not possible to have a client to work with multiple relays and almost each relay implementation should have its own clients. For me this approach seems against of flexibility of switching between different clients or even relays.

sepehr-safari commented 3 weeks ago

currently (especially after this PR), we can't have a client that works with multiple different relays, and we can't have a relay that works with multiple different clients.

ismyhc commented 3 weeks ago

1. Clarify Group Creation Process: Initially, the NIP indicates no direct method for creating groups. However, we have a "create-group" event that any user can send. To clarify this process, it’s important to outline a straightforward set of rules to enable users and clients to easily form groups, leaving it to the relays to decide if they will accept or limit these events, thereby granting or restricting group creation access.

We can add this later if necessary. I don't think this NIP should specify rules for creation of groups. In fact I am against the existence of the create-group, I think groups should be created on a proprietary relay HTML page or manually by relay admins on behalf of manual requests from users. But having an unspecified create-group is a good compromise. Relays decide what to allow what to not allow.

2. Empower "create-group" Event: It’s recommended to include all metadata fields in a single "create-group" event, allowing the relay to decide whether to create the group. Relays may impose certain restrictions on group creation, such as limiting by pubkey, specific words in the group name, or other criteria, or even fully denying group creation through the "create-group" event.

It's bad to have multiple ways of doing the same thing -- such as setting group metadata. Also relays shouldn't be denying group creation from their name, that would be very easy to escape these restrictions if they existed, so I don't think we should do that.

3. Handle “h” tags for “create-group” Event: Currently, there’s some ambiguity around how to handle "h" tags for "create-group" events. Most relays expect these tags for all types of moderation events, including "create-group," yet before a group is created, there’s no "h" tag or group ID available.

If I'm not mistaken I believe create-group should include an h tag with the desired group id. The user or client creating the group decides on that, the relay decides if it allows that or not.

4. Defining Relationships Between Roles and Permissions: Initially, permissions were included, but this PR removed them, leaving only roles without linked permissions. It’s crucial to clearly define roles, permissions, and their relationships. Each user may have multiple roles, with each role granting specific, predefined permissions. We need to clarify the structure of roles, permissions, and their connections to users, and streamline the process of adding or removing roles and permissions through well-defined events. Note that using the "add-user" event to handle roles can lead to confusion and misuse and is considered a bad practice.

The reason why this PR is removing those hardcoded permissions is that they are super annoying and shouldn't be specified in the NIP. It's not possible to specify all the nuances of permissions in the NIP, they are way too many. Ultimately the relay will have to decide what the permission "set-role" means, for example, and there are many grey areas. Even a half-assed specification like we had here was already way too cumbersome to implement on the relay side. It's much better to just not specify anything and let each relay make up their own rules, so they can be very simple, none or super complicated as per their wish.

I believe without having some minimum standard of the roles and permissions, it is not possible to have a client to work with multiple relays and almost each relay implementation should have its own clients. For me this approach seems against of flexibility of switching between different clients or even relays.

As I've written my relay implementation as I client you will be able to determine by group through 39003 what the roles are and what permissions they have. 39003 is part of this pull request. Although, in the spec, specifying the actual permissions a role has was something I added.

@sepehr-safari this should solve your problems. I had the same issue when coding my relay implementation to this PR.

For instance, a 39003 on my relay would look like this:

{
  "content": "",
  "created_at": 1728676272,
  "id": "121489f55f3e59db7d60751604a1ba76d3896ea4285fa4c6873ee3d59ebe4523",
  "kind": 39003,
  "pubkey": "000009056fd123b6a2368f532ec707f9b368c0d1fed62f68be0baeceec776274",
  "sig": "a3088ef308a4a6ac73d7144692828001d769652d112cf13e8e724d161c55cfe680f0a883735c23bb87966bb76c3fe9c3d481a9cf3d98ffc4a52aa0efcf442ad7",
  "tags": [
    [
      "d",
      "01J9YJ56G6RHSTANHPA2WASDSD"
    ],
    [
      "owner",
      "Owner",
      "The owner role is like god mode for the group",
      "add-user",
      "edit-metadata",
      "delete-event",
      "remove-user",
      "delete-group"
    ],
    [
      "admin",
      "Admin",
      "The admin role can do everything except remove the owner, delete the group and change owner role",
      "add-user",
      "edit-metadata",
      "delete-event",
      "remove-user"
    ],
    [
      "moderator",
      "Moderator",
      "The moderator role can do everything except remove the owner and admin, delete the group, edit metadata and change roles",
      "add-user",
      "delete-event",
      "remove-user"
    ]
  ]
}

Other than that, I think the spec is good. @fiatjaf I would suggest we add that role permissions be added to each role in 39003. I believe it solves the issue others are raising here.

sepehr-safari commented 3 weeks ago

For instance, a 39003 on my relay would look like this:

as you mentioned, that would be YOUR implementation – others might implement it the other way! that's what I'm talking about. it's a protocol, things should have some standards.

ismyhc commented 3 weeks ago

For instance, a 39003 on my relay would look like this:

as you mentioned, that would be YOUR implementation – others might implement it the other way! that's what I'm talking about. it's a protocol, things should have some standards.

Correct, Ive suggested this to @fiatjaf as it solves your concern around this area because I ran into the same issue. I plan to have my implementation ready soon.

As far as I understand it there is only my nip29 server solution and fiatjaf's previous solution which I don't believe has been updated with this PR.

Ive implemented this PR and only added this.

fiatjaf commented 3 weeks ago

Can a moderator remove an admin? Can an admin delete a post from a moderator? Can an admin assign moderator status to an user? Can an admin remove admin status from another? Can an admin change the group from private to public? Can a moderator add an user that has been banned by an admin?

ismyhc commented 3 weeks ago

Can a moderator remove an admin? Can an admin delete a post from a moderator? Can an admin assign moderator status to an user? Can an admin remove admin status from another? Can an admin change the group from private to public? Can a moderator add an user that has been banned by an admin?

These decisions are coded into the relay. It would be to complicated to pass that info to the user via 39003. So instead I just give a general idea of what a role can perform, but not the nuances.

ismyhc commented 3 weeks ago

It looks like this on the relay side

fn owner_permissions() []string {
    permissions := [
        GroupPermission.add_user.str(),
        GroupPermission.edit_metadata.str(),
        GroupPermission.delete_event.str(),
        GroupPermission.remove_user.str(),
        GroupPermission.delete_group.str(),
    ]
    return permissions
}

fn admin_permissions() []string {
    permissions := [
        GroupPermission.add_user.str(), // Cannot add owner or change owner role
        GroupPermission.edit_metadata.str(),
        GroupPermission.delete_event.str(), // Cannot delete owner events
        GroupPermission.remove_user.str(), // Cannot remove owner
    ]
    return permissions
}

fn moderator_permissions() []string {
    permissions := [
        GroupPermission.add_user.str(), // Cannot add owner, admin or change owner role or admin role
        GroupPermission.delete_event.str(), // Cannot delete owner, admin event
        GroupPermission.remove_user.str(), // Cannot remove owner or admin
    ]
    return permissions
}
sepehr-safari commented 3 weeks ago

Can a moderator remove an admin? Can an admin delete a post from a moderator? Can an admin assign moderator status to an user? Can an admin remove admin status from another? Can an admin change the group from private to public? Can a moderator add an user that has been banned by an admin?

how about a basic field for "power" ?

Role: Owner Power: 4 Permissions: delete-event, edit-metadata, set-permissions

Role: Moderator Power: 3 Permissions: delete-event, set-permissions

Role: Admin Power: 2 Permissions: delete-event

Role: User Power: 0 Permissions: -

and these are not hardcoded in the NIP, group owners could set their own rules.

this Power thing reminds me of my good old TeamSpeak days!

pablof7z commented 3 weeks ago

currently (especially after this PR), we can't have a client that works with multiple different relays,

Why?

ismyhc commented 3 weeks ago

Can a moderator remove an admin? Can an admin delete a post from a moderator? Can an admin assign moderator status to an user? Can an admin remove admin status from another? Can an admin change the group from private to public? Can a moderator add an user that has been banned by an admin?

how about a basic field for "power" ?

Role: Owner

Power: 4 Permissions: delete-event, edit-metadata, set-permissions

Role: Moderator

Power: 3 Permissions: delete-event, set-permissions

Role: Admin

Power: 2 Permissions: delete-event Role: User Power: 0 Permissions: -

and these are not hardcoded in the NIP, group owners could set their own rules.

this Power thing reminds me of my good old TeamSpeak days!

Internally that is how I handle the permissions actually. It's not exposed to the client though.

fn (gr GroupRole) power() int {
    return match gr {
        .owner { 20 }
        .admin { 15 }
        .moderator { 10 }
        .member { 0 } // Non-admin member
    }
}

This allows me to do stuff like this in the relay code

requested_role := group_role_from_string(role_str)
if requestor_role.power() < requested_role.power() {
    reject('${pubkey} is not able to perform this role change ${group_id}', event_id)
    continue
}

I guess we could expose this in 39003, I would be in favor of it, but for the sake of moving forward maybe that should come later. Maybe after I release this relay implantation and work out the kinks. If it works well then we could add it.

fiatjaf commented 3 weeks ago

Please open a PR with the role definitions so we can discuss there.

fiatjaf commented 3 weeks ago

These decisions are coded into the relay. It would be to complicated to pass that info to the user via 39003. So instead I just give a general idea of what a role can perform, but not the nuances.

We would need a list of permissions in the NIP anyway, I don't know if it makes sense to have the list, but then not specify what they do. Maybe it's a good idea?, I can see it both sides.