 Chris Coutinho
|
daabd90359
|
fix(security): address critical security issues from PR #401 code review
Implemented 6 critical security fixes identified during PR #401 review:
1. Token Rotation Race Condition (Issue 1)
- Added in-progress marker pattern to prevent concurrent refresh
- Prevents token invalidation when multiple requests refresh simultaneously
- File: token_broker.py:324, 343-390
2. Hardcoded Localhost URL (Issue 2)
- Added getNextcloudBaseUrl() with fallback chain
- Supports overwrite.cli.url, trusted_domains, and localhost fallback
- File: IdpTokenRefresher.php:38-61, 116
3. Error Information Leakage (Issue 3)
- Replaced 13 instances of str(e) with sanitized errors
- Prevents exposure of stack traces, paths, and tokens
- File: management.py:368, 444, 492, 510, 546, 571, 625, 643, 695, 750, 919, 956, 1121
4. Input Validation Gaps (Issue 4)
- Added validation helpers: _parse_int_param, _parse_float_param, _validate_query_string
- Applied bounds checking to get_chunk_context and unified_search
- File: management.py:119-164, 807-835, 1197-1212
5. PHP Refresh Token Validation (Issue 5)
- Added explicit refresh_token presence check
- Prevents silent token rotation failures
- File: IdpTokenRefresher.php:122-132
6. Cookie Security Configuration (Issue 6)
- Added _should_use_secure_cookies() with auto-detection
- Supports explicit COOKIE_SECURE env var or auto-detect from NEXTCLOUD_HOST
- Files: browser_oauth_routes.py:27-44, 470; env.sample:54-57
Testing:
- Unit tests: 195 passed
- Integration tests: 102 passed, 4 skipped
- OAuth tests: 9 passed
- All linting and type checks passed
Follow-up work tracked in issues #408-#417
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
2025-12-19 13:57:33 +01:00 |
|