feat(auth): verifie le lien de reset des le chargement, sans le consommer
Ajoute GET /auth/reset-password/validate (lecture seule, sans rate limit : le jeton est un secret de 256 bits non brute-forcable) pour que la page reset-password redirige immediatement vers /login si le lien est invalide ou expire, plutot que d'attendre la soumission du formulaire. La verification a la soumission (confirm_password_reset) reste la seule source de verite atomique.
This commit is contained in:
@@ -27,6 +27,7 @@ from app.schemas.auth import (
|
||||
PasswordChangeRequest,
|
||||
PrincipalResponse,
|
||||
ResetPasswordRequest,
|
||||
ResetTokenValidationResponse,
|
||||
TokenResponse,
|
||||
)
|
||||
from app.schemas.errors import ErrorResponse
|
||||
@@ -320,6 +321,15 @@ async def forgot_password(
|
||||
) from erreur
|
||||
|
||||
|
||||
@router.get(
|
||||
"/reset-password/validate",
|
||||
response_model=ResetTokenValidationResponse,
|
||||
summary="Vérifie sans le consommer si un lien de réinitialisation est encore valide",
|
||||
)
|
||||
async def validate_reset_token(token: str, service: AuthServiceDep) -> ResetTokenValidationResponse:
|
||||
return ResetTokenValidationResponse(valid=await service.is_reset_token_valid(token=token))
|
||||
|
||||
|
||||
@router.post(
|
||||
"/reset-password",
|
||||
response_model=TokenResponse,
|
||||
|
||||
@@ -6,7 +6,7 @@ from dataclasses import dataclass
|
||||
from datetime import datetime
|
||||
from uuid import UUID
|
||||
|
||||
from sqlalchemy import func, update
|
||||
from sqlalchemy import func, select, update
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.models.password_reset_token import PasswordResetToken
|
||||
@@ -58,6 +58,16 @@ class PasswordResetTokenRepository:
|
||||
return None
|
||||
return ConsumedResetToken(id=ligne.id, user_id=ligne.user_id)
|
||||
|
||||
# Piège : simple SELECT, volontairement pas atomique avec la consommation. Sert seulement
|
||||
# au feedback UX (jeton encore valide ?) ; `consume()` reste la seule source de vérité.
|
||||
async def exists_valid(self, token_hash: bytes) -> bool:
|
||||
requete = select(PasswordResetToken.id).where(
|
||||
PasswordResetToken.token_hash == token_hash,
|
||||
PasswordResetToken.consumed_at.is_(None),
|
||||
PasswordResetToken.expires_at > func.clock_timestamp(),
|
||||
)
|
||||
return (await self._session.execute(requete)).first() is not None
|
||||
|
||||
async def invalidate_all_for_user(self, user_id: UUID) -> int:
|
||||
resultat = await self._session.execute(
|
||||
update(PasswordResetToken)
|
||||
|
||||
@@ -84,6 +84,10 @@ class PrincipalResponse(BaseModel):
|
||||
return cls.model_validate(principal)
|
||||
|
||||
|
||||
class ResetTokenValidationResponse(BaseModel):
|
||||
valid: bool
|
||||
|
||||
|
||||
class TokenResponse(BaseModel):
|
||||
access_token: str
|
||||
token_type: Literal["bearer"] = "bearer" # noqa: S105
|
||||
|
||||
@@ -280,6 +280,13 @@ class AuthService:
|
||||
except Exception:
|
||||
logger.exception("auth.password_reset.mail_failed")
|
||||
|
||||
# Piège : lecture seule, pas d'appel à `consume()`. Aucune limitation de débit n'est
|
||||
# nécessaire ici : le jeton est un secret de 256 bits (`generate_refresh_secret`), donc
|
||||
# non brute-forçable, et cette route n'apprend rien sur l'existence d'un compte ou d'un
|
||||
# email, seulement si le lien déjà en main du visiteur est encore valide.
|
||||
async def is_reset_token_valid(self, token: str) -> bool:
|
||||
return await self._reset_tokens.exists_valid(fingerprint_refresh(token))
|
||||
|
||||
async def confirm_password_reset(
|
||||
self, *, token: str, new_password: str, client_ip: str | None, user_agent: str | None
|
||||
) -> AuthenticatedSession:
|
||||
|
||||
@@ -28,12 +28,16 @@ PRINCIPAL = Principal(
|
||||
|
||||
|
||||
class FauxService:
|
||||
def __init__(self, erreur: Exception | None = None) -> None:
|
||||
def __init__(self, erreur: Exception | None = None, *, jeton_valide: bool = True) -> None:
|
||||
self._erreur = erreur
|
||||
self._jeton_valide = jeton_valide
|
||||
|
||||
async def refresh(self, **_: object) -> AuthenticatedSession:
|
||||
return await self.authenticate()
|
||||
|
||||
async def is_reset_token_valid(self, **_: object) -> bool:
|
||||
return self._jeton_valide
|
||||
|
||||
async def logout(self, **_: object) -> None:
|
||||
return None
|
||||
|
||||
@@ -259,6 +263,38 @@ async def test_forgot_password_rejects_a_malformed_email(
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fake_auth_service_reset_validity(app: FastAPI) -> Iterator[list[bool]]:
|
||||
programme = [True]
|
||||
app.dependency_overrides[get_auth_service] = lambda: FauxService(jeton_valide=programme[0])
|
||||
yield programme
|
||||
app.dependency_overrides.pop(get_auth_service, None)
|
||||
|
||||
|
||||
async def test_validate_reset_token_reports_a_living_token(
|
||||
fake_auth_service_reset_validity: list[bool], client: AsyncClient
|
||||
) -> None:
|
||||
response = await client.get(
|
||||
"/api/v1/auth/reset-password/validate", params={"token": "un-secret-opaque"}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"valid": True}
|
||||
|
||||
|
||||
async def test_validate_reset_token_reports_an_invalid_or_expired_token(
|
||||
fake_auth_service_reset_validity: list[bool], client: AsyncClient
|
||||
) -> None:
|
||||
fake_auth_service_reset_validity[0] = False
|
||||
|
||||
response = await client.get(
|
||||
"/api/v1/auth/reset-password/validate", params={"token": "un-secret-perime"}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"valid": False}
|
||||
|
||||
|
||||
async def test_reset_password_returns_the_token_and_the_cookie_on_success(
|
||||
fake_auth_service: list[Exception | None], client: AsyncClient
|
||||
) -> None:
|
||||
|
||||
@@ -89,6 +89,34 @@ async def test_invalidate_all_for_user_only_touches_living_tokens(
|
||||
assert second_passage == 0
|
||||
|
||||
|
||||
async def test_exists_valid_is_true_for_a_living_token(session: AsyncSession) -> None:
|
||||
depot = PasswordResetTokenRepository(session)
|
||||
secret = await un_jeton(depot, await un_compte(session))
|
||||
|
||||
assert await depot.exists_valid(fingerprint_refresh(secret)) is True
|
||||
|
||||
|
||||
async def test_exists_valid_is_false_for_an_expired_token(session: AsyncSession) -> None:
|
||||
depot = PasswordResetTokenRepository(session)
|
||||
secret = await un_jeton(depot, await un_compte(session), duree=-timedelta(minutes=1))
|
||||
|
||||
assert await depot.exists_valid(fingerprint_refresh(secret)) is False
|
||||
|
||||
|
||||
async def test_exists_valid_is_false_once_the_token_is_consumed(session: AsyncSession) -> None:
|
||||
depot = PasswordResetTokenRepository(session)
|
||||
secret = await un_jeton(depot, await un_compte(session))
|
||||
await depot.consume(fingerprint_refresh(secret))
|
||||
|
||||
assert await depot.exists_valid(fingerprint_refresh(secret)) is False
|
||||
|
||||
|
||||
async def test_exists_valid_is_false_for_an_unknown_fingerprint(session: AsyncSession) -> None:
|
||||
depot = PasswordResetTokenRepository(session)
|
||||
|
||||
assert await depot.exists_valid(fingerprint_refresh(generate_refresh_secret())) is False
|
||||
|
||||
|
||||
async def test_the_database_refuses_two_tokens_sharing_a_fingerprint(
|
||||
session: AsyncSession,
|
||||
) -> None:
|
||||
|
||||
@@ -181,8 +181,11 @@ class FausseTransaction:
|
||||
|
||||
|
||||
class FauxDepotJetonsReset:
|
||||
def __init__(self, revendique: ConsumedResetToken | None = None) -> None:
|
||||
def __init__(
|
||||
self, revendique: ConsumedResetToken | None = None, *, valide: bool = False
|
||||
) -> None:
|
||||
self.revendique = revendique
|
||||
self.valide = valide
|
||||
self.crees: list[UUID] = []
|
||||
self.invalidations: list[UUID] = []
|
||||
|
||||
@@ -192,6 +195,9 @@ class FauxDepotJetonsReset:
|
||||
async def consume(self, token_hash: bytes) -> ConsumedResetToken | None:
|
||||
return self.revendique
|
||||
|
||||
async def exists_valid(self, token_hash: bytes) -> bool:
|
||||
return self.valide
|
||||
|
||||
async def invalidate_all_for_user(self, user_id: UUID) -> int:
|
||||
self.invalidations.append(user_id)
|
||||
return len(self.invalidations)
|
||||
@@ -670,6 +676,14 @@ async def test_confirm_password_reset_revokes_every_session_then_reopens_the_cur
|
||||
assert "auth.password_reset_self_service" in attirail.audit.lignes[0][0]
|
||||
|
||||
|
||||
async def test_is_reset_token_valid_reflects_the_repository() -> None:
|
||||
attirail_valide = fabrique_service(jetons_reset=FauxDepotJetonsReset(valide=True))
|
||||
attirail_invalide = fabrique_service(jetons_reset=FauxDepotJetonsReset(valide=False))
|
||||
|
||||
assert await attirail_valide.service.is_reset_token_valid("un-secret-opaque") is True
|
||||
assert await attirail_invalide.service.is_reset_token_valid("un-secret-opaque") is False
|
||||
|
||||
|
||||
async def test_confirm_password_reset_rejects_a_token_for_an_account_disabled_since() -> None:
|
||||
compte = FauxCompte(is_active=False)
|
||||
jetons_reset = FauxDepotJetonsReset(
|
||||
|
||||
@@ -83,4 +83,17 @@ describe('AuthService', () => {
|
||||
|
||||
expect(result).toEqual(tokenResponse.principal);
|
||||
});
|
||||
|
||||
it('vérifie la validité du jeton de reset via GET /auth/reset-password/validate', () => {
|
||||
let result: { valid: boolean } | undefined;
|
||||
service.validateResetToken('un-secret-opaque').subscribe((r) => (result = r));
|
||||
|
||||
const req = httpMock.expectOne(
|
||||
`${environment.apiUrl}/auth/reset-password/validate?token=un-secret-opaque`
|
||||
);
|
||||
expect(req.request.method).toBe('GET');
|
||||
req.flush({ valid: true });
|
||||
|
||||
expect(result).toEqual({ valid: true });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -83,4 +83,10 @@ export class AuthService {
|
||||
.post<TokenResponse>(`${environment.apiUrl}/auth/reset-password`, payload, { withCredentials: true })
|
||||
.pipe(tap((response) => this.setSession(response)));
|
||||
}
|
||||
|
||||
validateResetToken(token: string): Observable<{ valid: boolean }> {
|
||||
return this.http.get<{ valid: boolean }>(`${environment.apiUrl}/auth/reset-password/validate`, {
|
||||
params: { token },
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
<form class="auth-card" [formGroup]="form" (ngSubmit)="onSubmit()">
|
||||
<h1>Nouveau mot de passe</h1>
|
||||
|
||||
@if (hasToken) {
|
||||
@if (hasToken && !isCheckingToken()) {
|
||||
<p class="auth-subtitle">Choisissez votre nouveau mot de passe</p>
|
||||
|
||||
<label for="new_password">Nouveau mot de passe</label>
|
||||
@@ -23,6 +23,10 @@
|
||||
</button>
|
||||
}
|
||||
|
||||
@if (hasToken && isCheckingToken()) {
|
||||
<p class="auth-subtitle">Vérification du lien...</p>
|
||||
}
|
||||
|
||||
<p class="auth-link"><a routerLink="/forgot-password">Redemander un lien</a></p>
|
||||
</form>
|
||||
</div>
|
||||
|
||||
@@ -12,7 +12,13 @@ function configure(token: string | null) {
|
||||
return TestBed.configureTestingModule({
|
||||
imports: [ResetPassword, ReactiveFormsModule],
|
||||
providers: [
|
||||
{ provide: AuthService, useValue: { resetPassword: vi.fn() } },
|
||||
{
|
||||
provide: AuthService,
|
||||
useValue: {
|
||||
resetPassword: vi.fn(),
|
||||
validateResetToken: vi.fn().mockReturnValue(of({ valid: true })),
|
||||
},
|
||||
},
|
||||
{ provide: Router, useValue: { navigate: vi.fn() } },
|
||||
{
|
||||
provide: ActivatedRoute,
|
||||
@@ -36,6 +42,32 @@ describe('ResetPassword', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('vérifie le jeton sans le consommer dès le chargement de la page', async () => {
|
||||
await configure('un-secret-opaque');
|
||||
const fixture = TestBed.createComponent(ResetPassword);
|
||||
const auth = TestBed.inject(AuthService) as unknown as { validateResetToken: ReturnType<typeof vi.fn> };
|
||||
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(auth.validateResetToken).toHaveBeenCalledWith('un-secret-opaque');
|
||||
expect(fixture.componentInstance.isCheckingToken()).toBe(false);
|
||||
});
|
||||
|
||||
it('redirige immédiatement vers /login si la vérification signale un jeton invalide', async () => {
|
||||
await configure('un-secret-perime');
|
||||
TestBed.overrideProvider(AuthService, {
|
||||
useValue: { resetPassword: vi.fn(), validateResetToken: vi.fn().mockReturnValue(of({ valid: false })) },
|
||||
});
|
||||
const fixture = TestBed.createComponent(ResetPassword);
|
||||
const router = TestBed.inject(Router) as unknown as { navigate: ReturnType<typeof vi.fn> };
|
||||
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(router.navigate).toHaveBeenCalledWith(['/login'], {
|
||||
queryParams: { motif: MOTIF_LIEN_RESET_INVALIDE },
|
||||
});
|
||||
});
|
||||
|
||||
it('ne soumet pas si le mot de passe ne respecte pas la politique de complexité', async () => {
|
||||
await configure('un-secret-opaque');
|
||||
const fixture = TestBed.createComponent(ResetPassword);
|
||||
|
||||
@@ -33,11 +33,23 @@ export class ResetPassword implements OnInit {
|
||||
});
|
||||
|
||||
password = toSignal(this.form.controls.new_password.valueChanges, { initialValue: '' });
|
||||
isCheckingToken = signal(this.hasToken);
|
||||
|
||||
ngOnInit(): void {
|
||||
if (!this.hasToken) {
|
||||
this.redirigeVersLoginLienInvalide();
|
||||
return;
|
||||
}
|
||||
|
||||
this.auth.validateResetToken(this.token).subscribe({
|
||||
next: ({ valid }) => {
|
||||
this.isCheckingToken.set(false);
|
||||
if (!valid) {
|
||||
this.redirigeVersLoginLienInvalide();
|
||||
}
|
||||
},
|
||||
error: () => this.isCheckingToken.set(false),
|
||||
});
|
||||
}
|
||||
|
||||
onSubmit(): void {
|
||||
|
||||
Reference in New Issue
Block a user