chnm / serendipomatic

http://serendipomatic.org/
26 stars 9 forks source link

bookmarklet to select text on an arbitrary web page and send to serendipomatic #147

Open rlskoeser opened 10 years ago

rlskoeser commented 10 years ago

I needed to write a bookmarklet for work and as a simpler first case to get familiar with the technology I thought I'd write a bookmarklet for serendipomatic - current functionality is to take selected text on the current page and submit it as if you had copied and pasted it into the text form.

The one caveat is that the current site doesn't allow this because Django has CSRF (Cross Site Request Forgery) protection turned on by default. I think it's probably fine to turn this off for the main form on the front page since we're not passing any sensitive data or making any updates based on it, but was hoping someone else could confirm that. Anybody know enough to weigh in? Maybe @mialondon or @scottkleinman or @moltude ?

Django CSRF docs are here: https://docs.djangoproject.com/en/dev/ref/contrib/csrf/ There's a stack overflow question about when/why this is dangerous: http://stackoverflow.com/questions/9956356/in-what-case-can-csrf-exempt-be-dangerous

Assuming someone else concurs this sounds ok, I'll make this view CSRF exempt, and add the javascript to the site so it can be called/installed from there. We'll probably need to figure out where the bookmarklet should be made available, along with some brief text to explain how to install and use.

scottkleinman commented 10 years ago

My first sense is that this is OK; I can't think of any way the Serendip-o-matic site could be abused. For a while, we had a bookmarklet functioning as a link. The code is still in view.html, commented out. This was the Save to Zotero bookmarklethttp://www.zotero.org/downloadbookmarkletcode, which, if you'll recall, only worked as a link if you had Zotero Standalone open. Given that this does involve credentials, would disabling CSRF protection prevent us from implementing a Save to Zotero function later?

On 19 September 2013 12:13, Rebecca Sutton Koeser notifications@github.comwrote:

I needed to write a bookmarklet for work and as a simpler first case to get familiar with the technology I thought I'd write a bookmarklet for serendipomatic - current functionality is to take selected text on the current page and submit it as if you had copied and pasted it into the text form.

The one caveat is that the current site doesn't allow this because Django has CSRF (Cross Site Request Forgery) protection turned on by default. I think it's probably fine to turn this off for the main form on the front page since we're not passing any sensitive data or making any updates based on it, but was hoping someone else could confirm that. Anybody know enough to weigh in? Maybe @mialondon https://github.com/mialondon or @scottkleinman https://github.com/scottkleinman or @moltudehttps://github.com/moltude?

Django CSRF docs are here: https://docs.djangoproject.com/en/dev/ref/contrib/csrf/ There's a stack overflow question about when/why this is dangerous: http://stackoverflow.com/questions/9956356/in-what-case-can-csrf-exempt-be-dangerous

Assuming someone else concurs this sounds ok, I'll make this view CSRF exempt, and add the javascript to the site so it can be called/installed from there. We'll probably need to figure out where the bookmarklet should be made available, along with some brief text to explain how to install and use.

— Reply to this email directly or view it on GitHubhttps://github.com/chnm/serendipomatic/issues/147#issuecomment-24765649 .

Scott Kleinman Professor of English Director, Center for the Digital Humanities California State University, Northridge

rlskoeser commented 10 years ago

I was only planning to disable CSRF protection on the index view, which processes the form - however the zotero stuff does add a wrinkle, which probably suggests we should split the two out a bit more than they currently are. In my dev zotero auth branch, once you are logged in to zotero the form generates a list of items/groups/tags from your zotero library and then uses that to pull from your zotero library to feed the machine. --And we should probably keep that as protected as we can; in fact, splitting out the zotero input view would let us require that users be logged in to use that functionality, as well as making it much safer to remove CSRF protection from the text input form.

scottkleinman commented 10 years ago

This makes sense, but it sounds complicated to separate out the Zotero submission form (perhaps because I have not yet had enough coffee). Here are some possible alternatives.

Would either of these alternatives work?

On 25 September 2013 07:24, Rebecca Sutton Koeser notifications@github.comwrote:

I was only planning to disable CSRF protection on the index view, which processes the form - however the zotero stuff does add a wrinkle, which probably suggests we should split the two out a bit more than they currently are. In my dev zotero auth branch, once you are logged in to zotero the form generates a list of items/groups/tags from your zotero library and then uses that to pull from your zotero library to feed the machine. --And we should probably keep that as protected as we can; in fact, splitting out the zotero input view would let us require that users be logged in to use that functionality, as well as making it much safer to remove CSRF protection from the text input form.

— Reply to this email directly or view it on GitHubhttps://github.com/chnm/serendipomatic/issues/147#issuecomment-25089948 .

Scott Kleinman Professor of English Director, Center for the Digital Humanities California State University, Northridge

rlskoeser commented 10 years ago

Thanks for the feedback. I'll consider your suggestions... But the more I think about splitting out the zotero submission logic the more I think it makes sense - there really is not a lot of overlap between the text input handling and the zotero handling (and what overlap there is should be refactored out into reusable functions if it isn't already). Separating them might also help address some of the UX/flow that @mialondon was thinking through in her last comment on #111

mialondon commented 10 years ago

I've finally snuck a minute to look at this!

I think a) the bookmarklet sounds like a good idea and b) so does splitting out the zotero logic.