FriendsOfFlarum / webhooks

Flarum with outgoing webhooks
MIT License
21 stars 12 forks source link

filter events with groups other than admin&guest #12

Closed frankli0324 closed 4 years ago

frankli0324 commented 4 years ago

https://github.com/ReFlar/webhooks/blob/fb308b6f7cf55db6d2ce8d0a02f94573c51be5d2/js/src/admin/components/WebhookEditModal.js#L98

this is wierd.

I tried modifying the database, and group id that are not in ['1', '2'] worked as expected, so why filtering the groups?

dsevillamartin commented 4 years ago

The code only has logic for administrator and guest - when you set the group ID to anything other than admin or guest, it's definitely not working as you'd expect.

Checking if a group had a permission proved to be hard and I wasn't able to figure that out.

frankli0324 commented 4 years ago

webhooks can only be set by admins, why worry about permissions? permissions on what exactly? a bit confused..

It did trigger the webhook though... I tested 3 of the events I need. what could be wrong? am I missing something? image

I disabled the url check temporarily

dsevillamartin commented 4 years ago

That permission is not for who can trigger or edit webhooks. It's to make sure that only content that guests or admins can see triggers a webhooks, e.g. discussions in restricted tag that guests can't see won't be put in a public discord channel

frankli0324 commented 4 years ago

I think that's up to the admin to decide.. at least I wouldn't set up such a webhook that exposes private discussions... proper tag permissions should do

dsevillamartin commented 4 years ago

It is the admin that decides though...? I'm confused as to what you think that drop-down does. It's not a new permission - it's a permission check to avoid having restricted content made public. Tag permissions only work with users, so the check for when a user is a guest is used.

dsevillamartin commented 4 years ago

Before the last update, tag permissions or private discussions didn't prevent webhooks from sending them.

frankli0324 commented 4 years ago

It's not a new permission

no, I don't mean this, I mean by setting up tag permissions and the endpoints properly, admins just wouldn't link a public channel with some private event

frankli0324 commented 4 years ago

in my case, I want to trigger a webhook whenever someone creates a new post, however I have multiple user groups here where group_id of a normal user is 5

different user groups has different tag permissions, they can access different tags, and I wanted to set up a webhook for a specific user group

frankli0324 commented 4 years ago

my original understanding was that option describes the "perspective" of a webhook, the webhook "access as" the user group specified

well... I looked into the source code and found that it's a simpler check than I expected..

I can work on a PR that implements what I described if you want, or could you write the explanation into the README? I personally found it confusing.

frankli0324 commented 4 years ago

okay.. I see what you mean,

proved to be hard

reading through the flarum docs, flarum source, and tags extension source.

The only simple(quoted), solution I came up with is changing the selection box into multiple selection, and create a mock user on creating a webhook for permission checking, which is awful just to think with.

frankli0324 commented 4 years ago

guess that I have to write my own filtering rules

dsevillamartin commented 4 years ago

I attempted to use a mock user, but for whatever reason it didn't work as intended. I spent a while trying out different things but was unable to make it work.

Apologies for the confusion.