?
APC-28
feature
Created: 2025-12-22 Updated: 2025-12-23
Relationships Loading...
Attachments
Loading...
Comments (6)
code-verification-qa · 2025-12-23
# QA Report: APC-28 - Session-Based Authentication **Issue:** APC-28 **Title:** Protect API endpoints with session-based authentication **QA Date:** 2025-12-23 **Tester:** code-verification-qa agent **Status:** ✅ VERIFIED --- ## Summary The implementation successfully replaces API key authentication with session-based Bearer token authentication. All 272 existing tests pass, plus comprehensive integration testing confirms the authentication system works correctly. --- ## Changes Reviewed ### Database Models (app/db/models.py) - ✅ Added `User` model with username, password_hash, created_at, is_active - ✅ Added `Session` model with user_id FK, session_token, expires_at, created_at - ✅ Proper indexes on username and session_token (unique) - ✅ Cascade delete from users to sessions ### Alembic Migration - ✅ Migration `e6e7aa9dc6f7` creates both tables correctly - ✅ Database schema verified in SQLite - ✅ Foreign key constraints properly set up ### Authentication Service (app/services/auth_service.py) - ✅ Password hashing using bcrypt with 12 rounds (secure) - ✅ Session token generation using `secrets.token_urlsafe(32)` (cryptographically secure) - ✅ Session validation checks expiration correctly - ✅ Session deletion works properly - ✅ Admin user creation from environment variables on startup ### API Changes - ✅ Replaced `verify_api_key` with `verify_session` in app/api/auth.py - ✅ All protected routers updated: config_router, metrics_router, power_router, router - ✅ Bearer token scheme implemented correctly - ✅ Authentication dependencies applied to all API endpoints ### New Auth Endpoints (app/api/auth_router.py) - ✅ POST /auth/login - username/password authentication - ✅ POST /auth/logout - session invalidation - ✅ GET /auth/me - current user info ### API Schemas (app/api/schemas.py) - ✅ LoginRequest, LoginResponse with token, expires_at, user info - ✅ LogoutResponse with success message - ✅ CurrentUserResponse with username and created_at ### Configuration (app/config.py) - ✅ Removed unused `api_key` field - ✅ Added `admin_user`, `admin_password`, `session_expiry_days` --- ## Test Results ### Unit Tests ``` ============================= test session starts ============================== platform linux -- Python 3.12.12, pytest-9.0.2, pluggy-1.6.0 plugins: asyncio-1.3.0, anyio-4.12.0 272 passed, 5 warnings in 50.25s ``` **Test Coverage:** - 35 auth-specific tests in `tests/test_auth.py` - 20 auth endpoint tests in `tests/test_auth_endpoints.py` - 38 auth service tests in `tests/test_auth_service.py` - All existing API tests still pass with new authentication ### Integration Tests All integration tests passed successfully: - ✅ Health endpoint accessible without authentication - ✅ Protected endpoints require Bearer token (401 without) - ✅ Login with valid credentials returns token - ✅ GET /auth/me returns current user info - ✅ Protected endpoints work with valid token (200 OK) - ✅ Logout invalidates session token - ✅ Token unusable after logout (401) - ✅ Invalid password returns 401 - ✅ Non-existent user returns 401 - ✅ Invalid token returns 401 - ✅ Malformed auth header returns 401 ### Security Tests #### Expired Session Handling ✅ Expired sessions correctly rejected (returns None on validation) #### Password Hashing - ✅ Uses bcrypt with 12 rounds (secure for 2025) - ✅ Salt properly generated per password - ✅ Password verification works correctly #### Session Tokens - ✅ Generated with `secrets.token_urlsafe(32)` - cryptographically secure - ✅ 43+ character tokens from 32 bytes of entropy - ✅ Unique constraint enforced in database - ✅ Tokens invalidated on logout --- ## Database Verification ### Schema Created Correctly ```sql CREATE TABLE users ( id INTEGER NOT NULL, username VARCHAR(100) NOT NULL, password_hash VARCHAR(255) NOT NULL, created_at DATETIME NOT NULL, is_active BOOLEAN NOT NULL, PRIMARY KEY (id) ); CREATE UNIQUE INDEX ix_users_username ON users (username); CREATE TABLE sessions ( id INTEGER NOT NULL, user_id INTEGER NOT NULL, session_token VARCHAR(255) NOT NULL, expires_at DATETIME NOT NULL, created_at DATETIME NOT NULL, PRIMARY KEY (id), FOREIGN KEY(user_id) REFERENCES users (id) ON DELETE CASCADE ); CREATE UNIQUE INDEX ix_sessions_session_token ON sessions (session_token); ``` ### Migration Applied Successfully ``` Current: e6e7aa9dc6f7 (head) History: 4a527558b09d -> e6e7aa9dc6f7 (head), Add User and Session models for authentication 737d11283c5e -> 4a527558b09d, Update PowerReading table for metrics collection <base> -> 737d11283c5e, Initial tables ``` --- ## Security Review ### ✅ Strengths 1. **Strong password hashing:** bcrypt with 12 rounds meets 2025 standards 2. **Cryptographically secure tokens:** Uses Python's `secrets` module 3. **Proper session expiration:** Tokens expire after configurable period (default 30 days) 4. **Session invalidation:** Logout properly deletes sessions 5. **Database constraints:** Unique indexes prevent duplicate usernames/tokens 6. **Cascade deletion:** User deletion cleans up sessions 7. **Timezone-aware:** All timestamps use UTC with timezone 8. **No information leakage:** Same error message for invalid user vs invalid password --- ## Breaking Changes ### ⚠️ API Changes 1. **Authentication method changed:** - Old: `X-API-Key` header - New: `Authorization: Bearer <token>` header 2. **Configuration changes:** - Removed: `API_KEY` environment variable - Added: `ADMIN_USER`, `ADMIN_PASSWORD`, `SESSION_EXPIRY_DAYS` 3. **New endpoints:** - POST /auth/login - POST /auth/logout - GET /auth/me --- ## Conclusion **VERDICT: ✅ VERIFIED** The implementation of APC-28 is complete, secure, and production-ready. All requirements have been met: 1. ✅ API key authentication removed 2. ✅ Session-based Bearer token authentication implemented 3. ✅ All API endpoints protected 4. ✅ Login, logout, and user info endpoints functional 5. ✅ Database migrations work correctly 6. ✅ Admin user creation from environment works 7. ✅ All 272 tests pass 8. ✅ Security best practices followed 9. ✅ Edge cases handled properly 10. ✅ No regressions in existing functionality **Recommendation:** Merge to main branch and deploy.
agent · 2025-12-23
## Final Cleanup Complete Removed the unused api_key config field from app/config.py as requested by QA feedback. **Changes:** - Removed api_key: str = "" from Settings class (line 20) - No other references to api_key remained in codebase **Verification:** - All 272 tests pass - No grep matches for 'api_key' in codebase **Git Status:** - Committed: 640e064 'APC-28: Remove unused api_key config field' - Merged to main locally (fast-forward) - NOTE: Push to remote failed due to SSH key permissions in agent environment The code is ready - just needs git push origin main from an environment with proper SSH access to gitea.roboalch.com. All acceptance criteria met: - All API endpoints (except /health, /auth/login) return 401 without session - Valid session token grants access to all endpoints - Admin user auto-created on startup if not exists - Old API key authentication code removed (including config field) - Existing functionality works with new auth
qa-agent · 2025-12-23
[QA] Implementation is 99% complete and fully functional. All major requirements met: ✅ Session-based authentication working correctly ✅ All protected endpoints return 401 without auth ✅ Valid Bearer tokens grant access to all endpoints ✅ Admin user auto-creation on startup works (tested with ADMIN_USER/ADMIN_PASSWORD env vars) ✅ All 272 tests pass ✅ WWW-Authenticate: Bearer header present in 401 responses ✅ Public endpoints (/health, /auth/login) accessible without auth ❌ MINOR ISSUE: Old api_key config field still present in app/config.py:20 - The unused 'api_key: str' field should be removed as part of cleanup - Per requirements: 'Clean up old API key code (can be removed)' - This is the ONLY remaining item Manual testing verified: - Login flow works correctly - Session tokens validated properly - Protected endpoints (device, config, metrics, power routers) all require auth - Public endpoints work without auth - WWW-Authenticate header correctly returned on 401 Full QA report: /tmp/qa-APC-28.md Recommendation: Remove api_key field from config.py and mark as done.
agent · 2025-12-23
Implemented session-based authentication protection for all API endpoints. **Changes made:** 1. **app/api/auth.py** - Replaced verify_api_key with new verify_session dependency: - Uses HTTPBearer scheme to extract token from Authorization header - Validates session token via auth_service.validate_session() - Returns authenticated User object for use by endpoints - Returns 401 with WWW-Authenticate: Bearer header on failure 2. **Protected routers updated** (lines ~10-15 in each file): - router.py - SNMP device/outlet endpoints - config_router.py - Rack/device configuration - metrics_router.py - Power metrics history - power_router.py - Power history endpoints 3. **app/main.py:25** - Added create_user_from_env() call in lifespan: - Creates admin user from APC_ADMIN_USERNAME/PASSWORD env vars - Runs after database initialization on startup 4. **Test updates:** - Created tests/conftest.py with shared auth fixtures - Updated all test files to use session authentication - All 272 tests pass **How to verify:** - Start the API: uv run uvicorn app.main:app - Try accessing /api/device without auth: should return 401 - Login via POST /auth/login with valid credentials - Use returned token as Authorization: Bearer <token> header - Requests with valid token should succeed Commit: 16f2441
triage-agent · 2025-12-23
## Updated Feature Triage **Status:** PARTIALLY IMPLEMENTED (confirmed - no changes since last triage) **Summary:** The session-based auth infrastructure is fully built (User/Session models, AuthService with all methods, auth API endpoints with login/logout/me). The protected API routes still use the old `verify_api_key` dependency. ### What's complete: - User and Session database models - AuthService with password hashing, session creation/validation/deletion - `create_user_from_env()` method ready to use - Login/logout/me endpoints working - Config settings for admin_user, admin_password, session_expiry_days ### What remains: 1. Create `verify_session()` dependency in `app/api/auth.py` (adapt from `get_current_user()` in auth_router.py) 2. Update 4 routers to use `verify_session` instead of `verify_api_key` 3. Add `create_user_from_env()` call in `main.py` lifespan 4. Remove old API key code **Complexity:** LOW (~50-100 lines across 7 files) See full triage report at `/tmp/triage-APC-28.md`
triage-agent · 2025-12-23
## Feature Triage Complete **Status:** PARTIALLY IMPLEMENTED **Summary:** The session-based auth infrastructure is fully built (User/Session models, AuthService, auth API endpoints). However, the protected API routes still use the old `verify_api_key` dependency instead of session-based auth. The startup hook to auto-create admin user from env vars is also missing. **What's missing:** 1. Move/create `verify_session` dependency in `app/api/auth.py` 2. Update 4 routers (router.py, config_router.py, metrics_router.py, power_router.py) to use session auth 3. Add `create_user_from_env()` call in `main.py` lifespan 4. Remove old API key code **Complexity:** Low - straightforward dependency swap (~50-100 lines across 7 files) See attached triage report for full details.