From 01ab76a50bc1f52d35682d6687cbbad1bf2bfdb4 Mon Sep 17 00:00:00 2001
From: Eevee <git@veekun.com>
Date: Sun, 7 Nov 2010 23:29:35 -0800
Subject: [PATCH] CSRF protection.  #361

---
 splinext/users/__init__.py                       | 18 ++++++------------
 splinext/users/controllers/accounts.py           |  2 ++
 splinext/users/controllers/users.py              | 20 +++++++++++++++++++-
 splinext/users/templates/users/profile_edit.mako |  2 +-
 splinext/users/templates/widgets/user_state.mako |  2 +-
 5 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/splinext/users/__init__.py b/splinext/users/__init__.py
index e395542..8839e9d 100644
--- a/splinext/users/__init__.py
+++ b/splinext/users/__init__.py
@@ -12,12 +12,8 @@ from splinext.users import model as users_model
 
 def add_routes_hook(map, *args, **kwargs):
     """Hook to inject some of our behavior into the routes configuration."""
-    def id_is_numeric(environ, result):
-        try:
-            int(result['id'])
-            return True
-        except (KeyError, ValueError):
-            return False
+    require_GET = dict(conditions=dict(method=['GET']))
+    require_POST = dict(conditions=dict(method=['POST']))
 
     # Login, logout
     map.connect('/accounts/login', controller='accounts', action='login')
@@ -26,15 +22,13 @@ def add_routes_hook(map, *args, **kwargs):
     map.connect('/accounts/logout', controller='accounts', action='logout')
 
     # Self-admin
-    map.connect('/users/{id};{name}/edit', controller='users', action='profile_edit',
-        conditions=dict(function=id_is_numeric))
+    map.connect(r'/users/{id:\d+};{name}/edit', controller='users', action='profile_edit', **require_GET)
+    map.connect(r'/users/{id:\d+};{name}/edit', controller='users', action='profile_edit_commit', **require_POST)
 
     # Public user pages
     map.connect('/users', controller='users', action='list')
-    map.connect('/users/{id};{name}', controller='users', action='profile',
-        conditions=dict(function=id_is_numeric))
-    map.connect('/users/{id}', controller='users', action='profile',
-        conditions=dict(function=id_is_numeric))
+    map.connect(r'/users/{id:\d+};{name}', controller='users', action='profile')
+    map.connect(r'/users/{id:\d+}', controller='users', action='profile')
 
     # Big-boy admin
     map.connect('/admin/users/permissions', controller='admin_users', action='permissions')
diff --git a/splinext/users/controllers/accounts.py b/splinext/users/controllers/accounts.py
index 45d3b1e..55b6319 100644
--- a/splinext/users/controllers/accounts.py
+++ b/splinext/users/controllers/accounts.py
@@ -7,6 +7,7 @@ from sqlalchemy.orm.exc import NoResultFound
 
 from pylons import config, request, response, session, tmpl_context as c, url
 from pylons.controllers.util import abort, redirect
+from pylons.decorators.secure import authenticate_form
 from routes import request_config
 
 from spline.model import meta
@@ -108,6 +109,7 @@ class AccountsController(BaseController):
 
         redirect(url('/'), code=303)
 
+    @authenticate_form
     def logout(self):
         """Logs the user out."""
 
diff --git a/splinext/users/controllers/users.py b/splinext/users/controllers/users.py
index 5e3826c..4f46179 100644
--- a/splinext/users/controllers/users.py
+++ b/splinext/users/controllers/users.py
@@ -5,6 +5,7 @@ from wtforms import Form, ValidationError, fields, validators, widgets
 
 from pylons import config, request, response, session, tmpl_context as c, url
 from pylons.controllers.util import abort, redirect
+from pylons.decorators.secure import authenticate_form
 from routes import request_config
 from sqlalchemy.orm.exc import NoResultFound
 
@@ -73,7 +74,24 @@ class UsersController(BaseController):
             name=c.page_user.name,
         )
 
-        if request.method != 'POST' or not c.form.validate():
+        return render('/users/profile_edit.mako')
+
+    @authenticate_form
+    def profile_edit_commit(self, id, name=None):
+        """Save profile changes."""
+        c.page_user = meta.Session.query(users_model.User).get(id)
+        if not c.page_user:
+            abort(404)
+
+        # XXX could use some real permissions
+        if c.page_user != c.user:
+            abort(403)
+
+        c.form = ProfileEditForm(request.params,
+            name=c.page_user.name,
+        )
+
+        if not c.form.validate():
             return render('/users/profile_edit.mako')
 
 
diff --git a/splinext/users/templates/users/profile_edit.mako b/splinext/users/templates/users/profile_edit.mako
index 0f81df0..b70cebe 100644
--- a/splinext/users/templates/users/profile_edit.mako
+++ b/splinext/users/templates/users/profile_edit.mako
@@ -7,7 +7,7 @@
 
 <h1>Edit ${c.page_user.name}'s profile</h1>
 
-${h.form(url.current())}
+${h.secure_form(url.current(controller='users', action='profile_edit_commit'))}
 <dl>
     <dt>Color ID</dt>
     <dd>${userlib.color_bar(c.page_user)}</dd>
diff --git a/splinext/users/templates/widgets/user_state.mako b/splinext/users/templates/widgets/user_state.mako
index adf816b..58be697 100644
--- a/splinext/users/templates/widgets/user_state.mako
+++ b/splinext/users/templates/widgets/user_state.mako
@@ -1,6 +1,6 @@
 <%namespace name="userlib" file="/users/lib.mako" />
 % if c.user:
-${h.form(url(controller='accounts', action='logout'), id='user')}
+${h.secure_form(url(controller='accounts', action='logout'), id='user')}
     Logged in as <a href="${url(controller='users', action='profile', id=c.user.id, name=c.user.name)}">${userlib.color_bar(c.user)} ${c.user.name}</a>.
     <input type="submit" value="Log out">
 ${h.end_form()}
-- 
2.7.4