Commit 83d2142
feat(security): implement JWT token security improvements (#4371)
* feat(security): implement JWT token security improvements
Implements comprehensive JWT token security enhancements addressing
X-Force Red security audit findings:
Security Features:
- Reduced token lifetime from ~70 days to 20 minutes (configurable 5-1440 min)
- Server-side token blocklist with Redis caching and database persistence
- Idle timeout enforcement (60 minutes default, configurable)
- Logout endpoint with immediate token invalidation
- Activity tracking with automatic updates
- Token revocation on logout, expiry, and idle timeout
- Comprehensive audit logging for security events
Implementation:
- Added TokenBlocklistService for token revocation management
- Enhanced TokenRevocation model with token_expiry and last_activity fields
- Added idle timeout checking in get_current_user()
- Implemented /auth/logout endpoint with proper dependency injection
- Added security configuration with validation (TOKEN_LIFETIME_MINUTES, TOKEN_IDLE_TIMEOUT_MINUTES)
- Created Alembic migrations for database schema changes
Testing:
- 47 tests passing (39 unit + 8 edge cases + 11 integration)
- 88% coverage on token_blocklist_service.py
- 84% coverage on routers/auth.py
- 86% overall coverage for JWT security modules
- Comprehensive integration tests for all security flows
- Edge case tests for error handling paths
Documentation:
- Added JWT_TOKEN_SECURITY_IMPLEMENTATION.md with complete implementation guide
- Added test_jwt_token_security.md with test documentation
- Updated .secrets.baseline for security scanning
Closes #4317
Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
* fix: linting and coverage issues, enhanced /admin/logout to revoke tokens
Signed-off-by: Jonathan Springer <jps@s390x.com>
* fix: resolve linting issues and pre-commit checks
- Remove duplicate datetime/timezone imports in admin.py
- Replace __enter__() calls with proper context managers in token_blocklist_service.py
- Add pylint disable comments for func.count false positives
- Fix dict comprehension to use dict() constructor
- Add newline at end of test_jwt_token_security.md
Signed-off-by: Jonathan Springer <jps@s390x.com>
* test: add unit test for logout SecretStr handling and fix pylint issues
- Add test for routers/auth.py line 239 (SecretStr.get_secret_value)
- Fix 3 pylint R1705 (no-else-return) violations in token_blocklist_service.py
- Update secrets baseline after merge
- Add SimpleNamespace import to test_auth.py
Improves diff-cover from 90% to 97.4% for routers/auth.py
Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
* fix(security): make idle timeout actually enforce
The idle-timeout block in get_current_user only read last_activity
from the JWT, but no issuance code wrote it — so the check ran zero
times on real tokens. Read activity from Redis first (update_activity
was already writing there but had no consumer), fall back to the JWT
last_activity claim, fall back to iat. Emit last_activity=iat in
create_access_token for first-request bootstrap.
Folds in remaining PR-review blockers:
- bb43712cae28 alembic merge resolves dual-head from rebase past
the head referenced by cae28b15a507
- TOKEN_EXPIRY 10080->20 min default documented in CHANGELOG
Behavior Changes with migration & rollback guidance
- drop dead refresh_token_expiry config field + 3 doc references
- /auth/logout current_user: dict -> EmailUser (matches actual
return type of get_current_user)
- /admin/logout test rewritten with TestClient + CSRF deny-path
regression (was asyncio.run on MagicMock — bypassed middleware
and would pass even if the route was unregistered)
Coverage on diff: 91% -> ~100%. New unit tests cover every branch
of the idle-timeout block plus the Redis-success and fresh-session
paths in TokenBlocklistService that were previously unreached.
Refs #4317, #4371
Signed-off-by: Jonathan Springer <jps@s390x.com>
---------
Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
Co-authored-by: Jonathan Springer <jps@s390x.com>1 parent c87de35 commit 83d2142
22 files changed
Lines changed: 3707 additions & 32 deletions
File tree
- docs/security
- mcpgateway
- alembic/versions
- routers
- services
- tests
- integration
- security
- unit/mcpgateway
- routers
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| |||
250 | 250 | | |
251 | 251 | | |
252 | 252 | | |
253 | | - | |
| 253 | + | |
254 | 254 | | |
255 | 255 | | |
256 | 256 | | |
257 | 257 | | |
258 | 258 | | |
259 | 259 | | |
260 | 260 | | |
261 | | - | |
| 261 | + | |
262 | 262 | | |
263 | 263 | | |
264 | 264 | | |
265 | 265 | | |
266 | 266 | | |
267 | 267 | | |
268 | 268 | | |
269 | | - | |
| 269 | + | |
270 | 270 | | |
271 | 271 | | |
272 | 272 | | |
273 | 273 | | |
274 | 274 | | |
275 | 275 | | |
276 | 276 | | |
277 | | - | |
| 277 | + | |
278 | 278 | | |
279 | 279 | | |
280 | 280 | | |
281 | 281 | | |
282 | 282 | | |
283 | 283 | | |
284 | 284 | | |
285 | | - | |
| 285 | + | |
286 | 286 | | |
287 | 287 | | |
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
291 | 291 | | |
292 | 292 | | |
293 | | - | |
| 293 | + | |
294 | 294 | | |
295 | 295 | | |
296 | 296 | | |
297 | 297 | | |
298 | 298 | | |
299 | 299 | | |
300 | 300 | | |
301 | | - | |
| 301 | + | |
302 | 302 | | |
303 | 303 | | |
304 | 304 | | |
305 | 305 | | |
306 | 306 | | |
307 | 307 | | |
308 | 308 | | |
309 | | - | |
| 309 | + | |
310 | 310 | | |
311 | 311 | | |
312 | 312 | | |
| |||
4196 | 4196 | | |
4197 | 4197 | | |
4198 | 4198 | | |
4199 | | - | |
| 4199 | + | |
4200 | 4200 | | |
4201 | 4201 | | |
4202 | 4202 | | |
4203 | 4203 | | |
4204 | 4204 | | |
4205 | 4205 | | |
4206 | 4206 | | |
4207 | | - | |
| 4207 | + | |
4208 | 4208 | | |
4209 | 4209 | | |
4210 | 4210 | | |
4211 | 4211 | | |
4212 | 4212 | | |
4213 | 4213 | | |
4214 | 4214 | | |
4215 | | - | |
| 4215 | + | |
4216 | 4216 | | |
4217 | 4217 | | |
4218 | 4218 | | |
| |||
4862 | 4862 | | |
4863 | 4863 | | |
4864 | 4864 | | |
4865 | | - | |
| 4865 | + | |
4866 | 4866 | | |
4867 | 4867 | | |
4868 | 4868 | | |
| |||
4964 | 4964 | | |
4965 | 4965 | | |
4966 | 4966 | | |
4967 | | - | |
| 4967 | + | |
4968 | 4968 | | |
4969 | 4969 | | |
4970 | 4970 | | |
| |||
4974 | 4974 | | |
4975 | 4975 | | |
4976 | 4976 | | |
4977 | | - | |
| 4977 | + | |
4978 | 4978 | | |
4979 | 4979 | | |
4980 | 4980 | | |
4981 | 4981 | | |
4982 | 4982 | | |
4983 | 4983 | | |
4984 | 4984 | | |
4985 | | - | |
| 4985 | + | |
4986 | 4986 | | |
4987 | 4987 | | |
4988 | 4988 | | |
4989 | 4989 | | |
4990 | 4990 | | |
4991 | 4991 | | |
4992 | 4992 | | |
4993 | | - | |
| 4993 | + | |
4994 | 4994 | | |
4995 | 4995 | | |
4996 | 4996 | | |
| |||
7162 | 7162 | | |
7163 | 7163 | | |
7164 | 7164 | | |
7165 | | - | |
| 7165 | + | |
7166 | 7166 | | |
7167 | 7167 | | |
7168 | 7168 | | |
7169 | 7169 | | |
7170 | 7170 | | |
7171 | 7171 | | |
7172 | 7172 | | |
7173 | | - | |
| 7173 | + | |
7174 | 7174 | | |
7175 | 7175 | | |
7176 | 7176 | | |
| |||
9134 | 9134 | | |
9135 | 9135 | | |
9136 | 9136 | | |
9137 | | - | |
| 9137 | + | |
9138 | 9138 | | |
9139 | 9139 | | |
9140 | 9140 | | |
9141 | 9141 | | |
9142 | 9142 | | |
9143 | 9143 | | |
9144 | 9144 | | |
9145 | | - | |
| 9145 | + | |
9146 | 9146 | | |
9147 | 9147 | | |
9148 | 9148 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
15 | 29 | | |
16 | 30 | | |
17 | 31 | | |
| |||
0 commit comments