Repository: falconry/falcon
Pull request #1557 (2019-06-27 11:20:42)

Adding SameSite keyword
This is PR for #1556 

Added `same_site` keyword to `Response.set_cookie` method.

5onic: @vytas7 this PR is still in the WIP stage.

Also, I have one more question - Should we handle [here](https://github.com/falconry/falcon/pull/1557/commits/9893e15b2906a61ba25d6a42621b3e9994e74417#diff-4a6833e063be95e9d02ddb25e9a8d8d3R428) whether a value is equal to `Lax` or `Strict`? because my God tells me that, we need to do it, and raise an error if a value wasn't equal.
mikeyusko: @RonRothman @vytas7 Sorry for extremely delay with that Pull request, I plan to fix the review comments in 2-3 days. After that it will be ready for code review.
mikeyusko: Hm, I can't understand why I the PR failed, on my local machine everything were passed.
vytas7: @mikeyusko Tests seem to fail on all Python environments because of quite mundane reasons.
Is it possible that you have some uncommitted fixture or tweak in your local checkout that helps in passing tests, or some local library on your system? Could you take a look?
mikeyusko: @vytas7 Ok, I did an investigation, and unfortunately, we will able to add `SameSite` cookie since python3.8 and later.

The reason is - we're using a built-in `http` package, and the cookie was introduced in the python3.8 version.

```
Python 3.7.2 (v3.7.2:9a3ffc0492, Dec 24 2018, 02:44:43)
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from http import cookies
>>> cookies.Morsel._reserved
{'expires': 'expires', 'path': 'Path', 'comment': 'Comment', 'domain': 'Domain', 'max-age': 'Max-Age', 'secure': 'Secure', 'httponly': 'HttpOnly', 'version': 'Version'}
```

```
Python 3.8.0a1 (v3.8.0a1:e75eeb00b5, Feb  3 2019, 12:33:49)
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from http import cookies
>>> cookies.Morsel._reserved
{'expires': 'expires', 'path': 'Path', 'comment': 'Comment', 'domain': 'Domain', 'max-age': 'Max-Age', 'secure': 'Secure', 'httponly': 'HttpOnly', 'version': 'Version', 'samesite': 'SameSite'}
>>>
```

inirudebwoy: Hi @mikeyusko , 
There is a new option coming for `SameSite` according to this article https://blog.chromium.org/2019/10/developers-get-ready-for-new.html

AFAIK your code will support it but the docstring needs to be corrected.

kgriffs: Hi @mikeyusko, just checking in to see where we you with this? I'd love to get it over the finish line soon if we can.
mikeyusko: > Hi @mikeyusko, just checking in to see where we you with this? I'd love to get it over the finish line soon if we can.

Hi @kgriffs I'm going to finish it by the end of the week.
vytas7: Re the absence of [`Morsel.samesite`](https://docs.python.org/3.8/library/http.cookies.html#http.cookies.Morsel) in Python < 3.8.0... I guess we could ship our own `Morsel` class to enable our test client? Particularly considering PyPy is probably a couple of light-years apart from 3.8.0 compatibility...

If we don't provide any alternatives, any API user testing on pre-3.8.0 would run into the very same issue in their own unit tests.
kgriffs: @mikeyusko I've got some time to work on this. If you don't mind, I can add a couple patches to address remaining comments.
codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1557?src=pr&el=h1) Report
> Merging [#1557](https://codecov.io/gh/falconry/falcon/pull/1557?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/6982394b2e1b26e2b6ebbcb5dcc938ca23690b5a?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `93.75%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1557/graphs/tree.svg?width=650&token=zOtkfNQ3h6&height=150&src=pr)](https://codecov.io/gh/falconry/falcon/pull/1557?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1557      +/-   ##
==========================================
- Coverage     100%   99.96%   -0.04%     
==========================================
  Files          40       40              
  Lines        2595     2602       +7     
  Branches      394      396       +2     
==========================================
+ Hits         2595     2601       +6     
- Partials        0        1       +1
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1557?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/response.py](https://codecov.io/gh/falconry/falcon/pull/1557/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbnNlLnB5) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/testing/client.py](https://codecov.io/gh/falconry/falcon/pull/1557/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvY2xpZW50LnB5) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/util/\_\_init\_\_.py](https://codecov.io/gh/falconry/falcon/pull/1557/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvX19pbml0X18ucHk=) | `85.71% <66.66%> (-14.29%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1557?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1557?src=pr&el=footer). Last update [6982394...66c3d9a](https://codecov.io/gh/falconry/falcon/pull/1557?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/falcon/response.py b/falcon/response.py
index 177f7a7..3042a5d 100644
--- a/falcon/response.py
+++ b/falcon/response.py
@@ -14,7 +14,6 @@
 
 """Response class."""
 
-from http import cookies as http_cookies
 import mimetypes
 
 from falcon import DEFAULT_MEDIA_TYPE
@@ -28,7 +27,7 @@ from falcon.response_helpers import (
     header_property,
     is_ascii_encodable,
 )
-from falcon.util import dt_to_http, structures, TimezoneGMT
+from falcon.util import dt_to_http, http_cookies, structures, TimezoneGMT
 from falcon.util.uri import encode as uri_encode
 from falcon.util.uri import encode_value as uri_encode_value
 
@@ -41,6 +40,9 @@ _STREAM_LEN_REMOVED_MSG = (
 )
 
 
+_RESERVED_SAMESITE_VALUES = frozenset({'lax', 'strict', 'none'})
+
+
 class Response:
     """Represents an HTTP response to a client request.
 
@@ -279,7 +281,7 @@ class Response:
         self._headers['content-length'] = str(content_length)
 
     def set_cookie(self, name, value, expires=None, max_age=None,
-                   domain=None, path=None, secure=None, http_only=True):
+                   domain=None, path=None, secure=None, http_only=True, same_site=None):
         """Set a response cookie.
 
         Note:
@@ -357,6 +359,18 @@ class Response:
 
                 (See also: RFC 6265, Section 4.1.2.6)
 
+            same_site (str): Helps protect against CSRF attacks by restricting
+                when a cookie will be attached to the request by the user agent.
+                When set to ``'Strict'``, the cookie will only be sent along
+                with "same-site" requests.  If the value is ``'Lax'``, the
+                cookie will be sent with same-site requests, and with
+                "cross-site" top-level navigations.  If the value is ``'None'``,
+                the cookie will be sent with same-site and cross-site requests.
+                Finally, when this attribute is not set on the cookie, the
+                attribute will be treated as if it had been set to ``'None'``.
+
+                (See also: `Same-Site RFC Draft`_)
+
         Raises:
             KeyError: `name` is not a valid cookie name.
             ValueError: `value` is not a valid cookie value.
@@ -364,12 +378,15 @@ class Response:
         .. _RFC 6265:
             http://tools.ietf.org/html/rfc6265
 
+        .. _Same-Site RFC Draft:
+            https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.2.7
+
         """
 
         if not is_ascii_encodable(name):
-            raise KeyError('"name" is not ascii encodable')
+            raise KeyError('name is not ascii encodable')
         if not is_ascii_encodable(value):
-            raise ValueError('"value" is not ascii encodable')
+            raise ValueError('value is not ascii encodable')
 
         value = str(value)
 
@@ -414,10 +431,7 @@ class Response:
         if path:
             self._cookies[name]['path'] = path
 
-        if secure is None:
-            is_secure = self.options.secure_cookies_by_default
-        else:
-            is_secure = secure
+        is_secure = self.options.secure_cookies_by_default if secure is None else secure
 
         if is_secure:
             self._cookies[name]['secure'] = True
@@ -425,6 +439,17 @@ class Response:
         if http_only:
             self._cookies[name]['httponly'] = http_only
 
+        # PERF(kgriffs): Morsel.__setitem__() will lowercase this anyway,
+        #   so we can just pass this in and when __setitem__() calls
+        #   lower() it will be very slightly faster.
+        if same_site:
+            same_site = same_site.lower()
+
+            if same_site not in _RESERVED_SAMESITE_VALUES:
+                raise ValueError("same_site must be set to either 'lax', 'strict', or 'none'")
+
+            self._cookies[name]['samesite'] = same_site.capitalize()
+
     def unset_cookie(self, name):
         """Unset a cookie in the response
 
diff --git a/falcon/util/__init__.py b/falcon/util/__init__.py
index 5f02a77..80e31ec 100644
--- a/falcon/util/__init__.py
+++ b/falcon/util/__init__.py
@@ -20,9 +20,18 @@ Conversely, the `uri` module must be imported explicitly::
 
 """
 
+from http import cookies as http_cookies
 import json  # NOQA
 
 # Hoist misc. utils
 from falcon.util.structures import *  # NOQA
 from falcon.util.misc import *  # NOQA
 from falcon.util.time import *  # NOQA
+
+
+# NOTE(kgriffs): Backport support for the new 'SameSite' attribute
+#   for Python versions prior to 3.8. We do it this way because
+#   SimpleCookie does not give us a simple way to specify our own
+#   subclass of Morsel.
+if 'samesite' not in http_cookies.Morsel._reserved:  # pragma: no cover
+    http_cookies.Morsel._reserved['samesite'] = 'SameSite'


Test patch:
diff --git a/falcon/testing/client.py b/falcon/testing/client.py
index 0da2cf6..dd64d9b 100644
--- a/falcon/testing/client.py
+++ b/falcon/testing/client.py
@@ -18,14 +18,13 @@ This package includes utilities for simulating HTTP requests against a
 WSGI callable, without having to stand up a WSGI server.
 """
 
-from http import cookies as http_cookies
 import warnings
 import wsgiref.validate
 
 from falcon.constants import MEDIA_JSON
 from falcon.testing import helpers
 from falcon.testing.srmock import StartResponseMock
-from falcon.util import CaseInsensitiveDict, http_date_to_dt, to_query_str
+from falcon.util import CaseInsensitiveDict, http_cookies, http_date_to_dt, to_query_str
 from falcon.util import json as util_json
 
 
@@ -184,6 +183,7 @@ class Cookie:
             'max_age',
             'secure',
             'httponly',
+            'samesite'
         ):
             value = morsel[name.replace('_', '-')] or None
             setattr(self, '_' + name, value)
@@ -223,6 +223,10 @@ class Cookie:
     def http_only(self):
         return bool(self._httponly)
 
+    @property
+    def same_site(self):
+        return self._samesite if self._samesite else None
+
 
 def simulate_request(app, method='GET', path='/', query_string=None,
                      headers=None, body=None, json=None, file_wrapper=None,
diff --git a/tests/test_cookies.py b/tests/test_cookies.py
index f48f48b..1256e71 100644
--- a/tests/test_cookies.py
+++ b/tests/test_cookies.py
@@ -40,7 +40,10 @@ class CookieResource:
 
     def on_post(self, req, resp):
         e = datetime(year=2050, month=1, day=1)  # naive
-        resp.set_cookie('foo', 'bar', http_only=False, secure=False, expires=e)
+        resp.set_cookie('foo', 'bar',
+                        http_only=False,
+                        secure=False,
+                        expires=e)
         resp.unset_cookie('bad')
 
     def on_put(self, req, resp):
@@ -58,11 +61,26 @@ class CookieResourceMaxAgeFloatString:
             'foostring', 'bar', max_age='15', secure=False, http_only=False)
 
 
+class CookieResourceSameSite:
+    def on_get(self, req, resp):
+        resp.set_cookie('foo', 'bar', same_site='Lax')
+
+    def on_post(self, req, resp):
+        resp.set_cookie('bar', 'foo', same_site='STRICT')
+
+    def on_put(self, req, resp):
+        resp.set_cookie('baz', 'foo', same_site='none')
+
+    def on_delete(self, req, resp):
+        resp.set_cookie('baz', 'foo', same_site='')
+
+
 @pytest.fixture()
 def client():
     app = falcon.API()
     app.add_route('/', CookieResource())
     app.add_route('/test-convert', CookieResourceMaxAgeFloatString())
+    app.add_route('/same-site', CookieResourceSameSite())
 
     return testing.TestClient(app)
 
@@ -76,6 +94,7 @@ def test_response_base_case(client):
     result = client.simulate_get('/')
 
     cookie = result.cookies['foo']
+
     assert cookie.name == 'foo'
     assert cookie.value == 'bar'
     assert cookie.domain == 'example.com'
@@ -370,3 +389,54 @@ def test_non_ascii_value(value):
         assert not isinstance(e, UnicodeEncodeError)
     else:
         pytest.fail('set_bad_cookie_value did not fail as expected')
+
+
+def test_lax_same_site_value(client):
+    result = client.simulate_get('/same-site')
+    cookie = result.cookies['foo']
+
+    assert cookie.same_site == 'Lax'
+
+
+def test_strict_same_site_value(client):
+    result = client.simulate_post('/same-site')
+    cookie = result.cookies['bar']
+
+    assert cookie.same_site == 'Strict'
+
+
+def test_none_same_site_value(client):
+    result = client.simulate_put('/same-site')
+    cookie = result.cookies['baz']
+
+    assert cookie.same_site == 'None'
+
+
+def test_same_site_empty_string(client):
+    result = client.simulate_delete('/same-site')
+    cookie = result.cookies['baz']
+
+    assert cookie.same_site is None
+
+
+@pytest.mark.parametrize(
+    'same_site', ['laX', 'lax', 'STRICT', 'strict', 'None', 'none']
+)
+def test_same_site_value_case_insensitive(same_site):
+    resp = falcon.Response()
+    resp.set_cookie('foo', 'bar', same_site=same_site)
+
+    # NOTE(kgriffs): Verify directly, unit-test style, since we
+    #   already tested end-to-end above.
+    morsel = resp._cookies['foo']
+    assert morsel['samesite'].lower() == same_site.lower()
+
+
+@pytest.mark.parametrize(
+    'same_site', ['bogus', 'laxx', 'stric']
+)
+def test_invalid_same_site_value(same_site):
+    resp = falcon.Response()
+
+    with pytest.raises(ValueError):
+        resp.set_cookie('foo', 'bar', same_site=same_site)