Merge branch 'master' into pr/2853

14 files changed, 207 insertions, 11 deletions

diff --git a/docs/tutorials/wiki/authorization.rst b/docs/tutorials/wiki/authorization.rst
index 44097b35b..67af83b25 100644
--- a/docs/tutorials/wiki/authorization.rst
+++ b/docs/tutorials/wiki/authorization.rst
@@ -18,6 +18,7 @@ require permission, instead of a default "403 Forbidden" page.
 
 We will implement the access control with the following steps:
 
+* Add password hashing dependencies.
 * Add users and groups (``security.py``, a new module).
 * Add an :term:`ACL` (``models.py``).
 * Add an :term:`authentication policy` and an :term:`authorization policy`
@@ -38,11 +39,32 @@ Then we will add the login and logout feature:
 Access control
 --------------
 
+
+Add dependencies
+~~~~~~~~~~~~~~~~
+
+Just like in :ref:`wiki_defining_views`, we need a new dependency. We need to add the `bcrypt <https://pypi.python.org/pypi/bcrypt>`_ package, to our tutorial package's ``setup.py`` file by assigning this dependency to the ``requires`` parameter in the ``setup()`` function.
+
+Open ``setup.py`` and edit it to look like the following:
+
+.. literalinclude:: src/authorization/setup.py
+   :linenos:
+   :emphasize-lines: 21
+   :language: python
+
+Only the highlighted line needs to be added.
+
+Do not forget to run ``pip install -e .`` just like in :ref:`wiki-running-pip-install`.
+
+.. note::
+
+   We are using the ``bcrypt`` package from PyPI to hash our passwords securely. There are other one-way hash algorithms for passwords if bcrypt is an issue on your system. Just make sure that it's an algorithm approved for storing passwords versus a generic one-way hash.
+
+
 Add users and groups
 ~~~~~~~~~~~~~~~~~~~~
 
-Create a new ``tutorial/security.py`` module with the
-following content:
+Create a new ``tutorial/security.py`` module with the following content:
 
 .. literalinclude:: src/authorization/tutorial/security.py
    :linenos:
@@ -61,7 +83,20 @@ request)`` returns ``None``.  We will use ``groupfinder()`` as an
 :term:`authentication policy` "callback" that will provide the
 :term:`principal` or principals for a user.
 
-In a production system, user and group data will most often come from a
+There are two helper methods that will help us later to authenticate users.
+The first is ``hash_password`` which takes a raw password and transforms it using
+bcrypt into an irreversible representation, a process known as "hashing". The
+second method, ``check_password``, will allow us to compare the hashed value of the
+submitted password against the hashed value of the password stored in the user's
+record. If the two hashed values match, then the submitted
+password is valid, and we can authenticate the user.
+
+We hash passwords so that it is impossible to decrypt and use them to
+authenticate in the application. If we stored passwords foolishly in clear text,
+then anyone with access to the database could retrieve any password to authenticate
+as any user.
+
+In a production system, user and group data will most often be saved and come from a
 database, but here we use "dummy" data to represent user and groups sources.
 
 Add an ACL
diff --git a/docs/tutorials/wiki/definingviews.rst b/docs/tutorials/wiki/definingviews.rst
index ac94d8059..3859d2cad 100644
--- a/docs/tutorials/wiki/definingviews.rst
+++ b/docs/tutorials/wiki/definingviews.rst
@@ -52,6 +52,7 @@ Open ``setup.py`` and edit it to look like the following:
 
 Only the highlighted line needs to be added.
 
+.. _wiki-running-pip-install:
 
 Running ``pip install -e .``
 ============================
diff --git a/docs/tutorials/wiki/src/authorization/setup.py b/docs/tutorials/wiki/src/authorization/setup.py
index beeed75c9..68e3c0abd 100644
--- a/docs/tutorials/wiki/src/authorization/setup.py
+++ b/docs/tutorials/wiki/src/authorization/setup.py
@@ -18,6 +18,7 @@ requires = [
     'ZODB3',
     'waitress',
     'docutils',
+    'bcrypt',
     ]
 
 tests_require = [
diff --git a/docs/tutorials/wiki/src/authorization/tutorial/security.py b/docs/tutorials/wiki/src/authorization/tutorial/security.py
index d88c9c71f..cbb3acd5d 100644
--- a/docs/tutorials/wiki/src/authorization/tutorial/security.py
+++ b/docs/tutorials/wiki/src/authorization/tutorial/security.py
@@ -1,5 +1,18 @@
-USERS = {'editor':'editor',
-          'viewer':'viewer'}
+import bcrypt
+
+
+def hash_password(pw):
+    hashed_pw = bcrypt.hashpw(pw.encode('utf-8'), bcrypt.gensalt())
+    # return unicode instead of bytes because databases handle it better
+    return hashed_pw.decode('utf-8')
+
+def check_password(expected_hash, pw):
+    if expected_hash is not None:
+        return bcrypt.checkpw(pw.encode('utf-8'), expected_hash.encode('utf-8'))
+    return False
+
+USERS = {'editor': hash_password('editor'),
+          'viewer': hash_password('viewer')}
 GROUPS = {'editor':['group:editors']}
 
 def groupfinder(userid, request):
diff --git a/docs/tutorials/wiki/src/authorization/tutorial/views.py b/docs/tutorials/wiki/src/authorization/tutorial/views.py
index c271d2cc1..e4560dfe1 100644
--- a/docs/tutorials/wiki/src/authorization/tutorial/views.py
+++ b/docs/tutorials/wiki/src/authorization/tutorial/views.py
@@ -14,7 +14,7 @@ from pyramid.security import (
     )
 
 
-from .security import USERS
+from .security import USERS, check_password
 from .models import Page
 
 # regular expression used to find WikiWords
@@ -94,7 +94,7 @@ def login(request):
     if 'form.submitted' in request.params:
         login = request.params['login']
         password = request.params['password']
-        if USERS.get(login) == password:
+        if check_password(USERS.get(login), password):
             headers = remember(request, login)
             return HTTPFound(location=came_from,
                              headers=headers)
diff --git a/docs/tutorials/wiki/src/tests/setup.py b/docs/tutorials/wiki/src/tests/setup.py
index beeed75c9..68e3c0abd 100644
--- a/docs/tutorials/wiki/src/tests/setup.py
+++ b/docs/tutorials/wiki/src/tests/setup.py
@@ -18,6 +18,7 @@ requires = [
     'ZODB3',
     'waitress',
     'docutils',
+    'bcrypt',
     ]
 
 tests_require = [
diff --git a/docs/tutorials/wiki/src/tests/tutorial/security.py b/docs/tutorials/wiki/src/tests/tutorial/security.py
index d88c9c71f..cbb3acd5d 100644
--- a/docs/tutorials/wiki/src/tests/tutorial/security.py
+++ b/docs/tutorials/wiki/src/tests/tutorial/security.py
@@ -1,5 +1,18 @@
-USERS = {'editor':'editor',
-          'viewer':'viewer'}
+import bcrypt
+
+
+def hash_password(pw):
+    hashed_pw = bcrypt.hashpw(pw.encode('utf-8'), bcrypt.gensalt())
+    # return unicode instead of bytes because databases handle it better
+    return hashed_pw.decode('utf-8')
+
+def check_password(expected_hash, pw):
+    if expected_hash is not None:
+        return bcrypt.checkpw(pw.encode('utf-8'), expected_hash.encode('utf-8'))
+    return False
+
+USERS = {'editor': hash_password('editor'),
+          'viewer': hash_password('viewer')}
 GROUPS = {'editor':['group:editors']}
 
 def groupfinder(userid, request):
diff --git a/docs/tutorials/wiki/src/tests/tutorial/tests.py b/docs/tutorials/wiki/src/tests/tutorial/tests.py
index 04beaea44..098e9c1bd 100644
--- a/docs/tutorials/wiki/src/tests/tutorial/tests.py
+++ b/docs/tutorials/wiki/src/tests/tutorial/tests.py
@@ -122,6 +122,17 @@ class EditPageTests(unittest.TestCase):
         self.assertEqual(response.location, 'http://example.com/')
         self.assertEqual(context.data, 'Hello yo!')
 
+class SecurityTests(unittest.TestCase):
+    def test_hashing(self):
+        from .security import hash_password, check_password
+        password = 'secretpassword'
+        hashed_password = hash_password(password)
+        self.assertTrue(check_password(hashed_password, password))
+
+        self.assertFalse(check_password(hashed_password, 'attackerpassword'))
+
+        self.assertFalse(check_password(None, password))
+
 class FunctionalTests(unittest.TestCase):
 
     viewer_login = '/login?login=viewer&password=viewer' \
diff --git a/docs/tutorials/wiki/src/tests/tutorial/views.py b/docs/tutorials/wiki/src/tests/tutorial/views.py
index c271d2cc1..e4560dfe1 100644
--- a/docs/tutorials/wiki/src/tests/tutorial/views.py
+++ b/docs/tutorials/wiki/src/tests/tutorial/views.py
@@ -14,7 +14,7 @@ from pyramid.security import (
     )
 
 
-from .security import USERS
+from .security import USERS, check_password
 from .models import Page
 
 # regular expression used to find WikiWords
@@ -94,7 +94,7 @@ def login(request):
     if 'form.submitted' in request.params:
         login = request.params['login']
         password = request.params['password']
-        if USERS.get(login) == password:
+        if check_password(USERS.get(login), password):
             headers = remember(request, login)
             return HTTPFound(location=came_from,
                              headers=headers)
diff --git a/docs/tutorials/wiki2/src/tests/tutorial/scripts/initializedb.py b/docs/tutorials/wiki2/src/tests/tutorial/scripts/initializedb.py
index f3c0a6fef..c860ef8cf 100644
--- a/docs/tutorials/wiki2/src/tests/tutorial/scripts/initializedb.py
+++ b/docs/tutorials/wiki2/src/tests/tutorial/scripts/initializedb.py
@@ -28,6 +28,7 @@ def usage(argv):
 def main(argv=sys.argv):
     if len(argv) < 2:
         usage(argv)
+        return
     config_uri = argv[1]
     options = parse_vars(argv[2:])
     setup_logging(config_uri)
diff --git a/docs/tutorials/wiki2/src/tests/tutorial/tests/test_functional.py b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_functional.py
index 715768b2e..0250e71c9 100644
--- a/docs/tutorials/wiki2/src/tests/tutorial/tests/test_functional.py
+++ b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_functional.py
@@ -11,6 +11,9 @@ class FunctionalTests(unittest.TestCase):
     basic_wrong_login = (
         '/login?login=basic&password=incorrect'
         '&next=FrontPage&form.submitted=Login')
+    basic_login_no_next = (
+        '/login?login=basic&password=basic'
+        '&form.submitted=Login')
     editor_login = (
         '/login?login=editor&password=editor'
         '&next=FrontPage&form.submitted=Login')
@@ -68,6 +71,10 @@ class FunctionalTests(unittest.TestCase):
         res = self.testapp.get(self.basic_login, status=302)
         self.assertEqual(res.location, 'http://localhost/FrontPage')
 
+    def test_successful_log_in_no_next(self):
+        res = self.testapp.get(self.basic_login_no_next, status=302)
+        self.assertEqual(res.location, 'http://localhost/')
+
     def test_failed_log_in(self):
         res = self.testapp.get(self.basic_wrong_login, status=200)
         self.assertTrue(b'login' in res.body)
@@ -120,3 +127,8 @@ class FunctionalTests(unittest.TestCase):
         self.testapp.get(self.editor_login, status=302)
         res = self.testapp.get('/FrontPage', status=200)
         self.assertTrue(b'FrontPage' in res.body)
+
+    def test_redirect_to_edit_for_existing_page(self):
+        self.testapp.get(self.editor_login, status=302)
+        res = self.testapp.get('/add_page/FrontPage', status=302)
+        self.assertTrue(b'FrontPage' in res.body)
diff --git a/docs/tutorials/wiki2/src/tests/tutorial/tests/test_initdb.py b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_initdb.py
new file mode 100644
index 000000000..97511d5e8
--- /dev/null
+++ b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_initdb.py
@@ -0,0 +1,20 @@
+import mock
+import unittest
+
+
+class TestInitializeDB(unittest.TestCase):
+
+    @mock.patch('tutorial.scripts.initializedb.sys')
+    def test_usage(self, mocked_sys):
+        from ..scripts.initializedb import main
+        main(argv=['foo'])
+        mocked_sys.exit.assert_called_with(1)
+
+    @mock.patch('tutorial.scripts.initializedb.get_tm_session')
+    @mock.patch('tutorial.scripts.initializedb.sys')
+    def test_run(self, mocked_sys, mocked_session):
+        from ..scripts.initializedb import main
+        main(argv=['foo', 'development.ini'])
+        mocked_session.assert_called_once()
+
+
diff --git a/docs/tutorials/wiki2/src/tests/tutorial/tests/test_security.py b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_security.py
new file mode 100644
index 000000000..4c3b72946
--- /dev/null
+++ b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_security.py
@@ -0,0 +1,21 @@
+import mock
+import unittest
+
+
+class TestMyAuthenticationPolicy(unittest.TestCase):
+
+    def test_no_user(self):
+        request = mock.Mock()
+        request.user = None
+
+        from ..security import MyAuthenticationPolicy
+        policy = MyAuthenticationPolicy(None)
+        self.assertEqual(policy.authenticated_userid(request), None)
+
+    def test_authenticated_user(self):
+        request = mock.Mock()
+        request.user.id = 'foo'
+
+        from ..security import MyAuthenticationPolicy
+        policy = MyAuthenticationPolicy(None)
+        self.assertEqual(policy.authenticated_userid(request), 'foo')
diff --git a/docs/tutorials/wiki2/src/tests/tutorial/tests/test_user_model.py b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_user_model.py
new file mode 100644
index 000000000..9490ac990
--- /dev/null
+++ b/docs/tutorials/wiki2/src/tests/tutorial/tests/test_user_model.py
@@ -0,0 +1,67 @@
+import unittest
+import transaction
+
+from pyramid import testing
+
+
+class BaseTest(unittest.TestCase):
+
+    def setUp(self):
+        from ..models import get_tm_session
+        self.config = testing.setUp(settings={
+            'sqlalchemy.url': 'sqlite:///:memory:'
+        })
+        self.config.include('..models')
+        self.config.include('..routes')
+
+        session_factory = self.config.registry['dbsession_factory']
+        self.session = get_tm_session(session_factory, transaction.manager)
+
+        self.init_database()
+
+    def init_database(self):
+        from ..models.meta import Base
+        session_factory = self.config.registry['dbsession_factory']
+        engine = session_factory.kw['bind']
+        Base.metadata.create_all(engine)
+
+    def tearDown(self):
+        testing.tearDown()
+        transaction.abort()
+
+    def makeUser(self, name, role):
+        from ..models import User
+        return User(name=name, role=role)
+
+
+class TestSetPassword(BaseTest):
+
+    def test_password_hash_saved(self):
+        user = self.makeUser(name='foo', role='bar')
+        self.assertFalse(user.password_hash)
+
+        user.set_password('secret')
+        self.assertTrue(user.password_hash)
+
+
+class TestCheckPassword(BaseTest):
+
+    def test_password_hash_not_set(self):
+        user = self.makeUser(name='foo', role='bar')
+        self.assertFalse(user.password_hash)
+
+        self.assertFalse(user.check_password('secret'))
+
+    def test_correct_password(self):
+        user = self.makeUser(name='foo', role='bar')
+        user.set_password('secret')
+        self.assertTrue(user.password_hash)
+
+        self.assertTrue(user.check_password('secret'))
+
+    def test_incorrect_password(self):
+        user = self.makeUser(name='foo', role='bar')
+        user.set_password('secret')
+        self.assertTrue(user.password_hash)
+
+        self.assertFalse(user.check_password('incorrect'))