Fix issues from QA review: hash upgrade ordering, interceptor scope, guard
- W-01: Move is_active check before hash upgrade so disabled accounts don't get their password hash silently mutated on rejected login - W-02: Narrow interceptor exclusion to specific auth endpoints instead of blanket /auth/* prefix (future-proofs against new auth routes) - W-03: Add null guard on optimistic setQueryData to handle undefined cache gracefully instead of spreading undefined - S-01: Clear loginError when switching from register back to login mode - S-03: Add detail dict to auth.login_blocked_inactive audit event Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
c4c06be148
commit
1aeb725410
@ -333,19 +333,20 @@ async def login(
|
|||||||
await db.commit()
|
await db.commit()
|
||||||
raise HTTPException(status_code=401, detail="Invalid username or password")
|
raise HTTPException(status_code=401, detail="Invalid username or password")
|
||||||
|
|
||||||
if new_hash:
|
|
||||||
user.password_hash = new_hash
|
|
||||||
|
|
||||||
# Block disabled accounts — checked AFTER password verification to avoid
|
# Block disabled accounts — checked AFTER password verification to avoid
|
||||||
# leaking account-state info, and BEFORE _record_successful_login so
|
# leaking account-state info, and BEFORE _record_successful_login so
|
||||||
# last_login_at and lockout counters are not reset for inactive users.
|
# last_login_at and lockout counters are not reset for inactive users.
|
||||||
if not user.is_active:
|
if not user.is_active:
|
||||||
await log_audit_event(
|
await log_audit_event(
|
||||||
db, action="auth.login_blocked_inactive", actor_id=user.id, ip=client_ip,
|
db, action="auth.login_blocked_inactive", actor_id=user.id,
|
||||||
|
detail={"reason": "account_disabled"}, ip=client_ip,
|
||||||
)
|
)
|
||||||
await db.commit()
|
await db.commit()
|
||||||
raise HTTPException(status_code=403, detail="Account is disabled. Contact an administrator.")
|
raise HTTPException(status_code=403, detail="Account is disabled. Contact an administrator.")
|
||||||
|
|
||||||
|
if new_hash:
|
||||||
|
user.password_hash = new_hash
|
||||||
|
|
||||||
await _record_successful_login(db, user)
|
await _record_successful_login(db, user)
|
||||||
|
|
||||||
# SEC-03: MFA enforcement — block login entirely until MFA setup completes
|
# SEC-03: MFA enforcement — block login entirely until MFA setup completes
|
||||||
|
|||||||
@ -641,6 +641,7 @@ export default function LockScreen() {
|
|||||||
setUsername('');
|
setUsername('');
|
||||||
setPassword('');
|
setPassword('');
|
||||||
setConfirmPassword('');
|
setConfirmPassword('');
|
||||||
|
setLoginError(null);
|
||||||
}}
|
}}
|
||||||
className="text-xs text-muted-foreground hover:text-foreground transition-colors"
|
className="text-xs text-muted-foreground hover:text-foreground transition-colors"
|
||||||
>
|
>
|
||||||
|
|||||||
@ -36,10 +36,10 @@ export function useAuth() {
|
|||||||
setMfaSetupRequired(false);
|
setMfaSetupRequired(false);
|
||||||
// Optimistically mark authenticated to prevent form flash during refetch
|
// Optimistically mark authenticated to prevent form flash during refetch
|
||||||
if ('authenticated' in data && data.authenticated && !('must_change_password' in data && data.must_change_password)) {
|
if ('authenticated' in data && data.authenticated && !('must_change_password' in data && data.must_change_password)) {
|
||||||
queryClient.setQueryData(['auth'], (old: AuthStatus | undefined) => ({
|
queryClient.setQueryData(['auth'], (old: AuthStatus | undefined) => {
|
||||||
...old!,
|
if (!old) return old; // let invalidateQueries handle it
|
||||||
authenticated: true,
|
return { ...old, authenticated: true };
|
||||||
}));
|
});
|
||||||
}
|
}
|
||||||
queryClient.invalidateQueries({ queryKey: ['auth'] });
|
queryClient.invalidateQueries({ queryKey: ['auth'] });
|
||||||
}
|
}
|
||||||
|
|||||||
@ -15,7 +15,8 @@ api.interceptors.response.use(
|
|||||||
if (error.response?.status === 401) {
|
if (error.response?.status === 401) {
|
||||||
const url = error.config?.url || '';
|
const url = error.config?.url || '';
|
||||||
// Don't redirect on auth endpoints — they legitimately return 401
|
// Don't redirect on auth endpoints — they legitimately return 401
|
||||||
if (!url.startsWith('/auth/')) {
|
const authEndpoints = ['/auth/login', '/auth/register', '/auth/setup', '/auth/verify-password', '/auth/change-password'];
|
||||||
|
if (!authEndpoints.some(ep => url.startsWith(ep))) {
|
||||||
window.location.href = '/login';
|
window.location.href = '/login';
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user