[#1941] More concise way to ensure HttpOnly

ckan · Nov 20, 2014 · 150f452 · 150f452
1 parent ff39eec
commit 150f452
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 153 deletions.
diff --git a/ckan/lib/auth_tkt.py b/ckan/lib/auth_tkt.py
@@ -15,51 +15,22 @@ def __init__(self, httponly, *args, **kwargs):
         super(CkanAuthTktCookiePlugin, self).__init__(*args, **kwargs)
         self.httponly = httponly
 
-    def _ensure_httponly_for_cookies(self, cookies):
-        '''
-        Take a list of cookie tuples items and ensure HttpOnly is set
-        correctly.
-        '''
-        # change tuples to lists
-        cookie_list = [list(c) for c in cookies]
-        # ensure httponly is set correctly for each cookie
-        for cookie in cookie_list:
-            cookie[1] = _set_substring(cookie[1], '; HttpOnly', self.httponly)
-        # change lists back to tuples and return
-        return [tuple(c) for c in cookie_list]
-
     def _get_cookies(self, *args, **kwargs):
         '''
         Override method in superclass to ensure HttpOnly is set appropriately.
         '''
         super_cookies = super(CkanAuthTktCookiePlugin, self). \
             _get_cookies(*args, **kwargs)
 
-        cookies = self._ensure_httponly_for_cookies(super_cookies)
+        cookies = []
+        for k, v in super_cookies:
+            replace_with = '; HttpOnly' if self.httponly else ''
+            v = v.replace('; HttpOnly', '') + replace_with
+            cookies.append((k, v))
 
         return cookies
 
 
-def _set_substring(value, substring, presence=True):
-    '''
-    Ensure presence/absence of substring in value.
-
-    If presence is True, ensure value contains substring. Append substring to
-    value if absent.
-
-    If presence is False, ensure value does not contain substring. Remove
-    substring from value is present.
-    '''
-    has_substring = (substring in value)
-    if has_substring and not presence:
-        # remove substring
-        value = value.replace(substring, '')
-    elif not has_substring and presence:
-        # add substring
-        value += substring
-    return value
-
-
 def make_plugin(secret=None,
                 secretfile=None,
                 cookie_name='auth_tkt',

diff --git a/ckan/new_tests/lib/test_auth_tkt.py b/ckan/new_tests/lib/test_auth_tkt.py
@@ -1,123 +1,5 @@
 from ckan.new_tests import helpers
-from ckan.lib.auth_tkt import (CkanAuthTktCookiePlugin,
-                               _set_substring,
-                               make_plugin)
-
-
-class TestSetSubstring(object):
-
-    '''Tests for auth_tkt._set_substring method.'''
-
-    def test_set_substring__value_has_substring_and_substring_should_be_present(self):
-        '''Substring should be retained in value'''
-        value = "I love my kitten, it is sweet."
-        substring = "kitten"
-        presence = True
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == value
-
-    def test_set_substring__value_has_substring_and_substring_should_not_be_present(self):
-        '''Substring should be removed from value.'''
-        value = "I love my kitten, it is sweet."
-        substring = "kitten"
-        presence = False
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == "I love my , it is sweet."
-
-    def test_set_substring__value_doesnot_have_substring_and_substring_should_be_present(self):
-        '''Substring is appended to value.'''
-        value = "I wish I had a "
-        substring = "kitten"
-        presence = True
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == 'I wish I had a kitten'
-
-    def test_set_substring__value_doesnot_have_substring_and_substring_should_not_be_present(self):
-        '''Substring isn't appended to value.'''
-        value = "I don't have one."
-        substring = "kitten"
-        presence = False
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == "I don't have one."
-
-
-class TestEnsureHttpOnlyForCookies(object):
-
-    '''Tests for CkanAuthTktCookiePlugin._ensure_httponly_for_cookies method'''
-
-    def _make_plugin(self, httponly):
-        '''Only httponly needs to be set.'''
-        return CkanAuthTktCookiePlugin(httponly=httponly,
-                                       secret=None,
-                                       cookie_name='auth_tkt',
-                                       secure=False,
-                                       include_ip=False,
-                                       timeout=None,
-                                       reissue_time=None,
-                                       userid_checker=None)
-
-    def test_ensure_httponly_for_cookies__should_have_httponly(self):
-        '''Cookie values should contain HttpOnly.'''
-        plugin = self._make_plugin(httponly=True)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        expected_cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost; HttpOnly')
-        ]
-        assert ensured_cookies == expected_cookies
-
-    def test_ensure_httponly_for_cookies__should_have_httponly_already_do(self):
-        '''
-        Cookie values should contain HttpOnly, they already to so nothing
-        should change.
-        '''
-        plugin = self._make_plugin(httponly=True)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost; HttpOnly')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        assert ensured_cookies == cookies
-
-    def test_ensure_httponly_for_cookies__should_not_have_httponly_already_absent(self):
-        '''
-        Cookie values should not contain HttpOnly. They don't so nothing
-        should change.
-        '''
-        plugin = self._make_plugin(httponly=False)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        assert ensured_cookies == cookies
-
-    def test_ensure_httponly_for_cookies__should_not_have_httponly(self):
-        '''
-        Cookie values should not contain HttpOnly, they do so it should be
-        removed.
-        '''
-        plugin = self._make_plugin(httponly=False)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost; HttpOnly')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        expected_cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost')
-        ]
-        assert ensured_cookies == expected_cookies
+from ckan.lib.auth_tkt import CkanAuthTktCookiePlugin, make_plugin
 
 
 class TestCkanAuthTktCookiePlugin(object):