Skip to content

Conversation

@codesungrape
Copy link
Collaborator

Description

Trello card: https://trello.com/c/MkArdXgF

This PR addresses the requirement to secure write-access endpoints (POST, PUT, DELETE) to prevent unauthorized modifications to the book collection.

As per the discussion in the Trello card, this work implements a simple and robust API key authentication strategy. This was chosen as a pragmatic first step to meet the immediate security need, with a more complex user-based solution like OAuth2 being considered for a future iteration.

The implementation includes:

  • API Key Authentication: A @require_api_key decorator was created to protect routes. It uses a constant-time comparison (hmac.compare_digest) to prevent timing attacks, and features enhanced logging for unauthorized access attempts.
  • Standardized Error Handling (Breaking Change): A global, structured error handler (handle_http_exception) was introduced. All HTTP exceptions now return a consistent JSON object ({ "error": { "code", "name", "message" } }), which is a major improvement for API clients.
  • Comprehensive Test Suite: A full suite of tests was added for the new security layer. The tests were then refactored using pytest.mark.parametrize to eliminate repetition and improve maintainability.
  • Full Documentation Update: The openapi.yml documentation has been thoroughly updated to reflect the new security requirements on protected routes and, most importantly, the new structured error response schema.

Type of change

[x] New feature (non-breaking change which adds functionality)
[x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[x] This change requires a documentation update
[x] Code refactor (improving code quality without changing functionality)

How Has This Been Tested?

  • Automated testing using fixtures for the test application and client, security failure scenarios which covers cases of invalid key, missing key, server key not configured for every protected endpoint.
  • Manual Testing (e.g., DELETE /books/{id}) without a valid API key.
    Example using curl:
    Generated bash
# Attempt to delete a book with an invalid key
curl -i -X DELETE http://localhost:5000/books/some-id \
-H "X-API-KEY: this-is-a-wrong-key"
  • CI/CD with GitActions

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] My individual commit messages are descriptive and follow our commit guidelines
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes
[x] Any dependent changes have been merged and published in downstream modules

Any other information:

IMPORTANT: This is a BREAKING CHANGE for two reasons:

  1. Authentication is now required: The POST /books, PUT /books/{id}, and DELETE /books/{id} endpoints, which were previously open, now require a valid X-API-KEY header. Any client not providing this header will receive a 401 Unauthorized error.
  2. Error Response Format has changed: The structure of all error responses from the API has been standardized. This will affect any client that was parsing the old error format.

Before this change:
Generated json

{
    "error": "Invalid API key."
}

After this change, example of the new structured error response:
Generated json

{
  "error": {
    "code": 401,
    "name": "Unauthorized",
    "message": "Invalid API key."
  }
}

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements API key authentication for write-access endpoints (POST, PUT, DELETE) and standardizes error response formats across the API. The changes include introducing a decorator-based authentication system, centralizing Flask app configuration, and comprehensive test updates.

Key changes:

  • Adds API key authentication to protect write operations (POST /books, PUT /books/{id}, DELETE /books/{id})
  • Introduces structured error responses with consistent JSON format including code, name, and message fields
  • Refactors application configuration to use a centralized Config class and supports test configuration overrides

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
app/init.py Updates create_app to use Config class and accept test configuration overrides
app/config.py New centralized configuration module loading environment variables
app/routes.py Adds @require_api_key decorators to write endpoints and structured error handling
app/utils/api_security.py New security module with API key validation decorator
openapi.yml Updates API documentation with security requirements and standardized error schemas
tests/conftest.py Centralizes test fixtures and adds shared test app configuration
tests/test_*.py Updates all tests to include required API key headers
pytest.ini Adds pytest configuration for Python path

codesungrape and others added 2 commits July 21, 2025 16:35
Fix header merge to preserve content-type in test requests

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@codesungrape codesungrape merged commit 46cf629 into main Jul 22, 2025
2 checks passed
@codesungrape codesungrape deleted the Secure-routes-with-write-access-to-database branch July 22, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants