Serve the EAJE attestation proof via the X-Generate-Proof header - #296
Serve the EAJE attestation proof via the X-Generate-Proof header#296Un3x wants to merge 6 commits into
Conversation
2212cb8 to
68df22d
Compare
|
Itération 3 : rendu PDF conforme à la maquette (retour de test sandbox du 27/07, détail sur API-6792). |
813c7f9 to
5e6c16a
Compare
dff89f5 to
20d62d0
Compare
|
Historique replié pour la review : les 8 commits itératifs d'hier tiennent en 2 (arbre final inchangé, diff strictement identique). |
|
Il y a probablement des choses à redire, c'est néanmonis testé et validé en fonctionnement en sandbox. L'app y est déployé actuellement, vous pouvez faire des requêtes curl récup l'attestation et scanner le QR code pour récup la vérification. A priori, besoin d'aucune modification en staging ni de fake data on génère un vrai pdf sur de la fausse données et c'est ok, facile à tracé parce que ca ne répond à nos mocks qui sont tracable. |
|
Est-ce que tu peux upload un pdf directement ici, ainsi que le curl ? |
|
La description fonctionnelle de cette PR est incompréhensible, peux-tu clarifier ? |
skelz0r
left a comment
There was a problem hiding this comment.
J'ai plusieurs gros points:
- C'est beaucoup trop couplé entre le type de données et le pdf (2 coms) tout en étant très générique ;
- La spec a complètement divergé (ajout d'un header qui permet d'avoir un lien de vérif et/ou un pdf, dans la même réponse sans ajouter de nouvel endpoint) : pourquoi ? Le design me semble plus pauvre que la spec initiale, d'autant plus qu'on complexifie pas mal de choses en procédant de cette manière. Le but n'est pas de produire un nouveau type de réponse mais de la preuve, je trouve ça très étrange de l'exposer dans un endpoint de données (à la limite via une extension aurait été plus logique mais bon..)
- Les commits messages sont cryptiques, cela devrait être autoportant
Le 2. m'a excessivement brainfuck, pourquoi ? Cela devrait être dans le commentaire initial de la PR.
|
Btw j'ai lu commit / commit, et je me suis arrêté quand j'ai constaté le point 2. |
pour le moment je n'ai testé que en sandbox avec des données réelles donc non, mais je peux générer un fake en local pour illustrer |
|
Deploy prerequisite: provision |
ac4ea28 to
10e6a41
Compare
10e6a41 to
a3293d6
Compare
a3293d6 to
cd5868a
Compare
skelz0r
left a comment
There was a problem hiding this comment.
Je request change parce que je pense que l'interface est encore trop compliquée, et qu'il faudrait à mon sens mettre plus de convention pour moins de configuration + mettre potentiellement ça dans un interactor (ce qui serait plus simple à tester au passage).
Un point anecdotique: y'a tout une partie (non critique/importante sur le raw binary du pdf) qui pourrait être revert sur les clients, mais c'est peu important imo. T'as fait tourner claude sur le specs.md pour être sûr que ça passe encore sur le staging ?
| ### Binary responses | ||
|
|
||
| Some endpoints return a binary document instead of the JSON envelope (e.g. | ||
| `application/pdf` attestations). Clients MUST parse the body as JSON only when | ||
| the response `Content-Type` is a JSON media type **or absent**; otherwise `raw` | ||
| exposes the bytes verbatim, `data` is null/undefined and `links`/`meta` are | ||
| empty. | ||
|
|
There was a problem hiding this comment.
| ### Binary responses | |
| Some endpoints return a binary document instead of the JSON envelope (e.g. | |
| `application/pdf` attestations). Clients MUST parse the body as JSON only when | |
| the response `Content-Type` is a JSON media type **or absent**; otherwise `raw` | |
| exposes the bytes verbatim, `data` is null/undefined and `links`/`meta` are | |
| empty. |
ce n'est plus vrai à priori
There was a problem hiding this comment.
Supprimée. Le SDK ne reçoit jamais de binaire : l'attestation arrive comme un lien dans links, suivi hors client et sans auth. Le code de gestion du raw part aussi des deux clients.
| expect(r.rateLimit).toBe(rl); | ||
| }); | ||
|
|
||
| it('exposes a binary raw body verbatim without envelope fields', () => { |
There was a problem hiding this comment.
Supprimé, avec le test équivalent côté client-base et le code de parsing binaire des deux SDKs.
| @@ -0,0 +1,225 @@ | |||
| class AttestationPDF | |||
There was a problem hiding this comment.
Ce nom est encore trop générique.
Des pistes:
ProofPDFBuilderProofAttestationPDFBuilderProofAttestationPDFRenderer
Y'a un dossier builders dans app/ btw ça peut finir là bas.
There was a problem hiding this comment.
Alors l'attestation c'est pas la proof, c'est l'attestation. Mais ok pour le nom et le dossier ca me semble mieux.
There was a problem hiding this comment.
Attestation reste tout de même trop générique. PayloadAttestation ..?
| @@ -0,0 +1,69 @@ | |||
| class CNAV::AttestationToken | |||
There was a problem hiding this comment.
Je trouve que l'interface d'implémentation est encore compliqué ici.
Pour moi ici ça devrait être "générique", niveau interface j'aurais plus vu un truc de ce style:
# controller
include APIParticulier::GeneratesAttestationProof
def proof_attributes
{
'Nom de famille' => data['allocataires'].first['nom'].first(3),
'Paramètres calcul participation familiale' => data['parametres_calcul_participation_familiale']
}
end
def proof_attestation_attributes
# tout ce qu'il y a dans le pdf
[
{
title: "Allocataire",
attributes: [
{ key: "Nom de naissance", value: data['allocataires].first['nom'] }
]
]
endLe reste (data provider, siret, emission date..) tu l'infères du retriever.
A voir si on déporte pas ces constructions dans un nouvel interactor avec une interface à implanter, on inclut le APIParticulier::GeneratesAttestationProof partout et on build la preuve et le pdf ssi une clé proof_attestation_attributes existe sur le retriever.
J'ai cru comprendre à la lecture du code et nos échanges ultérieurs que la complexité résidait aussi dans la taille de la preuve, y'a moyen de réduire drastiquement avec du msgpack + deflect ici ? Surtout qu'il y a peu d'attributs.
Sinon ton approche de mettre dans commons/data/ est pas mal mais encore pas assez CoC -> faut inférer du nom de la classe le fichier sinon à chaque ajout on se retrouve à rajouter dans .expand, imo du genre:
# commons/data/proof_attestations_data.yml
shared:
cnav:
participation_familiale_eaje:
# propal pour plus haut -> on envoi juste un tableau de valeurs et on utilise l'index, comme ça on se débarasse du clé->valeur.
proof_attributes:
- 'Nom de naissance'
- 'Date de naissance'tout le reste est dans la construction dans l'interactor maintenant, y'a que la proof à mettre.
c'est pas encore idéal mais on s'approche d'un truc plus simple je trouve.
There was a problem hiding this comment.
J'avoue que je me suis un peu pris la tête sur le juste milieu de config / convention / généralisation alors qu'on a qu'un seul endpoint. Moi j'ai pas d'input particulier qui me dit que ce sera utilisé ailleurs, j'ai essayé de faire un truc "ajustable" pour que ce soit le cas sans forcément le mettre en place d'hors et déjà.
There was a problem hiding this comment.
Pour msgpack et deflect j'ai pas mis parce qu'avec la suppression des enfants en fait on a des tailles de payload stable et raisonnable. J'pourrais mesurer que en sandbox mais à priori pas besoin (en tout cas pas nécessaire)
Not yet, j'veux faire des tests en sandbox avant mais j'avais besoin que les clés soit mergés et deploy sur les repos et avec la migration des creds de admin sur ansible ben ca a un poil retardé. |
|
Je corrige ce que j'ai sous ma botte déjà et je valide en sandbox + les specs.md et comme ca on itère au besoin |
A holder must be able to prove an attestation is genuine without us storing it or being reachable, so the token carries its own payload, purpose and expiry. Each retriever's field set is declared in proof_attestations.yml rather than hardcoded.
Pure rubocop autocorrect, split out so it does not pad the review of the attestation work that follows.
d9729b0 to
588c4a9
Compare
The printed document must stand alone, so it embeds the proof token as a QR code pointing at the public verification page. Labels come from the retriever's declaration so no layout code is per-endpoint.
Existing JSON consumers must keep their exact response, so the proof is opt-in via X-Generate-Proof and served from its own throttled endpoint. The endpoint fiche records that the field set is frozen per API version.
Anyone holding a printed attestation can check it against the public site with no credentials and no call to the API — the page decrypts the token it is given and renders what it contains.
A server-side signature change without SDK support ships clients that lie to their users, so the proof header and endpoint land in SPECS.md, the shared commons and both vendored trees in one step.
588c4a9 to
59c2abd
Compare
Replace the dedicated attestation endpoint with the
X-Generate-Proof: proof-only|pdfheader on the EAJE identite call: proof link + visual code inmeta, short-lived self-contained PDF link inlinks— one provider call, no extra scope.site/at/attestations/verification/:token(native DSFR, Rails-native rate limit, tracker-free layout), decrypting with a dedicatedattestation_encryptor_*key shared by both apps/api/attestations/:token.pdfrenders the PDF from its self-contained token — 410 stale, 404 forged; footer QR is now also a clickable linkgenerate_proof:in both clients (scaffolds now emitin: headerparams)linksURL the caller follows outside the SDK, unauthenticatedGeneric vs EAJE. The proof mechanism is generic by data, not by hooks. A retriever declares two ordered lists —
proof_attributes(what the 5-year verification token carries, minimised per RSSI §4.4) andproof_attestation_attributes(what the PDF shows) — andAPIParticulier::BuildAttestationProofinfers the rest: siret, issue date, habilitation, and the declaration itself, found from the retriever's class name (CNAV::ParticipationFamilialeEAJE→cnav.participation_familiale_eajeincommons/data/proof_attestations.yml— one file for every endpoint, so adding one never touches.expand). Both YAML maps are positional, their order matching the retriever's arrays: that is what removes the key→value plumbing from the declaration side. The tokens themselves keep explicit readable keys — they are read back five years later by another application, where a silent remap would be undetectable.CNAV::AttestationToken, itsDOCUMENTdescriptor and the single-letter key map are gone;AttestationPDFBuilder(inapp/builders/) receives its declaration instead of reading global config, and the download controller and verification page are now driven by thedeclaration_keyinside the token rather than wired to EAJE.Attestation content is frozen per API version. Adding, removing or renaming a field on the PDF goes through a new versioned route, never an in-place change; reordering fields visually does not. Documented on the endpoint fiche because it is a constraint integrators need: it is a documentary requirement — two documents carrying the same API version cannot show different field sets, whether for the integrator, for CNAF, or for the agent checking the document — and it is independent of how the token is encoded.
Two API-surface fixes (breaking if done after merge):
meta.pdf_link_expires_in→pdf_link_expires_at(the value is an epoch, not a duration). And the/api/attestationsthrottle goes 5→60/min/IP: the PDF link is fetched by daycare software (machine, single IP, batch), and it is already non-guessable and self-expiring (5 min); the human verification page keeps its native 5/min.Sample (staging mock persona, fake data): attestation PDF attached below — reproducible with:
Closes API-6792 → https://linear.app/pole-api/issue/API-6792/ajout-dun-pdf-signe
Avant de deploy il faut merge : https://github.com/etalab/very_ansible/pull/828 et apply