From e9b76013c3fa4cff84acbc602086c93ac797c268 Mon Sep 17 00:00:00 2001 From: Sindre Stephansen Date: Tue, 10 Mar 2020 11:31:40 +0100 Subject: [PATCH] Implement CSRF protection Fixes #16 --- src/app/templates/apply.html | 15 +++++----- src/app/templates/login.html | 9 +++--- src/app/templates/new_project.html | 1 + src/app/templates/project.html | 5 ++-- src/app/templates/register.html | 3 +- src/app/views/app.py | 9 +++--- src/app/views/apply.py | 15 +++++----- src/app/views/login.py | 3 +- src/app/views/new_project.py | 48 +++++++++++++++--------------- src/app/views/project.py | 14 ++++----- src/app/views/register.py | 12 ++++---- src/app/views/utils.py | 48 +++++++++++++++++++++++++++--- 12 files changed, 114 insertions(+), 68 deletions(-) diff --git a/src/app/templates/apply.html b/src/app/templates/apply.html index 0bc9776..0b61e04 100644 --- a/src/app/templates/apply.html +++ b/src/app/templates/apply.html @@ -6,9 +6,9 @@ $def with (nav, apply_form, get_apply_permission_form, project, applicants, perm - + - + $:nav

Apply for this project

@@ -19,8 +19,9 @@ $def with (nav, apply_form, get_apply_permission_form, project, applicants, perm

Description: $project[4]

Status: $project[5]

- +
+ $:csrf_field()

Add Users

$:apply_form.render()

Users to apply:

@@ -29,8 +30,8 @@ $def with (nav, apply_form, get_apply_permission_form, project, applicants, perm
User: $applicants[i][1] $ apply_permissions_form = get_apply_permissions_form(i, permissions[i][0], permissions[i][1], permissions[i][2], applicants[i]) - $:apply_permissions_form.render() + $:apply_permissions_form.render()
-
- - \ No newline at end of file + + + diff --git a/src/app/templates/login.html b/src/app/templates/login.html index e6735d8..c98949a 100644 --- a/src/app/templates/login.html +++ b/src/app/templates/login.html @@ -9,20 +9,21 @@ $def with (nav, login_form, message) - + $:nav $if not session.username:

Log In

+ $:csrf_field() $:login_form.render() -
+ $else:

Logged in as $session.username

- +

$:message

Honeybee - \ No newline at end of file + diff --git a/src/app/templates/new_project.html b/src/app/templates/new_project.html index 244d094..521dc2b 100644 --- a/src/app/templates/new_project.html +++ b/src/app/templates/new_project.html @@ -14,6 +14,7 @@ $def with (nav, project_form, project_buttons, messasge)

Add project!

+ $:csrf_field()
$:project_form.render()
diff --git a/src/app/templates/project.html b/src/app/templates/project.html index ee54be5..0ce3447 100644 --- a/src/app/templates/project.html +++ b/src/app/templates/project.html @@ -9,7 +9,7 @@ $def with (nav, project_form, project, tasks, permissions, categories) - + $:nav $if len(project) and (permissions[0] or project[5] == "open" or session.userid == project[2]) and session.username: @@ -37,6 +37,7 @@ $def with (nav, project_form, project, tasks, permissions, categories) $filename[0].split("/")[-1]
+ $:csrf_field() $project_form.taskid.set_value(task[0]) $:project_form.taskid.render() $if (task[5] == "waiting for delivery" or task[5] == "declined"): @@ -53,4 +54,4 @@ $def with (nav, project_form, project, tasks, permissions, categories) $else:

You do not have permissions to view this project

- \ No newline at end of file + diff --git a/src/app/templates/register.html b/src/app/templates/register.html index 11d27c8..5b586cb 100644 --- a/src/app/templates/register.html +++ b/src/app/templates/register.html @@ -13,8 +13,9 @@ $def with (nav, register_form, message) $:nav

Register user!

- + + $:csrf_field() $:register_form.render()
diff --git a/src/app/views/app.py b/src/app/views/app.py index 5ccd8f8..ae1523f 100644 --- a/src/app/views/app.py +++ b/src/app/views/app.py @@ -1,6 +1,6 @@ import os import web -from views.utils import get_nav_bar +from views.utils import get_nav_bar, csrf_field from views.login import Login from views.logout import Logout from views.register import Register @@ -22,7 +22,7 @@ urls = ( '/project', 'Project', '/apply', 'Apply', ) - + # Initialize application using the web py framework app = web.application(urls, globals()) @@ -42,6 +42,9 @@ else: # Add session to global variables render._add_global(session, 'session') +# Add CSRF field to global variables +web.template.Template.globals['csrf_field'] = csrf_field + # Make the session available cross modules through webctx def session_hook(): web.ctx.session = session @@ -50,5 +53,3 @@ def session_hook(): app.add_processor(web.loadhook(session_hook)) app = app.wsgifunc() - - diff --git a/src/app/views/apply.py b/src/app/views/apply.py index c39e37e..30f716c 100644 --- a/src/app/views/apply.py +++ b/src/app/views/apply.py @@ -1,7 +1,7 @@ import web import models.project from models.user import get_user_name_by_id -from views.utils import get_nav_bar, get_element_count +from views.utils import get_nav_bar, get_element_count, csrf_protected from views.forms import get_apply_form, get_apply_permissions_form # Get html templates @@ -34,6 +34,7 @@ class Apply: return render.apply(nav, apply_form, get_apply_permissions_form, project, applicants, permissions) + @csrf_protected def POST(self): """ Post an application to the view, adding selected users to a project @@ -46,7 +47,7 @@ class Apply: applicants = [session.username] apply_form = get_apply_form() apply_permission_form = get_apply_permissions_form() - + # Prepare globals render = web.template.render('templates/', globals={"get_apply_permissions_form":get_apply_permissions_form, 'session':session}) if data.projectid: @@ -54,12 +55,12 @@ class Apply: if data.add_user: applicants, permissions = self.get_applicants(data, "add_user") - return render.apply(nav, apply_form, get_apply_permissions_form, project, applicants,permissions) + return render.apply(nav, apply_form, get_apply_permissions_form, project, applicants,permissions) elif data.remove_user: applicants, permissions = self.get_applicants(data, "remove_user") - return render.apply(nav, apply_form, get_apply_permissions_form, project, applicants, permissions) - + return render.apply(nav, apply_form, get_apply_permissions_form, project, applicants, permissions) + # Set users as working on project and set project status in progress elif data.apply: applicants, permissions = self.get_applicants(data, "") @@ -67,7 +68,7 @@ class Apply: models.project.set_projects_user(data.projectid, str(applicant[0]), permission[0], permission[1], permission[2]) models.project.update_project_status(data.projectid, "in progress") raise web.seeother(('/project?projectid=' + str(data.projectid))) - + def get_applicants(self, data, operation): """ Get applicants and corresponding permissions from the input data with and operation @@ -123,5 +124,3 @@ class Apply: permissions.append(["TRUE", "FALSE", "FALSE"]) return applicants, permissions - - diff --git a/src/app/views/login.py b/src/app/views/login.py index a7a1b70..58b52c5 100644 --- a/src/app/views/login.py +++ b/src/app/views/login.py @@ -1,8 +1,8 @@ import web from views.forms import login_form +from views.utils import get_nav_bar, csrf_protected import models.session import models.user -from views.utils import get_nav_bar import random import string import hashlib @@ -34,6 +34,7 @@ class Login(): return render.login(nav, login_form, "") + @csrf_protected def POST(self): """ Log in to the web application and register the session diff --git a/src/app/views/new_project.py b/src/app/views/new_project.py index 80df20c..06f7f50 100644 --- a/src/app/views/new_project.py +++ b/src/app/views/new_project.py @@ -1,9 +1,9 @@ import web from web import form from views.forms import get_task_form_elements, get_project_form_elements, get_user_form_elements, project_buttons +from views.utils import get_nav_bar, get_element_count, csrf_protected import models.project import models.user -from views.utils import get_nav_bar, get_element_count # Get html templates render = web.template.render('templates/') @@ -13,7 +13,7 @@ class New_project: def GET(self): """ Get the project registration form - + :return: New project page """ session = web.ctx.session @@ -26,43 +26,44 @@ class New_project: project_form = form.Form(*(project_form_elements + task_form_elements + user_form_elements)) return render.new_project(nav, project_form, project_buttons, "") + @csrf_protected def POST(self): """ Create a new project :return: Redirect to main page - """ + """ session = web.ctx.session nav = get_nav_bar(session) # Try the different URL input parameters to determine how to generate the form - data = web.input(add_user=None, remove_user=None, + data = web.input(add_user=None, remove_user=None, add_task=None, remove_task = None, create_project=None) # Add a set of task fields to the form if data.add_task: project_form = self.compose_form(data, "add_task") - return render.new_project(nav, project_form, project_buttons, "") - + return render.new_project(nav, project_form, project_buttons, "") + # Remove a set of task fields from the form if data.remove_task: project_form = self.compose_form(data, "remove_task") - return render.new_project(nav, project_form, project_buttons, "") - + return render.new_project(nav, project_form, project_buttons, "") + if data.add_user: project_form = self.compose_form(data, "add_user") - return render.new_project(nav, project_form, project_buttons, "") - + return render.new_project(nav, project_form, project_buttons, "") + if data.remove_user: project_form = self.compose_form(data, "remove_user") - return render.new_project(nav, project_form, project_buttons, "") - + return render.new_project(nav, project_form, project_buttons, "") + # Post the form data and save the project in the database if data.create_project: - + project_form = self.compose_form(data, None) if not project_form.validates(): - return render.new_project(nav, project_form, project_buttons, "") + return render.new_project(nav, project_form, project_buttons, "") task_count = get_element_count(data, "task_title_") user_count = get_element_count(data, "user_name_") @@ -74,18 +75,18 @@ class New_project: # Validate the input user names for i in range(0, user_count): - if len(data["user_name_"+str(i)]) and not models.user.get_user_id_by_name(data["user_name_"+str(i)]): + if len(data["user_name_"+str(i)]) and not models.user.get_user_id_by_name(data["user_name_"+str(i)]): return render.new_project(nav, project_form, project_buttons, "Invalid user: " + data["user_name_"+str(i)]) # Save the project to the database - projectid = models.project.set_project(data.category_name, str(session.userid), + projectid = models.project.set_project(data.category_name, str(session.userid), data.project_title, data.project_description, status) # Save the tasks in the database for i in range(0, task_count): - models.project.set_task(str(projectid), (data["task_title_" + str(i)]), + models.project.set_task(str(projectid), (data["task_title_" + str(i)]), (data["task_description_" + str(i)]), (data["budget_" + str(i)])) - + # Save the users in the database given that the input field is not empty for i in range(0, user_count): if len(data["user_name_"+str(i)]): @@ -108,10 +109,10 @@ class New_project: modify = "TRUE" except Exception as e: modify = "FALSE" - pass + pass models.project.set_projects_user(str(projectid), str(userid), read, write, modify) raise web.seeother('/?projects=my') - + def compose_form(self, data, operation): """ Compose a new project form by adding or removing a task @@ -128,14 +129,14 @@ class New_project: task_count -= 1 if operation == "remove_user" and user_count >=1: user_count -= 1 - + # Recreate project form fields project_form_elements = get_project_form_elements(data.project_title, data.project_description, data.category_name) # Recreate task form fields task_form_elements = () for i in range(0, task_count): - old_task_form_element = get_task_form_elements(i, data["task_title_"+str(i)], + old_task_form_element = get_task_form_elements(i, data["task_title_"+str(i)], data["task_description_"+str(i)], data["budget_"+str(i)]) task_form_elements = (task_form_elements + old_task_form_element) @@ -165,7 +166,7 @@ class New_project: user_form_elements = (user_form_elements + old_user_form_element) if operation == "add_task": - new_task_form_elements = get_task_form_elements(task_count) + new_task_form_elements = get_task_form_elements(task_count) project_form = form.Form(*(project_form_elements + task_form_elements + new_task_form_elements + user_form_elements)) return project_form @@ -176,4 +177,3 @@ class New_project: project_form = form.Form(*(project_form_elements + task_form_elements + user_form_elements)) return project_form - \ No newline at end of file diff --git a/src/app/views/project.py b/src/app/views/project.py index 66ccd70..d8ad8fc 100644 --- a/src/app/views/project.py +++ b/src/app/views/project.py @@ -1,6 +1,6 @@ import web import models.project -from views.utils import get_nav_bar +from views.utils import get_nav_bar, csrf_protected from views.forms import project_form import os from time import sleep @@ -17,7 +17,7 @@ class Project: :return: Project info page """ - + # Get session session = web.ctx.session # Get navbar @@ -41,12 +41,13 @@ class Project: render = web.template.render('templates/', globals={'get_task_files':models.project.get_task_files, 'session':session}) return render.project(nav, project_form, project, tasks,permissions, categories) + @csrf_protected def POST(self): # Get session session = web.ctx.session data = web.input(myfile={}, deliver=None, accepted=None, declined=None, projectid=0) fileitem = data['myfile'] - + permissions = models.project.get_user_permissions(str(session.userid), data.projectid) categories = models.project.get_categories() tasks = models.project.get_tasks_by_project_id(data.projectid) @@ -81,7 +82,7 @@ class Project: task_waiting = False task_delivered = False for task in tasks: - if task[0] == int(data.taskid): + if task[0] == int(data.taskid): if(task[5] == "waiting for delivery" or task[5] == "declined"): task_waiting = True if(task[5] == 'accepted'): @@ -90,7 +91,7 @@ class Project: # Deliver task if data.deliver and not task_delivered: models.project.update_task_status(data.taskid, "delivered") - + # Accept task delivery elif data.accepted: models.project.update_task_status(data.taskid, "accepted") @@ -107,6 +108,5 @@ class Project: # Decline task delivery elif data.declined: models.project.update_task_status(data.taskid, "declined") - - raise web.seeother(('/project?projectid=' + data.projectid)) + raise web.seeother(('/project?projectid=' + data.projectid)) diff --git a/src/app/views/register.py b/src/app/views/register.py index b9803f1..cd79330 100644 --- a/src/app/views/register.py +++ b/src/app/views/register.py @@ -1,8 +1,8 @@ import web from views.forms import register_form +from views.utils import get_nav_bar, csrf_protected import models.register import models.user -from views.utils import get_nav_bar import hashlib import re @@ -22,6 +22,7 @@ class Register: nav = get_nav_bar(session) return render.register(nav, register_form, "") + @csrf_protected def POST(self): """ Handle input data and register new user in database @@ -40,10 +41,9 @@ class Register: if models.user.get_user_id_by_name(data.username): return render.register(nav, register, "Invalid user, already exists.") - models.register.set_user(data.username, - hashlib.md5(b'TDT4237' + data.password.encode('utf-8')).hexdigest(), - data.full_name, data.company, data.email, data.street_address, + models.register.set_user(data.username, + hashlib.md5(b'TDT4237' + data.password.encode('utf-8')).hexdigest(), + data.full_name, data.company, data.email, data.street_address, data.city, data.state, data.postal_code, data.country) - - return render.register(nav, register_form, "User registered!") + return render.register(nav, register_form, "User registered!") diff --git a/src/app/views/utils.py b/src/app/views/utils.py index 3cf62a2..8b8a6d3 100644 --- a/src/app/views/utils.py +++ b/src/app/views/utils.py @@ -1,3 +1,6 @@ +import web +from uuid import uuid4 + def get_nav_bar(session): """ @@ -20,13 +23,13 @@ def get_nav_bar(session): result += '' return result - + def get_element_count(data, element): """ - Determine the number of tasks created by removing - the four other elements from count and divide by the + Determine the number of tasks created by removing + the four other elements from count and divide by the number of variables in one task. - + :param data: The data object from web.input :return: The number of tasks opened by the client """ @@ -38,3 +41,40 @@ def get_element_count(data, element): except: break return task_count + + +def csrf_token(): + """ + Get the CSRF token for the session + """ + session = web.ctx.session + + if 'csrf_token' not in session: + session.csrf_token = uuid4().hex + + return session.csrf_token + + +def csrf_field(): + """ + Return a HTML form field for the CSRF token + """ + return f'' + + +def csrf_protected(f): + """ + Decorate a function to do a CSRF check. + """ + def decorated(*args, **kwargs): + session = web.ctx.session + inp = web.input() + if not ('csrf_token' in inp and inp.csrf_token == session.pop('csrf_token', None)): + raise web.HTTPError( + '400 Bad request', + {'content-type': 'text/html'}, + 'Cross-site request forgery attempt', + ) + return f(*args, **kwargs) + + return decorated