Merge pull request #2282 from davidism/session-cookie-domain

Refactor session cookie domain

CHANGES

@@ -34,6 +34,12 @@ Major release, unreleased
   type is invalid. (`#2256`_)
 - Add ``routes`` CLI command to output routes registered on the application.
   (`#2259`_)
+- Show warning when session cookie domain is a bare hostname or an IP
+  address, as these may not behave properly in some browsers, such as Chrome.
+  (`#2282`_)
+- Allow IP address as exact session cookie domain. (`#2282`_)
+- ``SESSION_COOKIE_DOMAIN`` is set if it is detected through ``SERVER_NAME``.
+  (`#2282`_)
 
 .. _#1489: https://github.com/pallets/flask/pull/1489
 .. _#1898: https://github.com/pallets/flask/pull/1898
@@ -43,6 +49,7 @@ Major release, unreleased
 .. _#2254: https://github.com/pallets/flask/pull/2254
 .. _#2256: https://github.com/pallets/flask/pull/2256
 .. _#2259: https://github.com/pallets/flask/pull/2259
+.. _#2282: https://github.com/pallets/flask/pull/2282
 
 Version 0.12.1
 --------------

flask/helpers.py

@@ -10,6 +10,7 @@
 """
 
 import os
+import socket
 import sys
 import pkgutil
 import posixpath
@@ -976,3 +977,24 @@ def total_seconds(td):
     :rtype: int
     """
     return td.days * 60 * 60 * 24 + td.seconds
+
+
+def is_ip(value):
+    """Determine if the given string is an IP address.
+
+    :param value: value to check
+    :type value: str
+
+    :return: True if string is an IP address
+    :rtype: bool
+    """
+
+    for family in (socket.AF_INET, socket.AF_INET6):
+        try:
+            socket.inet_pton(family, value)
+        except socket.error:
+            pass
+        else:
+            return True
+
+    return False

flask/sessions.py

@@ -11,13 +11,14 @@
 
 import uuid
 import hashlib
+import warnings
 from base64 import b64encode, b64decode
 from datetime import datetime
 from werkzeug.http import http_date, parse_date
 from werkzeug.datastructures import CallbackDict
 from . import Markup, json
 from ._compat import iteritems, text_type
-from .helpers import total_seconds
+from .helpers import total_seconds, is_ip
 
 from itsdangerous import URLSafeTimedSerializer, BadSignature
 
@@ -200,30 +201,62 @@ class SessionInterface(object):
         return isinstance(obj, self.null_session_class)
 
     def get_cookie_domain(self, app):
-        """Helpful helper method that returns the cookie domain that should
-        be used for the session cookie if session cookies are used.
+        """Returns the domain that should be set for the session cookie.
+        
+        Uses ``SESSION_COOKIE_DOMAIN`` if it is configured, otherwise
+        falls back to detecting the domain based on ``SERVER_NAME``.
+        
+        Once detected (or if not set at all), ``SESSION_COOKIE_DOMAIN`` is
+        updated to avoid re-running the logic.
         """
-        if app.config['SESSION_COOKIE_DOMAIN'] is not None:
-            return app.config['SESSION_COOKIE_DOMAIN']
-        if app.config['SERVER_NAME'] is not None:
-            # chop off the port which is usually not supported by browsers
-            rv = '.' + app.config['SERVER_NAME'].rsplit(':', 1)[0]
-
-            # Google chrome does not like cookies set to .localhost, so
-            # we just go with no domain then.  Flask documents anyways that
-            # cross domain cookies need a fully qualified domain name
-            if rv == '.localhost':
-                rv = None
-
-            # If we infer the cookie domain from the server name we need
-            # to check if we are in a subpath.  In that case we can't
-            # set a cross domain cookie.
-            if rv is not None:
-                path = self.get_cookie_path(app)
-                if path != '/':
-                    rv = rv.lstrip('.')
-
-            return rv
+
+        rv = app.config['SESSION_COOKIE_DOMAIN']
+
+        # set explicitly, or cached from SERVER_NAME detection
+        # if False, return None
+        if rv is not None:
+            return rv if rv else None
+
+        rv = app.config['SERVER_NAME']
+
+        # server name not set, cache False to return none next time
+        if not rv:
+            app.config['SESSION_COOKIE_DOMAIN'] = False
+            return None
+
+        # chop off the port which is usually not supported by browsers
+        # remove any leading '.' since we'll add that later
+        rv = rv.rsplit(':', 1)[0].lstrip('.')
+
+        if '.' not in rv:
+            # Chrome doesn't allow names without a '.'
+            # this should only come up with localhost
+            # hack around this by not setting the name, and show a warning
+            warnings.warn(
+                '"{rv}" is not a valid cookie domain, it must contain a ".".'
+                ' Add an entry to your hosts file, for example'
+                ' "{rv}.localdomain", and use that instead.'.format(rv=rv)
+            )
+            app.config['SESSION_COOKIE_DOMAIN'] = False
+            return None
+
+        ip = is_ip(rv)
+
+        if ip:
+            warnings.warn(
+                'The session cookie domain is an IP address. This may not work'
+                ' as intended in some browsers. Add an entry to your hosts'
+                ' file, for example "localhost.localdomain", and use that'
+                ' instead.'
+            )
+
+        # if this is not an ip and app is mounted at the root, allow subdomain
+        # matching by adding a '.' prefix
+        if self.get_cookie_path(app) == '/' and not ip:
+            rv = '.' + rv
+
+        app.config['SESSION_COOKIE_DOMAIN'] = rv
+        return rv
 
     def get_cookie_path(self, app):
         """Returns the path for which the cookie should be valid.  The

tests/test_basic.py

@@ -351,6 +351,42 @@ def test_session_using_session_settings():
     assert 'httponly' not in cookie
 
 
+def test_session_localhost_warning(recwarn):
+    app = flask.Flask(__name__)
+    app.config.update(
+        SECRET_KEY='testing',
+        SERVER_NAME='localhost:5000',
+    )
+
+    @app.route('/')
+    def index():
+        flask.session['testing'] = 42
+        return 'testing'
+
+    rv = app.test_client().get('/', 'http://localhost:5000/')
+    assert 'domain' not in rv.headers['set-cookie'].lower()
+    w = recwarn.pop(UserWarning)
+    assert '"localhost" is not a valid cookie domain' in str(w.message)
+
+
+def test_session_ip_warning(recwarn):
+    app = flask.Flask(__name__)
+    app.config.update(
+        SECRET_KEY='testing',
+        SERVER_NAME='127.0.0.1:5000',
+    )
+
+    @app.route('/')
+    def index():
+        flask.session['testing'] = 42
+        return 'testing'
+
+    rv = app.test_client().get('/', 'http://127.0.0.1:5000/')
+    assert 'domain=127.0.0.1' in rv.headers['set-cookie'].lower()
+    w = recwarn.pop(UserWarning)
+    assert 'cookie domain is an IP' in str(w.message)
+
+
 def test_missing_session():
     app = flask.Flask(__name__)