Add delete request endpoint that creates a channel event for FBA chan…#834
Add delete request endpoint that creates a channel event for FBA chan…#834
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #834 +/- ##
=======================================
Coverage 70.25% 70.25%
=======================================
Files 114 114
Lines 13106 13141 +35
=======================================
+ Hits 9207 9232 +25
- Misses 3208 3216 +8
- Partials 691 693 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@norkans7 can you put the changes to write event UUIDs in a separate PR? |
norkans7
force-pushed
the
channel-events-delete-request
branch
from
55bd95f to
2ce0341
Compare
norkans7
force-pushed
the
channel-events-delete-request
branch
from
2ce0341 to
2a6d194
Compare
|
@rowanseymour I moved that to #835 and rebased this on that branch |
norkans7
force-pushed
the
channel-events-delete-request
branch
from
2a6d194 to
1c71f27
Compare
|
contains #846 |
norkans7
force-pushed
the
channel-events-delete-request
branch
from
1c71f27 to
72bc8c9
Compare
| data := make([]any, 0, 2) | ||
|
|
||
| payloadJson, _ := json.Marshal(payload) | ||
| sentry.CaptureMessage(fmt.Sprintf("Data Deletion Request: %s", payloadJson)) |
There was a problem hiding this comment.
I think this should be on the mailroom side
| return nil, err | ||
| } | ||
|
|
||
| confirmationURL := fmt.Sprintf("https://%s/channels/events/read/%s/", h.Server().Config().Domain, event.UUID()) |
There was a problem hiding this comment.
@norkans7 @ericnewcomer need to make a decision on this URL.. is it maybe odd to have event UUID in the URL and be the confirmation code? Should we use contact UUID instead? Is there a vulnerability there allowing enumeration of contact UUIDs? My inclination is a really generic URL like https://<domain>/public/forgetme/ with a UUID as the confirmation code that they enter.
There was a problem hiding this comment.
I think https://<domain>/public/forgetme/ with the confirmation code being the event UUID is fine
so the view will be a form and we will not query the contacts
There was a problem hiding this comment.
I find that once we submit the form, we need a page to redirect to so i think we need to keep the event read view for that.
And that approach gives us flexibility to change that behavior with changing the link we send for the confirmation
There was a problem hiding this comment.
I used the same view with a get param
https://github.com/nyaruka/rapidpro/pull/5928/files
norkans7
force-pushed
the
channel-events-delete-request
branch
from
72bc8c9 to
5690d30
Compare
norkans7
force-pushed
the
channel-events-delete-request
branch
from
5690d30 to
1c888d3
Compare
| // GetChannel returns the channel | ||
| func (h *handler) GetChannel(ctx context.Context, r *http.Request) (courier.Channel, error) { | ||
| if r.Method == http.MethodGet { | ||
| if r.Method == http.MethodGet || r.URL.Path == "/c/fba/delete" { |
There was a problem hiding this comment.
We use the same URL for all channels and this lookup the channel for the message however for the delete request we do not get info about the channel at all to be able to look it up so will ignore and the URN we get will be unique since they page scoped
There was a problem hiding this comment.
but how can we look up a URN without a channel? Don't we need channel to get org_id ?
There was a problem hiding this comment.
Right, this will not work without a valid channel, the request I expect we get on that also cannot allow us to lookup the channel
I guess we remove the check for the contacts exists in the DB and rely on the URN that we know if page scoped
There was a problem hiding this comment.
We can't create a channel event without a channel... and we don't gain much from making it mailroom's problem to determine if a URN exists - we don't have an index on URN without org id. Argh.
2 participants
…nels