Merge pull request #872 from uc-cdis/feat/httponlycookies

feat(cookies): always set httponly flag on cookies (also update deps) PXP-7593
uc-cdis · Feb 10, 2021 · d0fe6fc · d0fe6fc
2 parents 1bcbc45 + 473aef9
commit d0fe6fc
Show file tree

Hide file tree

Showing 7 changed files with 233 additions and 169 deletions.
diff --git a/fence/__init__.py b/fence/__init__.py
@@ -412,7 +412,7 @@ def set_csrf(response):
     """
     if not flask.request.cookies.get("csrftoken"):
         secure = config.get("SESSION_COOKIE_SECURE", True)
-        response.set_cookie("csrftoken", random_str(40), secure=secure)
+        response.set_cookie("csrftoken", random_str(40), secure=secure, httponly=True)
 
     if flask.request.method in ["POST", "PUT", "DELETE"]:
         current_session.commit()

diff --git a/fence/utils.py b/fence/utils.py
@@ -191,7 +191,7 @@ def clear_cookies(response):
     Set all cookies to empty and expired.
     """
     for cookie_name in list(flask.request.cookies.keys()):
-        response.set_cookie(cookie_name, "", expires=0)
+        response.set_cookie(cookie_name, "", expires=0, httponly=True)
 
 
 def get_error_params(error, description):

diff --git a/poetry.lock b/poetry.lock
diff --git a/tests/link/test_link.py b/tests/link/test_link.py
@@ -176,7 +176,9 @@ def test_google_link_auth_return(
     )
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     # simulate successfully authed reponse with user email
     google_auth_get_user_info_mock.return_value = {"email": google_account}
@@ -251,7 +253,9 @@ def test_patch_google_link(
     db_session.commit()
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     r = client.patch(
         "/link/google", headers={"Authorization": "Bearer " + encoded_credentials_jwt}
@@ -349,7 +353,9 @@ def test_patch_google_link_account_not_in_token(
     db_session.commit()
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     r = client.patch(
         "/link/google", headers={"Authorization": "Bearer " + encoded_credentials_jwt}
@@ -399,7 +405,9 @@ def test_patch_google_link_account_doesnt_exist(
     )
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     r = client.patch(
         "/link/google", headers={"Authorization": "Bearer " + encoded_credentials_jwt}
@@ -462,7 +470,9 @@ def test_google_link_g_account_exists(
     db_session.commit()
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     # simulate successfully authed reponse with user email
     google_auth_get_user_info_mock.return_value = {"email": google_account}
@@ -535,7 +545,9 @@ def test_google_link_g_account_access_extension(
     db_session.commit()
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     # simulate successfully authed reponse with user email
     google_auth_get_user_info_mock.return_value = {"email": google_account}
@@ -616,7 +628,9 @@ def test_google_link_g_account_exists_linked_to_different_user(
     db_session.commit()
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     # simulate successfully authed reponse with user email
     google_auth_get_user_info_mock.return_value = {"email": google_account}
@@ -678,7 +692,9 @@ def test_google_link_no_proxy_group(
     db_session.commit()
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     # simulate successfully authed reponse with user email
     google_auth_get_user_info_mock.return_value = {"email": google_account}
@@ -758,7 +774,9 @@ def test_google_link_when_google_mocked(
     )
 
     # manually set cookie for initial session
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     headers = {"Authorization": "Bearer " + encoded_creds_jwt.jwt}
 

diff --git a/tests/login/test_google_login.py b/tests/login/test_google_login.py
@@ -26,7 +26,9 @@ def test_google_login_http_headers_are_less_than_4k_for_user_with_many_projects(
             "redirect": "https://localhost/user/oauth2/authorize?client_id=7f7kAS4MJraUuo77d7RWHr4mZ6bvGtuzup7hw46I&response_type=id_token&redirect_uri=https://webapp.example/fence&scope=openid+user+data+google_credentials&nonce=randomvalue"
         },
     )
-    client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+    client.set_cookie(
+        "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+    )
 
     user_projects = {
         "test": {

diff --git a/tests/session/test_session.py b/tests/session/test_session.py
@@ -58,7 +58,9 @@ def test_valid_session(app):
     # the username
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+        client.set_cookie(
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
         with client.session_transaction() as session:
             assert session["username"] == username
 
@@ -75,7 +77,9 @@ def test_valid_session_modified(app):
     # the username
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+        client.set_cookie(
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
         with client.session_transaction() as session:
 
             assert session["username"] == username
@@ -100,7 +104,9 @@ def test_expired_session_lifetime(app):
 
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+        client.set_cookie(
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
         with client.session_transaction() as session:
             # make sure we don't have the username when opening
             # the session, since it has expired
@@ -127,7 +133,9 @@ def test_expired_session_timeout(app):
 
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+        client.set_cookie(
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
         with client.session_transaction() as session:
             # make sure we don't have the username when opening
             # the session, since it has expired
@@ -145,7 +153,9 @@ def test_session_cleared(app):
     # the username
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+        client.set_cookie(
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
         with client.session_transaction() as session:
             session["username"] = username
             session.clear()
@@ -161,7 +171,9 @@ def test_invalid_session_cookie(app):
     # the username
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+        client.set_cookie(
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
         with client.session_transaction() as session:
             # main test is that we haven't raised an exception by this point
 
@@ -199,9 +211,14 @@ def test_valid_session_valid_access_token(
     # the username
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
         client.set_cookie(
-            "localhost", config["ACCESS_TOKEN_COOKIE_NAME"], test_access_jwt
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
+        client.set_cookie(
+            "localhost",
+            config["ACCESS_TOKEN_COOKIE_NAME"],
+            test_access_jwt,
+            httponly=True,
         )
 
         response = client.get("/user")
@@ -242,9 +259,14 @@ def test_valid_session_valid_access_token_diff_user(
 
     with app.test_client() as client:
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
         client.set_cookie(
-            "localhost", config["ACCESS_TOKEN_COOKIE_NAME"], test_access_jwt
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
+        client.set_cookie(
+            "localhost",
+            config["ACCESS_TOKEN_COOKIE_NAME"],
+            test_access_jwt,
+            httponly=True,
         )
 
         response = client.get("/user")

diff --git a/tests/test_logout.py b/tests/test_logout.py
@@ -70,7 +70,9 @@ def test_logout_fence(app, client, user_with_fence_provider, monkeypatch):
     # this redirect valid
     with mock.patch("fence.allowed_login_redirects", return_value={"some_site.com"}):
         # manually set cookie for initial session
-        client.set_cookie("localhost", config["SESSION_COOKIE_NAME"], test_session_jwt)
+        client.set_cookie(
+            "localhost", config["SESSION_COOKIE_NAME"], test_session_jwt, httponly=True
+        )
 
         r = client.get("/logout?next={}".format(redirect))
         assert r.status_code == 302