Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion flaskr/flaskr.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def close_db(error):
@app.route('/')
def show_entries():
db = get_db()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Description: The show_entries() function doesn't handle potential database errors, which could lead to unhandled exceptions. Wrap the database operations in a try-except block to catch and handle potential SQLite errors.

Severity: High

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix addresses the comment by wrapping the database operations in the show_entries() function within a try-except block. This modification handles potential SQLite errors, preventing unhandled exceptions. If a database error occurs, it logs the error and returns an error page or message, improving the application's robustness and user experience.

Suggested change
db = get_db()
@app.route('/')
def show_entries():
try:
db = get_db()
cur = db.execute('SELECT id, title, text FROM entries ORDER BY id DESC')
entries = cur.fetchall()
except sqlite3.Error as e:
# Log the error and return an error page or message
app.logger.error(f"Database error: {e}")
return render_template('error.html', error="Database error occurred"), 500
return render_template('show_entries.html', entries=entries)

cur = db.execute('SELECT title, text FROM entries ORDER BY id DESC')
cur = db.execute('SELECT id, title, text FROM entries ORDER BY id DESC')
entries = cur.fetchall()
return render_template('show_entries.html', entries=entries)

Expand All @@ -76,6 +76,17 @@ def add_entry():
return redirect(url_for('show_entries'))


@app.route('/delete/<int:entry_id>', methods=['POST'])
def delete_entry(entry_id):
if not session.get('logged_in'):
abort(401)
db = get_db()
db.execute('DELETE FROM entries WHERE id = ?', [entry_id])
db.commit()
flash('Entry was successfully deleted')
return redirect(url_for('show_entries'))


@app.route('/login', methods=['GET', 'POST'])
def login():
error = None
Expand Down
9 changes: 8 additions & 1 deletion flaskr/templates/show_entries.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@
{% endif %}
<ul class=entries>
{% for entry in entries %}
<li><h2>{{ entry.title }}</h2>{{ entry.text|safe }}
<li>
<h2>{{ entry.title }}</h2>{{ entry.text|safe }}
{% if session.logged_in %}
<form action="{{ url_for('delete_entry', entry_id=entry.id) }}" method=post class=delete-entry style="display: inline;">
<input type=submit value="Delete" onclick="return confirm('Are you sure you want to delete this entry?');">
</form>
{% endif %}
</li>
{% else %}
<li><em>Unbelievable. No entries here so far</em>
{% endfor %}
Expand Down
47 changes: 47 additions & 0 deletions tests/test_flaskr.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,53 @@ def test_show_entries(self):
# the database state is not guaranteed. In a real-world scenario,
# you might want to set up a known database state before running this test.

def test_delete_entry_unauthorized(self):
"""
Test that an unauthorized user cannot delete an entry.
"""
with app.test_client() as client:
# Try to delete an entry without being logged in
response = client.post('/delete/1', follow_redirects=True)

# Should get a 401 Unauthorized response
assert response.status_code == 401

def test_delete_entry_authorized(self):

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description: The test_delete_entry_authorized function is quite long and performs multiple operations. Consider breaking down the function into smaller, more focused test functions or using setup and teardown methods.

Severity: Low

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not able to suggest a fix for this review finding.

Request ID : 1d7a21f3-17b7-40fa-8f5c-94f691fe9b33

"""
Test that an authorized user can delete an entry.
"""
with app.test_client() as client:
# First login
client.post('/login', data={
'username': app.config['USERNAME'],
'password': app.config['PASSWORD']
})

# Add an entry to delete
client.post('/add', data={
'title': 'Test Entry to Delete',
'text': 'This entry will be deleted'
})

# Get the entries to find the ID of the one we just added
with app.app_context():
db = get_db()
entry = db.execute('SELECT id FROM entries WHERE title = ?',
['Test Entry to Delete']).fetchone()

if entry:
# Delete the entry
response = client.post(f'/delete/{entry["id"]}', follow_redirects=True)

# Check if deletion was successful
assert response.status_code == 200
assert b'Entry was successfully deleted' in response.data

# Verify the entry is no longer in the database
check = db.execute('SELECT * FROM entries WHERE id = ?',
[entry["id"]]).fetchone()
assert check is None



class AuthActions(object):
Expand Down