Update admin notification UI and allow for deleting notifications (#803)

* Show notification titles on the notification list page
* Allow for deleting notifications
* Update notification UI in admin panel
* Make /api/v1/notifications/<id> accessible to all
* Default `login_as_user()` and `register_user()` to fail on invalid credentials

CTFd/api/v1/notifications.py

@@ -50,3 +50,33 @@ class NotificantionList(Resource):
             'success': True,
             'data': response.data
         }
+
+
+@notifications_namespace.route('/<notification_id>')
+@notifications_namespace.param('notification_id', 'A Notification ID')
+class Notification(Resource):
+    def get(self, notification_id):
+        notif = Notifications.query.filter_by(id=notification_id).first_or_404()
+        schema = NotificationSchema()
+        response = schema.dump(notif)
+        if response.errors:
+            return {
+                'success': False,
+                'errors': response.errors
+            }, 400
+
+        return {
+            'success': True,
+            'data': response.data
+        }
+
+    @admins_only
+    def delete(self, notification_id):
+        notif = Notifications.query.filter_by(id=notification_id).first_or_404()
+        db.session.delete(notif)
+        db.session.commit()
+        db.session.close()
+
+        return {
+            'success': True,
+        }

CTFd/themes/admin/static/js/notifications/notifications.js

@@ -14,6 +14,31 @@ $(document).ready(function () {
             body: JSON.stringify(params)
         }).then(function (response) {
             return response.json();
+        }).then(function (response) {
+            if (response.success) {
+                setTimeout(function () {
+                    window.location.reload();
+                }, 3000);
+            }
+        });
+    });
+
+    $('.delete-notification').click(function (e) {
+        e.preventDefault();
+        var elem = $(this);
+        var notif_id = elem.attr("notif-id");
+
+        ezq({
+            title: 'Delete Notification',
+            body: "Are you sure you want to delete this notification?",
+            success: function () {
+                var delete_route = script_root + '/api/v1/notifications/' + notif_id;
+                $.delete(delete_route, {}, function (response) {
+                    if (response.success) {
+                        elem.parent().remove();
+                    }
+                });
+            }
         });
     });
 });

CTFd/themes/admin/templates/notifications.html

@@ -40,26 +40,20 @@
 		<div class="row">
 			<div class="col-md-12">
 				<hr>
-				<table class="table table-striped">
+				{% for notification in notifications %}
-					<thead>
+					<div class="card bg-light mb-4">
-						<tr>
+						<button type="button" notif-id="{{ notification.id }}" class="delete-notification close position-absolute p-3" style="right:0;" data-dismiss="alert" aria-label="Close">
-							<td width="10px"><b>ID</b></td>
+							<span aria-hidden="true">&times;</span>
-							<td><b>Title</b></td>
+						</button>
-							<td><b>Content</b></td>
+						<div class="card-body">
-							<td><b>Date</b></td>
+							<h3 class="card-title">{{ notification.title }}</h3>
-						</tr>
+							<blockquote class="blockquote mb-0">
-					</thead>
+								<p>{{ notification.content | safe }}</p>
-					<tbody>
+								<small class="text-muted">{{ notification.date | isoformat }}</small>
-					{% for notification in notifications %}
+							</blockquote>
-						<tr>
+						</div>
-							<td>{{ notification.id }}</td>
+					</div>
-							<td>{{ notification.title }}</td>
+				{% endfor %}
-							<td>{{ notification.content | safe }}</td>
-							<td>{{ notification.date | isoformat }}</td>
-						</tr>
-					{% endfor %}
-					</tbody>
-				</table>
 			</div>
 		</div>
 	</div>

CTFd/themes/core/templates/notifications.html

@@ -10,6 +10,7 @@
 		{% for notification in notifications %}
 		<div class="card bg-light mb-4">
 			<div class="card-body">
+				<h3 class="card-title">{{ notification.title }}</h3>
 				<blockquote class="blockquote mb-0">
 					<p>{{ notification.content | safe }}</p>
 					<small class="text-muted">{{ notification.date | isoformat }}</small>

tests/api/v1/test_hints.py

@@ -9,6 +9,7 @@ def test_api_hint_get_non_admin():
     """Can the users get /api/v1/hints if not admin"""
     app = create_ctfd()
     with app.app_context():
+        register_user(app)
         with login_as_user(app) as client:
             r = client.get('/api/v1/hints', json="")
             assert r.status_code == 403
@@ -30,6 +31,7 @@ def test_api_hint_post_non_admin():
     """Can the users post /api/v1/hints if not admin"""
     app = create_ctfd()
     with app.app_context():
+        register_user(app)
         with login_as_user(app) as client:
             r = client.post('/api/v1/hints', json="")
             assert r.status_code == 403

tests/api/v1/test_notifications.py

@@ -8,9 +8,26 @@ def test_api_notifications_get():
     """Can the users get /api/v1/notifications"""
     app = create_ctfd()
     with app.app_context():
+        register_user(app)
+        gen_notification(app.db)
         with login_as_user(app) as client:
             r = client.get('/api/v1/notifications', json="")
             assert r.status_code == 200
+            assert len(r.get_json()['data']) == 1
+    destroy_ctfd(app)
+
+
+def test_api_get_notification_detail():
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app)
+        gen_notification(app.db)
+        with login_as_user(app) as client:
+            r = client.get('/api/v1/notifications/1', json="")
+            assert r.status_code == 200
+            resp = r.get_json()
+            assert resp['data']['title'] == 'title'
+            assert resp['data']['content'] == 'content'
     destroy_ctfd(app)
 
 
@@ -18,6 +35,7 @@ def test_api_notifications_post_non_admin():
     """Can the users post /api/v1/notifications if not admin"""
     app = create_ctfd()
     with app.app_context():
+        register_user(app)
         with login_as_user(app) as client:
             r = client.post('/api/v1/notifications', json="")
             assert r.status_code == 403
@@ -35,3 +53,33 @@ def test_api_notifications_post_admin():
                 "content": "content"})
             assert r.status_code == 200
     destroy_ctfd(app)
+
+
+def test_api_delete_notifications_by_admin():
+    """Test that an admin can delete notifications"""
+    app = create_ctfd()
+    with app.app_context():
+        gen_challenge(app.db)
+        gen_notification(app.db)
+        assert Notifications.query.count() == 1
+        with login_as_user(app, name="admin") as client:
+            r = client.delete('/api/v1/notifications/1', json="")
+            assert r.status_code == 200
+            assert r.get_json()['success'] is True
+        assert Notifications.query.count() == 0
+    destroy_ctfd(app)
+
+
+def test_api_delete_notifications_by_user():
+    """Test that a non-admin cannot delete notifications"""
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app)
+        gen_challenge(app.db)
+        gen_notification(app.db)
+        assert Notifications.query.count() == 1
+        with login_as_user(app) as client:
+            r = client.delete('/api/v1/notifications/1', json="")
+            assert r.status_code == 403
+        assert Notifications.query.count() == 1
+    destroy_ctfd(app)

tests/api/v1/user/test_hints.py

@@ -112,7 +112,6 @@ def test_api_hint_admin_access():
         r = client.delete('/api/v1/hints/1')
         assert r.status_code == 302
         r_admin = admin.patch('/api/v1/hints/1', json={"cost": 2})
-        print(r_admin.get_json)
         assert r_admin.status_code == 200
         r_admin = admin.delete('/api/v1/hints/1')
         assert r_admin.status_code == 200

tests/helpers.py

@@ -55,7 +55,7 @@ def destroy_ctfd(app):
         drop_database(app.config['SQLALCHEMY_DATABASE_URI'])
 
 
-def register_user(app, name="user", email="user@ctfd.io", password="password"):
+def register_user(app, name="user", email="user@ctfd.io", password="password", raise_for_error=True):
     with app.app_context():
         with app.test_client() as client:
             r = client.get('/register')
@@ -67,6 +67,13 @@ def register_user(app, name="user", email="user@ctfd.io", password="password"):
                     "nonce": sess.get('nonce')
                 }
             client.post('/register', data=data)
+            if raise_for_error:
+                with client.session_transaction() as sess:
+                    assert sess['id']
+                    assert sess['name'] == name
+                    assert sess['type']
+                    assert sess['email']
+                    assert sess['nonce']
 
 
 def register_team(app, name="team", password="password"):
@@ -82,7 +89,7 @@ def register_team(app, name="team", password="password"):
             client.post('/teams/new', data=data)
 
 
-def login_as_user(app, name="user", password="password"):
+def login_as_user(app, name="user", password="password", raise_for_error=True):
     with app.app_context():
         with app.test_client() as client:
             r = client.get('/login')
@@ -93,6 +100,13 @@ def login_as_user(app, name="user", password="password"):
                     "nonce": sess.get('nonce')
                 }
             client.post('/login', data=data)
+            if raise_for_error:
+                with client.session_transaction() as sess:
+                    assert sess['id']
+                    assert sess['name']
+                    assert sess['type']
+                    assert sess['email']
+                    assert sess['nonce']
             return client
 
 
@@ -208,7 +222,7 @@ def gen_page(db, title, route, content, draft=False, auth_required=False, **kwar
     return page
 
 
-def gen_notification(db, title, content):
+def gen_notification(db, title='title', content='content'):
     notif = Notifications(title=title, content=content)
     db.session.add(notif)
     db.session.commit()

tests/test_plugin_utils.py

@@ -134,8 +134,8 @@ def test_register_user_page_menu_bar():
     with app.app_context():
         register_user_page_menu_bar(title='test_user_menu_link', route='/test_user_href')
 
-        client = login_as_user(app)
+        with app.test_client() as client:
-        r = client.get('/')
+            r = client.get('/')
 
         output = r.get_data(as_text=True)
         assert '/test_user_href' in output

tests/users/test_auth.py

@@ -34,8 +34,8 @@ def test_register_duplicate_username():
     """A user shouldn't be able to use an already registered team name"""
     app = create_ctfd()
     with app.app_context():
-        register_user(app, name="user1", email="user1@ctfd.io", password="password")
+        register_user(app, name="user1", email="user1@ctfd.io", password="password", raise_for_error=False)
-        register_user(app, name="user1", email="user2@ctfd.io", password="password")
+        register_user(app, name="user1", email="user2@ctfd.io", password="password", raise_for_error=False)
         user_count = Users.query.count()
         assert user_count == 2  # There's the admin user and the first created user
     destroy_ctfd(app)
@@ -45,8 +45,8 @@ def test_register_duplicate_email():
     """A user shouldn't be able to use an already registered email address"""
     app = create_ctfd()
     with app.app_context():
-        register_user(app, name="user1", email="user1@ctfd.io", password="password")
+        register_user(app, name="user1", email="user1@ctfd.io", password="password", raise_for_error=False)
-        register_user(app, name="user2", email="user1@ctfd.io", password="password")
+        register_user(app, name="user2", email="user1@ctfd.io", password="password", raise_for_error=False)
         user_count = Users.query.count()
         assert user_count == 2  # There's the admin user and the first created user
     destroy_ctfd(app)
@@ -57,7 +57,7 @@ def test_register_whitelisted_email():
     app = create_ctfd()
     with app.app_context():
         set_config('domain_whitelist', 'whitelisted.com, whitelisted.org, whitelisted.net')
-        register_user(app, name="not_whitelisted", email='user@nope.com')
+        register_user(app, name="not_whitelisted", email='user@nope.com', raise_for_error=False)
         assert Users.query.count() == 1
 
         register_user(app, name="user1", email='user@whitelisted.com')
@@ -76,7 +76,7 @@ def test_user_bad_login():
     app = create_ctfd()
     with app.app_context():
         register_user(app)
-        client = login_as_user(app, name="user", password="wrong_password")
+        client = login_as_user(app, name="user", password="wrong_password", raise_for_error=False)
         with client.session_transaction() as sess:
             assert sess.get('id') is None
         r = client.get('/profile')

tests/users/test_scoreboard.py

@@ -84,6 +84,7 @@ def test_top_10():
     with app.app_context():
         register_user(app, name="user1", email="user1@ctfd.io")
         register_user(app, name="user2", email="user2@ctfd.io")
+        register_user(app)
 
         chal1 = gen_challenge(app.db)
         flag1 = gen_flag(app.db, challenge_id=chal1.id, content='flag')