Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Description • 11 years ago

When using the Input API, it shouldn't create an anoncsrf cookie when you post feedback data.

	Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Comment 1 • 11 years ago

I'm pretty sure this isn't hard to do--it just needs looking at.

Bumping it to 2013q4.

	Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Comment 2 • 11 years ago

This is annoying, but I'm going to push it off to the next quarter.

Also, this is highly related to bug #947767. They're both csrf-token-with-api related.

	Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Comment 3 • 10 years ago

Moving this to 2014q2.

	Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Comment 4 • 10 years ago

Pushing it off again.

	Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Comment 5 • 10 years ago

There are two steps here:

1. Figure out what code is creating the anoncsrf cookie.

2. Depending on what the answer to 1 is, fix it so that it doesn't create cookies for API things.

	Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Comment 6 • 10 years ago

I'm pushing this to the backlog since this isn't crucial and this quarter is feeling pretty full.

	L. Guruprasad
	Comment 7 • 9 years ago

The cookie is set by django-session-csrf middleware due to the ANON_ALWAYS=True setting that we have configured in fjord/settings/base.py. Here is what the README file (https://github.com/mozilla/django-session-csrf/blob/master/README.rst) of the package says 

<snip>
If you want all views to have CSRF protection for anonymous users, use the following setting:

    ANON_ALWAYS

        always provide CSRF protection for anonymous users

        Default: False
</snip>

I tried overriding it by using the "anonymous_csrf_exempt" decorator, but that doesn't work as the setting seems to override it.

One way to solve this would be to remove ANON_ALWAYS setting (defaults to False) and manually add it to all the views that require the anonymous csrf protection. But that would be very cumbersome to say the least.

Another way would be to fork django-session-csrf (since it doesn't appear to be actively maintained) and add code changes to not apply the anonymous CSRF protection to views decorated with anonymous_csrf_exempt decorator even when ANON_ALWAYS is set to True. This looks to be a very simple code change which I can make.

OTOH, if it is possible to discuss this with the maintainer of the project and see if we can merge our changes, then nothing like it.

	L. Guruprasad
	Comment 8 • 9 years ago

django-session-csrf is a Mozilla project, so I guess we can discuss this with the maintainer and see if we can merge our proposed change.

	Will Kahn-Greene [:willkg] ET needinfo? me Reporter
	Comment 9 • 9 years ago

Looking at that repository, I doubt that there is a maintainer. There certainly isn't any activity.

I don't know much about django-session-csrf and I don't think I understand why we use it rather than the Django csrf stuff. I'll need to spend some time reading through the code in question. If we don't need it--if Django has upped its csrf game or django-session-csrf just isn't important for Input or some third reason--then we can nix it. If we do need it, then your idea about fixing the decorator sounds like a good one.

	L. Guruprasad
	Comment 10 • 9 years ago

I have made code changes by forking willkg's fork of django-session-csrf. Here is the pull request - https://github.com/willkg/django-session-csrf/pull/1/

What I have done is, I have created a new decorator anonymous_csrf_exempt_always (instead of changing the behaviour of the existing anonymous_csrf_exempt decorator) and made the changes to not set the anoncsrf cookie when the view is decorated with it. This will not break any of the existing code in django-session-csrf or fjord as it introduces a new decorator and makes changes to the behaviour only when it is used.

	Ricky Rosario [:rrosario, :r1cky]
	Comment 11 • 9 years ago

(In reply to lgp171188 from comment #10)
> I have made code changes by forking willkg's fork of django-session-csrf.
> Here is the pull request -
> https://github.com/willkg/django-session-csrf/pull/1/
> 
> What I have done is, I have created a new decorator
> anonymous_csrf_exempt_always (instead of changing the behaviour of the
> existing anonymous_csrf_exempt decorator) and made the changes to not set
> the anoncsrf cookie when the view is decorated with it. This will not break
> any of the existing code in django-session-csrf or fjord as it introduces a
> new decorator and makes changes to the behaviour only when it is used.

I think you can just use django's built-in csrf_exempt decorator:
https://docs.djangoproject.com/en/dev/ref/csrf/#django.views.decorators.csrf.csrf_exempt

	L. Guruprasad
	Comment 12 • 9 years ago

(In reply to Ricky Rosario [:rrosario, :r1cky] from comment #11)
> (In reply to lgp171188 from comment #10)
> > I have made code changes by forking willkg's fork of django-session-csrf.
> > Here is the pull request -
> > https://github.com/willkg/django-session-csrf/pull/1/
> > 
> > What I have done is, I have created a new decorator
> > anonymous_csrf_exempt_always (instead of changing the behaviour of the
> > existing anonymous_csrf_exempt decorator) and made the changes to not set
> > the anoncsrf cookie when the view is decorated with it. This will not break
> > any of the existing code in django-session-csrf or fjord as it introduces a
> > new decorator and makes changes to the behaviour only when it is used.
> 
> I think you can just use django's built-in csrf_exempt decorator:
> https://docs.djangoproject.com/en/dev/ref/csrf/#django.views.decorators.csrf.
> csrf_exempt

The csrf_exempt decorator provided by Django is indeed being used in the view that has this bug. See https://github.com/mozilla/fjord/blob/master/fjord/feedback/api_views.py#L137

The problem is, with ANON_ALWAYS=True configured in settings module, the csrf_exempt decorator is not honored by django-session-csrf. It may have been intentional or an accidental side effect and I am not sure about that.

I didn't want to change the existing behaviour of django-session-csrf in any way as I didn't want to risk breaking anything that uses it. That is why I have added a new decorator and customized the behavior only when it is used.

Of course, modifying django-session-csrf to honor csrf_exempt would be the cleanest and ideal solution. But for the reasons stated above, I haven't done it.

	Osmose [:osmose, :mkelly]
	Comment 13 • 7 years ago

The Input service has been decommissioned (see bug 1315316) and has been replaced by a redirect to an external vendor (SurveyGizmo). I'm bulk WONTFIXing Input bugs that do not appear to be relevant anymore.