Enforce separate "comment" permission on Sandstorm.

[kentonv]

I used to work on Google Docs sharing. One thing I helped implement there was the ability to grant "comment" permission independently of "read" and "write". Originally we only let editors comment, but users made it very clear that they wanted to grant comment permission without write permission. On the other hand, giving comment permission to all readers (as ep_comments currently does) is also not great, because it means public documents end up littered with spam.

Overhauling Etherpad's basic sharing model to include "comment-only" links (separate from the existing read-only links) would probably be a big project. I have not done that here.

However, on Sandstorm, the sharing model is controlled by the platform. The app merely declares what permissions exist, and then Sandstorm provides the UI for users to share various permissions. So, on Sandstorm, we can trivially add a "comment" permission, and I have done so.

This change extends ep_comments with the minimal changes needed to enforce the Sandstorm comment-only permission. I could maintain this as a fork, but the change should be harmless to merge upstream, and this would be a lot easier for me to maintain.

[JohnMcLear]

Instead of using the sandstorm namespace/variable this should be generic IE "header" as the variable name then the permission header attribute should be set by the admin in settings.json -- This would mean anyone could leverage this functionality irrespective of the service rewriting to Etherpad.

[kentonv]

I'm happy to restructure this to be more reusable, but I'll need to know exactly what you want to avoid "wrong rock" syndrome. I'm not quite sure I understand your comment.

I think you are saying:

• The user should be able to configure the name of the permissions header in settings.json. On Sandstorm it will be x-sandstorm-permissions, but in other environments there might be some other header (which happens to have the same comma-delimited format with permissions called comment and modify).
• clientVars.sandstormPermissions should be renamed to just clientVars.permissions, but will continue to be a list of permission names including comment and modify.

Is that right?

Alternatively, it might be more consistent with the current approach to add clientVars.noComment as a boolean field, similar to the existing readonly field, rather than a permissions field.

[JohnMcLear]

You nailed it on the head :)

[JohnMcLear]

*bump @kentonv

[kentonv]

Hey, sorry, I'm pretty swamped so it's likely I won't work on this again until the next time I sit down to update the Sandstorm Etherpad packaging, and even then I might just maintain it as a fork for now.

[JohnMcLear]

@kentonv want me to spend an hour or sorting it out and add it to my SS hours?

[kentonv]

Sure, if you're confident that you understand Sandstorm's requirements here. The relevant changes I made to Etherpad core are:

• kentonv/etherpad-lite@4bc50d5
• kentonv/etherpad-lite@1fb3bd1 (in particular PadMessageHandler.js and sandstorm-pkgdef.capnp; scroll to the bottom)

If I can remove those and use a settings.json change instead, that'd make me happy. But I'd caution that I don't think it's worth spending too much time generalizing unless there is some specific other use case that you're trying to generalize to cover; otherwise it might be wasted work on features no one else will ever use.

[JohnMcLear]

Btw this is pretty much done just been busy w/ other stuff so haven't committed anything -_-

[JohnMcLear]

Gah I'm not gonna get these commits merged, the TLDR was I moved everything to settings.json -- The soonest I can look into this again is late August. Sorry man things just got crazy busy here and I need to drop some tasks!

[kentonv]

Sure, no rush on this since I have a working solution that I can maintain as a fork.

[JohnMcLear]

I'm gonna close this with the goal to move the conversation to Etherpad core