Repository: CTFd/CTFd (python)

Issue:
Creating a page route starting with a /
Creating a page route that begins with a slash causes problems:

1. Content is not saved
2. Page cannot be removed using the /admin/pages interface

Text on /admin/pages?mode=create seems to suggest that the route should start with a slash.

I suggest stripping any initial slashes or adding validation to this field.

Pull request:
Allowing pages to have slashes in them but removing leading slashes
Closes #449
Pages can now have slashes in them as long as they are not leading. 


Patch:
diff --git a/CTFd/admin/pages.py b/CTFd/admin/pages.py
index b698dd3b..9751faa0 100644
--- a/CTFd/admin/pages.py
+++ b/CTFd/admin/pages.py
@@ -19,24 +19,27 @@ def admin_css():
     return '0'
 
 
-@admin_pages.route('/admin/pages', defaults={'route': None}, methods=['GET', 'POST'])
-@admin_pages.route('/admin/pages/<route>', methods=['GET', 'POST'])
+@admin_pages.route('/admin/pages', methods=['GET', 'POST'])
 @admins_only
-def admin_pages_view(route):
+def admin_pages_view():
+    route = request.args.get('route')
+
     if request.method == 'GET' and request.args.get('mode') == 'create':
         return render_template('admin/editor.html')
+
     if route and request.method == 'GET':
         page = Pages.query.filter_by(route=route).first()
         return render_template('admin/editor.html', page=page)
-    if route and request.method == 'POST':
+
+    if request.method == 'POST':
+        html = request.form['html']
+        route = request.form['route'].lstrip('/')
         page = Pages.query.filter_by(route=route).first()
         errors = []
-        html = request.form['html']
-        route = request.form['route']
         if not route:
             errors.append('Missing URL route')
         if errors:
-            page = Pages(html, '')
+            page = Pages(html, route)
             return render_template('/admin/editor.html', page=page)
         if page:
             page.route = route
@@ -53,10 +56,24 @@ def admin_pages_view(route):
         with app.app_context():
             cache.clear()
         return redirect(url_for('admin_pages.admin_pages_view'))
+
     pages = Pages.query.all()
     return render_template('admin/pages.html', routes=pages, css=utils.get_config('css'))
 
 
+@admin_pages.route('/admin/pages/delete', methods=['POST'])
+@admins_only
+def delete_page():
+    route = request.form['route']
+    page = Pages.query.filter_by(route=route).first_or_404()
+    db.session.delete(page)
+    db.session.commit()
+    db.session.close()
+    with app.app_context():
+        cache.clear()
+    return '1'
+
+
 @admin_pages.route('/admin/media', methods=['GET', 'POST', 'DELETE'])
 @admins_only
 def admin_pages_media():
@@ -77,15 +94,3 @@ def admin_pages_media():
     else:
         files = [{'id': f.id, 'location': f.location} for f in Files.query.filter_by(chal=None).all()]
         return jsonify({'results': files})
-
-
-@admin_pages.route('/admin/page/<pageroute>/delete', methods=['POST'])
-@admins_only
-def delete_page(pageroute):
-    page = Pages.query.filter_by(route=pageroute).first_or_404()
-    db.session.delete(page)
-    db.session.commit()
-    db.session.close()
-    with app.app_context():
-        cache.clear()
-    return '1'
diff --git a/CTFd/themes/admin/templates/editor.html b/CTFd/themes/admin/templates/editor.html
index 6b773aac..30e5b14d 100644
--- a/CTFd/themes/admin/templates/editor.html
+++ b/CTFd/themes/admin/templates/editor.html
@@ -272,7 +272,7 @@
 	});
 
 	$('#page-edit').submit(function (e){
-		$(this).attr('action', '{{ request.script_root }}/admin/pages/'+$('#route').val());
+		$(this).attr('action', '{{ request.script_root }}/admin/pages');
 	});
 
 	// Markdown Preview
diff --git a/CTFd/themes/admin/templates/pages.html b/CTFd/themes/admin/templates/pages.html
index c6d25443..bb257b5e 100644
--- a/CTFd/themes/admin/templates/pages.html
+++ b/CTFd/themes/admin/templates/pages.html
@@ -51,7 +51,7 @@
 			{% for route in routes %}
 				<tr name="{{ route.route }}">
 					<td class="route-name"><a
-							href="{{ request.script_root }}/admin/pages/{{ route.route }}">{{ route.route }}</a></td>
+							href="{{ request.script_root }}/admin/pages?route={{ route.route }}">{{ route.route }}</a></td>
 					<td class="text-center"><i class="fa fa-times"></i></td>
 				</tr>
 			{% endfor %}
@@ -103,7 +103,7 @@ function load_confirm_modal(route){
 	var modal = $('#confirm')
 	modal.find('input[name=route]').val(route)
 	modal.find('#confirm-route-name').text(route)
-	$('#confirm form').attr('action', '{{ request.script_root }}/admin/page/'+route+'/delete');
+	$('#confirm form').attr('action', '{{ request.script_root }}/admin/pages/delete');
 	$('#confirm').modal();
 }
 
diff --git a/CTFd/views.py b/CTFd/views.py
index 5f8bb91c..953e38fa 100644
--- a/CTFd/views.py
+++ b/CTFd/views.py
@@ -106,7 +106,7 @@ def custom_css():
 
 # Static HTML files
 @views.route("/", defaults={'template': 'index'})
-@views.route("/<template>")
+@views.route("/<path:template>")
 def static_html(template):
     try:
         return render_template('%s.html' % template)


Test patch:
diff --git a/tests/admin/test_pages.py b/tests/admin/test_pages.py
new file mode 100644
index 00000000..838617f6
--- /dev/null
+++ b/tests/admin/test_pages.py
@@ -0,0 +1,82 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+from tests.helpers import *
+from CTFd.models import Pages
+from CTFd.utils import get_config, set_config, override_template, sendmail, verify_email, ctf_started, ctf_ended
+from CTFd.plugins.challenges import get_chal_class
+from freezegun import freeze_time
+from mock import patch
+
+
+def test_admin_page_create():
+    """Can an admin create a page?"""
+    app = create_ctfd()
+    with app.app_context():
+        client = login_as_user(app, name="admin", password="password")
+        r = client.get('/admin/pages?mode=create')
+        assert r.status_code == 200
+        with client.session_transaction() as sess:
+            data = {
+                "route": "this-is-a-route",
+                "html": "This is some HTML",
+                "nonce": sess.get('nonce')
+            }
+        r = client.post('/admin/pages', data=data)
+        r = client.get('/admin/pages?route=this-is-a-route')
+        assert r.status_code == 200
+
+        r = client.get('/this-is-a-route')
+        assert r.status_code == 200
+        output = r.get_data(as_text=True)
+        assert "This is some HTML" in output
+    destroy_ctfd(app)
+
+
+def test_admin_page_update():
+    """Can an admin update a page?"""
+    app = create_ctfd()
+    with app.app_context():
+        client = login_as_user(app, name="admin", password="password")
+        r = client.get('/admin/pages?route=index')
+        assert r.status_code == 200
+        with client.session_transaction() as sess:
+            data = {
+                "route": "index",
+                "html": "New Index Page",
+                "nonce": sess.get('nonce')
+            }
+        r = client.post('/admin/pages', data=data)
+        r = client.get('/admin/pages?route=index')
+        assert r.status_code == 200
+        output = r.get_data(as_text=True)
+        assert "New Index Page" in output
+
+        r = client.get('/')
+        assert r.status_code == 200
+        output = r.get_data(as_text=True)
+        assert "New Index Page" in output
+    destroy_ctfd(app)
+
+
+def test_admin_page_delete():
+    """Can an admin delete a page?"""
+    app = create_ctfd()
+    with app.app_context():
+        client = login_as_user(app, name="admin", password="password")
+
+        with client.session_transaction() as sess:
+            data = {
+                "route": "index",
+                "nonce": sess.get('nonce')
+            }
+        r = client.post('/admin/pages/delete', data=data)
+        assert r.status_code == 200
+
+        r = client.get('/')
+        assert r.status_code == 404
+
+        count = Pages.query.count()
+        assert count == 0
+
+    destroy_ctfd(app)