Baip 316/do not allow api key and jwt simultaneously#33
Baip 316/do not allow api key and jwt simultaneously#33jiristrouhal wants to merge 4 commits intomasterfrom
Conversation
WalkthroughThis update enforces mutual exclusivity between JWT token and API key authentication methods, raising a 401 error if both are used simultaneously. It introduces a new test to verify this behavior, updates error message strings, and increments the project and OpenAPI specification version to 4.1.2. Minor formatting improvements are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SecurityController
participant AuthSystem
Client->>SecurityController: Request with JWT token and/or API key
SecurityController->>SecurityController: _raise_for_simultaneous_jwt_and_api_key()
alt Both JWT and API key present
SecurityController-->>Client: 401 Unauthorized (error message)
else Only one authentication method present
SecurityController->>AuthSystem: Validate credentials
AuthSystem-->>SecurityController: Auth result
SecurityController-->>Client: Success or failure response
end
Possibly related PRs
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
fleet_management_api/controllers/security_controller.py (1)
74-85: Implementation correctly enforces mutual exclusivity between authentication methods.The implementation properly detects when both authentication methods are present and raises a 401 error with a clear message. The approach of checking the raw query string for the API key parameter is robust.
Small performance optimization: You could consider caching the result of
request.query_string.decode()in a variable since this operation is performed twice.- api_key_used = "api_key" in request.query_string.decode() + query_string = request.query_string.decode() + api_key_used = "api_key" in query_stringtests/security/test_combining_auth_schemes.py (1)
1-37: Well-structured test case to verify the new authentication enforcement.This test effectively verifies that using both JWT token and API key authentication simultaneously returns a 401 error with an appropriate message.
-from connexion.exceptions import UnauthorizedThere's an unused import of
Unauthorizedfromconnexion.exceptionsthat can be removed.🧰 Tools
🪛 Ruff (0.8.2)
3-3:
connexion.exceptions.Unauthorizedimported but unusedRemove unused import:
connexion.exceptions.Unauthorized(F401)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
fleet_management_api/api_impl/controller_decorators.py(1 hunks)fleet_management_api/api_impl/tenants.py(1 hunks)fleet_management_api/controllers/security_controller.py(3 hunks)fleet_management_api/openapi/openapi.yaml(1 hunks)openapi/openapi.yaml(1 hunks)pyproject.toml(1 hunks)tests/controllers/car/test_car_state_controller.py(1 hunks)tests/security/test_combining_auth_schemes.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
fleet_management_api/controllers/security_controller.py (4)
fleet_management_api/api_impl/load_request.py (1)
api_key(50-51)fleet_management_api/models/car.py (2)
name(121-128)name(131-141)fleet_management_api/models/stop.py (2)
name(88-95)name(98-108)fleet_management_api/models/route.py (2)
name(74-81)name(84-94)
🪛 Ruff (0.8.2)
tests/security/test_combining_auth_schemes.py
3-3: connexion.exceptions.Unauthorized imported but unused
Remove unused import: connexion.exceptions.Unauthorized
(F401)
🔇 Additional comments (9)
openapi/openapi.yaml (1)
5-5: Version increment appropriately reflects the new security validation feature.The version update from 4.1.1 to 4.1.2 aligns with the implementation of the new security validation that prevents simultaneous use of API key and JWT authentication.
pyproject.toml (1)
3-3: Version update maintains consistency with OpenAPI specification.The project version update from 4.1.1 to 4.1.2 aligns with the corresponding OpenAPI specification version change, ensuring consistency across the project.
fleet_management_api/openapi/openapi.yaml (1)
12-12: Version update maintains consistency with main OpenAPI specification.This version increment from 4.1.1 to 4.1.2 ensures that the OpenAPI specification bundled with the Python package matches the main specification file.
fleet_management_api/api_impl/controller_decorators.py (1)
66-66: Improved error message clarity for tenant extraction failures.The error title change from "No tenants" to "Cannot extract tenants" provides clearer context about what went wrong during the tenant extraction process, which enhances troubleshooting and error reporting.
fleet_management_api/api_impl/tenants.py (1)
227-227: Improved code readability with consistent spacing.The addition of a blank line after the exception provides better visual separation between code blocks, enhancing readability while maintaining the same functionality.
tests/controllers/car/test_car_state_controller.py (1)
558-559: LGTM! URL updated to align with new authentication enforcement.The URL has been correctly updated to remove the
api_keyparameter, ensuring this test now uses only JWT token authentication via the Authorization header. This change aligns with the new enforcement where API key and JWT token cannot be used simultaneously.fleet_management_api/controllers/security_controller.py (3)
2-2: LGTM! Added necessary import for Request access.The connexion import is required to access the request object and exceptions for the new authentication validation.
24-24: Good addition of mutual exclusivity check.Adding the check at the beginning of the OAuth handler ensures requests with both authentication methods will be rejected early.
66-66: Good addition of mutual exclusivity check.Adding the check at the beginning of the API key handler ensures requests with both authentication methods will be rejected early.
|
Version tag has to be incremented further |
2 participants
During authentication, the API now checks, if multiple authentication schemes are used in the same request and if so, 401 response is returned.
Summary by CodeRabbit