italia / spid-laravel

SPID authentication package for Laravel
BSD 3-Clause "New" or "Revised" License
46 stars 19 forks source link

SPID Validator - Malformed request: idp value not present or wrong (this is likely due to a non-https request) #86

Closed vidrez closed 2 years ago

vidrez commented 2 years ago

Steps per riprodurre il problema

  1. Accedere al Validator (https://demo.spid.gov.it/validator) tramite SPID
  2. Response -> Check Response
  3. Invia Response al Service Provider (Test 01. Risposta Corretta)

Risultato Atteso

Ok

Risultato Ottenuto

Errore 500 - Malformed request: idp value not present or wrong (this is likely due to a non-https request)

Dettagli

Buongiorno, sto avendo problemi a fare i Response test utilizzando SPID Validator, da quanto ho letto nella issue #55 questo errore potrebbe essere riconducibile alla policy errata dei cookie di sessione ma controllando nei config la policy dei cookie è impostata su Lax, il sito su cui sto facendo i test ha l'https e controllando la risposta che mi arriva ricevo correttamente il cookie spid_idp.

Come si legge dalle specifiche sto facendo delle prove con php-saml 4.0.0, ho adattato la patch senza troppi problemi e tutto sembra funzionare correttamente, di fatto se faccio il login con SPID utilizzando il Test IDP non ho alcun problema, validando il metadata tutto ok anche lì, l'errore mi esce fuori unicamente facendo i response test sia in locale usando php-saml-check su docker che con l'ambiente demo online.

Ho fatto una prova anche con l'esempio fornito nel pacchetto su un'app di test creata da zero con php 7.4 e php-saml 3.4.1 ma ottengo gli stessi risultati.

Specifiche

pdavide commented 2 years ago

@vidrez non riesco a riprodurre l'errore usando spid-saml-check:master e spid-laravel:1.2.7-beta in un'installazione di prova. Hai ulteriori dettagli che possono aiutarci a capire l'origine dell'errore?

vidrez commented 2 years ago

Ho fatto qualche test in più e dai log ho notato un errore che mi era sfuggito.

Praticamente SPID Validator nella response non mi imposta il valore "Audience", lo dice anche con il messaggio di errore "I dati della Response sono parziali. Assicurarsi che il metadata sia stato caricato correttamente.", purtroppo non ho dato molto peso alla cosa pensando che fosse un valore "extra".

Spid Laravel invece si aspetta questo valore di Audience quindi non trovandolo giustamente va in errore con il messaggio "production.ERROR: SAML response validation error: empty or missing AudienceRestriction element ".

Quello che io non ho mai fatto però è stato chiudere il Validator dopo il primo messaggio di errore e riaprirlo impostando il valore di Audience prima di inviare la response, siccome dopo il primo tentativo di invio della response da parte di SPID Validator, se il risultato non è quello atteso, negli invii successivi che si imposti o meno il valore di Audience non farà nessuna differenza, in quanto SPID Validator imposta il cookie spid_idp con valore null invece di validator, risultando quindi nell'errore originale per cui ho aperto la issue.

Premetto che essendo l'app in "produzione" per gli errori mi vado a vedere direttamente i file log, quindi in sostanza il tutto è stato un mio errore di disattenzione che sorvolavo il primo errore riguardante l'AudienceRestriction e vedevo solo l'errore riguardo l'idp (il più recente che mi appariva), rimango però con qualche dubbio:

  1. Perché SPID Validator non riesce ad impostarmi in automatico il valore "Audience"? É un comportamento atteso? Possibile che abbia sbagliato io qualcosa nella generazione del metadata o qualche procedura per la validazione?
  2. Perché SPID Validator dopo il primo invio di response (con errore) non imposta più spid_idp (o meglio lo imposta con valore null)?

Queste domande non credo però abbiano niente a che fare con spid-laravel quindi penso si possa anche chiudere la issue

pdavide commented 2 years ago

Ciao @vidrez, l'attributo Audience mi sembra che sia obbligatorio da regole tecniche SPID, quindi va sondato come mail il Validator non lo imposta. Riguardo invece al cookie, è spid-laravel che lo imposta, non il validator, quindi va approfondito il perché lo trovi con valore null. Se nella tua ricerca scovi un bug oppure un modo per rendere i messaggi di errore più chiari possiamo apportare le modifiche necessarie. Nel frattempo chiudo la issue, ne puo aprire una nuova se emerge qualcosa che non va.

vidrez commented 1 year ago

Ho qualche aggiornamento sui due punti che non riuscivo a spiegarmi ma ho risolto quindi aggiungo rapidamente qui un commento che spero possa tornare utile anche ad altri.

Contesto: spid-saml-check 1.9.0 in locale su docker (con altre versioni più aggiornate avevo errori e non ho più provato a cambiare)

  1. Per il primo punto forse ho mancato io una parte nella documentazione oppure non ho capito al 100% la logica di funzionamento del validatore. Ad ogni modo usando l'idp "validator", SPID Validator si aspetta comunque che prima di iniziare la validazione della Response entri in Metadata SP a scaricare il metadata, in questo modo imposterà l'Audience correttamente per tutti i check. Anche scaricandolo prima il metadata, procedura necessaria per far funzionare correttamente il test idp, facendo l'accesso con l'idp validator bisogna andare a rifare il download.

  2. Per questo secondo punto ho trovato il "problema" nella gestione dei cookie di spid-laravel, anche se non lo considererei un problema, più che altro il validator è un eccezione. spid-laravel imposta i cookie (tra cui quello contenente l'idp) nel momento in cui invia la richiesta di login, dopodiché ad ogni check che viene fatto sulla response i cookie vengono dimenticati dentro la funzione acs, quindi già al secondo check si riceverà un errore che dice di non avere il valore dell'idp. Per risolvere in locale ho aggiunto una banale condizione per cui se l'idp è "validator" e nei config l'idp "validator" è effettivamente attivo allora non dimenticare i cookie.

Con questi due accorgimenti ora riesco a fare senza problemi tutti i check del validator.

pdavide commented 1 year ago

Ciao @vidrez grazie molte per il contributo. Rispetto al punto 2, ti andrebbe di fare una PR in modo da avere il fix disponibile per tutti?

vidrez commented 1 year ago

Certamente, nei prossimi giorni apro una PR a riguardo