security: cleanup obsolete token validation code (Action 5.1.1.6)
- Remove obsolete error logging in api/auth.ts that expected tokens in localStorage - Fix tokenRefresh.ts periodic refresh to not check tokens (httpOnly cookies not accessible) - Mark Actions 5.1.1.6-5.1.1.9 as complete in TODO list Actions 5.1.1.7-5.1.1.9 were already completed in previous actions: - 5.1.1.7: TokenStorage already returns null (httpOnly cookies not readable) - 5.1.1.8: tokenRefresh already works with cookies - 5.1.1.9: All token access goes through TokenStorage No localStorage.getItem/setItem calls for tokens remain (only removeItem for cleanup)
This commit is contained in:
senke
parent
6d08664b6b
commit
a4f7d4e577
3 changed files with 46 additions and 61 deletions
|
|
@ -1669,32 +1669,48 @@ Critical path dependencies:
|
|||
- Works with existing token refresh infrastructure
|
||||
- **Rollback**: Remove periodic refresh interval
|
||||
|
||||
- [ ] **Action 5.1.1.6**: Clean up localStorage token references
|
||||
- **Scope**: Search codebase for `localStorage.getItem('access_token')`, `localStorage.setItem('access_token')` - Remove all
|
||||
- **Dependencies**: Action 5.1.1.2 complete
|
||||
- [x] **Action 5.1.1.6**: Clean up localStorage token references
|
||||
- **Scope**: Search codebase for `localStorage.getItem('access_token')`, `localStorage.setItem('access_token')` - Remove all ✅
|
||||
- **Dependencies**: Action 5.1.1.2 complete ✅
|
||||
- **Risk**: LOW
|
||||
- **Validation**: No localStorage token access remains
|
||||
- **Validation**: ✅ No localStorage token access remains:
|
||||
- **Grep search**: No `localStorage.getItem('access_token')` or `localStorage.setItem('access_token')` calls found
|
||||
- **Only cleanup**: Only `localStorage.removeItem()` calls remain in `tokenStorage.ts` for legacy token cleanup (intentional)
|
||||
- **Removed obsolete code**: Removed error logging in `api/auth.ts` that expected tokens to be stored (lines 114-119, 234-239)
|
||||
- **Fixed tokenRefresh**: Removed obsolete token check in `tokenRefresh.ts` periodic refresh (line 235)
|
||||
- **Result**: All direct localStorage token access removed, only cleanup operations remain
|
||||
- **Rollback**: Restore localStorage access
|
||||
|
||||
- [ ] **Action 5.1.1.7**: Update TokenStorage to read from cookie only
|
||||
- **Scope**: `apps/web/src/services/tokenStorage.ts` - Remove localStorage, add cookie reading
|
||||
- **Dependencies**: Action 5.1.1.2 complete
|
||||
- [x] **Action 5.1.1.7**: Update TokenStorage to read from cookie only
|
||||
- **Scope**: `apps/web/src/services/tokenStorage.ts` - Remove localStorage, add cookie reading ✅
|
||||
- **Dependencies**: Action 5.1.1.2 complete ✅
|
||||
- **Risk**: MEDIUM
|
||||
- **Validation**: TokenStorage reads from cookie, no localStorage
|
||||
- **Validation**: ✅ TokenStorage reads from cookie, no localStorage:
|
||||
- **Already completed in Action 5.1.1.2**: TokenStorage.getAccessToken() returns null (httpOnly cookies not accessible)
|
||||
- **Already completed in Action 5.1.1.2**: TokenStorage.getRefreshToken() returns null (httpOnly cookies not accessible)
|
||||
- **Already completed in Action 5.1.1.2**: TokenStorage.setTokens() is a no-op (tokens set by backend in httpOnly cookies)
|
||||
- **Result**: TokenStorage works with httpOnly cookies (returns null since cookies not accessible from JS)
|
||||
- **Rollback**: Restore localStorage logic
|
||||
|
||||
- [ ] **Action 5.1.1.8**: Update tokenRefresh to work with cookies
|
||||
- **Scope**: `apps/web/src/services/tokenRefresh.ts` - Update to read/write cookies instead of localStorage
|
||||
- **Dependencies**: Action 5.1.1.7 complete
|
||||
- [x] **Action 5.1.1.8**: Update tokenRefresh to work with cookies
|
||||
- **Scope**: `apps/web/src/services/tokenRefresh.ts` - Update to read/write cookies instead of localStorage ✅
|
||||
- **Dependencies**: Action 5.1.1.7 complete ✅
|
||||
- **Risk**: MEDIUM
|
||||
- **Validation**: Token refresh works with cookies
|
||||
- **Validation**: ✅ Token refresh works with cookies:
|
||||
- **Already completed in Action 5.1.1.3**: refreshToken() sends empty body, cookies sent automatically via withCredentials
|
||||
- **Already completed in Action 5.1.1.3**: Removed token reading and expiration checks (can't check httpOnly cookies from JS)
|
||||
- **Fixed**: Removed obsolete token check in periodic refresh (line 235)
|
||||
- **Result**: Token refresh works entirely with httpOnly cookies
|
||||
- **Rollback**: Restore localStorage logic
|
||||
|
||||
- [ ] **Action 5.1.1.9**: Update all token access to use TokenStorage
|
||||
- **Scope**: Search for direct localStorage token access - Replace with TokenStorage methods
|
||||
- **Dependencies**: Action 5.1.1.7 complete
|
||||
- [x] **Action 5.1.1.9**: Update all token access to use TokenStorage
|
||||
- **Scope**: Search for direct localStorage token access - Replace with TokenStorage methods ✅
|
||||
- **Dependencies**: Action 5.1.1.7 complete ✅
|
||||
- **Risk**: MEDIUM
|
||||
- **Validation**: No direct localStorage token access
|
||||
- **Validation**: ✅ No direct localStorage token access:
|
||||
- **Grep search**: No direct `localStorage.getItem('access_token')` or `localStorage.setItem('access_token')` calls found
|
||||
- **All access via TokenStorage**: All token access goes through TokenStorage methods (which return null for httpOnly cookies)
|
||||
- **Result**: All token access uses TokenStorage API (no direct localStorage access)
|
||||
- **Rollback**: Restore direct access
|
||||
|
||||
### Sub-Epic 5.2: Pre-Validation 🟢
|
||||
|
|
|
|||
|
|
@ -106,18 +106,10 @@ export async function register(
|
|||
// NOTE: Si refresh_token est vide, le backend utilise probablement des cookies httpOnly
|
||||
// Dans ce cas, on stocke seulement l'access_token et on laisse le refresh se faire via cookie
|
||||
if (accessToken) {
|
||||
// Si refresh_token est présent, on le stocke, sinon on utilise une chaîne vide
|
||||
// Le refresh se fera via cookie httpOnly si disponible
|
||||
// SECURITY: Action 5.1.1.2 - Tokens are set in httpOnly cookies by backend
|
||||
// TokenStorage.setTokens is now a no-op (tokens in httpOnly cookies, not accessible to JS)
|
||||
TokenStorage.setTokens(accessToken, refreshToken || '');
|
||||
|
||||
// Vérifier que les tokens sont bien stockés (pour debug)
|
||||
const storedToken = TokenStorage.getAccessToken();
|
||||
if (!storedToken) {
|
||||
logger.error(
|
||||
'[AUTH] Failed to store token in localStorage after setTokens',
|
||||
);
|
||||
}
|
||||
|
||||
// INT-016: Initialiser le refresh proactif après register
|
||||
initializeProactiveRefresh();
|
||||
}
|
||||
|
|
@ -226,18 +218,10 @@ export async function login(data: LoginRequest): Promise<LoginResponse> {
|
|||
// NOTE: Si refresh_token est vide, le backend utilise probablement des cookies httpOnly
|
||||
// Dans ce cas, on stocke seulement l'access_token et on laisse le refresh se faire via cookie
|
||||
if (accessToken) {
|
||||
// Si refresh_token est présent, on le stocke, sinon on utilise une chaîne vide
|
||||
// Le refresh se fera via cookie httpOnly si disponible
|
||||
// SECURITY: Action 5.1.1.2 - Tokens are set in httpOnly cookies by backend
|
||||
// TokenStorage.setTokens is now a no-op (tokens in httpOnly cookies, not accessible to JS)
|
||||
TokenStorage.setTokens(accessToken, refreshToken || '');
|
||||
|
||||
// Vérifier que les tokens sont bien stockés (pour debug)
|
||||
const storedToken = TokenStorage.getAccessToken();
|
||||
if (!storedToken) {
|
||||
logger.error(
|
||||
'[AUTH] Failed to store token in localStorage after setTokens',
|
||||
);
|
||||
}
|
||||
|
||||
// Stocker le flag remember_me pour référence future
|
||||
if (data.remember_me) {
|
||||
localStorage.setItem('remember_me', 'true');
|
||||
|
|
|
|||
|
|
@ -231,33 +231,18 @@ function startPeriodicRefresh(): void {
|
|||
}
|
||||
|
||||
// Set up interval to refresh every 4 minutes
|
||||
// SECURITY: Action 5.1.1.2 - Can't check token expiration from httpOnly cookies, just refresh periodically
|
||||
proactiveRefreshInterval = setInterval(() => {
|
||||
const currentToken = TokenStorage.getAccessToken();
|
||||
if (currentToken) {
|
||||
// Check if token is still valid before refreshing
|
||||
const payload = decodeJWT(currentToken);
|
||||
if (payload?.exp) {
|
||||
const expirationTime = payload.exp * 1000;
|
||||
const now = Date.now();
|
||||
const timeUntilExpiration = expirationTime - now;
|
||||
|
||||
// Only refresh if token is still valid (not expired)
|
||||
if (timeUntilExpiration > 0) {
|
||||
refreshToken().catch((error) => {
|
||||
logger.warn('Periodic token refresh failed', {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
stack: error instanceof Error ? error.stack : undefined,
|
||||
});
|
||||
});
|
||||
} else {
|
||||
// Token expired, stop periodic refresh
|
||||
cancelProactiveRefresh();
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// No token, stop periodic refresh
|
||||
// Just attempt refresh - if it fails (401), the error handler will stop the interval
|
||||
refreshToken().catch((error) => {
|
||||
logger.warn('Periodic token refresh failed', {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
stack: error instanceof Error ? error.stack : undefined,
|
||||
});
|
||||
// If refresh fails (e.g., token expired), stop periodic refresh
|
||||
// The error handler in refreshToken will clear tokens and cancel refresh
|
||||
cancelProactiveRefresh();
|
||||
}
|
||||
});
|
||||
}, PROACTIVE_REFRESH_INTERVAL_MS);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue