Giter VIP home page Giter VIP logo

Comments (22)

medmunds avatar medmunds commented on August 20, 2024

It's a great idea. I'm not aware of anyone working on this right now -- thanks for offering.

Do you have thoughts about how incoming emails would show up in Django?

from djrill.

chrisfranklin avatar chrisfranklin commented on August 20, 2024

Not sure, In my app I personally pass them into django-helpdesk but in this case I think it might be sensible to provide an extra model. What are your feelings on the matter? Perhaps provide a settings such as Email_inbound_model to let the user specify their own model if they prefer, in which case we should provide an abstract model for them to inherit from.

from djrill.

chrisfranklin avatar chrisfranklin commented on August 20, 2024

It would also be nice to provide signals containing the email so that the user is free to implement the link with django-helpdesk or others but does not have to scour the mandrill documentation on web hooks. I think it could be a nice little time saver.

from djrill.

medmunds avatar medmunds commented on August 20, 2024

I would probably lean toward only sending signals, and not create any new models.

What happens with the inbound-email is going to be very app-specific, so sending the signal gives the host app flexibility to do whatever it wants: direct it to another app like django-helpdesk, turn it into a blog post, send an automatic response, or anything else. (The app can easily create or update one of its models in its signal handler, if that's what's desired.)

from djrill.

chrisfranklin avatar chrisfranklin commented on August 20, 2024

That settles it then, let me play with my code until I am happy then I will split it out and put in a pull request, thanks for getting back to me so quickly. I will implement signals for the various different web hooks that mandrill requires. To save opening another issue (although I can if you prefer) does the admin interface still have styling issues with admin themes such as grappelli and admin-tools?

from djrill.

medmunds avatar medmunds commented on August 20, 2024

Great, thanks. [Yes, please submit a separate issue for any admin problems -- easier to track that way. Wouldn't be at all surprised to hear it's broken.]

A couple of other thoughts on inbound handling...

One of the goals of Djrill is to make the Mandrill API more Pythonic (and more, uh, Djangotic?). So certainly it should parse the Mandrill webhook's JSON blob into Python data structures. But maybe it should also provide the signal handler with a Python email.message.Message? That would make it easy for apps to use all the email package functions, and would simplify sharing incoming-email code between Djrill and (e.g.,) IMAP.

I'm a little bit concerned about security. From the docs, it seems that Mandrill (and Mailchimp) don't make any attempt authenticate their calls to your app's webhooks: "... keep in mind that your endpoint is going to be wide-open on the internet, and you might not want others to be able to submit random data to your systems. At this time, aside from trying to keep the URL private, our best suggestion is to simply include a secret key in the URL your provide and check that GET parameter in your scripts" (from http://apidocs.mailchimp.com/webhooks/). Keeping the url private doesn't seem terribly robust. I wonder if there's anything else Djrill could do to help the app developer with securing the webhooks? Randomize and update a secret key, or ...? (It's really a shame Mandrill doesn't sign webhook calls with your api secret.)

from djrill.

chrisfranklin avatar chrisfranklin commented on August 20, 2024

Wow yeah I didn't see that, it's a very valid point, we can change the callback URL through the API so doing something like you suggest would be entirely possible. It would certainly be an option to change it based on time but I don't see the need. As long as the connection is going over HTTPS (which is a necessity for webhooks if we are honest, I will make this point in the included docs) then we could just set a DJRILL_CALLBACK_SECRET random string in settings that the user can update, much like the existing django secret. Would this work?

from djrill.

chrisfranklin avatar chrisfranklin commented on August 20, 2024

Perhaps it would be worth regenerating the secret with each new session but I think anything else would be overkill, It is possible I am wrong though =)

from djrill.

chrisfranklin avatar chrisfranklin commented on August 20, 2024

The opposite end of the spectrum would be to change it after every single request which would also be possible, mandrill seems to bunch messages and doesn't hit the API all that often, to be honest delivery is slow, the messages appear in the dashboard but do not get sent out to the webhooks for a while. This could be due to my development environment and my dodgy virgin internet connection. This means we would have plenty of time to regenerate a random key and register it with the API. This would only happen whenever emails were received rather than constantly on a timer.

from djrill.

medmunds avatar medmunds commented on August 20, 2024

Adding a fixed DJRILL_CALLBACK_SECRET sounds good. It adds as much "security" as you can reasonably get with the current scheme. And at least makes users aware of the potential issue.

I feel like updating the webhook regularly might be overcomplicated and error prone around the edges. There'd be potential to get out of sync, conflict with other apps on the same Mandrill account, etc.

This really needs to be addressed on the Mandrill side. (E.g., Pusher handles callback security by providing a HMAC SHA256 digest signed by your API secret.) I'm going to contact Mandrill support -- perhaps they have some sort of callback auth in the works, and just not documented yet.

from djrill.

medmunds avatar medmunds commented on August 20, 2024

Yeah, Mandrill support recommended securing callbacks by including a "secret" query parameter, same as the MailChimp recommendation. (And to be fair, this seems to be how a lot of other APIs handle callback authentication.)

Given the security implications, I'd suggest that Djrill require a DJRILL_WEBHOOK_SECRET if the user enables callbacks. (Raise ImproperlyConfigured if it's not set when adding the callback urls. And then raise SuspiciousOperation if a callback is called with a missing or bad secret param.)

(Oh, and I'm suggesting "WEBHOOK" in the setting name, to match Mandrill's terminology.)

from djrill.

jpadilla avatar jpadilla commented on August 20, 2024

@chrisfranklin any progress on this? Just came across having to deal with webhooks and inbound emails I was thinking of tackling it by simply creating a view that would dispatch signals depending on the incoming event type.

from djrill.

jpadilla avatar jpadilla commented on August 20, 2024

@chrisfranklin @medmunds for now this is what I'm doing https://gist.github.com/jpadilla/5351909

from djrill.

medmunds avatar medmunds commented on August 20, 2024

@jpadilla that looks helpful. Thanks!

I'm guessing there's also some other code that registers the signals and sets up the url paths to your views. If you want to put together a pull request, I'd be happy to roll it into an update.

(One key would be making sure Djrill still loads for people who aren't using the webhooks.)

from djrill.

jpadilla avatar jpadilla commented on August 20, 2024

@medmunds yea it consists of a signals.py that registers the signals as well. Still not sure if it should leave registering the url up to the user, what's your take on that? I also added an optional setting for the secret on a querystring. I'm also thinking of doing something like https://github.com/jpadilla/postmark-inbound-python but for Mandrill webhooks, including inbound as well, but that's something apart from Djrill.

from djrill.

medmunds avatar medmunds commented on August 20, 2024

On the url side, an approach I've seen is for the app to provide an urls.py, but leave it up to the user how (and whether) to include those urls in their main urls.py:

urlpatterns = patterns('',
    # ...
    (r'^djrill/', include('djrill.urls')),  # Mandrill webhooks
    # ...
)

Djrill's installation docs could suggest something like the above, but of course the user could choose a different prefix, or whatever.

from djrill.

medmunds avatar medmunds commented on August 20, 2024

... and as far as processing the event data into a new object (like postmark-inbound-python), I'd suggest keeping it pretty minimal in Djrill. (You could always provide mandrill-inbound-python as a separate-but-compatible package.)

The few bits of processing that would make sense to me:

  • Definitely parsing the json -- and you're already doing this.
  • Maybe converting Mandrill's arrays-of-key-value-pairs into python dicts. (The backend's (poorly-named) _expand_merge_vars does this in the opposite direction.)

Also, I wonder if it would be good for Djrill to send a different signal for each type of Mandrill webhook event? (Or at least split the "inbound mail" signal from the various "status of sent messages" events.) It seems unlikely an app would want to use the same code to handle "incoming message" and "outgoing message bounced".

from djrill.

jpadilla avatar jpadilla commented on August 20, 2024

@medmunds check out what I've got for now on this branch jpadilla@c60a7c6.

I added a mixin that provides the optional check of the DJRILL_WEBHOOK_SECRET. I personally dig the routes on Mandrill for incoming email so I know I'll be creating views using that mixin to handle specific inbound from different routes, resulting in simpler views. For other webhook events like send, open, click and generic inbound stuff you could use the same signal. I'm not against adding specific signals which would make it obvious what the signal is listening for, but I'm not sure if it should be specific signals or a generic one like right now, or both. Thoughts?

# Signals
@receiver(webhook_event)
def handle_webhook_event(event_type, data, *args, **kwargs):
    print event_type
    print data

I definitely want to get this done first and later worry about the other project(processing event data) which anyone could definitely be used to process the data from Djrill signals as well. I like the _expand_merge_vars idea too, so I'll keep that in mind.

from djrill.

jpadilla avatar jpadilla commented on August 20, 2024

@medmunds WIP https://github.com/jpadilla/mandrill-inbound-python

from djrill.

medmunds avatar medmunds commented on August 20, 2024

@jpadilla that looks like a nice start on mandrill-inbound-python. I do think it makes sense to keep it separate from Djrill. The two packages can work perfectly well together if users want them, and can also be used independently if users don't need both sets of functionality.

from djrill.

medmunds avatar medmunds commented on August 20, 2024

Thanks to @jpadilla we now have support for inbound email and other Mandrill webhooks, in the just-released Djrill 0.5.

from djrill.

jpadilla avatar jpadilla commented on August 20, 2024

@medmunds Thanks for all the help and for that fast merge. Cheers!

from djrill.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.